From: Claudio D. Gasparini Date: Wed, 10 Aug 2016 08:53:51 +0000 (+0200) Subject: BUG-6378: Fix PeerExportGroup X-Git-Tag: release/carbon~210 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=cfd681578182da51e732a2c0bf90ffd09507e687;p=bgpcep.git BUG-6378: Fix PeerExportGroup PeerExportGroup contains all peers. Fix By contain only peers correspending to same PeerExportGroup Role Change-Id: If3f795add270d598b74a62ee8e9597441bcea858 Signed-off-by: Claudio D. Gasparini --- diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java index 0514ae23e6..912349838b 100644 --- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java +++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java @@ -110,14 +110,15 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry { if(this.bestPath != null) { this.bestPath.stream().filter(path -> filterRoutes(path.getPeerId(), destPeer, peerPT, localTK, discPeers) && peersSupportsAddPathOrIsFirstBestPath(destPeerSupAddPath, isFirstBestPath(this.bestPath.indexOf(path)))) - .forEach(path -> writeRoutePath(destPeer, routeId, peerGroup, destPeerSupAddPath, path, rootPath, localTK, ribSup, tx)); + .forEach(path -> writeRoutePath(destPeer, routeId, peerPT, peerGroup, destPeerSupAddPath, path, rootPath, localTK, ribSup, tx)); } } - private void writeRoutePath(final PeerId destPeer, final PathArgument routeId, final PeerExportGroup peerGroup, final boolean destPeerSupAddPath, + private void writeRoutePath(final PeerId destPeer, final PathArgument routeId, final ExportPolicyPeerTracker peerPT, + final PeerExportGroup peerGroup, final boolean destPeerSupAddPath, final BestPath path, final YangInstanceIdentifier rootPath, final TablesKey localTK, final RIBSupport ribSup, final DOMDataWriteTransaction tx) { final PathArgument routeIdAddPath = ribSup.getRouteIdAddPath(path.getPathId(), routeId); - final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(path.getPeerId(), path.getAttributes()); + final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT,path.getPeerId()), path.getAttributes()); if (destPeerSupAddPath) { writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPath, localTK), effectiveAttributes, createValue(routeIdAddPath, path), ribSup, tx); } else { @@ -164,7 +165,7 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry { for (final PeerRole role : PeerRole.values()) { final PeerExportGroup peerGroup = peerPT.getPeerGroup(role); if (peerGroup != null) { - final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(routePeerId, attributes); + final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT, routePeerId), attributes); for (final Map.Entry pid : peerGroup.getPeers()) { final PeerId destPeer = pid.getKey(); final boolean destPeerSupAddPath = peerPT.isAddPathSupportedByPeer(destPeer); 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 f8fc1cc867..9d984ee7e0 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 @@ -120,7 +120,7 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry { if (this.bestPath != null) { final BaseBestPath path = this.bestPath; if (filterRoutes(path.getPeerId(), destPeer, peerPT, localTK, discPeers)) { - final ContainerNode effAttrib = peerGroup.effectiveAttributes(path.getPeerId(), path.getAttributes()); + final ContainerNode effAttrib = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT,path.getPeerId()), path.getAttributes()); writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK), effAttrib, createValue(routeId, path), ribSup, tx); } } @@ -179,7 +179,7 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry { for (final PeerRole role : PeerRole.values()) { final PeerExportGroup peerGroup = peerPT.getPeerGroup(role); if (peerGroup != null) { - final ContainerNode effAttrib = peerGroup.effectiveAttributes(routePeerId, attributes); + final ContainerNode effAttrib = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT,routePeerId), attributes); peerGroup.getPeers().stream() .filter(pid -> filterRoutes(routePeerId, pid.getKey(), peerPT, localTK, discPeers)) .forEach(pid -> update(pid.getKey(), getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeId, localTK), effAttrib, value, ribSup, tx)); diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java index 93328d24db..1bc617d110 100644 --- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java +++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java @@ -13,9 +13,11 @@ import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction; import org.opendaylight.protocol.bgp.mode.api.RouteEntry; import org.opendaylight.protocol.bgp.rib.spi.CacheDisconnectedPeers; import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker; +import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup; import org.opendaylight.protocol.bgp.rib.spi.RIBSupport; import org.opendaylight.protocol.bgp.rib.spi.RibSupportUtils; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.bgp.rib.rib.peer.AdjRibOut; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.Tables; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.TablesKey; @@ -71,4 +73,14 @@ public abstract class AbstractRouteEntry implements RouteEntry { return ribSup.routePath(rootPath.node(AdjRibOut.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(localTK)) .node(ROUTES_IDENTIFIER), routeId); } + + protected PeerRole getRoutePeerIdRole(final ExportPolicyPeerTracker peerPT, final PeerId routePeerId) { + for (final PeerRole role : PeerRole.values()) { + final PeerExportGroup peerGroup = peerPT.getPeerGroup(role); + if(peerGroup != null && peerGroup.containsPeer(routePeerId)) { + return role; + } + } + return null; + } } diff --git a/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java b/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java index bad58b6439..85fe15e6d4 100644 --- a/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java +++ b/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java @@ -170,11 +170,12 @@ public class AbstractRouteEntryTest { } private void mockExportGroup() { - Mockito.doReturn(this.attributes).when(this.peg).effectiveAttributes(Mockito.any(PeerId.class), Mockito.any(ContainerNode.class)); - Mockito.doReturn(null).when(this.pegNot).effectiveAttributes(Mockito.any(PeerId.class), Mockito.any(ContainerNode.class)); + Mockito.doReturn(this.attributes).when(this.peg).effectiveAttributes(Mockito.any(PeerRole.class), Mockito.any(ContainerNode.class)); + Mockito.doReturn(null).when(this.pegNot).effectiveAttributes(Mockito.any(PeerRole.class), Mockito.any(ContainerNode.class)); Map peers = new HashMap<>(); Mockito.doReturn(ImmutableList.copyOf(peers.entrySet())).when(this.pegNot).getPeers(); + Mockito.doReturn(true).when(this.pegNot).containsPeer(Mockito.any(PeerId.class)); peers.put(PEER_ID, new PeerExportGroup.PeerExporTuple(PEER_YII, PeerRole.Ibgp)); peers.put(PEER_ID2, new PeerExportGroup.PeerExporTuple(PEER_YII2, PeerRole.Ibgp)); diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java index 197c9c793f..f7eec81f32 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java @@ -64,12 +64,13 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker { return Collections.emptyMap(); } - final Map immutablePeers = ImmutableMap.copyOf(peerPathRoles.entrySet().stream() - .collect(toMap(peer -> IdentifierUtils.peerKeyToPeerId(peer.getKey()), peer -> new PeerExporTuple(peer.getKey(), peer.getValue())))); + final Map> immutablePeers = peerPathRoles.entrySet().stream() + .collect(Collectors.groupingBy(Map.Entry::getValue, toMap(peer -> IdentifierUtils.peerKeyToPeerId(peer + .getKey()), peer -> new PeerExporTuple(peer.getKey(), peer.getValue())))); final Map ret = peerPathRoles.values().stream().collect(Collectors.toSet()).stream().filter(role -> role != PeerRole.Internal) - .collect(toMap(identity(), role -> new PeerExportGroupImpl(immutablePeers, this.policyDatabase.exportPolicyForRole(role)), - (oldKey, newKey) -> oldKey, () -> new EnumMap<>(PeerRole.class))); + .collect(toMap(identity(), role -> new PeerExportGroupImpl(ImmutableMap.copyOf(immutablePeers.get(role)), + this.policyDatabase.exportPolicyForRole(role)), (oldKey, newKey) -> oldKey, () -> new EnumMap<>(PeerRole.class))); return ret; } diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java index 53bdbdc322..69ba399ce1 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java @@ -12,6 +12,7 @@ import java.util.Collection; import java.util.Map; import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; final class PeerExportGroupImpl implements PeerExportGroup { @@ -24,13 +25,17 @@ final class PeerExportGroupImpl implements PeerExportGroup { } @Override - public ContainerNode effectiveAttributes(final PeerId sourcePeerId, final ContainerNode attributes) { - final PeerExporTuple peer = this.peers.get(sourcePeerId); - return attributes == null || peer == null ? null : policy.effectiveAttributes(peer.getRole(), attributes); + public ContainerNode effectiveAttributes(final PeerRole role, final ContainerNode attributes) { + return attributes == null || role == null ? null : policy.effectiveAttributes(role, attributes); } @Override public Collection> getPeers() { return peers.entrySet(); } + + @Override + public boolean containsPeer(final PeerId routePeerId) { + return this.peers.containsKey(routePeerId); + } } \ No newline at end of file diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java index b9c8287898..d8c538e0e7 100644 --- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java +++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java @@ -40,14 +40,16 @@ public interface PeerExportGroup { /** * Transform outgoing attributes according to policy per Peer * - * @param sourcePeerId root Peer + * @param role root Peer role * @param attributes attributes container * @return return attributes container after apply policy */ - ContainerNode effectiveAttributes(PeerId sourcePeerId, ContainerNode attributes); + ContainerNode effectiveAttributes(PeerRole role, ContainerNode attributes); /** * @return map of peer */ Collection> getPeers(); + + boolean containsPeer(PeerId routePeerId); }