From 1e29a1046ff3d16660c3074df0a6a4b279cd951e Mon Sep 17 00:00:00 2001 From: "Claudio D. Gasparini" Date: Fri, 24 Mar 2017 20:26:08 +0100 Subject: [PATCH] BUG-7976: Race between peer removal and routes update There is a race condition when we are advertizing routes, and a session peer goes down which will end in removing the peer from DS. Since advertizement and removal are done by 2 different tx, we need to do it blocking mode, and not route should be advertized until peer is removed and vice versa, peer should not be removed until routes are advertized. Otherwise toure update will try to update a peer/path which is not longer present. Fix by rework PeerExportGroup & ExportPolicyPeerTracker, now instead of generate PeerExportGroup each time a peer is reg/unreg, we make PeerExportGroup work as a blocking registry to solve previous issue. Change-Id: I46a27871bfa2aa2a632e3bfb76061105c62bb6d2 Signed-off-by: Claudio D. Gasparini --- .../impl/add/AddPathAbstractRouteEntry.java | 9 +- .../impl/base/BaseAbstractRouteEntry.java | 16 ++- .../bgp/mode/impl/AbstractRouteEntryTest.java | 121 ++++++++++-------- .../rib/impl/ExportPolicyPeerTrackerImpl.java | 45 +++---- .../bgp/rib/impl/PeerExportGroupImpl.java | 55 +++++++- .../rib/impl/spi/PeerExportGroupRegistry.java | 34 +++++ .../protocol/bgp/rib/spi/PeerExportGroup.java | 15 +++ 7 files changed, 202 insertions(+), 93 deletions(-) create mode 100644 bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/spi/PeerExportGroupRegistry.java 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 012d196d40..6fad073559 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 @@ -195,19 +195,18 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry { final PeerExportGroup peerGroup = peerPT.getPeerGroup(role); if (peerGroup != null) { final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT, routePeerId), attributes); - for (final Map.Entry pid : peerGroup.getPeers()) { - final PeerId destPeer = pid.getKey(); + peerGroup.forEach((destPeer, rootPath) -> { final boolean destPeerSupAddPath = peerPT.isAddPathSupportedByPeer(destPeer); if (filterRoutes(routePeerId, destPeer, peerPT, localTK, role) && peersSupportsAddPathOrIsFirstBestPath(destPeerSupAddPath, isFirstBestPath)) { if (destPeerSupAddPath) { - update(destPeer, getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeIdAddPath, localTK), effectiveAttributes, + update(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPath, localTK), effectiveAttributes, addPathValue, ribSup, tx); } else if(!this.oldNonAddPathBestPathTheSame){ - update(destPeer, getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeId, localTK), effectiveAttributes, value, ribSup, tx); + update(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK), effectiveAttributes, value, ribSup, tx); } } - } + }); } } } 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 395e51cb48..3f87a4d6af 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 @@ -163,8 +163,8 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry { @VisibleForTesting 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 DOMDataWriteTransaction tx) { + final PathArgument routeId, final PeerId routePeerId, final ExportPolicyPeerTracker peerPT, + final TablesKey localTK, final RIBSupport ribSup, 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 @@ -177,10 +177,14 @@ 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(getRoutePeerIdRole(peerPT, routePeerId), attributes); - peerGroup.getPeers().stream() - .filter(pid -> filterRoutes(routePeerId, pid.getKey(), peerPT, localTK, getRoutePeerIdRole(peerPT, pid.getKey()))) - .forEach(pid -> update(pid.getKey(), getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeId, localTK), effAttrib, value, ribSup, tx)); + final ContainerNode effAttrib = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT, routePeerId), + attributes); + peerGroup.forEach((destPeer, rootPath) -> { + if (!filterRoutes(routePeerId, destPeer, peerPT, localTK, getRoutePeerIdRole(peerPT, destPeer))) { + return; + } + update(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK), effAttrib, value, ribSup, tx); + }); } } } 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 ae78af6755..a6f5f91155 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 @@ -8,23 +8,28 @@ package org.opendaylight.protocol.bgp.mode.impl; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import static org.opendaylight.protocol.bgp.mode.impl.base.BasePathSelectorTest.ATTRS_EXTENSION_Q; import static org.opendaylight.protocol.bgp.mode.impl.base.BasePathSelectorTest.SEGMENTS_NID; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.UnsignedInteger; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction; import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker; import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup; +import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup.PeerExporTuple; 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.inet.rev150305.ipv4.routes.Ipv4Routes; @@ -54,7 +59,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.schema.Builders; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; -public class AbstractRouteEntryTest { +public abstract class AbstractRouteEntryTest { protected static final long REMOTE_PATH_ID = 1; protected static final PeerId PEER_ID = new PeerId("bgp://42.42.42.42"); protected static final YangInstanceIdentifier PEER_YII2 = YangInstanceIdentifier.of(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet:test", "2015-03-05", "peer2")); @@ -65,7 +70,6 @@ public class AbstractRouteEntryTest { .node(LocRib.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(TABLES_KEY)).getPathArguments()); private static final long PATH_ID = 1; private static final PeerId PEER_ID2 = new PeerId("bgp://43.43.43.43"); - private static final PeerId PEER_DISCONNECTED = new PeerId("bgp://44.44.44.44"); private static final String PREFIX = "1.2.3.4/32"; private static final String PREFIX2 = "2.2.2.2/32"; private static final YangInstanceIdentifier PEER_YII = YangInstanceIdentifier.of(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet:test", "2015-03-05", "peer1")); @@ -108,7 +112,7 @@ public class AbstractRouteEntryTest { protected void setUp() { MockitoAnnotations.initMocks(this); this.yIIChanges = new ArrayList<>(); - this.peerPT = Mockito.mock(ExportPolicyPeerTracker.class); + this.peerPT = mock(ExportPolicyPeerTracker.class); this.attributes = createAttr(); this.locRibTargetYii = LOC_RIB_TARGET.node(ROUTES_IDENTIFIER); this.locRibOutTargetYii = PEER_YII.node(AdjRibOut.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(TABLES_KEY)).node(ROUTES_IDENTIFIER); @@ -129,89 +133,100 @@ public class AbstractRouteEntryTest { } private void mockTransactionChain() { - Mockito.doAnswer(invocation -> { + doAnswer(invocation -> { final Object[] args = invocation.getArguments(); - yIIChanges.add((YangInstanceIdentifier) args[1]); + this.yIIChanges.add((YangInstanceIdentifier) args[1]); return args[1]; - }).when(this.tx).put(Mockito.any(LogicalDatastoreType.class), Mockito.any(YangInstanceIdentifier.class), Mockito.any(NormalizedNode.class)); + }).when(this.tx).put(any(LogicalDatastoreType.class), any(YangInstanceIdentifier.class), any(NormalizedNode.class)); - Mockito.doAnswer(invocation -> { + doAnswer(invocation -> { final Object[] args = invocation.getArguments(); - if (routePaYii.equals(args[1])) { - yIIChanges.remove(routePaYii); - } else if (routePaAddPathYii.equals(args[1])) { - yIIChanges.remove(routePaAddPathYii); - } else if (routeRiboutYii.equals(args[1])) { - yIIChanges.remove(routeRiboutYii); - yIIChanges.remove(routeAddRiboutAttYii); - } else if (routeAddRiboutYii.equals(args[1])) { - yIIChanges.remove(routeAddRiboutYii); - yIIChanges.remove(routeAddRiboutAttYii); + if (this.routePaYii.equals(args[1])) { + this.yIIChanges.remove(this.routePaYii); + } else if (this.routePaAddPathYii.equals(args[1])) { + this.yIIChanges.remove(this.routePaAddPathYii); + } else if (this.routeRiboutYii.equals(args[1])) { + this.yIIChanges.remove(this.routeRiboutYii); + this.yIIChanges.remove(this.routeAddRiboutAttYii); + } else if (this.routeAddRiboutYii.equals(args[1])) { + this.yIIChanges.remove(this.routeAddRiboutYii); + this.yIIChanges.remove(this.routeAddRiboutAttYii); } return args[1]; - }).when(this.tx).delete(Mockito.any(LogicalDatastoreType.class), Mockito.any(YangInstanceIdentifier.class)); + }).when(this.tx).delete(any(LogicalDatastoreType.class), any(YangInstanceIdentifier.class)); } private void mockExportGroup() { - 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)); + doReturn(this.attributes).when(this.peg).effectiveAttributes(any(PeerRole.class), any(ContainerNode.class)); + doReturn(null).when(this.pegNot).effectiveAttributes(any(PeerRole.class), any(ContainerNode.class)); + + final Map peers = new HashMap<>(); + doAnswer(invocation -> { + final BiConsumer action = (BiConsumer) invocation.getArguments()[0]; + for (final Map.Entry pid : peers.entrySet()) { + action.accept(pid.getKey(), pid.getValue().getYii()); + } + return null; + }).when(this.pegNot).forEach(any()); + doReturn(true).when(this.pegNot).containsPeer(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)); - - Mockito.doReturn(ImmutableList.copyOf(peers.entrySet())).when(this.peg).getPeers(); + doAnswer(invocation -> { + final BiConsumer action = (BiConsumer) invocation.getArguments()[0]; + for (final Map.Entry pid : peers.entrySet()) { + action.accept(pid.getKey(), pid.getValue().getYii()); + } + return null; + }).when(this.peg).forEach(any()); } private void mockExportPolicies() { - Mockito.doReturn(true).when(this.peerPT).isTableSupported(PEER_ID); - Mockito.doReturn(false).when(this.peerPT).isTableSupported(PEER_ID2); - Mockito.doAnswer(invocation -> { + doReturn(true).when(this.peerPT).isTableSupported(PEER_ID); + doReturn(false).when(this.peerPT).isTableSupported(PEER_ID2); + doAnswer(invocation -> { final Object[] args = invocation.getArguments(); if (PeerRole.Ibgp.equals(args[0])) { - return peg; + return this.peg; } else if (PeerRole.Ebgp.equals(args[0])) { - return pegNot; + return this.pegNot; } else { return null; } - }).when(this.peerPT).getPeerGroup(Mockito.any(PeerRole.class)); + }).when(this.peerPT).getPeerGroup(any(PeerRole.class)); - Mockito.doReturn(true).when(this.peerPT).isAddPathSupportedByPeer(PEER_ID); - Mockito.doReturn(false).when(this.peerPT).isAddPathSupportedByPeer(PEER_ID2); + doReturn(true).when(this.peerPT).isAddPathSupportedByPeer(PEER_ID); + doReturn(false).when(this.peerPT).isAddPathSupportedByPeer(PEER_ID2); } private void mockRibSupport() { - Mockito.doReturn(ROUTE_ATTRIBUTES_IDENTIFIER).when(this.ribSupport).routeAttributesIdentifier(); - Mockito.doReturn(ROUTE_ID_PA_ADD_PATH).when(this.ribSupport).getRouteIdAddPath(Mockito.any(Long.class), Mockito.eq(ROUTE_ID_PA_ADD_PATH)); - Mockito.doReturn(null).when(this.ribSupport).getRouteIdAddPath(Mockito.any(Long.class), Mockito.eq(ROUTE_ID_PA)); - Mockito.doAnswer(invocation -> { + doReturn(ROUTE_ATTRIBUTES_IDENTIFIER).when(this.ribSupport).routeAttributesIdentifier(); + doReturn(ROUTE_ID_PA_ADD_PATH).when(this.ribSupport).getRouteIdAddPath(any(Long.class), eq(ROUTE_ID_PA_ADD_PATH)); + doReturn(null).when(this.ribSupport).getRouteIdAddPath(any(Long.class), eq(ROUTE_ID_PA)); + doAnswer(invocation -> { final Object[] args = invocation.getArguments(); final YangInstanceIdentifier yii = (YangInstanceIdentifier) args[0]; final PathArgument paa = (PathArgument) args[1]; if (ROUTE_ID_PA.equals(paa)) { - if (yii.equals(locRibTargetYii)) { - return routePaYii; - } else if (yii.equals(locRibOutTargetYii)) { - return routeRiboutYii; - } else if (yii.equals(locRibOutTargetYiiPeer2)) { - return routeRiboutYiiPeer2; + if (yii.equals(this.locRibTargetYii)) { + return this.routePaYii; + } else if (yii.equals(this.locRibOutTargetYii)) { + return this.routeRiboutYii; + } else if (yii.equals(this.locRibOutTargetYiiPeer2)) { + return this.routeRiboutYiiPeer2; } } else if (ROUTE_ID_PA_ADD_PATH.equals(paa)) { - if (yii.equals(locRibTargetYii)) { - return routePaAddPathYii; - } else if (yii.equals(locRibOutTargetYii)) { - return routeAddRiboutYii; - } else if (yii.equals(locRibOutTargetYiiPeer2)) { - return routeAddRiboutYiiPeer2; + if (yii.equals(this.locRibTargetYii)) { + return this.routePaAddPathYii; + } else if (yii.equals(this.locRibOutTargetYii)) { + return this.routeAddRiboutYii; + } else if (yii.equals(this.locRibOutTargetYiiPeer2)) { + return this.routeAddRiboutYiiPeer2; } } return null; - }).when(this.ribSupport).routePath(Mockito.any(YangInstanceIdentifier.class), Mockito.any(PathArgument.class)); + }).when(this.ribSupport).routePath(any(YangInstanceIdentifier.class), any(PathArgument.class)); } private NormalizedNode createAttr() { 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 1807dfe11e..be0f6a4c32 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 @@ -7,23 +7,17 @@ */ package org.opendaylight.protocol.bgp.rib.impl; -import static java.util.function.Function.identity; -import static java.util.stream.Collectors.toMap; - import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; -import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; +import org.opendaylight.protocol.bgp.rib.impl.spi.PeerExportGroupRegistry; import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker; -import org.opendaylight.protocol.bgp.rib.spi.IdentifierUtils; import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup; import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup.PeerExporTuple; import org.opendaylight.protocol.concepts.AbstractRegistration; @@ -47,28 +41,36 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker { private final Set peerTables = new HashSet<>(); private final PolicyDatabase policyDatabase; private final TablesKey localTableKey; - private volatile Map groups = Collections.emptyMap(); + @GuardedBy("this") + private final Map groups = new EnumMap<>(PeerRole.class); ExportPolicyPeerTrackerImpl(final PolicyDatabase policyDatabase, final TablesKey localTablesKey) { this.policyDatabase = Preconditions.checkNotNull(policyDatabase); this.localTableKey = localTablesKey; } - private synchronized void createGroups(final Map peerPathRoles) { - if (!peerPathRoles.isEmpty()) { - 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())))); + private synchronized AbstractRegistration addToExportGroups(final PeerId peerId, + final YangInstanceIdentifier peerPath, final PeerRole peerRole) { + final PeerExportGroupRegistry peerExp = this.groups.computeIfAbsent(peerRole, + k -> new PeerExportGroupImpl(this.policyDatabase.exportPolicyForRole(peerRole))); - this.groups = peerPathRoles.values().stream().collect(Collectors.toSet()).stream() - .collect(toMap(identity(), role -> new PeerExportGroupImpl(ImmutableMap.copyOf(immutablePeers.get(role)), - this.policyDatabase.exportPolicyForRole(role)), (oldKey, newKey) -> oldKey, () -> new EnumMap<>(PeerRole.class))); - } + final AbstractRegistration registration = peerExp.registerPeer(peerId, new PeerExporTuple(peerPath, peerRole)); + + return new AbstractRegistration() { + @Override + protected void removeRegistration() { + registration.close(); + if(ExportPolicyPeerTrackerImpl.this.groups.get(peerRole).isEmpty()) { + ExportPolicyPeerTrackerImpl.this.groups.remove(peerRole); + } + } + }; } @Override - public synchronized AbstractRegistration registerPeer(final PeerId peerId, final SendReceive sendReceive, final YangInstanceIdentifier peerPath, - final PeerRole peerRole, final Optional optSimpleRoutingPolicy) { + public synchronized AbstractRegistration registerPeer(final PeerId peerId, final SendReceive sendReceive, + final YangInstanceIdentifier peerPath, final PeerRole peerRole, + final Optional optSimpleRoutingPolicy) { if (sendReceive != null) { this.peerAddPathTables.put(peerId, sendReceive); LOG.debug("Supported Add BestPath table {} added to peer {}", sendReceive, peerId); @@ -79,7 +81,7 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker { } this.peerRoles.put(peerPath, peerRole); LOG.debug("Supported table {} added to peer {} role {}", this.localTableKey, peerId, peerRole); - createGroups(this.peerRoles); + final AbstractRegistration registration = addToExportGroups(peerId, peerPath, peerRole); final Object lock = this; return new AbstractRegistration() { @@ -93,7 +95,7 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker { ExportPolicyPeerTrackerImpl.this.peerTables.remove(peerId); LOG.debug("Removed peer {} from supported table {}", peerId, ExportPolicyPeerTrackerImpl.this.localTableKey); ExportPolicyPeerTrackerImpl.this.peerRoles.remove(peerPath); - createGroups(ExportPolicyPeerTrackerImpl.this.peerRoles); + registration.close(); } } }; @@ -119,5 +121,4 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker { final SendReceive sendReceive = this.peerAddPathTables.get(peerId); return sendReceive != null && (sendReceive.equals(SendReceive.Both) || sendReceive.equals(SendReceive.Receive)); } - } \ No newline at end of file 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 7a5b42f156..dd6dbc66d2 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 @@ -9,18 +9,23 @@ package org.opendaylight.protocol.bgp.rib.impl; import com.google.common.base.Preconditions; import java.util.Collection; +import java.util.HashMap; import java.util.Map; -import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup; +import java.util.function.BiConsumer; +import javax.annotation.concurrent.GuardedBy; +import org.opendaylight.protocol.bgp.rib.impl.spi.PeerExportGroupRegistry; +import org.opendaylight.protocol.concepts.AbstractRegistration; 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.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; -final class PeerExportGroupImpl implements PeerExportGroup { - private final Map peers; +final class PeerExportGroupImpl implements PeerExportGroupRegistry { + @GuardedBy("this") + private final Map peers = new HashMap<>(); private final AbstractExportPolicy policy; - public PeerExportGroupImpl(final Map peers, final AbstractExportPolicy policy) { - this.peers = Preconditions.checkNotNull(peers); + public PeerExportGroupImpl(final AbstractExportPolicy policy) { this.policy = Preconditions.checkNotNull(policy); } @@ -31,11 +36,47 @@ final class PeerExportGroupImpl implements PeerExportGroup { @Override public Collection> getPeers() { - return this.peers.entrySet(); + synchronized (this.peers) { + return this.peers.entrySet(); + } } @Override public boolean containsPeer(final PeerId routePeerId) { - return this.peers.containsKey(routePeerId); + synchronized (this.peers) { + return this.peers.containsKey(routePeerId); + } + } + + @Override + public void forEach(final BiConsumer action) { + synchronized (this.peers) { + for (final Map.Entry pid : this.peers.entrySet()) { + action.accept(pid.getKey(), pid.getValue().getYii()); + } + } + } + + @Override + public AbstractRegistration registerPeer(final PeerId peerId, final PeerExporTuple peerExporTuple) { + synchronized (this.peers) { + this.peers.put(peerId, peerExporTuple); + } + + return new AbstractRegistration() { + @Override + protected void removeRegistration() { + synchronized (PeerExportGroupImpl.this.peers) { + PeerExportGroupImpl.this.peers.remove(peerId); + } + } + }; + } + + @Override + public boolean isEmpty() { + synchronized (this.peers) { + return this.peers.isEmpty(); + } } } \ No newline at end of file diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/spi/PeerExportGroupRegistry.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/spi/PeerExportGroupRegistry.java new file mode 100644 index 0000000000..844ec94a76 --- /dev/null +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/spi/PeerExportGroupRegistry.java @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2017 Pantheon Technologies s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.protocol.bgp.rib.impl.spi; + +import com.google.common.annotations.Beta; +import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup; +import org.opendaylight.protocol.concepts.AbstractRegistration; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId; + +/** + * PeerExportGroup Registry. Register should be maintained in blocking mode to avoid race between + * updating the routes and notify to a peer which doesn't longer exist. BUG-7676 + */ +@Beta +public interface PeerExportGroupRegistry extends PeerExportGroup { + /** + * Register peer to this exportGroup + * + * @param peerId Peer Id + * @param peerExporTuple Peer Export Tuple + * @return registration ticket which take allows to unregister peer + */ + AbstractRegistration registerPeer(PeerId peerId, PeerExporTuple peerExporTuple); + + /** + * @return true if none peer is registered. + */ + boolean isEmpty(); +} 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 ff7e91798d..31bb7ed57c 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 @@ -10,6 +10,7 @@ package org.opendaylight.protocol.bgp.rib.spi; import java.util.Collection; import java.util.Map; +import java.util.function.BiConsumer; 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.YangInstanceIdentifier; @@ -48,8 +49,22 @@ public interface PeerExportGroup { /** * @return map of peer + * @deprecated Use {@link #forEach} */ + @Deprecated Collection> getPeers(); + /** + * + * @param routePeerId PeerId + * @return true if peer is present on this export group + */ boolean containsPeer(PeerId routePeerId); + + /** + * Applies the given action for each entry in this PeerExportGroup on synchronized mode + * + * @param action action to be applied + */ + void forEach(BiConsumer action); } -- 2.36.6