Issue: Longer time transactions with QOS

Root cause: In interface delete event, DSCP flow is deleted
            even if QoS is not configured

Fix: Remove flow only if QoS DSCP config applied on the port

Change-Id: Ifd51e2ac4224a36834bca4f80e0834ad19ea65bb
Signed-off-by: A Vamsikrishna <a.vamsikrishna@ericsson.com>

diff --git a/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosInterfaceStateChangeListener.java b/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosInterfaceStateChangeListener.java
index e3e68ecf5be592405f16cc19533301489beee3d6..66a10fa313b41ce89c4eec7493dd4da2351d9dd9 100644 (file)
--- a/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosInterfaceStateChangeListener.java
+++ b/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosInterfaceStateChangeListener.java
@@ -9,6 +9,7 @@
 package org.opendaylight.netvirt.qosservice;
 
 import com.google.common.base.Optional;
+import java.util.Collections;
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -16,6 +17,7 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.datastoreutils.AsyncClusteredDataTreeChangeListenerBase;
 import org.opendaylight.genius.mdsalutil.NwConstants;
+import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
 import org.opendaylight.netvirt.neutronvpn.interfaces.INeutronVpnManager;
 import org.opendaylight.netvirt.qosservice.recovery.QosServiceRecoveryHandler;
 import org.opendaylight.serviceutils.srm.RecoverableListener;
@@ -43,19 +45,22 @@ public class QosInterfaceStateChangeListener extends AsyncClusteredDataTreeChang
     private final QosAlertManager qosAlertManager;
     private final QosNeutronUtils qosNeutronUtils;
     private final INeutronVpnManager neutronVpnManager;
+    private final JobCoordinator jobCoordinator;
 
     @Inject
     public QosInterfaceStateChangeListener(final DataBroker dataBroker, final QosAlertManager qosAlertManager,
                                            final QosNeutronUtils qosNeutronUtils,
                                            final INeutronVpnManager neutronVpnManager,
                                            final ServiceRecoveryRegistry serviceRecoveryRegistry,
-                                           final QosServiceRecoveryHandler qosServiceRecoveryHandler) {
+                                           final QosServiceRecoveryHandler qosServiceRecoveryHandler,
+                                           final JobCoordinator jobCoordinator) {
         super(Interface.class, QosInterfaceStateChangeListener.class);
         this.dataBroker = dataBroker;
         this.uuidUtil = new UuidUtil();
         this.qosAlertManager = qosAlertManager;
         this.qosNeutronUtils = qosNeutronUtils;
         this.neutronVpnManager = neutronVpnManager;
+        this.jobCoordinator = jobCoordinator;
         serviceRecoveryRegistry.addRecoverableListener(qosServiceRecoveryHandler.buildServiceRegistryKey(),
                 this);
         LOG.trace("{} created",  getClass().getSimpleName());
@@ -123,9 +128,16 @@ public class QosInterfaceStateChangeListener extends AsyncClusteredDataTreeChang
             if (port != null) {
                 return Optional.fromJavaUtil(uuid.toJavaUtil().map(qosNeutronUtils::getNeutronPort));
             }
-            LOG.trace("Qos Service : interface {} clearing stale flow entries if any", portName);
-            qosNeutronUtils.removeStaleFlowEntry(intrf, NwConstants.ETHTYPE_IPV4);
-            qosNeutronUtils.removeStaleFlowEntry(intrf, NwConstants.ETHTYPE_IPV6);
+            if (qosNeutronUtils.isBindServiceDone(uuid)) {
+                LOG.trace("Qos Service : interface {} clearing stale flow entries if any", portName);
+                jobCoordinator.enqueueJob("QosPort-" + portName, () -> {
+                    qosNeutronUtils.removeStaleFlowEntry(intrf, NwConstants.ETHTYPE_IPV4);
+                    qosNeutronUtils.removeStaleFlowEntry(intrf, NwConstants.ETHTYPE_IPV6);
+                    qosNeutronUtils.unbindservice(portName);
+                    qosNeutronUtils.removeInterfaceInQosConfiguredPorts(uuid);
+                    return Collections.emptyList();
+                });
+            }
         }
         return Optional.absent();
     }

diff --git a/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java b/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java
index 878b2c863dd5a6fdaba4e8b9899e7bc825cd9714..a797eabc40dc6b444c898dcd2546d53286a00b55 100644 (file)
--- a/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java
+++ b/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java
@@ -508,43 +508,43 @@ public class QosNeutronUtils {
     }
 
     public void setPortDscpMarking(Port port, DscpmarkingRules dscpMark) {
-        if (!qosEosHandler.isQosClusterOwner()) {
-            LOG.trace("Not Qos Cluster Owner. Ignoring setting DSCP marking");
-            return;
-        }
 
         BigInteger dpnId = getDpnForInterface(port.getUuid().getValue());
         String ifName = port.getUuid().getValue();
-        Interface ifState = getInterfaceStateFromOperDS(ifName);
-        Short dscpValue = dscpMark.getDscpMark();
 
         if (dpnId.equals(BigInteger.ZERO)) {
             LOG.info("DPN ID for interface {} not found. Cannot set dscp value {} on port {}",
                     port.getUuid().getValue(), dscpMark, port.getUuid().getValue());
             return;
         }
-        int ipVersions = getIpVersions(port);
-        //1. OF rules
-        if (hasIpv4Addr(ipVersions)) {
-            LOG.trace("setting ipv4 flow for port: {}, dscp: {}", ifName, dscpValue);
-            addFlow(dpnId, dscpValue, ifName, NwConstants.ETHTYPE_IPV4, ifState);
-        }
-        if (hasIpv6Addr(ipVersions)) {
-            LOG.trace("setting ipv6 flow for port: {}, dscp: {}", ifName, dscpValue);
-            addFlow(dpnId, dscpValue, ifName, NwConstants.ETHTYPE_IPV6, ifState);
-        }
 
-        if (qosServiceConfiguredPorts.add(port.getUuid())) {
-            // bind qos service to interface
-            bindservice(ifName);
+        if (!qosEosHandler.isQosClusterOwner()) {
+            qosServiceConfiguredPorts.add(port.getUuid());
+            LOG.trace("Not Qos Cluster Owner. Ignoring setting DSCP marking");
+            return;
+        } else {
+            Interface ifState = getInterfaceStateFromOperDS(ifName);
+            Short dscpValue = dscpMark.getDscpMark();
+            int ipVersions = getIpVersions(port);
+            //1. OF rules
+            if (hasIpv4Addr(ipVersions)) {
+                LOG.trace("setting ipv4 flow for port: {}, dscp: {}", ifName, dscpValue);
+                addFlow(dpnId, dscpValue, ifName, NwConstants.ETHTYPE_IPV4, ifState);
+            }
+            if (hasIpv6Addr(ipVersions)) {
+                LOG.trace("setting ipv6 flow for port: {}, dscp: {}", ifName, dscpValue);
+                addFlow(dpnId, dscpValue, ifName, NwConstants.ETHTYPE_IPV6, ifState);
+            }
+
+            if (qosServiceConfiguredPorts.add(port.getUuid())) {
+                // bind qos service to interface
+                bindservice(ifName);
+            }
+
         }
     }
 
     public void unsetPortDscpMark(Port port) {
-        if (!qosEosHandler.isQosClusterOwner()) {
-            LOG.debug("Not Qos Cluster Owner. Ignoring unsetting DSCP marking");
-            return;
-        }
 
         BigInteger dpnId = getDpnForInterface(port.getUuid().getValue());
         String ifName = port.getUuid().getValue();
@@ -553,25 +553,30 @@ public class QosNeutronUtils {
             LOG.debug("DPN ID for port {} not found. Cannot unset dscp value", port.getUuid().getValue());
             return;
         }
-        LOG.trace("Removing dscp marking rule from Port {}", port.getUuid().getValue());
-        Interface intf = getInterfaceStateFromOperDS(ifName);
-        //unbind service from interface
-        unbindservice(ifName);
-        // 1. OF
-        int ipVersions = getIpVersions(port);
-        if (hasIpv4Addr(ipVersions)) {
-            removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV4, intf);
-        }
-        if (hasIpv6Addr(ipVersions)) {
-            removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV6, intf);
-        }
-        qosServiceConfiguredPorts.remove(port.getUuid());
-    }
 
-    public void unsetPortDscpMark(Port port, Interface intrf) {
         if (!qosEosHandler.isQosClusterOwner()) {
+            qosServiceConfiguredPorts.remove(port.getUuid());
+            LOG.debug("Not Qos Cluster Owner. Ignoring unsetting DSCP marking");
             return;
+        } else {
+            LOG.trace("Removing dscp marking rule from Port {}", port.getUuid().getValue());
+            Interface intf = getInterfaceStateFromOperDS(ifName);
+            //unbind service from interface
+            unbindservice(ifName);
+            // 1. OF
+            int ipVersions = getIpVersions(port);
+            if (hasIpv4Addr(ipVersions)) {
+                removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV4, intf);
+            }
+            if (hasIpv6Addr(ipVersions)) {
+                removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV6, intf);
+            }
+            qosServiceConfiguredPorts.remove(port.getUuid());
         }
+    }
+
+    public void unsetPortDscpMark(Port port, Interface intrf) {
+
         BigInteger dpnId = getDpIdFromInterface(intrf);
         String ifName = port.getUuid().getValue();
 
@@ -579,16 +584,21 @@ public class QosNeutronUtils {
             LOG.error("Unable to retrieve DPN Id for interface {}. Cannot unset dscp value on port", ifName);
             return;
         }
-        LOG.trace("Removing dscp marking rule from Port {}", port.getUuid().getValue());
-        unbindservice(ifName);
-        int ipVersions = getIpVersions(port);
-        if (hasIpv4Addr(ipVersions)) {
-            removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV4, intrf);
-        }
-        if (hasIpv6Addr(ipVersions)) {
-            removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV6, intrf);
+        if (!qosEosHandler.isQosClusterOwner()) {
+            qosServiceConfiguredPorts.remove(port.getUuid());
+            return;
+        } else {
+            LOG.trace("Removing dscp marking rule from Port {}", port.getUuid().getValue());
+            unbindservice(ifName);
+            int ipVersions = getIpVersions(port);
+            if (hasIpv4Addr(ipVersions)) {
+                removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV4, intrf);
+            }
+            if (hasIpv6Addr(ipVersions)) {
+                removeFlow(dpnId, ifName, NwConstants.ETHTYPE_IPV6, intrf);
+            }
+            qosServiceConfiguredPorts.remove(port.getUuid());
         }
-        qosServiceConfiguredPorts.remove(port.getUuid());
     }
 
     private static BigInteger getDpIdFromInterface(org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf
@@ -880,4 +890,17 @@ public class QosNeutronUtils {
     public boolean hasIpv6Addr(int versions) {
         return (versions & (1 << QosConstants.IPV6_ADDR_MASK_BIT)) != 0;
     }
+
+    public boolean isBindServiceDone(Optional<Uuid> uuid) {
+        if (uuid != null) {
+            return qosServiceConfiguredPorts.contains(uuid.get());
+        }
+        return false;
+    }
+
+    public void removeInterfaceInQosConfiguredPorts(Optional<Uuid> uuid) {
+        if (uuid != null) {
+            qosServiceConfiguredPorts.remove(uuid.get());
+        }
+    }
 }