From 2b1942bb6a5dd50f5a60b418986b64409201a509 Mon Sep 17 00:00:00 2001 From: "Claudio D. Gasparini" Date: Mon, 16 Nov 2015 12:30:36 +0100 Subject: [PATCH] BUG-4634: Prefix in withdrawn list is not ignored when announced in same msgUpd. An UPDATE message SHOULD NOT include the same address prefix in the WITHDRAWN ROUTES and Network Layer Reachability Information fields. However, a BGP speaker MUST be able to process UPDATE messages in this form. A BGP speaker SHOULD treat an UPDATE message of this form as though the WITHDRAWN ROUTES do not contain the address prefix. Fix by ingore common NLRI on withdrawn router Change-Id: Ic9ebd3ce13f042f0dfcd2b28815442f8484add16 Signed-off-by: Claudio D. Gasparini --- .../protocol/bgp/rib/impl/BGPPeer.java | 17 +++++++++++++---- .../protocol/bgp/rib/impl/PeerTest.java | 6 +++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java index 4bb8759824..d9942cb188 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java @@ -115,7 +115,8 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun // update AdjRibs final Attributes attrs = message.getAttributes(); MpReachNlri mpReach = null; - if (message.getNlri() != null) { + final boolean isAnyNlriAnnounced = message.getNlri() != null; + if (isAnyNlriAnnounced) { mpReach = prefixesToMpReach(message); } else if (attrs != null && attrs.getAugmentation(Attributes1.class) != null) { mpReach = attrs.getAugmentation(Attributes1.class).getMpReachNlri(); @@ -125,7 +126,7 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun } MpUnreachNlri mpUnreach = null; if (message.getWithdrawnRoutes() != null) { - mpUnreach = prefixesToMpUnreach(message); + mpUnreach = prefixesToMpUnreach(message, isAnyNlriAnnounced); } else if (attrs != null && attrs.getAugmentation(Attributes2.class) != null) { mpUnreach = attrs.getAugmentation(Attributes2.class).getMpUnreachNlri(); } @@ -169,12 +170,20 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun * Create MPUnreach for the prefixes to be handled in the same way as linkstate routes * * @param message Update message containing withdrawn routes + * @param isAnyNlriAnnounced * @return MpUnreachNlri with prefixes from the withdrawn routes field */ - private static MpUnreachNlri prefixesToMpUnreach(final Update message) { + private static MpUnreachNlri prefixesToMpUnreach(final Update message, final boolean isAnyNlriAnnounced) { final List prefixes = new ArrayList<>(); for (final Ipv4Prefix p : message.getWithdrawnRoutes().getWithdrawnRoutes()) { - prefixes.add(new Ipv4PrefixesBuilder().setPrefix(p).build()); + boolean nlriAnounced = false; + if(isAnyNlriAnnounced) { + nlriAnounced = message.getNlri().getNlri().contains(p); + } + + if(!nlriAnounced) { + prefixes.add(new Ipv4PrefixesBuilder().setPrefix(p).build()); + } } return new MpUnreachNlriBuilder().setAfi(Ipv4AddressFamily.class).setSafi(UnicastSubsequentAddressFamily.class).setWithdrawnRoutes( new WithdrawnRoutesBuilder().setDestinationType( diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/PeerTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/PeerTest.java index 5666e4dc1a..3a771767c0 100644 --- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/PeerTest.java +++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/PeerTest.java @@ -123,17 +123,17 @@ public class PeerTest extends AbstractRIBTestSetup { this.classic.onSessionUp(this.session); Assert.assertArrayEquals(new byte[] {1, 1, 1, 1}, this.classic.getRawIdentifier()); assertEquals("BGPPeer{name=testPeer, tables=[TablesKey [_afi=class org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.Ipv4AddressFamily, _safi=class org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.UnicastSubsequentAddressFamily]]}", this.classic.toString()); - final List prefs = Lists.newArrayList(new Ipv4Prefix("127.0.0.1/32"), new Ipv4Prefix("2.2.2.2/24")); + final List prefs = Lists.newArrayList(new Ipv4Prefix("8.0.1.0/28"), new Ipv4Prefix("127.0.0.1/32"), new Ipv4Prefix("2.2.2.2/24")); final UpdateBuilder ub = new UpdateBuilder(); ub.setNlri(new NlriBuilder().setNlri(prefs).build()); ub.setAttributes(new AttributesBuilder().build()); this.classic.onMessage(this.session, ub.build()); - assertEquals(2, this.routes.size()); + assertEquals(3, this.routes.size()); //create new peer so that it gets advertized routes from RIB try (final BGPPeer testingPeer = new BGPPeer("testingPeer", getRib())) { testingPeer.onSessionUp(this.session); - assertEquals(2, this.routes.size()); + assertEquals(3, this.routes.size()); assertEquals(1, testingPeer.getBgpPeerState().getSessionEstablishedCount().intValue()); assertEquals(1, testingPeer.getBgpPeerState().getRouteTable().size()); assertNotNull(testingPeer.getBgpSessionState()); -- 2.36.6