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 <cgaspari@cisco.com>

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 4bb87598245fcd5757aff1dee9e7968465277605..d9942cb1881d109c22e45766fe956bf1d3814e9c 100644 (file)
--- 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<Ipv4Prefixes> 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 5666e4dc1a0f260e02219116f284f78d035eeeea..3a771767c0f7af3fc8439e303d6c6f89442e1171 100644 (file)
--- 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<Ipv4Prefix> prefs = Lists.newArrayList(new Ipv4Prefix("127.0.0.1/32"), new Ipv4Prefix("2.2.2.2/24"));
+        final List<Ipv4Prefix> 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());