From: Faseela K Date: Wed, 30 Dec 2015 12:24:58 +0000 (+0530) Subject: Fixing issues with batching transactions during bind service X-Git-Tag: release/beryllium~72 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F50%2F31950%2F2;hp=60ff05a5b600fa05bacdfd364869d968364b983d;p=vpnservice.git 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 --- 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 af22f84b..e34e2b06 100644 --- 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> bindService(InstanceIdentifier instanceIdentifier, BoundServices boundServiceNew, DataBroker dataBroker) { List> 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 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 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 cc57ff0e..e18072c7 100644 --- 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 = servicesInfo.getBoundServices(); if (boundServices.isEmpty()) { // Remove entry from Ingress Table. @@ -101,14 +94,14 @@ public class FlowBasedServicesConfigUnbindHelper { List 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 2516304f..b1c4d489 100644 --- 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 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 0feb834d..bcaff79d 100644 --- 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 getMatchInfoForVlanPortAtIngressTable(BigInteger dpId, long portNo, long vlanId) { + public static List getMatchInfoForVlanPortAtIngressTable(BigInteger dpId, long portNo, Interface iface) { List 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 matches, int lportTag, short tableId) { List instructions = boundServiceNew.getAugmentation(StypeOpenflow.class).getInstruction(); int serviceInstructionsSize = instructions.size(); List instructionSet = new ArrayList(); + 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,