Bug 7048 - Update to OF port does not change 220 flow

Description : Wrong instance identifier was being used for unbind service.
Also, DJC synchronization key for unbind service should be parent interface, rather
than interface-name, else this will cause race conditions while deleting 220 flow.

Change-Id: I632791ed131cab96e1967542c1fa7e18fa92c519
Signed-off-by: Faseela K <faseela.k@ericsson.com>

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/IfmUtil.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/IfmUtil.java
index 7f62b3f323b568c032f185397c095506ff2fc474..31584dae6c10052c2eb11424cc41e71953ae0954 100755 (executable)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/IfmUtil.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/IfmUtil.java
@@ -487,10 +487,10 @@ public class IfmUtil {
     }
 
     public static void unbindService(DataBroker dataBroker, String interfaceName, InstanceIdentifier<BoundServices>
-            boundServicesInstanceIdentifier, Class<? extends ServiceModeBase> serviceMode){
+            boundServicesInstanceIdentifier, String parentInterface){
         LOG.info("Unbinding Service from : {}", interfaceName);
         DataStoreJobCoordinator dataStoreJobCoordinator = DataStoreJobCoordinator.getInstance();
-        dataStoreJobCoordinator.enqueueJob(interfaceName,
+        dataStoreJobCoordinator.enqueueJob(parentInterface,
                 () -> {
                     WriteTransaction t = dataBroker.newWriteOnlyTransaction();
                     t.delete(LogicalDatastoreType.CONFIGURATION, boundServicesInstanceIdentifier);

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/InterfacemgrProvider.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/InterfacemgrProvider.java
index 07ba6089f09e2e46a2aa05d15382323cfbcd36a3..5da5d58d2fbd7cb35b552ae52ac506a0baffe744 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/InterfacemgrProvider.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/InterfacemgrProvider.java
@@ -485,7 +485,7 @@ public class InterfacemgrProvider implements BindingAwareProvider, AutoCloseable
     @Override
     public void unbindService(String interfaceName, Class<? extends ServiceModeBase> serviceMode, BoundServices serviceInfo) {
         IfmUtil.unbindService(dataBroker, interfaceName,
-                FlowBasedServicesUtils.buildServiceId(interfaceName, serviceInfo.getServicePriority()), serviceMode);
+                FlowBasedServicesUtils.buildServiceId(interfaceName, serviceInfo.getServicePriority(), serviceMode), interfaceName);
     }
 
     @Override

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/listeners/InterfaceInventoryStateListener.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/listeners/InterfaceInventoryStateListener.java
index ed3b0de6335cadc3c100dba34d523e8c9c70f032..2a962b601d9a6d7b3950fdcb1de561bc938c50e6 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/listeners/InterfaceInventoryStateListener.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/listeners/InterfaceInventoryStateListener.java
@@ -149,7 +149,7 @@ public class InterfaceInventoryStateListener extends AsyncClusteredDataTreeChang
         boolean isNodePresent = InterfaceManagerCommonUtils.isNodePresent(dataBroker, nodeConnectorIdNew);
         DataStoreJobCoordinator coordinator = DataStoreJobCoordinator.getInstance();
         InterfaceStateRemoveWorker portStateRemoveWorker = new InterfaceStateRemoveWorker(idManager, nodeConnectorIdNew,
-                nodeConnectorIdOld, fcNodeConnectorNew, portName, isNodePresent, isNetworkEvent, true);
+                nodeConnectorIdOld, fcNodeConnectorNew, portName, portName, isNodePresent, isNetworkEvent, true);
         coordinator.enqueueJob(portName, portStateRemoveWorker, IfmConstants.JOB_MAX_RETRIES);
     }
 
@@ -248,6 +248,7 @@ public class InterfaceInventoryStateListener extends AsyncClusteredDataTreeChang
         private NodeConnectorId nodeConnectorIdOld;
         FlowCapableNodeConnector fcNodeConnectorOld;
         private final String interfaceName;
+        private final String parentInterface;
         private final IdManagerService idManager;
         private final boolean isNodePresent;
         private final boolean isNetworkEvent;
@@ -256,14 +257,16 @@ public class InterfaceInventoryStateListener extends AsyncClusteredDataTreeChang
         public InterfaceStateRemoveWorker(IdManagerService idManager, NodeConnectorId nodeConnectorIdNew,
                                           NodeConnectorId nodeConnectorIdOld,
                                           FlowCapableNodeConnector fcNodeConnectorOld,
-                                          String portName,
+                                          String interfaceName,
+                                          String parentInterface,
                                           boolean isNodePresent,
                                           boolean isNetworkEvent,
                                           boolean isParentInterface) {
             this.nodeConnectorIdNew = nodeConnectorIdNew;
             this.nodeConnectorIdOld = nodeConnectorIdOld;
             this.fcNodeConnectorOld = fcNodeConnectorOld;
-            this.interfaceName = portName;
+            this.interfaceName = interfaceName;
+            this.parentInterface = parentInterface;
             this.idManager = idManager;
             this.isNodePresent = isNodePresent;
             this.isNetworkEvent = isNetworkEvent;
@@ -286,13 +289,13 @@ public class InterfaceInventoryStateListener extends AsyncClusteredDataTreeChang
             }
 
             futures = OvsInterfaceStateRemoveHelper.removeInterfaceStateConfiguration(idManager, mdsalApiManager, alivenessMonitorService,
-                    nodeConnectorIdNew, nodeConnectorIdOld, dataBroker, interfaceName, fcNodeConnectorOld, isNodePresent);
+                    nodeConnectorIdNew, nodeConnectorIdOld, dataBroker, interfaceName, fcNodeConnectorOld, isNodePresent, parentInterface);
 
             List<InterfaceChildEntry> interfaceChildEntries = getInterfaceChildEntries(dataBroker, interfaceName);
             for (InterfaceChildEntry interfaceChildEntry : interfaceChildEntries) {
                 // Fetch all interfaces on this port and trigger remove worker for each of them
                 InterfaceStateRemoveWorker interfaceStateRemoveWorker = new InterfaceStateRemoveWorker(idManager, nodeConnectorIdNew,
-                        nodeConnectorIdOld, fcNodeConnectorOld, interfaceChildEntry.getChildInterface(), isNodePresent, isNetworkEvent, false);
+                        nodeConnectorIdOld, fcNodeConnectorOld, interfaceChildEntry.getChildInterface(), interfaceName, isNodePresent, isNetworkEvent, false);
                 DataStoreJobCoordinator.getInstance().enqueueJob(interfaceName, interfaceStateRemoveWorker);
             }
             return futures;

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsInterfaceConfigRemoveHelper.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsInterfaceConfigRemoveHelper.java
index f3e2c51d9c8ec2f7efdad0035809e656ff01f1aa..ff332176dcd182cc4c0dcf6a1daff7b34248fe48 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsInterfaceConfigRemoveHelper.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsInterfaceConfigRemoveHelper.java
@@ -108,7 +108,7 @@ public class OvsInterfaceConfigRemoveHelper {
             LOG.debug("removing interface state for  vlan trunk member {}", interfaceChildEntry.getChildInterface());
             InterfaceManagerCommonUtils.deleteInterfaceStateInformation(interfaceChildEntry.getChildInterface(), defaultOperationalShardTransaction, idManagerService);
             FlowBasedServicesUtils.removeIngressFlow(interfaceChildEntry.getChildInterface(), dpId, dataBroker, futures);
-            FlowBasedServicesUtils.unbindDefaultEgressDispatcherService(dataBroker, interfaceName);
+            FlowBasedServicesUtils.unbindDefaultEgressDispatcherService(dataBroker, interfaceName,interfaceParentEntry.getParentInterface());
         }
     }
 

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsVlanMemberConfigRemoveHelper.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsVlanMemberConfigRemoveHelper.java
index edb74f6ba4dea18ed4ec3ad04ef7068b7e55d9a0..91892533b31afa3abbbb7bd0d073f48687b9b178 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsVlanMemberConfigRemoveHelper.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/confighelpers/OvsVlanMemberConfigRemoveHelper.java
@@ -71,7 +71,7 @@ public class OvsVlanMemberConfigRemoveHelper {
                     IfmUtil.buildStateInterfaceId(interfaceOld.getName());
             defaultOperShardTransaction.delete(LogicalDatastoreType.OPERATIONAL, ifStateId);
             FlowBasedServicesUtils.removeIngressFlow(interfaceOld.getName(), dpId, dataBroker, futures);
-            FlowBasedServicesUtils.unbindDefaultEgressDispatcherService(dataBroker, interfaceOld.getName());
+            FlowBasedServicesUtils.unbindDefaultEgressDispatcherService(dataBroker, interfaceOld.getName(), parentRefs.getParentInterface());
         }
 
         futures.add(defaultConfigShardTransaction.submit());

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/statehelpers/OvsInterfaceStateRemoveHelper.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/statehelpers/OvsInterfaceStateRemoveHelper.java
index feee7e5b0e6c17f94515458419cc6c3046aa75b7..cdde1500e154cc101c47e8af33abb467d3828aff 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/statehelpers/OvsInterfaceStateRemoveHelper.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/renderer/ovs/statehelpers/OvsInterfaceStateRemoveHelper.java
@@ -37,7 +37,8 @@ public class OvsInterfaceStateRemoveHelper {
                                                                                  NodeConnectorId nodeConnectorIdNew, NodeConnectorId nodeConnectorIdOld,
                                                                                  DataBroker dataBroker, String interfaceName,
                                                                                  FlowCapableNodeConnector fcNodeConnectorOld,
-                                                                                 boolean isNodePresent) {
+                                                                                 boolean isNodePresent,
+                                                                                 String parentInterface) {
         LOG.debug("Removing interface-state information for interface: {} {}", interfaceName, isNodePresent);
         List<ListenableFuture<Void>> futures = new ArrayList<>();
         WriteTransaction defaultOperationalShardTransaction = dataBroker.newWriteOnlyTransaction();
@@ -73,7 +74,7 @@ public class OvsInterfaceStateRemoveHelper {
             // skip this check for non-unique ports(Ex: br-int,br-ex)
             if(iface != null || (iface == null && !interfaceName.contains(fcNodeConnectorOld.getName()))) {
                 FlowBasedServicesUtils.removeIngressFlow(interfaceName, dpId, dataBroker, futures);
-                FlowBasedServicesUtils.unbindDefaultEgressDispatcherService(dataBroker, interfaceName);
+                FlowBasedServicesUtils.unbindDefaultEgressDispatcherService(dataBroker, interfaceName, parentInterface);
             }
 
             // Delete the Vpn Interface from DpnToInterface Op DS.

diff --git a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java
index fd27e5793fe7c996ffffbca1f08be9e2a59288fd..b73a1f340378f8a9b79016015d6591660989df8d 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java
+++ b/interfacemanager/interfacemanager-impl/src/main/java/org/opendaylight/genius/interfacemanager/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java
@@ -325,10 +325,16 @@ public class FlowBasedServicesUtils {
                 .child(BoundServices.class, new BoundServicesKey(serviceIndex)).build();
     }
 
-    public static void unbindDefaultEgressDispatcherService(DataBroker dataBroker, String interfaceName) {
+    public static InstanceIdentifier<BoundServices> buildServiceId(String interfaceName, short serviceIndex, Class<? extends ServiceModeBase> serviceMode) {
+        return InstanceIdentifier.builder(ServiceBindings.class).child(ServicesInfo.class,
+                new ServicesInfoKey(interfaceName, serviceMode))
+                .child(BoundServices.class, new BoundServicesKey(serviceIndex)).build();
+    }
+
+    public static void unbindDefaultEgressDispatcherService(DataBroker dataBroker, String interfaceName, String parentInterface) {
         IfmUtil.unbindService(dataBroker, interfaceName, buildServiceId(interfaceName,
-                ServiceIndex.getIndex(NwConstants.DEFAULT_EGRESS_SERVICE_NAME, NwConstants.DEFAULT_EGRESS_SERVICE_INDEX)),
-                ServiceModeEgress.class);
+                ServiceIndex.getIndex(NwConstants.DEFAULT_EGRESS_SERVICE_NAME, NwConstants.DEFAULT_EGRESS_SERVICE_INDEX),
+                ServiceModeEgress.class), parentInterface);
     }
 
     public static void bindDefaultEgressDispatcherService(DataBroker dataBroker, List<ListenableFuture<Void>> futures,

diff --git a/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/InterfaceManagerTestUtil.java b/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/InterfaceManagerTestUtil.java
index fb3b35fa5549cd4301a7bded03cad207674df026..7e6206ffc0540522eb8d8b54a44111b3c9df9bfc 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/InterfaceManagerTestUtil.java
+++ b/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/InterfaceManagerTestUtil.java
@@ -248,7 +248,8 @@ public class InterfaceManagerTestUtil {
         NodeConnectorBuilder ncBuilder = new NodeConnectorBuilder()
                 .setId(ncId)
                 .setKey(new NodeConnectorKey(ncId));
-        FlowCapableNodeConnectorBuilder flowCapableNodeConnectorBuilder = new FlowCapableNodeConnectorBuilder().setHardwareAddress(MacAddress.getDefaultInstance("AA:AA:AA:AA:AA:AA"));
+        FlowCapableNodeConnectorBuilder flowCapableNodeConnectorBuilder = new FlowCapableNodeConnectorBuilder().
+                setHardwareAddress(MacAddress.getDefaultInstance("AA:AA:AA:AA:AA:AA")).setName(InterfaceManagerTestUtil.interfaceName);
         ncBuilder.addAugmentation(FlowCapableNodeConnector.class,flowCapableNodeConnectorBuilder.build());
         return ncBuilder.build();
     }

diff --git a/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/StateInterfaceTest.java b/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/StateInterfaceTest.java
index 7530f2abb242532b43e8ecc7360008cff893d5e3..90811d194f09e86fdbda442cf891521d7beb87db 100644 (file)
--- a/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/StateInterfaceTest.java
+++ b/interfacemanager/interfacemanager-impl/src/test/java/org/opendaylight/genius/interfacemanager/test/StateInterfaceTest.java
@@ -273,7 +273,7 @@ public class StateInterfaceTest {
         doReturn(Futures.immediateFuture(RpcResultBuilder.<Void>success().build())).when(idManager).releaseId(getIdInput);
         boolean isNodePresent = InterfaceManagerCommonUtils.isNodePresent(dataBroker, nodeConnectorId);
         removeHelper.removeInterfaceStateConfiguration(idManager, mdsalManager, alivenessMonitorService, nodeConnectorId,
-                nodeConnectorId, dataBroker, InterfaceManagerTestUtil.interfaceName, fcNodeConnectorNew, isNodePresent);
+                nodeConnectorId, dataBroker, InterfaceManagerTestUtil.interfaceName, fcNodeConnectorNew, isNodePresent, fcNodeConnectorNew.getName());
 
         verify(mockWriteTx).delete(LogicalDatastoreType.OPERATIONAL, interfaceStateIdentifier);
 
@@ -306,7 +306,7 @@ public class StateInterfaceTest {
 
         boolean isNodePresent = InterfaceManagerCommonUtils.isNodePresent(dataBroker, nodeConnectorId);
         removeHelper.removeInterfaceStateConfiguration(idManager, mdsalManager, alivenessMonitorService, nodeConnectorId,
-                nodeConnectorId, dataBroker, InterfaceManagerTestUtil.interfaceName, fcNodeConnectorNew, isNodePresent);
+                nodeConnectorId, dataBroker, InterfaceManagerTestUtil.interfaceName, fcNodeConnectorNew, isNodePresent, fcNodeConnectorNew.getName());
 
     }