Napt Switch Over not happening

Description : With using heat template for clean-up, it's been observed
that when the interface is removed from router north-bound event is
received, the ietf-interface entry in config DS been already deleted. As a
result, the router-interface remove event returns with only deletion of
interface-to-router mapping and not deleting any entry from
neutron-router-dpns(as unable to obtained dpn for the interface).

When interface-state south bound event is triggered, interface-to-router
mapping is already deleted and this event also returns without deleting
entry from neutron-router-dpns. As a result, the earlier continues to act
as napt switch for a given router even through it doesn't has any router
presence.

Changes are done to prevent immediate deletion of interface-to-router
mapping during north-bound event, so that this interface-to-router data is
available during south-bound event resulting in proper deletion of
neutron-router-dpns entry and triggers Napt Switch Re-election.

Issue : NETVIRT-1614

Change-Id: Ib9e8a6ea2b59ad073b9e6e3551bd6df1bd44fd96
Signed-off-by: Chetan Arakere Gowdru <chetan.arakere@altencalsoftlabs.com>

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatConstants.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatConstants.java
index 4b631dfefc44fac39df1b66194c97c4522fb1a4b..1c9641efdbf446bd4247c00556a4b084e5d763fe 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatConstants.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatConstants.java
@@ -50,6 +50,7 @@ public interface NatConstants {
     String NAT_DJC_PREFIX = "NAT-";
     int NAT_DJC_MAX_RETRIES = 3;
     int DEFAULT_VPN_INTERNAL_TUNNEL_TABLE_PRIORITY = 8;
+    String NETWORK_ROUTER_INTERFACE = "network:router_interface";
     // Flow Actions
     int ADD_FLOW = 0;
     int DEL_FLOW = 1;

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatRouterInterfaceListener.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatRouterInterfaceListener.java
index 19b043d2e165b7e719e621b801b7d7b9f721c124..53c99eb304e8028b3e7956d996a556e2dcfd671c 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatRouterInterfaceListener.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatRouterInterfaceListener.java
@@ -85,7 +85,10 @@ public class NatRouterInterfaceListener
         LOG.trace("add : Add event - key: {}, value: {}", interfaceInfo.key(), interfaceInfo);
         final String routerId = identifier.firstKeyOf(RouterInterfaces.class).getRouterId().getValue();
         final String interfaceName = interfaceInfo.getInterfaceId();
-
+        if (NatUtil.isRouterInterfacePort(dataBroker, interfaceName)) {
+            LOG.info("ADD: Ignoring Router Interface Port {} for processing of router {}", interfaceName, routerId);
+            return;
+        }
         try {
             MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION,
                 NatUtil.getRouterInterfaceId(interfaceName), getRouterInterface(interfaceName, routerId));
@@ -96,7 +99,7 @@ public class NatRouterInterfaceListener
         org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces
             .state.Interface interfaceState = NatUtil.getInterfaceStateFromOperDS(dataBroker, interfaceName);
         if (interfaceState != null) {
-            BigInteger dpId = NatUtil.getDpnForInterface(interfaceManager, interfaceName);
+            BigInteger dpId = NatUtil.getDpIdFromInterface(interfaceState);
             if (dpId.equals(BigInteger.ZERO)) {
                 LOG.warn("ADD : Could not retrieve dp id for interface {} to handle router {} association model",
                         interfaceName, routerId);
@@ -113,6 +116,7 @@ public class NatRouterInterfaceListener
             } finally {
                 lock.unlock();
             }
+            LOG.info("ADD: Added neutron-router-dpns mapping for interface {} of router {}", interfaceName, routerId);
         } else {
             LOG.info("add : Interface {} not yet operational to handle router interface add event in router {}",
                     interfaceName, routerId);
@@ -124,34 +128,41 @@ public class NatRouterInterfaceListener
         LOG.trace("remove : Remove event - key: {}, value: {}", interfaceInfo.key(), interfaceInfo);
         final String routerId = identifier.firstKeyOf(RouterInterfaces.class).getRouterId().getValue();
         final String interfaceName = interfaceInfo.getInterfaceId();
-
-        //Delete the RouterInterfaces maintained in the ODL:L3VPN configuration model
-        ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
-            confTx -> confTx.delete(NatUtil.getRouterInterfaceId(interfaceName))), LOG,
-            "Error handling NAT router interface removal");
-
-        BigInteger dpId = NatUtil.getDpnForInterface(interfaceManager, interfaceName);
-        if (dpId.equals(BigInteger.ZERO)) {
-            LOG.warn("REMOVE : Could not retrieve DPN ID for interface {} to handle router {} dissociation model",
+        org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.state.Interface
+            interfaceState = NatUtil.getInterfaceStateFromOperDS(dataBroker, interfaceName);
+        if (interfaceState != null) {
+            BigInteger dpId = NatUtil.getDpIdFromInterface(interfaceState);
+            if (dpId.equals(BigInteger.ZERO)) {
+                LOG.warn(
+                    "REMOVE : Could not retrieve DPN ID for interface {} to handle router {} dissociation model",
                     interfaceName, routerId);
-            return;
-        }
+                return;
+            }
 
-        final ReentrantLock lock = NatUtil.lockForNat(dpId);
-        lock.lock();
-        try {
-            if (NatUtil.isSnatEnabledForRouterId(dataBroker, routerId)) {
-                NatUtil.removeSnatEntriesForPort(dataBroker, naptManager, mdsalManager, neutronVpnService,
-                    interfaceName, routerId);
+            final ReentrantLock lock = NatUtil.lockForNat(dpId);
+            lock.lock();
+            try {
+                if (NatUtil.isSnatEnabledForRouterId(dataBroker, routerId)) {
+                    NatUtil.removeSnatEntriesForPort(dataBroker, naptManager, mdsalManager, neutronVpnService,
+                        interfaceName, routerId);
+                }
+                ListenableFutures.addErrorLogging(
+                    txRunner.callWithNewReadWriteTransactionAndSubmit(OPERATIONAL, operTx -> {
+                        //Delete the NeutronRouterDpnMap from the ODL:L3VPN operational model
+                        NatUtil
+                            .removeFromNeutronRouterDpnsMap(routerId, interfaceName, dpId, operTx);
+                        //Delete the DpnRouterMap from the ODL:L3VPN operational model
+                        NatUtil.removeFromDpnRoutersMap(dataBroker, routerId, interfaceName, dpId,
+                            interfaceManager, operTx);
+                    }), LOG, "Error handling NAT router interface removal");
+                //Delete the RouterInterfaces maintained in the ODL:L3VPN configuration model
+                ListenableFutures
+                    .addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
+                        confTx -> confTx.delete(NatUtil.getRouterInterfaceId(interfaceName))), LOG,
+                        "Error handling NAT router interface removal");
+            } finally {
+                lock.unlock();
             }
-            ListenableFutures.addErrorLogging(txRunner.callWithNewReadWriteTransactionAndSubmit(OPERATIONAL, operTx -> {
-                //Delete the NeutronRouterDpnMap from the ODL:L3VPN operational model
-                NatUtil.removeFromNeutronRouterDpnsMap(routerId, interfaceName, dpId, operTx);
-                //Delete the DpnRouterMap from the ODL:L3VPN operational model
-                NatUtil.removeFromDpnRoutersMap(dataBroker, routerId, interfaceName, dpId, interfaceManager, operTx);
-            }), LOG, "Error handling NAT router interface removal");
-        } finally {
-            lock.unlock();
         }
     }
 

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatSouthboundEventHandlers.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatSouthboundEventHandlers.java
index 881ff6d558b99c597dc59a752fdb164b06154907..0895d42198fa785b33388d13c53928bc513579b0 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatSouthboundEventHandlers.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatSouthboundEventHandlers.java
@@ -29,6 +29,7 @@ import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.genius.datastoreutils.SingleTransactionDataBroker;
 import org.opendaylight.genius.infra.Datastore.Operational;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunner;
@@ -102,12 +103,27 @@ public class NatSouthboundEventHandlers {
     public void handleAdd(String interfaceName, BigInteger intfDpnId,
                           RouterInterface routerInterface, @Nullable VipState vipState) {
         String routerName = routerInterface.getRouterName();
-        NatInterfaceStateAddWorker natIfaceStateAddWorker = new NatInterfaceStateAddWorker(interfaceName,
+        if (NatUtil.validateIsIntefacePartofRouter(dataBroker, routerName, interfaceName)) {
+            NatInterfaceStateAddWorker natIfaceStateAddWorker = new NatInterfaceStateAddWorker(
+                interfaceName,
                 intfDpnId, routerName);
-        coordinator.enqueueJob(NAT_DS + "-" + interfaceName, natIfaceStateAddWorker);
+            coordinator.enqueueJob(NAT_DS + "-" + interfaceName, natIfaceStateAddWorker);
 
-        NatFlowAddWorker natFlowAddWorker = new NatFlowAddWorker(interfaceName, routerName, intfDpnId, vipState);
-        coordinator.enqueueJob(NAT_DS + "-" + interfaceName, natFlowAddWorker, NatConstants.NAT_DJC_MAX_RETRIES);
+            NatFlowAddWorker natFlowAddWorker = new NatFlowAddWorker(interfaceName, routerName,
+                intfDpnId, vipState);
+            coordinator.enqueueJob(NAT_DS + "-" + interfaceName, natFlowAddWorker,
+                NatConstants.NAT_DJC_MAX_RETRIES);
+        } else {
+            LOG.error("NAT Service : Router {} not valid for interface {} during add. Deleting it from "
+                + "router-interfaces DS", routerName, routerInterface);
+            try {
+                SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                    NatUtil.getRouterInterfaceId(interfaceName));
+            } catch (TransactionCommitFailedException e) {
+                LOG.error("NAT Service : Failed to stale Router Interface entry for interface {}",
+                    interfaceName, e);
+            }
+        }
     }
 
     public void handleRemove(String interfaceName, BigInteger intfDpnId, RouterInterface routerInterface) {
@@ -119,17 +135,33 @@ public class NatSouthboundEventHandlers {
         NatFlowRemoveWorker natFlowRemoveWorker = new NatFlowRemoveWorker(interfaceName, intfDpnId, routerName);
         coordinator.enqueueJob(NAT_DS + "-" + interfaceName, natFlowRemoveWorker,
                 NatConstants.NAT_DJC_MAX_RETRIES);
+        // Validate whether the routerInterface is still part of given router in router-interface-map
+        if (!NatUtil.validateIsIntefacePartofRouter(dataBroker, routerName, interfaceName)) {
+            LOG.error("NAT Service : Router {} not valid for interface {} during remove. "
+                + "Deleting it from router-interfaces DS", routerName, routerInterface);
+            try {
+                SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                    NatUtil.getRouterInterfaceId(interfaceName));
+            } catch (TransactionCommitFailedException e) {
+                LOG.error("NAT Service : Failed to stale Router Interface entry for interface {}",
+                    interfaceName, e);
+            }
+        }
     }
 
     public void handleUpdate(Interface original, Interface update,
                              BigInteger intfDpnId, RouterInterface routerInterface) {
         String routerName = routerInterface.getRouterName();
-        NatInterfaceStateUpdateWorker natIfaceStateupdateWorker = new NatInterfaceStateUpdateWorker(original,
+        if (NatUtil.validateIsIntefacePartofRouter(dataBroker, routerName, update.getName())) {
+            NatInterfaceStateUpdateWorker natIfaceStateupdateWorker = new NatInterfaceStateUpdateWorker(
+                original,
                 update, intfDpnId, routerName);
-        coordinator.enqueueJob(NAT_DS + "-" + update.getName(), natIfaceStateupdateWorker);
-        NatFlowUpdateWorker natFlowUpdateWorker = new NatFlowUpdateWorker(original, update, routerName);
-        coordinator.enqueueJob(NAT_DS + "-" + update.getName(), natFlowUpdateWorker,
+            coordinator.enqueueJob(NAT_DS + "-" + update.getName(), natIfaceStateupdateWorker);
+            NatFlowUpdateWorker natFlowUpdateWorker = new NatFlowUpdateWorker(original, update,
+                routerName);
+            coordinator.enqueueJob(NAT_DS + "-" + update.getName(), natFlowUpdateWorker,
                 NatConstants.NAT_DJC_MAX_RETRIES);
+        }
     }
 
     void handleRouterInterfacesUpEvent(String routerName, String interfaceName, BigInteger dpId,

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
index 7d651ec48da74f97fabf57df84b48666fb053e5d..88ab42cba6a896c0395fc6e27b1eb9f5487b2fdb 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
@@ -256,6 +256,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.l3.rev150712.router
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.l3.rev150712.routers.attributes.routers.RouterKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.port.attributes.FixedIps;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.Port;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.PortKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.rev150712.Neutron;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.subnets.rev150712.subnets.attributes.Subnets;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.subnets.rev150712.subnets.attributes.subnets.Subnet;
@@ -361,6 +362,52 @@ public final class NatUtil {
         return vpnId;
     }
 
+    public static Boolean validateIsIntefacePartofRouter(DataBroker broker, String routerName, String interfaceName) {
+        InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router.interfaces
+            .map.router.interfaces.Interfaces> vmInterfaceIdentifier = getRoutersInterfacesIdentifier(routerName,
+            interfaceName);
+
+        Optional<org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router.interfaces
+            .map.router.interfaces.Interfaces> routerInterfacesData;
+        try {
+            routerInterfacesData = SingleTransactionDataBroker.syncReadOptional(broker,
+                LogicalDatastoreType.CONFIGURATION, vmInterfaceIdentifier);
+        } catch (ReadFailedException e) {
+            LOG.error("Read Failed Exception While read RouterInterface data for router {}", routerName, e);
+            routerInterfacesData = Optional.absent();
+        }
+        if (routerInterfacesData.isPresent()) {
+            return Boolean.TRUE;
+        } else {
+            return Boolean.FALSE;
+        }
+    }
+
+    private static InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn
+        .rev150602.router.interfaces
+        .map.router.interfaces.Interfaces> getRoutersInterfacesIdentifier(String routerName, String interfaceName) {
+        return InstanceIdentifier.builder(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn
+            .rev150602.RouterInterfacesMap.class)
+            .child(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router
+                    .interfaces.map.RouterInterfaces.class,
+                new org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router
+                    .interfaces.map.RouterInterfacesKey(new Uuid(routerName)))
+            .child(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router.interfaces
+                    .map.router.interfaces.Interfaces.class,
+                new org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router.interfaces
+                    .map.router.interfaces.InterfacesKey(interfaceName)).build();
+    }
+
+    private static InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn
+        .rev150602.router.interfaces.map.RouterInterfaces> getRoutersInterfacesIdentifier(String routerName) {
+        return InstanceIdentifier.builder(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn
+            .rev150602.RouterInterfacesMap.class)
+            .child(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router
+                    .interfaces.map.RouterInterfaces.class,
+                new org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router
+                    .interfaces.map.RouterInterfacesKey(new Uuid(routerName))).build();
+    }
+
     static InstanceIdentifier<RouterPorts> getRouterPortsId(String routerId) {
         return InstanceIdentifier.builder(FloatingIpInfo.class)
             .child(RouterPorts.class, new RouterPortsKey(routerId)).build();
@@ -1340,16 +1387,6 @@ public final class NatUtil {
         operTx.delete(routersListIdentifier);
     }
 
-    private static InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn
-        .rev150602.router.interfaces.map.RouterInterfaces> getRoutersInterfacesIdentifier(String routerName) {
-        return InstanceIdentifier.builder(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn
-            .rev150602.RouterInterfacesMap.class)
-            .child(org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router
-                    .interfaces.map.RouterInterfaces.class,
-                new org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.neutronvpn.rev150602.router
-                    .interfaces.map.RouterInterfacesKey(new Uuid(routerName))).build();
-    }
-
     private static InstanceIdentifier<RoutersList> getRoutersList(BigInteger dpnId, String routerName) {
         return InstanceIdentifier.builder(DpnRouters.class)
             .child(DpnRoutersList.class, new DpnRoutersListKey(dpnId))
@@ -2607,6 +2644,36 @@ public final class NatUtil {
         return null;
     }
 
+    public static Boolean isRouterInterfacePort(DataBroker broker, String ifaceName) {
+        Port neutronPort = getNeutronPort(broker, ifaceName);
+        if (neutronPort == null) {
+            return Boolean.TRUE;
+        } else {
+            return (NatConstants.NETWORK_ROUTER_INTERFACE.equalsIgnoreCase(neutronPort.getDeviceOwner()) ? Boolean.TRUE
+                : Boolean.FALSE);
+        }
+    }
+
+    private static Port getNeutronPort(DataBroker broker, String ifaceName) {
+        InstanceIdentifier<Port>
+            portsIdentifier = InstanceIdentifier.create(Neutron.class)
+            .child(org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.Ports.class)
+            .child(Port.class, new PortKey(new Uuid(ifaceName)));
+        Optional<Port> portsOptional;
+        try {
+            portsOptional = SingleTransactionDataBroker
+                .syncReadOptional(broker, LogicalDatastoreType.CONFIGURATION, portsIdentifier);
+        } catch (ReadFailedException e) {
+            LOG.error("Read Failed Exception While Reading Neutron Port for {}", ifaceName, e);
+            portsOptional = Optional.absent();
+        }
+        if (!portsOptional.isPresent()) {
+            LOG.error("getNeutronPort : No neutron ports found for interface {}", ifaceName);
+            return null;
+        }
+        return portsOptional.get();
+    }
+
     static ReentrantLock lockForNat(final BigInteger dataPath) {
         // FIXME: wrap this in an Identifier
         return JvmGlobalLocks.getLockForString(NatConstants.NAT_DJC_PREFIX + dataPath);