summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Bays <robert@vyatta.com>2011-06-06 11:51:46 -0700
committerRobert Bays <robert@vyatta.com>2011-06-06 11:51:46 -0700
commitbc5905e60a4cebfaeb5b180472ae2c6633ba6da3 (patch)
tree9d6bcdfaa7813e6458c629903bb4ae2f5bbffa9a
parentc625b415bb3e087e7df8ed054617526b0358f79c (diff)
downloadvyatta-cfg-quagga-bc5905e60a4cebfaeb5b180472ae2c6633ba6da3.tar.gz
vyatta-cfg-quagga-bc5905e60a4cebfaeb5b180472ae2c6633ba6da3.zip
Bug 6841: a neighbor becomes inactivated in the routing engine after its peer-group is removed
Bug 6981: BGP commit failure with shutdown and peer-group attributes rework of the peer-groups implementation.
-rwxr-xr-x[-rw-r--r--]lib/Vyatta/Quagga/Config.pm75
-rwxr-xr-xscripts/bgp/vyatta-bgp.pl129
2 files changed, 144 insertions, 60 deletions
diff --git a/lib/Vyatta/Quagga/Config.pm b/lib/Vyatta/Quagga/Config.pm
index 3c3187e0..f07ffa11 100644..100755
--- a/lib/Vyatta/Quagga/Config.pm
+++ b/lib/Vyatta/Quagga/Config.pm
@@ -122,6 +122,18 @@ sub deleteConfigTreeRecursive {
return 0;
}
+# Explicitly re-add an unchanged node to the list to be re-inserted into Quagga
+# Note that this method is not recursive!
+sub reInsertNode
+{
+ my ($self, $level) = @_;
+
+ _pdebug(1, "_reAddNode: level: $level");
+ if (! _qtree("$level", 'oneshot')) {
+ return 0;
+ }
+}
+
### End Public methods -
### Private methods
sub _pdebug {
@@ -289,7 +301,8 @@ sub _qVarReplace {
my $node = shift;
my $qcommand = shift;
- _pdebug(2, "_qVarReplace entry: node - $node\n_qVarReplace entry: qcommand - $qcommand");
+ _pdebug(2, "_qVarReplace entry: node - $node");
+ _pdebug(2, "_qVarReplace entry: qcommand - $qcommand");
my @nodes = split /\s/, $node;
my @qcommands = split /\s/, $qcommand;
@@ -367,34 +380,39 @@ sub _qCommandFind {
# translate the adds/changes in a Vyatta config tree into Quagga vtysh commands.
# recursively walks the tree.
# input: $1 - the level of the Vyatta config tree to start at
-# $2 - the action (set|delete)
+# ex: protocols bgp 1 neighbor 1.1.1.1 remote-as 10
+# $2 - the action (set|delete|oneshot)
# output: none - creates the %vtysh that contains the Quagga add commands
sub _qtree {
my ($level, $action) = @_;
my @nodes;
- my ($qcom, $vtysh);
-
- $qcom = $_qcomref;
-
- # Would love to reference a global config and just reset Levels,
- # but Vyatta::Config isn't recursion safe.
+ my $oneshot = 0;
+ my $qcom = $_qcomref;
+ my $vtysh = \%_vtysh;
my $config = new Vyatta::Config;
$config->setLevel($level);
-
- # setup references for set or delete action
- if ($action eq 'set') {
- $vtysh = \%_vtysh;
+
+ # setup data structures and references for various action types
+ if ($action eq 'oneshot') {
+ # split the last parameter out of the level string
+ $level =~ s/\s+/ /g;
+ my $index = rindex($level, " ");
+ my $node = substr($level, $index + 1);
+ $level = substr($level, 0, (-1 * length($node)));
+ chop $level;
+ $config->setLevel($level);
+ push @nodes, $node;
+ $action = 'set';
+ $oneshot = 1;
+ } elsif ($action eq 'set') {
@nodes = $config->listNodes();
- }
- else {
+ } else {
$vtysh = \%_vtyshdel;
@nodes = $config->listDeleted();
# handle special case for multi-nodes values being deleted
- # listDeleted() doesn't return the node as deleted if it is a multi
- # unless all values are deleted.
- # TODO: fix listDeleted() in Config.pm
- # This is really, really fugly.
+ # listDeleted() doesn't return the node as being deleted if
+ # it is a multi unless all values are deleted. This is suboptimal.
my @all_nodes = $config->listNodes();
foreach my $node (@all_nodes) {
my @array = split /\s+/, $level;
@@ -409,7 +427,6 @@ sub _qtree {
}
}
}
-
} ## end else {
_pdebug(1, "_qtree - action: $action\tlevel: $level");
@@ -420,8 +437,9 @@ sub _qtree {
_pdebug(2, "_qtree - foreach node loop - node $node");
# for set action, need to check that the node was actually changed. Otherwise
- # we end up re-writing every node to Quagga every commit, which is bad. Mmm' ok?
- if (($action eq 'del') || ($config->isChanged("$node"))) {
+ # we end up re-writing every node to Quagga every commit. For the other action
+ # types, just do it.
+ if ($config->isChanged("$node") || ($action eq 'del') || $oneshot) {
# is there a Quagga command template?
# TODO: need to add function reference support to qcom hash for complicated nodes
my $qcommand = _qCommandFind("$level $node", $action, $qcom);
@@ -460,20 +478,21 @@ sub _qtree {
foreach my $val (@vals) {
my $var = _qVarReplace("$level $node $val", $qcom->{$qcommand}->{$action});
push @{$vtysh->{"$qcommand"}}, "$level $node $val \036 $var";
- _pdebug(1, "_qtree leaf node command: set $level $action $node $val \n\t\t\t\t\t$var");
+ _pdebug(1, "_qtree leaf node command: $action $level $node $val \n\t\t\t\t\t$var");
}
- }
-
- else {
+ } else {
my $var = _qVarReplace("$level $node", $qcom->{$qcommand}->{$action});
push @{$vtysh->{"$qcommand"}}, "$level $node \036 $var";
- _pdebug(1, "_qtree node command: set $level $action $node \n\t\t\t\t$var");
+ _pdebug(1, "_qtree node command: $action $level $node \n\t\t\t\t$var");
}
}
}
+
# recurse to next level in tree
- _qtree("$level $node", 'del');
- _qtree("$level $node", 'set');
+ if (! $oneshot) {
+ _qtree("$level $node", 'del');
+ _qtree("$level $node", 'set');
+ }
}
}
diff --git a/scripts/bgp/vyatta-bgp.pl b/scripts/bgp/vyatta-bgp.pl
index 2ccf4724..73d35a98 100755
--- a/scripts/bgp/vyatta-bgp.pl
+++ b/scripts/bgp/vyatta-bgp.pl
@@ -406,7 +406,8 @@ my %qcom = (
},
'protocols bgp var neighbor var peer-group' => {
set => 'router bgp #3 ; neighbor #5 peer-group #7',
- del => 'router bgp #3 ; no neighbor #5 peer-group #7',
+ del => 'router bgp #3 ; no neighbor #5 peer-group #7 ; neighbor #5 activate',
+ noerr => 'del',
},
'protocols bgp var neighbor var port' => {
set => 'router bgp #3 ; neighbor #5 port #7',
@@ -1041,6 +1042,8 @@ my %qcom = (
},
);
+my $qconfig = new Vyatta::Quagga::Config('protocols', \%qcom);
+
my ( $pg, $as, $neighbor );
my ( $main, $peername, $isneighbor, $checkpeergroups, $checksource, $isiBGPpeer, $wasiBGPpeer, $confedibgpasn);
@@ -1093,7 +1096,7 @@ sub check_peergroup_name {
# Quagga treats the first byte as a potential IPv6 address
# so we can't use it as a peer group name. So let's check for it.
if (/^[A-Fa-f]{1,4}$/) {
- die "malformed peer-group name $neighbor\n";
+ die "malformed peer-group name $neighbor\n";
}
}
@@ -1165,9 +1168,72 @@ sub bgp_type_change {
}
}
+# delete a neighbor from a peer-group
+sub neighborFromPeerGroup
+{
+ my ($level) = @_;
+
+ my @overwritelist = ('allowas-in', 'allowas-in number', 'capability dynamic', 'capability orf',
+ 'disable-connected-check', 'distribute-list import',
+ 'disable-capability-negotiation', 'ebgp-multihop', 'filter-list import',
+ 'maximum-prefix', 'override-capability', 'passive', 'password', 'port',
+ 'prefix-list import', 'route-map import', 'shutdown',
+ 'soft-reconfiguration inbound', 'strict-capability-match',
+ 'update-source', 'weight');
+
+ my $config = new Vyatta::Config;
+ $config->setLevel("protocols bgp $level");
+
+ # make sure to re-add the overwrite nodes if they exist.
+ foreach my $node (@overwritelist) {
+ if ($config->exists($node)) {
+ $qconfig->reInsertNode("protocols bgp $level $node");
+ }
+ }
+}
+
+# add a neighbor to a peer-group
+sub neighborToPeerGroup
+{
+ my ($level) = @_;
+
+ my @bannedlist = ('advertisement-interval', 'attribute-unchanged', 'capability orf',
+ 'default-originate', 'distribute-list export', 'filter-list export',
+ 'local-as', 'nexthop-self', 'prefix-list export', 'remove-private-as',
+ 'route-map export', 'route-reflector-client', 'route-server-client',
+ 'disable-send-community', 'timers', 'ttl-security', 'unsuppress-map');
+
+ my @overwritelist = ('allowas-in', 'allowas-in number', 'capability dynamic', 'capability orf',
+ 'disable-connected-check', 'distribute-list import',
+ 'disable-capability-negotiation', 'ebgp-multihop', 'filter-list import',
+ 'maximum-prefix', 'override-capability', 'passive', 'password', 'port',
+ 'prefix-list import', 'route-map import', 'shutdown',
+ 'soft-reconfiguration inbound', 'strict-capability-match',
+ 'update-source', 'weight');
+
+ my $config = new Vyatta::Config;
+ $config->setLevel("protocols bgp $level");
+
+ # first check that the neighbor doesn't contain anything in the bannedlist
+ foreach my $node (@bannedlist) {
+ if ($config->exists($node)) {
+ print "[ protocols bgp $level ]\n parameter $node is incompatible with neighbors in a peer-group\n";
+ exit 1;
+ }
+ }
+
+ # now make sure to re-add the overwrite nodes if they exist.
+ foreach my $node (@overwritelist) {
+ if ($config->exists($node)) {
+ $qconfig->reInsertNode("protocols bgp $level $node");
+ }
+ }
+}
+
# check that changed neighbors have a remote-as or peer-group defined
# and that all permutations of parameters and BGP type are correct
-sub check_remote_as {
+sub check_neighbor_parameters
+{
my $config = new Vyatta::Config;
$config->setLevel('protocols bgp');
@@ -1217,45 +1283,43 @@ sub check_remote_as {
# check neighbor if remote-as or peer-group has been changed
my @neighbors = $config->listNodes("$as neighbor");
+
foreach my $neighbor (@neighbors) {
- next unless ( $config->isChanged("$as neighbor $neighbor remote-as") ||
- $config->isDeleted("$as neighbor $neighbor remote-as") ||
- $config->isChanged("$as neighbor $neighbor peer-group") ||
- $config->isDeleted("$as neighbor $neighbor peer-group") );
-
- if ($config->isDeleted("$as neighbor $neighbor remote-as")) {
- my @neighbor_params = undef;
- @neighbor_params = $config->listNodes("$as neighbor $neighbor");
- die "[protocols bgp $as neighbor $neighbor]\n must delete the neighbor first if changing the remote-as\n"
- if (@neighbor_params);
- }
+ next unless ($config->isChanged("$as neighbor $neighbor remote-as") ||
+ $config->isChanged("$as neighbor $neighbor peer-group") ||
+ $config->isDeleted("$as neighbor $neighbor remote-as") ||
+ $config->isDeleted("$as neighbor $neighbor peer-group") );
- # First check that we have a remote-as defined in the neighbor or that
- # the neighbor is a member of a peer-group that has a remote-as defined
+ # remote-as checks: Make sure the neighbor has a remote-as defined locally or in the peer-group
my ($remoteas, $peergroup, $peergroupas);
$remoteas = $config->returnValue("$as neighbor $neighbor remote-as");
if ($config->exists("$as neighbor $neighbor peer-group")) {
- $peergroup = $config->returnValue("$as neighbor $neighbor peer-group");
+ $peergroup = $config->returnValue("$as neighbor $neighbor peer-group");
if ($config->exists("$as peer-group $peergroup remote-as")) {
- $peergroupas = $config->returnValue("$as peer-group $peergroup remote-as");
+ $peergroupas = $config->returnValue("$as peer-group $peergroup remote-as");
}
}
- die "[protocols bgp $as neighbor $neighbor]\n must define a remote-as or peer-group\n"
- unless ($peergroup || $remoteas);
+ die "[ protocols bgp $as neighbor $neighbor ]\n must set remote-as or peer-group with remote-as defined\n"
+ unless ($remoteas || $peergroupas);
- die "[protocols bgp $as neighbor $neighbor]\n remote-as should not be defined in both neighbor and peer-group\n"
+ die "[ protocols bgp $as neighbor $neighbor ]\n remote-as should not be defined in both neighbor and peer-group\n"
if ($remoteas && $peergroupas);
-
- die "[protocols bgp $as neighbor $neighbor]\n must define a remote-as in neighbor or peer-group $peergroup\n"
- if ( (! $remoteas) && (! $peergroupas) );
+ ## end remote-as checks
- # now check if changing remote-as type from/to i/eBGP
+ # If the peer-group has changed since the last commit, update overwritable nodes
+ # We do this because Quagga removes nodes silently while vyatta-cfg does not. These
+ # functions actually make Vyatta implentation of peer-groups more consistent.
+ if ($config->isChanged("$as neighbor $neighbor peer-group")) {
+ neighborToPeerGroup("$as neighbor $neighbor");
+ } elsif ($config->isDeleted("$as neighbor $neighbor peer-group")) {
+ neighborFromPeerGroup("$as neighbor $neighbor");
+ }
+
+ # Check if changing BGP peer type from/to i/eBGP
my $error = bgp_type_change($neighbor, $as, "neighbor");
if ($error) { die "[protocols bgp $as neighbor $neighbor]\n $error\n"; }
-
} ## end foreach my $neighbor (@neighbors)
-
} ## end foreach my $as (@asns)
}
@@ -1364,21 +1428,22 @@ sub check_source {
}
}
-sub main {
+sub main
+{
# initialize the Quagga Config object with data from Vyatta config tree
- my $qconfig = new Vyatta::Quagga::Config('protocols', \%qcom);
+ # my $qconfig = new Vyatta::Quagga::Config('protocols', \%qcom);
# debug routines
- #$qconfig->setDebugLevel('3');
+ $qconfig->setDebugLevel('3');
#$qconfig->_reInitialize();
# check that all changed neighbors have a proper remote-as or peer-group defined
# and that migrations to/from iBGP eBGP are valid
- check_remote_as();
+ check_neighbor_parameters();
## deletes with priority
# delete everything in neighbor, ordered nodes last
- my @ordered = ('remote-as', 'shutdown', 'route-map', 'prefix-list', 'filter-list', 'distribute-list', 'unsuppress-map');
+ my @ordered = ('remote-as', 'peer-group', 'shutdown', 'route-map', 'prefix-list', 'filter-list', 'distribute-list', 'unsuppress-map');
# notice the extra space in the level string. keeps the parent from being deleted.
$qconfig->deleteConfigTreeRecursive('protocols bgp var neighbor var ', undef, \@ordered) || die "exiting $?\n";
$qconfig->deleteConfigTreeRecursive('protocols bgp var peer-group var ', undef, \@ordered) || die "exiting $?\n";