BUG-4827: Modify ipv4-routes list model, to include path-id as part of the key

Add path Id as a part of the key for ipv4-routes. This will force us to use
path-Id when writing on Loc-Rib even when add-path selection mode are not
used. Therefore we need to adapt the code on base selection mode
to take in consideration wheter model can support Add-path or not
and proced accordingly.

Change-Id: I94bc6523ff788c236008df892569d3ea04cadb65
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>

diff --git a/bgp/inet/src/main/yang/bgp-inet.yang b/bgp/inet/src/main/yang/bgp-inet.yang
index 55a5f5cc4e62714676e35b61183d2947ac3c05a6..cdcc58489e42b2edf92cb822a201f83eab3c7956 100644 (file)
--- a/bgp/inet/src/main/yang/bgp-inet.yang
+++ b/bgp/inet/src/main/yang/bgp-inet.yang
@@ -64,7 +64,7 @@ module bgp-inet {
             list ipv4-route {
                 uses ipv4-prefix;
 
-                key prefix;
+                key "prefix path-id";
 
                 uses bgp-rib:route;
             }

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPath.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPath.java
index 0f9bb058d5b48331c9f2472486ef6ebcdc6806db..eeeedc47fcf3212c15f7df24c803137f53d8e592 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPath.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPath.java
@@ -31,4 +31,9 @@ public interface BestPath {
      * @return the path attributes
      */
     ContainerNode getAttributes();
+    /**
+     *
+     * @return pathId
+     */
+    long getPathId();
 }
\ No newline at end of file

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/PathSelectionMode.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/PathSelectionMode.java
index 4e7b2a5bceef490d7de3b960dcf246fdce125bfa..b1ee6a1d1a8ce7df6d848547a21d295e8e3e2541 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/PathSelectionMode.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/PathSelectionMode.java
@@ -8,9 +8,6 @@
 package org.opendaylight.protocol.bgp.mode.api;
 
 public interface PathSelectionMode extends AutoCloseable {
-    /**
-     *
-     */
     /**
      * Create a RouteEntry
      * @param isComplex true if is complex

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
index 7ba5acc7b1b9abf44b419b79e6f75bbd61f96a9e..8845a04dd2f94502f872369465c727d304b30ba5 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
@@ -9,7 +9,6 @@ package org.opendaylight.protocol.bgp.mode.impl.base;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.primitives.UnsignedInteger;
-import java.util.Map;
 import java.util.Optional;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
@@ -66,7 +65,7 @@ abstract class BaseAbstractRouteEntry implements RouteEntry {
      * Remove route
      *
      * @param routerId router ID in unsigned integer
-     * @param offset offset Offset of removed route
+     * @param offset of removed route
      * @return true if its the last route
      */
     final boolean removeRoute(final UnsignedInteger routerId, final int offset) {
@@ -112,13 +111,12 @@ abstract class BaseAbstractRouteEntry implements RouteEntry {
     @Override
     public void updateRoute(final TablesKey localTK, final ExportPolicyPeerTracker peerPT, final YangInstanceIdentifier locRibTarget, final RIBSupport ribSup,
         final CacheDisconnectedPeers discPeers, final DOMDataWriteTransaction tx, final PathArgument routeIdPA) {
-        final YangInstanceIdentifier writePath = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdPA);
         if (this.removedBestPath.isPresent()) {
-            removePathFromDataStore(this.removedBestPath.get(), routeIdPA, writePath, peerPT, localTK, ribSup, discPeers, tx);
+            removePathFromDataStore(this.removedBestPath.get(), routeIdPA, locRibTarget, peerPT, localTK, ribSup, discPeers, tx);
             this.removedBestPath = Optional.empty();
         }
         if (this.bestPath.isPresent()) {
-            addPathToDataStore(this.bestPath.get(), routeIdPA, writePath, ribSup, peerPT, localTK, discPeers, tx);
+            addPathToDataStore(this.bestPath.get(), routeIdPA, locRibTarget, ribSup, peerPT, localTK, discPeers, tx);
         }
     }
 
@@ -127,24 +125,40 @@ abstract class BaseAbstractRouteEntry implements RouteEntry {
         final PeerExportGroup peerGroup, final TablesKey localTK, final RIBSupport ribSup, final DOMDataWriteTransaction tx) {
         final BaseBestPath path = this.bestPath.get();
         final ContainerNode effAttrib = peerGroup.effectiveAttributes(path.getPeerId(), path.getAttributes());
-        writeRoute(destPeer, getRouteTarget(ribSup, rootPath, routeId, localTK), effAttrib, createValue(routeId, path), ribSup, tx);
+        writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK), effAttrib, createValue(routeId, path), ribSup, tx);
     }
 
-    private void removePathFromDataStore(final BestPath path, final PathArgument routeIdPathArgument, final YangInstanceIdentifier writePath,
+    private void removePathFromDataStore(final BestPath path, final PathArgument routeIdPA, final YangInstanceIdentifier locRibTarget,
         final ExportPolicyPeerTracker peerPT, final TablesKey localTK, final RIBSupport ribSup, final CacheDisconnectedPeers discPeers,
         final DOMDataWriteTransaction tx) {
         LOG.trace("Best Path removed {}", path);
-        fillLocRib(writePath, tx, null);
-        fillAdjRibsOut(tx, null, null, routeIdPathArgument, path.getPeerId(), peerPT, localTK, ribSup, discPeers);
+        final PathArgument routeIdAddPath = ribSup.getRouteIdAddPath(path.getPathId(), routeIdPA);
+        final YangInstanceIdentifier pathTarget = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdPA);
+        YangInstanceIdentifier pathAddPathTarget = null;
+        if (routeIdAddPath != null) {
+            pathAddPathTarget = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdAddPath);
+        }
+        fillLocRib(pathAddPathTarget == null ? pathTarget : pathAddPathTarget, null, tx);
+        fillAdjRibsOut(null, null, routeIdPA, path.getPeerId(), peerPT, localTK, ribSup, discPeers, tx);
     }
 
-    private void addPathToDataStore(final BestPath path, final PathArgument routeIdPathArgument, final YangInstanceIdentifier writePath,
+    private void addPathToDataStore(final BestPath path, final PathArgument routeIdPA, final YangInstanceIdentifier locRibTarget,
         final RIBSupport ribSup, final ExportPolicyPeerTracker peerPT, final TablesKey localTK, final CacheDisconnectedPeers discPeers,
         final DOMDataWriteTransaction tx) {
-        final NormalizedNode<?, ?> value = createValue(routeIdPathArgument, path);
-        LOG.trace("Selected best value {}", value);
-        fillLocRib(writePath, tx, value);
-        fillAdjRibsOut(tx, path.getAttributes(), value, routeIdPathArgument, path.getPeerId(), peerPT, localTK, ribSup, discPeers);
+        final PathArgument routeIdAddPath = ribSup.getRouteIdAddPath(path.getPathId(), routeIdPA);
+        final YangInstanceIdentifier pathTarget = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdPA);
+        final NormalizedNode<?, ?> value = createValue(routeIdPA, path);
+        NormalizedNode<?, ?> addPathValue = null;
+        YangInstanceIdentifier pathAddPathTarget = null;
+        if (routeIdAddPath == null) {
+            LOG.trace("Selected best value {}", value);
+        } else {
+            pathAddPathTarget = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdAddPath);
+            addPathValue = createValue(routeIdAddPath, path);
+            LOG.trace("Selected best value {}", addPathValue);
+        }
+        fillLocRib(pathAddPathTarget == null ? pathTarget : pathAddPathTarget, addPathValue == null ? value : addPathValue, tx);
+        fillAdjRibsOut(path.getAttributes(), value, routeIdPA, path.getPeerId(), peerPT, localTK, ribSup, discPeers, tx);
     }
 
     private boolean writeRoute(final PeerId destPeer, final YangInstanceIdentifier routeTarget, final ContainerNode effAttrib,
@@ -162,20 +176,20 @@ abstract class BaseAbstractRouteEntry implements RouteEntry {
         return this.offsets;
     }
 
-    private void fillLocRib(final YangInstanceIdentifier writePath, final DOMDataWriteTransaction tx, final NormalizedNode<?, ?> value) {
+    private void fillLocRib(final YangInstanceIdentifier routesYangId, final NormalizedNode<?, ?> value, final DOMDataWriteTransaction tx) {
         if (value != null) {
             LOG.debug("Write route to LocRib {}", value);
-            tx.put(LogicalDatastoreType.OPERATIONAL, writePath, value);
+            tx.put(LogicalDatastoreType.OPERATIONAL, routesYangId, value);
         } else {
-            LOG.debug("Delete route from LocRib {}", writePath);
-            tx.delete(LogicalDatastoreType.OPERATIONAL, writePath);
+            LOG.debug("Delete route from LocRib {}", routesYangId);
+            tx.delete(LogicalDatastoreType.OPERATIONAL, routesYangId);
         }
     }
 
     @VisibleForTesting
-    private void fillAdjRibsOut(final DOMDataWriteTransaction tx, final ContainerNode attributes, final NormalizedNode<?, ?> value,
+    private void fillAdjRibsOut(final ContainerNode attributes, final NormalizedNode<?, ?> value,
         final PathArgument routeId, final PeerId routePeerId, final ExportPolicyPeerTracker peerPT, final TablesKey localTK, final RIBSupport ribSup,
-        final CacheDisconnectedPeers discPeers) {
+        final CacheDisconnectedPeers discPeers, final DOMDataWriteTransaction tx) {
         /*
          * We need to keep track of routers and populate adj-ribs-out, too. If we do not, we need to
          * expose from which client a particular route was learned from in the local RIB, and have
@@ -193,17 +207,16 @@ abstract class BaseAbstractRouteEntry implements RouteEntry {
             if (peerGroup != null) {
                 final ContainerNode effAttrib = peerGroup.effectiveAttributes(routePeerId, attributes);
                 peerGroup.getPeers().stream()
-                    .filter(pid -> filterRoutes(pid, routePeerId, peerPT, localTK, discPeers))
-                    .forEach(pid -> update(pid.getKey(), getRouteTarget(ribSup, pid.getValue(), routeId, localTK), effAttrib, value,
+                    .filter(pid -> filterRoutes(routePeerId, pid.getKey(), peerPT, localTK, discPeers))
+                    .forEach(pid -> update(pid.getKey(), getAdjRibOutYII(ribSup, pid.getValue(), routeId, localTK), effAttrib, value,
                         ribSup, tx));
             }
         }
     }
 
-    private boolean filterRoutes(final Map.Entry<PeerId, YangInstanceIdentifier> pid, final PeerId destPeer, final ExportPolicyPeerTracker peerPT,
+    private boolean filterRoutes(final PeerId rootPeer, final PeerId destPeer, final ExportPolicyPeerTracker peerPT,
         final TablesKey localTK, final CacheDisconnectedPeers discPeers) {
-        final PeerId peerId = pid.getKey();
-        return !peerId.equals(destPeer) && isTableSupported(destPeer, peerPT, localTK) && !discPeers.isPeerDisconnected(destPeer);
+        return !rootPeer.equals(destPeer) && isTableSupported(destPeer, peerPT, localTK) && !discPeers.isPeerDisconnected(destPeer);
     }
 
     private void update(final PeerId destPeer, final YangInstanceIdentifier routeTarget, final ContainerNode effAttrib,
@@ -223,9 +236,9 @@ abstract class BaseAbstractRouteEntry implements RouteEntry {
     }
 
 
-    private YangInstanceIdentifier getRouteTarget(final RIBSupport ribSup, final YangInstanceIdentifier rootPath, final PathArgument routeId,
+    private YangInstanceIdentifier getAdjRibOutYII(final RIBSupport ribSup, final YangInstanceIdentifier rootPath, final PathArgument routeId,
         final TablesKey localTK) {
-        return ribSup.routePath(rootPath.node(AdjRibOut.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(localTK)).
-            node(ROUTES_IDENTIFIER), routeId);
+        return ribSup.routePath(rootPath.node(AdjRibOut.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(localTK))
+            .node(ROUTES_IDENTIFIER), routeId);
     }
 }
\ No newline at end of file

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseBestPath.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseBestPath.java
index f34cdec7443d95dbbae2199c38e835bda603ef49..26bd65796cbf805bf4ad3f014d91792d66047994 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseBestPath.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseBestPath.java
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.protocol.bgp.mode.impl.base;
 
-import com.google.common.base.MoreObjects;
 import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.base.Preconditions;
 import com.google.common.primitives.UnsignedInteger;
@@ -18,7 +17,9 @@ import org.opendaylight.protocol.bgp.rib.spi.RouterIds;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
 
 final class BaseBestPath extends AbstractBestPath {
+    private static final long PATH_ID = 0;
     private final UnsignedInteger routerId;
+
     BaseBestPath(@Nonnull final UnsignedInteger routerId, @Nonnull final BestPathState state) {
         super(state);
         this.routerId = Preconditions.checkNotNull(routerId);
@@ -34,15 +35,16 @@ final class BaseBestPath extends AbstractBestPath {
         return RouterIds.createPeerId(this.routerId);
     }
 
-    private ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
-        toStringHelper.add("routerId", this.routerId);
-        toStringHelper.add("state", this.state);
-        return toStringHelper;
+    @Override
+    public long getPathId() {
+        return PATH_ID;
     }
 
     @Override
-    public String toString() {
-        return addToStringAttributes(MoreObjects.toStringHelper(this)).toString();
+    protected ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
+        toStringHelper.add("routerId", this.routerId);
+        toStringHelper.add("state", this.state);
+        return toStringHelper;
     }
 
     @Override

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPath.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPath.java
index 1bab3427f9b17bf6ffcad7e832b6ecf749914f27..caace246f7616dbbdb0c4e5af15b6d1b618da8eb 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPath.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPath.java
@@ -9,6 +9,7 @@
 package org.opendaylight.protocol.bgp.mode.spi;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import org.opendaylight.protocol.bgp.mode.api.BestPath;
 import org.opendaylight.protocol.bgp.mode.api.BestPathState;
@@ -21,6 +22,8 @@ public abstract class AbstractBestPath implements BestPath {
         this.state = Preconditions.checkNotNull(state);
     }
 
+    protected abstract MoreObjects.ToStringHelper addToStringAttributes(final MoreObjects.ToStringHelper toStringHelper);
+
     @VisibleForTesting
     public final BestPathState getState() {
         return this.state;
@@ -30,4 +33,9 @@ public abstract class AbstractBestPath implements BestPath {
     public final ContainerNode getAttributes() {
         return this.state.getAttributes();
     }
+
+    @Override
+    public final String toString() {
+        return addToStringAttributes(MoreObjects.toStringHelper(this)).toString();
+    }
 }
\ No newline at end of file

diff --git a/bgp/topology-provider/src/test/java/org/opendaylight/bgpcep/bgp/topology/provider/Ipv4ReachabilityTopologyBuilderTest.java b/bgp/topology-provider/src/test/java/org/opendaylight/bgpcep/bgp/topology/provider/Ipv4ReachabilityTopologyBuilderTest.java
index c3c2d41a1c034376e85ef44243587152d548ea2b..08acbe61b08910fbfac49b4cec52a1ee5cbcec8e 100644 (file)
--- a/bgp/topology-provider/src/test/java/org/opendaylight/bgpcep/bgp/topology/provider/Ipv4ReachabilityTopologyBuilderTest.java
+++ b/bgp/topology-provider/src/test/java/org/opendaylight/bgpcep/bgp/topology/provider/Ipv4ReachabilityTopologyBuilderTest.java
@@ -24,6 +24,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.inet
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.inet.rev150305.ipv4.routes.ipv4.routes.Ipv4Route;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.inet.rev150305.ipv4.routes.ipv4.routes.Ipv4RouteBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.inet.rev150305.ipv4.routes.ipv4.routes.Ipv4RouteKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.PathId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.path.attributes.Attributes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.path.attributes.AttributesBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.Tables;
@@ -39,6 +40,7 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 public class Ipv4ReachabilityTopologyBuilderTest extends AbstractTopologyBuilderTest {
 
     private static final String ROUTE_IP4PREFIX = "127.1.0.0/32";
+    private static final long ROUTE_IP4PATH_ID = 1;
     private static final String NEXT_HOP = "127.1.0.1";
     private static final String NEW_NEXT_HOP = "127.1.0.2";
 
@@ -51,7 +53,8 @@ public class Ipv4ReachabilityTopologyBuilderTest extends AbstractTopologyBuilder
         this.ipv4TopoBuilder = new Ipv4ReachabilityTopologyBuilder(dataBroker, LOC_RIB_REF, TEST_TOPOLOGY_ID);
         this.ipv4TopoBuilder.start(dataBroker, Ipv4AddressFamily.class, UnicastSubsequentAddressFamily.class);
         final InstanceIdentifier<Tables> path = this.ipv4TopoBuilder.tableInstanceIdentifier(Ipv4AddressFamily.class, UnicastSubsequentAddressFamily.class);
-        this.ipv4RouteIID = path.builder().child((Class)Ipv4Routes.class).child(Ipv4Route.class, new Ipv4RouteKey(new Ipv4Prefix(ROUTE_IP4PREFIX))).build();
+        this.ipv4RouteIID = path.builder().child((Class)Ipv4Routes.class).child(Ipv4Route.class, new Ipv4RouteKey(new PathId(ROUTE_IP4PATH_ID),
+            new Ipv4Prefix(ROUTE_IP4PREFIX))).build();
     }
 
     @Test
@@ -95,7 +98,8 @@ public class Ipv4ReachabilityTopologyBuilderTest extends AbstractTopologyBuilder
         final Attributes attribute = new AttributesBuilder()
             .setCNextHop(new Ipv4NextHopCaseBuilder().setIpv4NextHop(new Ipv4NextHopBuilder().setGlobal(new Ipv4Address(nextHop)).build()).build())
             .build();
-        return new Ipv4RouteBuilder().setKey(new Ipv4RouteKey(new Ipv4Prefix(ROUTE_IP4PREFIX))).setPrefix(new Ipv4Prefix(ROUTE_IP4PREFIX)).setAttributes(attribute).build();
+        return new Ipv4RouteBuilder().setKey(new Ipv4RouteKey(new PathId(ROUTE_IP4PATH_ID), new Ipv4Prefix(ROUTE_IP4PREFIX)))
+            .setPrefix(new Ipv4Prefix(ROUTE_IP4PREFIX)).setAttributes(attribute).build();
     }
 
 }