Fixing issues with batching transactions during bind service

- whenever a service with higher priority is bound to an interface, there
  is a sequence of flow installation and deletion which were batched into
  the same transaction. This transaction batching is not working as they
  are all on the same subtree and hence
  moving them to separate transactions.

- Some minor code optimisations and cleanups

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

diff --git a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigBindHelper.java b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigBindHelper.java
index af22f84bf8fd9bac092b9ab37779964f41031297..e34e2b069a93d52a933e2a9a73b727994e090f5f 100644 (file)
--- a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigBindHelper.java
+++ b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigBindHelper.java
@@ -40,7 +40,7 @@ public class FlowBasedServicesConfigBindHelper {
     public static List<ListenableFuture<Void>> bindService(InstanceIdentifier<BoundServices> instanceIdentifier,
                                                            BoundServices boundServiceNew, DataBroker dataBroker) {
         List<ListenableFuture<Void>> futures = new ArrayList<>();
-        WriteTransaction t = dataBroker.newWriteOnlyTransaction();
+        WriteTransaction transaction = dataBroker.newWriteOnlyTransaction();
         String interfaceName =
                 InstanceIdentifier.keyOf(instanceIdentifier.firstIdentifierOf(ServicesInfo.class)).getInterfaceName();
 
@@ -77,22 +77,18 @@ public class FlowBasedServicesConfigBindHelper {
             int vlanId = 0;
             List<MatchInfo> matches = null;
             if (iface.getType().isAssignableFrom(L2vlan.class)) {
-                IfL2vlan l2vlan = iface.getAugmentation(IfL2vlan.class);
-                if( l2vlan != null){
-                    vlanId = l2vlan.getVlanId().getValue();
-                }
-                matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, vlanId);
+                matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, iface);
             } else if (iface.getType().isAssignableFrom(Tunnel.class)){
                 matches = FlowBasedServicesUtils.getMatchInfoForTunnelPortAtIngressTable (dpId, portNo, iface);
             }
 
             if (matches != null) {
-                FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface.getName(), vlanId, boundServiceNew,
-                        dataBroker, t, matches, ifState.getIfIndex(), IfmConstants.VLAN_INTERFACE_INGRESS_TABLE);
+                FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface, boundServiceNew,
+                        transaction, matches, ifState.getIfIndex(), IfmConstants.VLAN_INTERFACE_INGRESS_TABLE);
             }
 
-            if (t != null) {
-                futures.add(t.submit());
+            if (transaction != null) {
+                futures.add(transaction.submit());
             }
             return futures;
         }
@@ -111,35 +107,37 @@ public class FlowBasedServicesConfigBindHelper {
                     highestPriority = boundService.getServicePriority();
                 }
             }
-            LOG.error("Reached unexpected part 1 of the code when handling bind service for interface: {}, when binding" +
-                    "service: {}", iface, boundServiceNew);
         }
 
         if (!isCurrentServiceHighestPriority) {
-            FlowBasedServicesUtils.installLPortDispatcherFlow(dpId, boundServiceNew, iface, t,
+            FlowBasedServicesUtils.installLPortDispatcherFlow(dpId, boundServiceNew, iface, transaction,
                     ifState.getIfIndex());
         } else {
             BoundServices serviceToReplace = tmpServicesMap.get(highestPriority);
-            FlowBasedServicesUtils.installLPortDispatcherFlow(dpId, serviceToReplace, iface, t,
+            FlowBasedServicesUtils.installLPortDispatcherFlow(dpId, serviceToReplace, iface, transaction,
                     ifState.getIfIndex());
-            int vlanId = 0;
             List<MatchInfo> matches = null;
             if (iface.getType().isAssignableFrom(L2vlan.class)) {
-                vlanId = iface.getAugmentation(IfL2vlan.class).getVlanId().getValue();
-                matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, vlanId);
+                matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, iface);
             } else if (iface.getType().isAssignableFrom(Tunnel.class)){
                 matches = FlowBasedServicesUtils.getMatchInfoForTunnelPortAtIngressTable (dpId, portNo, iface);
             }
 
             if (matches != null) {
-                FlowBasedServicesUtils.removeIngressFlow(iface, serviceToReplace, dpId, t);
-                FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface.getName(), vlanId, boundServiceNew, dataBroker, t,
+
+                WriteTransaction removeFlowTransaction = dataBroker.newWriteOnlyTransaction();
+                FlowBasedServicesUtils.removeIngressFlow(iface, serviceToReplace, dpId, removeFlowTransaction);
+                futures.add(removeFlowTransaction.submit());
+
+                WriteTransaction installFlowTransaction = dataBroker.newWriteOnlyTransaction();
+                FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface, boundServiceNew, installFlowTransaction,
                         matches, ifState.getIfIndex(), IfmConstants.VLAN_INTERFACE_INGRESS_TABLE);
+                futures.add(installFlowTransaction.submit());
             }
         }
 
-        if (t != null) {
-            futures.add(t.submit());
+        if (transaction != null) {
+            futures.add(transaction.submit());
         }
         return futures;
     }

diff --git a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigUnbindHelper.java b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigUnbindHelper.java
index cc57ff0e24bfe263d28dbcc4834585ce2050fb6a..e18072c7164b70898f1da9482fac8c53bf677054 100644 (file)
--- a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigUnbindHelper.java
+++ b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/confighelpers/FlowBasedServicesConfigUnbindHelper.java
@@ -65,13 +65,6 @@ public class FlowBasedServicesConfigUnbindHelper {
         long portNo = Long.parseLong(IfmUtil.getPortNoFromNodeConnectorId(nodeConnectorId));
         BigInteger dpId = new BigInteger(IfmUtil.getDpnFromNodeConnectorId(nodeConnectorId));
 
-        int vlanId = 0;
-        if (iface.getType().isAssignableFrom(L2vlan.class)) {
-            IfL2vlan l2vlan = iface.getAugmentation(IfL2vlan.class);
-            if(l2vlan != null) {
-                vlanId = iface.getAugmentation(IfL2vlan.class).getVlanId().getValue();
-            }
-        }
         List<BoundServices> boundServices = servicesInfo.getBoundServices();
         if (boundServices.isEmpty()) {
             // Remove entry from Ingress Table.
@@ -101,14 +94,14 @@ public class FlowBasedServicesConfigUnbindHelper {
 
         List<MatchInfo> matches = null;
         if (iface.getType().isAssignableFrom(L2vlan.class)) {
-            matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, vlanId);
+            matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, iface);
         } else if (iface.getType().isAssignableFrom(Tunnel.class)){
             matches = FlowBasedServicesUtils.getMatchInfoForTunnelPortAtIngressTable (dpId, portNo, iface);
         }
 
         BoundServices toBeMoved = tmpServicesMap.get(highestPriority);
         FlowBasedServicesUtils.removeIngressFlow(iface, boundServiceOld, dpId, t);
-        FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface.getName(), vlanId, toBeMoved, dataBroker, t,
+        FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface, toBeMoved, t,
                 matches, ifState.getIfIndex(), IfmConstants.VLAN_INTERFACE_INGRESS_TABLE);
         FlowBasedServicesUtils.removeLPortDispatcherFlow(dpId, iface, toBeMoved, t);
 

diff --git a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/statehelpers/FlowBasedServicesStateBindHelper.java b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/statehelpers/FlowBasedServicesStateBindHelper.java
index 2516304f233f2a9e0f90ab08548120c6385a26f4..b1c4d4898325500c6c2203f7dd74f75a9645ea20 100644 (file)
--- a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/statehelpers/FlowBasedServicesStateBindHelper.java
+++ b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/statehelpers/FlowBasedServicesStateBindHelper.java
@@ -64,21 +64,16 @@ public class FlowBasedServicesStateBindHelper {
         NodeConnectorId nodeConnectorId = FlowBasedServicesUtils.getNodeConnectorIdFromInterface(iface, dataBroker);
         long portNo = Long.parseLong(IfmUtil.getPortNoFromNodeConnectorId(nodeConnectorId));
         BigInteger dpId = new BigInteger(IfmUtil.getDpnFromNodeConnectorId(nodeConnectorId));
-        int vlanId = 0;
         List<MatchInfo> matches = null;
         if (iface.getType().isAssignableFrom(L2vlan.class)) {
-            IfL2vlan l2vlan = iface.getAugmentation(IfL2vlan.class);
-            if(l2vlan != null) {
-                vlanId = l2vlan.getVlanId().getValue();
-            }
-            matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, vlanId);
+            matches = FlowBasedServicesUtils.getMatchInfoForVlanPortAtIngressTable(dpId, portNo, iface);
         } else if (iface.getType().isAssignableFrom(Tunnel.class)){
             matches = FlowBasedServicesUtils.getMatchInfoForTunnelPortAtIngressTable (dpId, portNo, iface);
         }
 
         if (matches != null) {
-            FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface.getName(), vlanId, highestPriorityBoundService,
-                    dataBroker, t, matches, ifaceState.getIfIndex(), IfmConstants.VLAN_INTERFACE_INGRESS_TABLE);
+            FlowBasedServicesUtils.installInterfaceIngressFlow(dpId, iface, highestPriorityBoundService,
+                    t, matches, ifaceState.getIfIndex(), IfmConstants.VLAN_INTERFACE_INGRESS_TABLE);
         }
 
         for (BoundServices boundService : allServices) {

diff --git a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java
index 0feb834d8c3f44d6b629f27ac23917e19909e984..bcaff79de63a9576a743178a472dd57bab00b6b1 100644 (file)
--- a/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java
+++ b/interfacemgr/interfacemgr-impl/src/main/java/org/opendaylight/vpnservice/interfacemgr/servicebindings/flowbased/utilities/FlowBasedServicesUtils.java
@@ -35,6 +35,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.serviceb
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.servicebinding.rev151015.service.bindings.ServicesInfo;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.servicebinding.rev151015.service.bindings.ServicesInfoKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.servicebinding.rev151015.service.bindings.services.info.BoundServices;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.interfacemgr.rev150331.IfL2vlan;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -70,9 +71,14 @@ public class FlowBasedServicesUtils {
         return null;
     }
 
-    public static List<MatchInfo> getMatchInfoForVlanPortAtIngressTable(BigInteger dpId, long portNo, long vlanId) {
+    public static List<MatchInfo> getMatchInfoForVlanPortAtIngressTable(BigInteger dpId, long portNo, Interface iface) {
         List<MatchInfo> matches = new ArrayList<>();
         matches.add(new MatchInfo(MatchFieldType.in_port, new BigInteger[] {dpId, BigInteger.valueOf(portNo)}));
+        int vlanId = 0;
+        IfL2vlan l2vlan = iface.getAugmentation(IfL2vlan.class);
+        if(l2vlan != null){
+            vlanId = l2vlan.getVlanId().getValue();
+        }
         if (vlanId > 0) {
             LOG.error("VlanId matching support is not fully available in Be.");
             matches.add(new MatchInfo(MatchFieldType.vlan_vid, new long[]{vlanId}));
@@ -116,14 +122,19 @@ public class FlowBasedServicesUtils {
         return 0L;
     }
 
-    public static void installInterfaceIngressFlow(BigInteger dpId, String interfaceName, int vlanId,
+    public static void installInterfaceIngressFlow(BigInteger dpId, Interface iface,
                                                    BoundServices boundServiceNew,
-                                                   DataBroker dataBroker, WriteTransaction t,
+                                                   WriteTransaction t,
                                                    List<MatchInfo> matches, int lportTag, short tableId) {
         List<Instruction> instructions = boundServiceNew.getAugmentation(StypeOpenflow.class).getInstruction();
 
         int serviceInstructionsSize = instructions.size();
         List<Instruction> instructionSet = new ArrayList<Instruction>();
+        int vlanId = 0;
+        IfL2vlan l2vlan = iface.getAugmentation(IfL2vlan.class);
+        if(l2vlan != null){
+            vlanId = l2vlan.getVlanId().getValue();
+        }
         if (vlanId != 0) {
             // incrementing instructionSize and using it as actionKey. Because it won't clash with any other instructions
             int actionKey = ++serviceInstructionsSize;
@@ -153,7 +164,7 @@ public class FlowBasedServicesUtils {
         }
 
         String serviceRef = boundServiceNew.getServiceName();
-        String flowRef = getFlowRef(dpId, interfaceName, boundServiceNew);
+        String flowRef = getFlowRef(dpId, iface.getName(), boundServiceNew);
         StypeOpenflow stypeOpenflow = boundServiceNew.getAugmentation(StypeOpenflow.class);
         Flow ingressFlow = MDSALUtil.buildFlowNew(tableId, flowRef,
                 stypeOpenflow.getFlowPriority(), serviceRef, 0, 0,