BGPCEP-672: Fix key storage un adj-rib-out

list key must not be null, therefore
path-id should always be included.
Reserving PATH-ID 0 for non-supporting
add-path.

Change-Id: I9b0af92e91e7c0b1c7281230a804c7adeff2dbca
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

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 ef91b92b0a17faf57da06385a8aad446b67039b2..d6854b0b20e9c244ddd4d502d798aef568bf381c 100644 (file)
--- 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
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.protocol.bgp.mode.impl.add;
 
+import static org.opendaylight.protocol.bgp.parser.spi.PathIdUtil.NON_PATH_ID;
+
 import com.google.common.collect.Lists;
 import com.google.common.primitives.UnsignedInteger;
 import java.util.ArrayList;
@@ -132,7 +134,9 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry {
         if (this.removedPaths != null) {
             this.removedPaths.forEach(removedPath -> {
                 final PathArgument routeIdAddPath = ribSupport.getRouteIdAddPath(removedPath.getPathId(), routeIdPA);
-                fillAdjRibsOut(true, null, null, null, routeIdPA, routeIdAddPath,
+                final PathArgument routeIdAddPathDefault = ribSupport.getRouteIdAddPath(NON_PATH_ID, routeIdPA);
+                fillAdjRibsOut(true, null, null, null,
+                        routeIdAddPathDefault, routeIdAddPath,
                         RouterIds.createPeerId(removedPath.getRouteId()), peerPT, localTK, ribSupport, tx);
             });
             this.removedPaths = null;
@@ -164,35 +168,36 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry {
         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(getRoutePeerIdRole(peerPT,path.getPeerId()), path.getAttributes());
+        PathArgument routeIdAddPath;
         if (destPeerSupAddPath) {
-            writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPath, localTK), effectiveAttributes,
-                    createValue(routeIdAddPath, path), ribSup, tx);
+            routeIdAddPath = ribSup.getRouteIdAddPath(path.getPathId(), routeId);
         } else {
-            writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK), effectiveAttributes,
-                    createValue(routeId, path), ribSup, tx);
+            routeIdAddPath = ribSup.getRouteIdAddPath(NON_PATH_ID, routeId);
         }
+        writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPath, localTK), effectiveAttributes,
+                createValue(routeIdAddPath, path), ribSup, tx);
     }
 
     private void addPathToDataStore(final BestPath path, final boolean isFirstBestPath, final PathArgument routeIdPA,
             final YangInstanceIdentifier locRibTarget, final RIBSupport ribSup, final ExportPolicyPeerTracker peerPT,
             final TablesKey localTK, final DOMDataWriteTransaction tx) {
         final PathArgument routeIdAddPath = ribSup.getRouteIdAddPath(path.getPathId(), routeIdPA);
+        final PathArgument routeIdAddPathDefault = ribSup.getRouteIdAddPath(NON_PATH_ID, routeIdPA);
         final YangInstanceIdentifier pathAddPathTarget = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER),
                 routeIdAddPath);
         final MapEntryNode addPathValue = createValue(routeIdAddPath, path);
-        final MapEntryNode value = createValue(routeIdPA, path);
+        final MapEntryNode defaultValue = createValue(routeIdAddPathDefault, path);
         LOG.trace("Selected best value {}", addPathValue);
         fillLocRib(pathAddPathTarget, addPathValue, tx);
-        fillAdjRibsOut(isFirstBestPath, path.getAttributes(), value, addPathValue, routeIdPA, routeIdAddPath,
-                path.getPeerId(), peerPT, localTK,
-            ribSup, tx);
+        fillAdjRibsOut(isFirstBestPath, path.getAttributes(), defaultValue, addPathValue, routeIdAddPathDefault,
+            routeIdAddPath, path.getPeerId(), peerPT, localTK, ribSup, tx);
     }
 
     private void fillAdjRibsOut(final boolean isFirstBestPath, final ContainerNode attributes,
-            final NormalizedNode<?, ?> value, final MapEntryNode addPathValue, final PathArgument routeId,
+            final MapEntryNode defaultValue, final MapEntryNode addPathValue,
+            final PathArgument routeIdAddPathDefault,
             final PathArgument routeIdAddPath, final PeerId routePeerId, final ExportPolicyPeerTracker peerPT,
             final TablesKey localTK, final RIBSupport ribSup, final DOMDataWriteTransaction tx) {
         /*
@@ -217,8 +222,8 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry {
                             update(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPath, localTK),
                                     effectiveAttributes, addPathValue, ribSup, tx);
                         } else if (!this.oldNonAddPathBestPathTheSame) {
-                            update(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK),
-                                    effectiveAttributes, value, ribSup, tx);
+                            update(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPathDefault, localTK),
+                                    effectiveAttributes, defaultValue, 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 e3381a83c961482682c96bdabf9c7857999d401b..4cae936af56f007ce20a079998ebefb2d5857fc5 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
@@ -23,6 +23,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes;
 import org.slf4j.Logger;
@@ -119,10 +120,14 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry {
             final BaseBestPath path = this.bestPath;
             final PeerRole destPeerRole = getRoutePeerIdRole(peerPT, destPeer);
             if (filterRoutes(path.getPeerId(), destPeer, peerPT, localTK, destPeerRole)) {
+                PathArgument routeIdDest = ribSupport.getRouteIdAddPath(path.getPathId(), routeId);
+                if (routeIdDest == null) {
+                    routeIdDest = routeId;
+                }
                 final ContainerNode effAttrib = peerGroup.effectiveAttributes(
                         getRoutePeerIdRole(peerPT, path.getPeerId()), path.getAttributes());
-                writeRoute(destPeer, getAdjRibOutYII(ribSupport, rootPath, routeId, localTK), effAttrib,
-                        createValue(routeId, path), ribSupport, tx);
+                writeRoute(destPeer, getAdjRibOutYII(ribSupport, rootPath, routeIdDest, localTK), effAttrib,
+                        createValue(routeIdDest, path), ribSupport, tx);
             }
         }
     }
@@ -131,34 +136,29 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry {
             final YangInstanceIdentifier locRibTarget, final ExportPolicyPeerTracker peerPT,
             final TablesKey localTK, final RIBSupport ribSup, final DOMDataWriteTransaction tx) {
         LOG.trace("Best Path removed {}", path);
-        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);
+        PathArgument routeIdTarget = ribSup.getRouteIdAddPath(path.getPathId(), routeIdPA);
+        if (routeIdTarget == null) {
+            routeIdTarget = routeIdPA;
         }
-        fillLocRib(pathAddPathTarget == null ? pathTarget : pathAddPathTarget, null, tx);
-        fillAdjRibsOut(null, null, routeIdPA, path.getPeerId(), peerPT, localTK, ribSup, tx);
+        fillLocRib(ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdTarget), null, tx);
+        fillAdjRibsOut(null, null, routeIdTarget, path.getPeerId(), peerPT, localTK, ribSup, tx);
     }
 
     private void addPathToDataStore(final BestPath path, final PathArgument routeIdPA,
             final YangInstanceIdentifier locRibTarget, final RIBSupport ribSup, final ExportPolicyPeerTracker peerPT,
             final TablesKey localTK, final DOMDataWriteTransaction tx) {
-        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);
+        PathArgument routeIdDest = ribSup.getRouteIdAddPath(path.getPathId(), routeIdPA);
+        if (routeIdDest == null) {
+            routeIdDest = routeIdPA;
         }
-        fillLocRib(pathAddPathTarget == null ? pathTarget : pathAddPathTarget,
-                addPathValue == null ? value : addPathValue, tx);
-        fillAdjRibsOut(path.getAttributes(), value, routeIdPA, path.getPeerId(), peerPT, localTK, ribSup, tx);
+
+        final MapEntryNode value = createValue(routeIdDest, path);
+        LOG.trace("Selected best value {}", value);
+
+        final YangInstanceIdentifier pathAddPathTarget
+                = ribSup.routePath(locRibTarget.node(ROUTES_IDENTIFIER), routeIdDest);
+        fillLocRib(pathAddPathTarget, value, tx);
+        fillAdjRibsOut(path.getAttributes(), value, routeIdDest, path.getPeerId(), peerPT, localTK, ribSup, tx);
     }
 
     final OffsetMap getOffsets() {
@@ -166,7 +166,7 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry {
     }
 
     @VisibleForTesting
-    private void fillAdjRibsOut(final ContainerNode attributes, final NormalizedNode<?, ?> value,
+    private void fillAdjRibsOut(final ContainerNode attributes, final MapEntryNode value,
             final PathArgument routeId, final PeerId routePeerId, final ExportPolicyPeerTracker peerPT,
             final TablesKey localTK, final RIBSupport ribSup, final DOMDataWriteTransaction 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 ec58b69229d0d3310a86c466d3214341121d7dae..ec2a4a0256edb2bf81c0a750cbab896e49183f7a 100644 (file)
--- 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
@@ -91,7 +91,7 @@ public abstract class AbstractRouteEntry implements RouteEntry {
                 .node(RibSupportUtils.toYangTablesKey(localTK)).node(ROUTES_IDENTIFIER), routeId);
     }
 
-    protected PeerRole getRoutePeerIdRole(final ExportPolicyPeerTracker peerPT, final PeerId routePeerId) {
+    protected static 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)) {

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
index bdd4fb8000ddc4cbf47c5ad4228fedf7a8c45322..4c68e9a8ecf14c022a3fdc20dd91acc5e74fa9e1 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
@@ -204,6 +204,9 @@ final class LocRibWriter implements AutoCloseable, TotalPrefixesCounter, TotalPa
         if (!table.getDataBefore().isPresent() && this.exportPolicyPeerTracker.isTableSupported(peerIdOfNewPeer)) {
             this.exportPolicyPeerTracker.registerPeerAsInitialized(peerIdOfNewPeer);
             LOG.debug("Peer {} table has been created, inserting existent routes", peerIdOfNewPeer);
+            if (this.routeEntries.isEmpty()) {
+                return;
+            }
             final PeerRole newPeerRole = this.exportPolicyPeerTracker.getRole(IdentifierUtils.peerPath(rootPath));
             final PeerExportGroup peerGroup = this.exportPolicyPeerTracker.getPeerGroup(newPeerRole);
             this.routeEntries.forEach((key, value) -> value.writeRoute(peerIdOfNewPeer, key,

diff --git a/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java b/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java
index 081fc8719ddc5d7e14b02040742f7cdaf9314604..9199d78b9596932b9903e4542e03a4fef49a0089 100644 (file)
--- a/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java
+++ b/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java
@@ -147,6 +147,7 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
     }
 
     private class ConfigLoaderImplRunnable implements Runnable {
+        @GuardedBy("this")
         private final WatchService watchService;
 
         ConfigLoaderImplRunnable(final WatchService watchService) {
@@ -156,14 +157,14 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
         @Override
         public void run() {
             while (!Thread.currentThread().isInterrupted()) {
-                handleChanges(this.watchService);
+                handleChanges();
             }
         }
 
-        private synchronized void handleChanges(final WatchService watch) {
+        private synchronized void handleChanges() {
             final WatchKey key;
             try {
-                key = watch.take();
+                key = this.watchService.take();
             } catch (final InterruptedException | ClosedWatchServiceException e) {
                 if (!ConfigLoaderImpl.this.closed) {
                     LOG.warn(INTERRUPTED, e);