Commit f61a6288 authored by Leigh B Stoller's avatar Leigh B Stoller

We now save the switch path that assign computes, into the DB. We then

use this path when setting up the vlan, instead of recomputing the set
of trunks that are need. Assign does a much better job of this, so
throwing the info away is bad.

But, if there is no switch path, we still have to be careful cause the
switch infrastructure might have loops, and the existing algorithm did
not take that into account. And in fact, Utah has loops and this was
causing grief. I added a simple spanning tree function (Prim's Greedy)
to calculate a loop free set of trunks.

An added complication is if the vlans are modified on the command
line, and the there is a switch path in the DB. In this case we have
to throw that away, and revert to dumb loop free calculation.

Note that we also have to store the switch path in the vlans table,
since for swapmod/synctables, we need to know how to undo stale vlans
(which are no longer in the lans table).
parent be08fdbf
...@@ -37,6 +37,8 @@ use Exporter; ...@@ -37,6 +37,8 @@ use Exporter;
reserveVlanTag getReservedVlanTag clearReservedVlanTag reserveVlanTag getReservedVlanTag clearReservedVlanTag
convertPortFromString convertPortsFromStrings convertPortFromString convertPortsFromStrings
mapVlansToSwitches mapStaleVlansToSwitches mapVlansToSwitches mapStaleVlansToSwitches
getTrunksForVlan getExperimentTrunksForVlan
setSwitchTrunkPath
); );
use English; use English;
...@@ -45,6 +47,7 @@ use libtestbed; ...@@ -45,6 +47,7 @@ use libtestbed;
use libtblog qw(tbdie tbwarn tbreport SEV_ERROR); use libtblog qw(tbdie tbwarn tbreport SEV_ERROR);
use Experiment; use Experiment;
use Lan; use Lan;
use emutil qw(SpanningTree);
use strict; use strict;
use SNMP; use SNMP;
use Port; use Port;
...@@ -1064,6 +1067,130 @@ sub getTrunkPath($$$$) { ...@@ -1064,6 +1067,130 @@ sub getTrunkPath($$$$) {
} }
} }
#
# This is a replacement for getTrunksFromSwitches, used by the stack
# module. The idea is to use the path in the DB, as computed by
# assign. Fall back to old method method if no path defined in the DB.
# We need the switches in case we fall back, otherwise we do what
# the DB says (as computed by assign), ignoring the switches.
#
sub getTrunksForVlan($@)
{
my ($vlan_id, @switches) = @_;
my %trunks = getTrunks();
my $vlan = VLan->Lookup($vlan_id);
return ()
if (!defined($vlan));
print STDERR "getTrunksForVlan: $vlan_id: @switches\n";
#
# We want to use the path that is in the DB.
#
my $path = $vlan->GetSwitchPath();
if (!defined($path) || $path eq "") {
#
# Nothing defined in the DB, so fall back to old method.
#
# One switch, cannot be a path.
#
return ()
if (scalar(@switches) < 2);
my @trunks = getTrunksFromSwitches(\%trunks, @switches);
#
# Now form a spanning tree to ensure there are no loops.
#
@trunks = SpanningTree(\@trunks);
print STDERR " old style path: " .
join(" ", map { join(":", @$_) } @trunks) . "\n";
return @trunks;
}
print STDERR " DB path: $path\n";
my @path = ();
foreach my $p (split(" ", $path)) {
my ($a,$b) = split(":", $p);
if (!exists($trunks{$a})) {
print STDERR "No trunk entry for $a\n";
next;
}
if (!exists($trunks{$a}->{$b})) {
print STDERR "No trunk entry for $a:$b\n";
next;
}
push(@path, [$a, $b]);
}
if (@path) {
print STDERR " new style path: ".
join(" ", map { join(":", @$_) } @path) . "\n";
}
return @path;
}
#
# Same as above, only we want the recorded switch path from vlans table,
# since this might be a swapmod and the contents of the lans table is
# not how things are now, but how they will be later.
#
sub getExperimentTrunksForVlan($@)
{
my ($vlan_id, @switches) = @_;
my %trunks = getTrunks();
my $vlan = VLan->Lookup($vlan_id);
return ()
if (!defined($vlan));
print STDERR "getExperimentTrunksForVlan: $vlan_id: @switches\n";
#
# We want to use the path that is in the DB.
#
my $path = VLan->GetVlanSwitchPath($vlan);
if (!defined($path) || $path eq "") {
#
# Nothing defined in the DB, so fall back to old method.
#
# One switch, cannot be a path.
#
return ()
if (scalar(@switches) < 2);
my @trunks = getTrunksFromSwitches(\%trunks, @switches);
#
# Now form a spanning tree to ensure there are no loops.
#
@trunks = SpanningTree(\@trunks);
print STDERR " old style path: " .
join(" ", map { join(":", @$_) } @trunks) . "\n";
return @trunks;
}
print STDERR " DB path: $path\n";
my @path = ();
foreach my $p (split(" ", $path)) {
my ($a,$b) = split(":", $p);
if (!exists($trunks{$a})) {
print STDERR "No trunk entry for $a\n";
next;
}
if (!exists($trunks{$a}->{$b})) {
print STDERR "No trunk entry for $a:$b\n";
next;
}
push(@path, [$a, $b]);
}
if (@path) {
print STDERR " new style path: ".
join(" ", map { join(":", @$_) } @path) . "\n";
}
return @path;
}
# #
# Given a set of vlans, determine *exactly* what devices are needed # Given a set of vlans, determine *exactly* what devices are needed
# for the ports and any trunks that need to be crossed. This is done # for the ports and any trunks that need to be crossed. This is done
...@@ -1156,6 +1283,22 @@ sub mapPortsToSwitches(@) ...@@ -1156,6 +1283,22 @@ sub mapPortsToSwitches(@)
return @sorted; return @sorted;
} }
#
#
#
sub setSwitchTrunkPath($)
{
my ($vlan) = @_;
my %switches = ();
my @ports = getVlanPorts($vlan->lanid());
my %map = mapPortsToDevices(@ports);
my @trunks = getTrunksForVlan($vlan->lanid(), keys(%map));
my $path = join(" ", map { join(":", @$_) } @trunks);
return $vlan->SetSwitchPath($path);
}
# #
# Returns a list of trunks, in the form [src, dest], from a path (as returned # Returns a list of trunks, in the form [src, dest], from a path (as returned
# by getTrunkPath() ). For example, if the input is: # by getTrunkPath() ). For example, if the input is:
...@@ -1235,12 +1378,11 @@ sub getTrunksFromSwitches($@) { ...@@ -1235,12 +1378,11 @@ sub getTrunksFromSwitches($@) {
} }
# #
# Last, remove any duplicates from the list of trunks # Remove any duplicates from the list of trunks
# #
my @trunks = getUniqueTrunks(@trunkList); my @trunks = getUniqueTrunks(@trunkList);
return @trunks; return @trunks;
} }
# #
......
...@@ -319,7 +319,8 @@ sub setPortVlan($$@) { ...@@ -319,7 +319,8 @@ sub setPortVlan($$@) {
# Find out every switch which might have to transit this VLAN through # Find out every switch which might have to transit this VLAN through
# its trunks # its trunks
# #
@trunks = getTrunksFromSwitches(\%trunks, keys %switches); @trunks = getTrunksForVlan($vlan_id, keys(%switches));
foreach my $trunk (@trunks) { foreach my $trunk (@trunks) {
my ($src,$dst) = @$trunk; my ($src,$dst) = @$trunk;
$switches{$src} = $switches{$dst} = 1; $switches{$src} = $switches{$dst} = 1;
...@@ -779,6 +780,8 @@ sub removeVlan($@) { ...@@ -779,6 +780,8 @@ sub removeVlan($@) {
if (!@vlan_ids) { if (!@vlan_ids) {
return 1; return 1;
} }
# First, get a list of all trunks
my %trunks = getTrunks();
my %vlan_numbers = $self->findVlans(@vlan_ids); my %vlan_numbers = $self->findVlans(@vlan_ids);
foreach my $vlan_id (@vlan_ids) { foreach my $vlan_id (@vlan_ids) {
...@@ -791,16 +794,31 @@ sub removeVlan($@) { ...@@ -791,16 +794,31 @@ sub removeVlan($@) {
return 0; return 0;
} }
#
# Next, figure out which switches this VLAN exists on.
#
# Since this may be a hand-created (not from the database) VLAN, the
# only way we can figure out which swtiches this VLAN spans is to
# ask them all.
#
my @switches = $self->switchesWithPortsInVlan($vlan_number);
#
# Next, get a list of the trunks that are used to move between these
# switches
#
my @trunks = getExperimentTrunksForVlan($vlan_id, @switches);
# #
# Prevent the VLAN from being sent across trunks. # Prevent the VLAN from being sent across trunks.
# #
if (!$self->setVlanOnTrunks($vlan_number,0)) { if (!$self->setVlanOnTrunks2($vlan_number, 0, \%trunks, @trunks)) {
warn "ERROR: Unable to remove VLAN $vlan_number from trunks!\n"; warn "ERROR: Unable to remove VLAN $vlan_number from trunks!\n";
# #
# We can keep going, 'cause we can still remove the VLAN # We can keep going, 'cause we can still remove the VLAN
# #
} }
} }
# #
...@@ -1111,21 +1129,24 @@ sub disableTrunking($$) { ...@@ -1111,21 +1129,24 @@ sub disableTrunking($$) {
} }
# #
# Not a 'public' function - only needs to get called by other functions in # Now a public function; for a vlan, add or remove it from the trunks
# this file, not external functions. # it needs to span the switches.
# #
# Enables or disables (depending on $value) a VLAN on all appropriate # Enables or disables (depending on $value).
# switches in a stack. Returns 1 on sucess, 0 on failure. # Returns 1 on success, 0 on failure.
#
# ONLY pass in @ports if you're SURE that they are the only ports in the
# VLAN - basically, only if you just created it. This is a shortcut, so
# that we don't have to ask all switches if they have any ports in the VLAN.
# #
sub setVlanOnTrunks($$$;@) { sub setVlanOnSwitchTrunks($$$) {
my $self = shift; my $self = shift;
my $vlan_number = shift; my $vlan_id = shift;
my $value = shift; my $enable = shift;
my @ports = @_;
my $vlan_number = $self->findVlan($vlan_id);
if (!$vlan_number) {
print STDERR
"ERROR: VLAN with identifier $vlan_id does not exist on stack " .
$self->{STACKID} . "\n" ;
return 0;
}
# #
# First, get a list of all trunks # First, get a list of all trunks
...@@ -1135,30 +1156,19 @@ sub setVlanOnTrunks($$$;@) { ...@@ -1135,30 +1156,19 @@ sub setVlanOnTrunks($$$;@) {
# #
# Next, figure out which switches this VLAN exists on # Next, figure out which switches this VLAN exists on
# #
my @switches; my @switches = $self->switchesWithPortsInVlan($vlan_number);
if (@ports) {
#
# I'd rather not have to go out to the switches to ask which ones
# have ports in the VLAN. So, if they gave me ports, I'll just
# trust that those are the only ones in the VLAN
#
@switches = getDeviceNames(@ports);
} else {
#
# Since this may be a hand-created (not from the database) VLAN, the
# only way we can figure out which swtiches this VLAN spans is to
# ask them all.
#
@switches = $self->switchesWithPortsInVlan($vlan_number);
}
# #
# Next, get a list of the trunks that are used to move between these # Next, get a list of the trunks that are used to move between these
# switches # switches. When disabling, we want the list from the DB if it exsists.
# #
my @trunks = getTrunksFromSwitches(\%trunks,@switches); my @trunks = ($enable ?
return $self->setVlanOnTrunks2($vlan_number,$value,\%trunks,@trunks); getTrunksForVlan($vlan_id, @switches) :
getExperimentTrunksForVlan($vlan_id, @switches));
return $self->setVlanOnTrunks2($vlan_number,$enable,\%trunks,@trunks);
} }
# #
# Enables or disables (depending on $value) a VLAN on all the supplied # Enables or disables (depending on $value) a VLAN on all the supplied
# trunks. Returns 1 on sucess, 0 on failure. # trunks. Returns 1 on sucess, 0 on failure.
......
...@@ -27,6 +27,7 @@ use EmulabFeatures; ...@@ -27,6 +27,7 @@ use EmulabFeatures;
use Lan; use Lan;
use English; use English;
use Getopt::Long; use Getopt::Long;
use emutil;
use strict; use strict;
# Optional alternate version of libraries. # Optional alternate version of libraries.
...@@ -190,7 +191,7 @@ GetOptions(\%opt, ...@@ -190,7 +191,7 @@ GetOptions(\%opt,
'y=s','x=s','z=s','F','L=s','O', 'D', 'R', 'f', 'X', 'Z', 'vlan_tag=i', 'y=s','x=s','z=s','F','L=s','O', 'D', 'R', 'f', 'X', 'Z', 'vlan_tag=i',
'of-disable=s', 'of-enable=s', 'of-controller=s', 'of-listener=s', 'of-disable=s', 'of-enable=s', 'of-controller=s', 'of-listener=s',
'o=s@{1,1}', 'redirect-err', 'blockmode', 'syncvlans', 'impotent', 'o=s@{1,1}', 'redirect-err', 'blockmode', 'syncvlans', 'impotent',
'shadow', 'skip-supplied'); 'shadow', 'skip-supplied', 'whol-magic', 'backtraceonwarning');
if ($opt{h}) { if ($opt{h}) {
exit &usage; exit &usage;
...@@ -203,7 +204,9 @@ if ($opt{h}) { ...@@ -203,7 +204,9 @@ if ($opt{h}) {
if ($opt{'redirect-err'}) { if ($opt{'redirect-err'}) {
open(STDERR, ">&STDOUT"); open(STDERR, ">&STDOUT");
} }
if ($opt{'backtraceonwarning'}) {
BackTraceOnWarning(1);
}
if ($opt{'impotent'}) { if ($opt{'impotent'}) {
$impotent = 1; $impotent = 1;
} }
...@@ -1677,13 +1680,16 @@ sub doRestorePortStatus($@) { ...@@ -1677,13 +1680,16 @@ sub doRestorePortStatus($@) {
$errors += $stack->setPortVlan($vlan_id, $port); $errors += $stack->setPortVlan($vlan_id, $port);
if ($errors) { return $errors; } if ($errors) { return $errors; }
#
# Do not worry about mucking with the switch trunk paths.
# This call exists to temporarily move ports back and forth.
#
if (defined($source_vlan) && $source_vlan->KeepInSync()) { if (defined($source_vlan) && $source_vlan->KeepInSync()) {
if ($source_vlan->DelPort($port)) { if ($source_vlan->DelPort($port)) {
print "Could not yank $port from $source_vlan\n"; print "Could not yank $port from $source_vlan\n";
$errors++; $errors++;
} }
my @tmp = ($port->toIfaceString()); VLan->RecordVlanInsertion($source_vlan->id(), $stack->{STACKID});
VLan->RecordVLanModification($source_vlan->id(), undef, \@tmp);
} }
if (defined($target_vlan) && $target_vlan->KeepInSync()) { if (defined($target_vlan) && $target_vlan->KeepInSync()) {
if ($target_vlan->AddPort($port)) { if ($target_vlan->AddPort($port)) {
...@@ -1967,11 +1973,6 @@ sub CreateOneVlan($$$@) ...@@ -1967,11 +1973,6 @@ sub CreateOneVlan($$$@)
if ($stack->setPortVlan($vlanid,@ports)) { if ($stack->setPortVlan($vlanid,@ports)) {
$errors++; $errors++;
} }
else {
# Convert the ports to node:iface for the next call.
my @conports = map {$_->toIfaceString()} @ports;
VLan->RecordVLanModification($vlanid, \@conports, undef);
}
} }
else { else {
my $vlan_number = $stack->createVlan($vlanid, $vlanid, \@ports); my $vlan_number = $stack->createVlan($vlanid, $vlanid, \@ports);
...@@ -1984,13 +1985,16 @@ sub CreateOneVlan($$$@) ...@@ -1984,13 +1985,16 @@ sub CreateOneVlan($$$@)
# #
$errors++; $errors++;
} }
else {
VLan->RecordVlanInsertion($vlanid, $stack->{STACKID});
}
} }
return $errors return $errors
if ($errors); if ($errors);
#
# Record an insertion to make sure the DB reflects the current state
# of the vlan.
#
VLan->RecordVlanInsertion($vlanid, $stack->{STACKID});
# #
# Set the speed and duplex of each interface depending on the # Set the speed and duplex of each interface depending on the
# value in the database # value in the database
...@@ -2233,12 +2237,6 @@ sub syncVlansFromTables($$) { ...@@ -2233,12 +2237,6 @@ sub syncVlansFromTables($$) {
if (@trunkports) { if (@trunkports) {
$stack->removeSomePortsFromTrunk($vlanid, @trunkports) $stack->removeSomePortsFromTrunk($vlanid, @trunkports)
or goto bad; or goto bad;
# Convert the ports to node:iface for the next call.
@trunkports = map {$_->toIfaceString()} @trunkports;
VLan->RecordVLanModification($vlanid,
undef, \@trunkports)
== 0 or goto bad;
} }
if (!$stack->removeVlan($vlanid)) { if (!$stack->removeVlan($vlanid)) {
print STDERR "Could not remove vlan: $vlanid\n"; print STDERR "Could not remove vlan: $vlanid\n";
...@@ -2283,17 +2281,18 @@ sub syncVlansFromTables($$) { ...@@ -2283,17 +2281,18 @@ sub syncVlansFromTables($$) {
next next
if (! (@staleports || @staletrunks)); if (! (@staleports || @staletrunks));
#
# Clear the switch trunks since we are removing ports, which
# changes that set.
#
$stack->setVlanOnSwitchTrunks($vlanid, 0);
if (@staleports) { if (@staleports) {
debug("Removing stale ports from vlan $vlanid: ".Port->toStrings(@staleports)."\n"); debug("Removing stale ports from vlan $vlanid: ".Port->toStrings(@staleports)."\n");
if (!$impotent) { if (!$impotent) {
$stack->removeSomePortsFromVlan($vlanid, @staleports) $stack->removeSomePortsFromVlan($vlanid, @staleports)
or goto bad; or goto bad;
# Convert the ports to node:iface for the next call.
@staleports = map {$_->toIfaceString()} @staleports;
VLan->RecordVLanModification($vlanid, undef, \@staleports) == 0
or goto bad;
} }
} }
if (@staletrunks) { if (@staletrunks) {
...@@ -2303,13 +2302,14 @@ sub syncVlansFromTables($$) { ...@@ -2303,13 +2302,14 @@ sub syncVlansFromTables($$) {
if (!$impotent) { if (!$impotent) {
$stack->removeSomePortsFromTrunk($vlanid, @staletrunks) $stack->removeSomePortsFromTrunk($vlanid, @staletrunks)
or goto bad; or goto bad;
# Convert the ports to node:iface for the next call.
@staletrunks = map {$_->toIfaceString()} @staletrunks;
VLan->RecordVLanModification($vlanid, undef, \@staletrunks) == 0
or goto bad;
} }
} }
#
# We can optimize this slightly by looking to see if this
# vlan is going to get new ports below, and waiting till then.
#
$stack->setVlanOnSwitchTrunks($vlanid, 1);
VLan->RecordVlanInsertion($vlanid, $stack->{STACKID});
} }
# #
...@@ -2430,6 +2430,12 @@ sub syncVlansFromTables($$) { ...@@ -2430,6 +2430,12 @@ sub syncVlansFromTables($$) {
next next
if ($impotent); if ($impotent);
#
# Clear the switch trunks since we are adding ports, which
# changes that set. The trunks are added back in CreateOneVlan.
#
$stack->setVlanOnSwitchTrunks($vlanid, 0);
if (CreateOneVlan($experiment, $stack, $vlanid, @newports)) { if (CreateOneVlan($experiment, $stack, $vlanid, @newports)) {
goto bad; goto bad;
} }
...@@ -2909,7 +2915,7 @@ sub doSyncVlansWithDB($) { ...@@ -2909,7 +2915,7 @@ sub doSyncVlansWithDB($) {
if (!@dbports) { if (!@dbports) {
print " Making it consistent\n"; print " Making it consistent\n";
foreach my $port (@swports) { foreach my $port (@swports) {
if ($vlan->AddPort($port)) { if (!defined($vlan->AddPort($port))) {
print "Could not add $port to $vlan\n"; print "Could not add $port to $vlan\n";
$errors++; $errors++;
} }
...@@ -2931,8 +2937,10 @@ sub doMakeVlan($$@) { ...@@ -2931,8 +2937,10 @@ sub doMakeVlan($$@) {
my $vlan_name = shift; my $vlan_name = shift;
my @ports = @_; my @ports = @_;
my $errors = 0; my $errors = 0;
my $vlanexists = 0;
my $target_vlan; my $target_vlan;
my $source_vlan; my $source_vlan;
my ($old_target_swpath, $old_source_swpath);
if (@$stacks > 1) { if (@$stacks > 1) {
die "VLAN creation across multiple stacks is not yet supported\n" . die "VLAN creation across multiple stacks is not yet supported\n" .
...@@ -3011,9 +3019,6 @@ sub doMakeVlan($$@) { ...@@ -3011,9 +3019,6 @@ sub doMakeVlan($$@) {
$target_vlan->MarkInternal(); $target_vlan->MarkInternal();
} }
} }
foreach my $port (@ports) {
$target_vlan->AddPort($port);
}
} }
else { else {
# #
...@@ -3027,6 +3032,9 @@ sub doMakeVlan($$@) { ...@@ -3027,6 +3032,9 @@ sub doMakeVlan($$@) {
# Pass to outer boss. # Pass to outer boss.
# #
if ($ELABINELAB) { if ($ELABINELAB) {
foreach my $port (@ports) {
$target_vlan->AddPort($port);
}
return RemoteDoVlansFromTables($experiment, $target_vlan->id()); return RemoteDoVlansFromTables($experiment, $target_vlan->id());
} }
...@@ -3070,10 +3078,67 @@ sub doMakeVlan($$@) { ...@@ -3070,10 +3078,67 @@ sub doMakeVlan($$@) {
my $device_id = ($target_vlan->IsInternal() ? my $device_id = ($target_vlan->IsInternal() ?
$target_vlan->vname() : $target_vlan->id()); $target_vlan->vname() : $target_vlan->id());
# Save for error recovery.
$vlanexists = $stack->vlanExists($device_id);
#
# Remove the current set of switch trunks since we have to generate
# a new set anyway. Do this for experiment vlans only though.
#
if (!$opt{'whol-magic'}) {
if (! $target_vlan->IsManual()) {
print STDERR "Disabling switch trunks on $target_vlan\n";
if (!$stack->setVlanOnSwitchTrunks($target_vlan->lanid(), 0)) {
print STDERR
"Could not disable switch trunks on $target_vlan\n";
goto bad;
}
}
# This causes fallback to snmpit switch path generation.
$old_target_swpath = $target_vlan->GetSwitchPath();
$target_vlan->ClrSwitchPath();
}
#
# Same goes for the source vlan, but worse since once we forget
# the port, there is no way to recover the path.
#
if (defined($source_vlan) &&
!$source_vlan->SameVlan($target_vlan) &&
$source_vlan->KeepInSync()) {
if (!$opt{'whol-magic'}) {
if (!$stack->setVlanOnSwitchTrunks($source_vlan->lanid(), 0)) {
print STDERR
"Could not disable switch trunks on $source_vlan\n";
goto bad;
}
# This causes fallback to snmpit switch path generation.
$old_source_swpath = $source_vlan->GetSwitchPath();
$source_vlan->ClrSwitchPath();
}
# Yank the ports out of the old vlan now.
foreach my $port (@ports) {