From: Claudio D. Gasparini Date: Fri, 11 Mar 2016 10:28:36 +0000 (+0100) Subject: BUG-4827: Modify ipv4-routes list model, to include path-id as part of the key X-Git-Tag: release/boron~250 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=02559d3d5f4be9e716a30580186740f4fded6c8e;p=bgpcep.git 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 --- diff --git a/bgp/inet/src/main/yang/bgp-inet.yang b/bgp/inet/src/main/yang/bgp-inet.yang index 55a5f5cc4e..cdcc58489e 100644 --- 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 0f9bb058d5..eeeedc47fc 100644 --- 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 4e7b2a5bce..b1ee6a1d1a 100644 --- 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 7ba5acc7b1..8845a04dd2 100644 --- 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 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 f34cdec744..26bd65796c 100644 --- 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 1bab3427f9..caace246f7 100644 --- 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 c3c2d41a1c..08acbe61b0 100644 --- 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 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(); } }