Bug 7533 : Fix for bind/unbind in DHCP service

- If the unbind is called from interface state listeners there could be
  chances of ending up in a race condition in service bind logic and hence
  the flow does not get removed.
- Moved the bind service call to interface config listener rather than state
  so that the service flows get programmed as soon as the interface is UP.

Change-Id: Iadd3743e47efed9f256612fe8bab7085d3de2d54
Signed-off-by: Kency Kurian <kency.kurian@ericsson.com>

diff --git a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceConfigListener.java b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceConfigListener.java
index ab8382fcd6b9d4c911418704c13e70a398d7f309..98656169fb665647e1019efa18aeb71fa1ea9682 100644 (file)
--- a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceConfigListener.java
+++ b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceConfigListener.java
@@ -8,14 +8,26 @@
 
 package org.opendaylight.netvirt.dhcpservice;
 
+import com.google.common.util.concurrent.ListenableFuture;
+
+import java.util.ArrayList;
+import java.util.List;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase;
 import org.opendaylight.genius.datastoreutils.DataStoreJobCoordinator;
+import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.netvirt.dhcpservice.api.DhcpMConstants;
-import org.opendaylight.netvirt.dhcpservice.jobs.DhcpInterfaceConfigRemoveJob;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.Interfaces;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.Interface;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.IfL2vlan;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.IfTunnel;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.ParentRefs;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.Port;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.subnets.rev150712.subnets.attributes.subnets.Subnet;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -28,12 +40,15 @@ public class DhcpInterfaceConfigListener
 
     private final DataBroker dataBroker;
     private final DhcpExternalTunnelManager dhcpExternalTunnelManager;
+    private final DhcpManager dhcpManager;
     private DataStoreJobCoordinator dataStoreJobCoordinator;
 
-    public DhcpInterfaceConfigListener(DataBroker dataBroker, DhcpExternalTunnelManager dhcpExternalTunnelManager) {
+    public DhcpInterfaceConfigListener(DataBroker dataBroker,
+            DhcpExternalTunnelManager dhcpExternalTunnelManager, DhcpManager dhcpManager) {
         super(Interface.class, DhcpInterfaceConfigListener.class);
         this.dataBroker = dataBroker;
         this.dhcpExternalTunnelManager = dhcpExternalTunnelManager;
+        this.dhcpManager = dhcpManager;
         registerListener(LogicalDatastoreType.CONFIGURATION, dataBroker);
         dataStoreJobCoordinator = DataStoreJobCoordinator.getInstance();
     }
@@ -46,8 +61,31 @@ public class DhcpInterfaceConfigListener
 
     @Override
     protected void remove(InstanceIdentifier<Interface> identifier, Interface del) {
-        DhcpInterfaceConfigRemoveJob job = new DhcpInterfaceConfigRemoveJob(dhcpExternalTunnelManager, dataBroker, del);
-        dataStoreJobCoordinator.enqueueJob(DhcpServiceUtils.getJobKey(del.getName()), job, DhcpMConstants.RETRY_COUNT );
+        dataStoreJobCoordinator.enqueueJob(DhcpServiceUtils.getJobKey(del.getName()), () -> {
+            List<ListenableFuture<Void>> futures = new ArrayList<>();
+            IfTunnel tunnelInterface = del.getAugmentation(IfTunnel.class);
+            IfL2vlan vlanInterface = del.getAugmentation(IfL2vlan.class);
+            String interfaceName = del.getName();
+            if (tunnelInterface != null && !tunnelInterface.isInternal()) {
+                IpAddress tunnelIp = tunnelInterface.getTunnelDestination();
+                ParentRefs interfce = del.getAugmentation(ParentRefs.class);
+                if (interfce != null) {
+                    if (LOG.isTraceEnabled()) {
+                        LOG.trace("Calling handleTunnelStateDown for tunnelIp {} and interface {}",
+                                tunnelIp, interfaceName);
+                    }
+                    dhcpExternalTunnelManager.handleTunnelStateDown(tunnelIp,
+                            interfce.getDatapathNodeIdentifier(), futures);
+                    return futures;
+                }
+            }
+            if (vlanInterface != null) {
+                WriteTransaction unbindTx = dataBroker.newWriteOnlyTransaction();
+                DhcpServiceUtils.unbindDhcpService(interfaceName, unbindTx);
+                futures.add(unbindTx.submit());
+            }
+            return futures;
+        }, DhcpMConstants.RETRY_COUNT );
     }
 
     @Override
@@ -57,6 +95,25 @@ public class DhcpInterfaceConfigListener
 
     @Override
     protected void add(InstanceIdentifier<Interface> identifier, Interface add) {
+        dataStoreJobCoordinator.enqueueJob(DhcpServiceUtils.getJobKey(add.getName()), () -> {
+            List<ListenableFuture<Void>> futures = new ArrayList<>();
+            String interfaceName = add.getName();
+            IfL2vlan vlanInterface = add.getAugmentation(IfL2vlan.class);
+            if (vlanInterface == null) {
+                return futures;
+            }
+            Port port = dhcpManager.getNeutronPort(interfaceName);
+            Subnet subnet = dhcpManager.getNeutronSubnet(port);
+            if (null != subnet && subnet.isEnableDhcp()) {
+                WriteTransaction bindServiceTx = dataBroker.newWriteOnlyTransaction();
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("Binding DHCP service for interface {}", interfaceName);
+                }
+                DhcpServiceUtils.bindDhcpService(interfaceName, NwConstants.DHCP_TABLE, bindServiceTx);
+                futures.add(bindServiceTx.submit());
+            }
+            return futures;
+        }, DhcpMConstants.RETRY_COUNT );
     }
 
     @Override
@@ -68,4 +125,4 @@ public class DhcpInterfaceConfigListener
     protected DhcpInterfaceConfigListener getDataTreeChangeListener() {
         return DhcpInterfaceConfigListener.this;
     }
-}
+}
\ No newline at end of file

diff --git a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceEventListener.java b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceEventListener.java
index 84fe722f169dcf5d949038537dd448d64d5655f0..dd91acd04aa2e3faaf9c45117e727e6e408b791d 100644 (file)
--- a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceEventListener.java
+++ b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpInterfaceEventListener.java
@@ -55,7 +55,7 @@ public class DhcpInterfaceEventListener
     @Override
     public void close() throws Exception {
         super.close();
-        LOG.info("Interface Manager Closed");
+        LOG.info("DhcpInterfaceEventListener Closed");
     }
 
     @Override

diff --git a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpManager.java b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpManager.java
index 4a8af1ad323d7117a7b76cd9749be634267b6a58..81275f816c749f22921612569bd8cf68ce37da51 100644 (file)
--- a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpManager.java
+++ b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/DhcpManager.java
@@ -65,7 +65,7 @@ public class DhcpManager {
         if (config.isControllerDhcpEnabled()) {
             dhcpInterfaceEventListener =
                     new DhcpInterfaceEventListener(this, broker, dhcpExternalTunnelManager, interfaceManager);
-            dhcpInterfaceConfigListener = new DhcpInterfaceConfigListener(broker, dhcpExternalTunnelManager);
+            dhcpInterfaceConfigListener = new DhcpInterfaceConfigListener(broker, dhcpExternalTunnelManager, this);
             LOG.info("DHCP Service initialized");
         }
     }

diff --git a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceAddJob.java b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceAddJob.java
index f1bf7a133cce1385e805fb29f03471ac3e4fbb06..a2e195285ec68560443ce3b7cf5e0ca7cb4406bf 100644 (file)
--- a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceAddJob.java
+++ b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceAddJob.java
@@ -10,17 +10,18 @@ package org.opendaylight.netvirt.dhcpservice.jobs;
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.ListenableFuture;
+
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Callable;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager;
 import org.opendaylight.genius.mdsalutil.MDSALDataStoreUtils;
 import org.opendaylight.genius.mdsalutil.MDSALUtil;
-import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.netvirt.dhcpservice.DhcpExternalTunnelManager;
 import org.opendaylight.netvirt.dhcpservice.DhcpManager;
 import org.opendaylight.netvirt.dhcpservice.DhcpServiceUtils;
@@ -101,10 +102,7 @@ public class DhcpInterfaceAddJob implements Callable<List<ListenableFuture<Void>
     private void installDhcpEntries(String interfaceName, BigInteger dpId, List<ListenableFuture<Void>> futures) {
         String vmMacAddress = getAndUpdateVmMacAddress(interfaceName);
         WriteTransaction flowTx = dataBroker.newWriteOnlyTransaction();
-        WriteTransaction bindServiceTx = dataBroker.newWriteOnlyTransaction();
-        DhcpServiceUtils.bindDhcpService(interfaceName, NwConstants.DHCP_TABLE, bindServiceTx);
         dhcpManager.installDhcpEntries(dpId, vmMacAddress, flowTx);
-        futures.add(bindServiceTx.submit());
         futures.add(flowTx.submit());
     }
 

diff --git a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceConfigRemoveJob.java b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceConfigRemoveJob.java
deleted file mode 100644 (file)
index b9dcc95..0000000
--- a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceConfigRemoveJob.java
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Copyright (c) 2016 Ericsson India Global Services Pvt Ltd. and others.  All rights reserved.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License v1.0 which accompanies this distribution,
- * and is available at http://www.eclipse.org/legal/epl-v10.html
- */
-package org.opendaylight.netvirt.dhcpservice.jobs;
-
-import com.google.common.util.concurrent.ListenableFuture;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.Callable;
-import org.opendaylight.controller.md.sal.binding.api.DataBroker;
-import org.opendaylight.netvirt.dhcpservice.DhcpExternalTunnelManager;
-import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
-import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.Interface;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.IfTunnel;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.ParentRefs;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-public class DhcpInterfaceConfigRemoveJob implements Callable<List<ListenableFuture<Void>>> {
-
-    private static final Logger LOG = LoggerFactory.getLogger(DhcpInterfaceConfigRemoveJob.class);
-    DhcpExternalTunnelManager dhcpExternalTunnelManager;
-    DataBroker dataBroker;
-    Interface iface;
-
-    public DhcpInterfaceConfigRemoveJob(DhcpExternalTunnelManager dhcpExternalTunnelManager, DataBroker dataBroker,
-            Interface iface) {
-        super();
-        this.dhcpExternalTunnelManager = dhcpExternalTunnelManager;
-        this.dataBroker = dataBroker;
-        this.iface = iface;
-    }
-
-    @Override
-    public List<ListenableFuture<Void>> call() throws Exception {
-        List<ListenableFuture<Void>> futures = new ArrayList<>();
-        IfTunnel tunnelInterface = iface.getAugmentation(IfTunnel.class);
-        if (tunnelInterface != null && !tunnelInterface.isInternal()) {
-            IpAddress tunnelIp = tunnelInterface.getTunnelDestination();
-            ParentRefs interfce = iface.getAugmentation(ParentRefs.class);
-            if (interfce != null) {
-                LOG.trace("Calling handleTunnelStateDown for tunnelIp {} and interface {}", tunnelIp, iface.getName());
-                dhcpExternalTunnelManager.handleTunnelStateDown(tunnelIp,
-                        interfce.getDatapathNodeIdentifier(), futures);
-            }
-        }
-        return futures;
-    }
-}
\ No newline at end of file

diff --git a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceRemoveJob.java b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceRemoveJob.java
index 4ca52d5f9367564e8e6cd7c699a2fe42320bb48e..e604bdd01ad3510c905a2aaa55233ca35769b7cd 100644 (file)
--- a/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceRemoveJob.java
+++ b/vpnservice/dhcpservice/dhcpservice-impl/src/main/java/org/opendaylight/netvirt/dhcpservice/jobs/DhcpInterfaceRemoveJob.java
@@ -88,11 +88,8 @@ public class DhcpInterfaceRemoveJob implements Callable<List<ListenableFuture<Vo
     private void unInstallDhcpEntries(String interfaceName, BigInteger dpId, List<ListenableFuture<Void>> futures) {
         String vmMacAddress = getAndRemoveVmMacAddress(interfaceName);
         WriteTransaction flowTx = dataBroker.newWriteOnlyTransaction();
-        WriteTransaction unbindTx = dataBroker.newWriteOnlyTransaction();
-        DhcpServiceUtils.unbindDhcpService(interfaceName, unbindTx);
         dhcpManager.unInstallDhcpEntries(dpId, vmMacAddress, flowTx);
         futures.add(flowTx.submit());
-        futures.add(unbindTx.submit());
     }
 
     private String getAndRemoveVmMacAddress(String interfaceName) {