From: Faseela K Date: Thu, 27 Oct 2016 13:41:09 +0000 (+0530) Subject: Bug 7048 - Update to OF port does not change 220 flow X-Git-Tag: release/carbon~386^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=7a71314dd6ae468162ddb34e30af1fa772c3ce18;p=genius.git 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 --- 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 7f62b3f32..31584dae6 100755 --- 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 - boundServicesInstanceIdentifier, Class 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 07ba6089f..5da5d58d2 100644 --- 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 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 ed3b0de63..2a962b601 100644 --- 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 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 f3e2c51d9..ff332176d 100644 --- 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 edb74f6ba..91892533b 100644 --- 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 feee7e5b0..cdde1500e 100644 --- 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> 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 fd27e5793..b73a1f340 100644 --- 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 buildServiceId(String interfaceName, short serviceIndex, Class 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> 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 fb3b35fa5..7e6206ffc 100644 --- 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 7530f2abb..90811d194 100644 --- 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.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()); }