Bug 7447: Unexpected flows from T21 to T44 for FIP
[netvirt.git] / vpnservice / natservice / natservice-impl / src / main / java / org / opendaylight / netvirt / natservice / internal / ExternalRoutersListener.java
index f757d51976a07dc98c89aeaa9b1183ebeb1e40d3..11e93f7d590715b1c38930d62e19e361d6ff9d55 100644 (file)
@@ -16,17 +16,18 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.JdkFutureAdapters;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.math.BigInteger;
-import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.DataChangeListener;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
+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.mdsalutil.ActionInfo;
@@ -45,6 +46,7 @@ import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager;
 import org.opendaylight.netvirt.bgpmanager.api.IBgpManager;
 import org.opendaylight.netvirt.fibmanager.api.IFibManager;
 import org.opendaylight.netvirt.fibmanager.api.RouteOrigin;
+import org.opendaylight.netvirt.vpnmanager.api.IVpnManager;
 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.inet.types.rev130715.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;
@@ -79,7 +81,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev16011
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.ext.routers.RoutersKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.external.ips.counter.ExternalCounters;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.external.ips.counter.external.counters.ExternalIpCounter;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.external.networks.Networks;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.intext.ip.map.ip.mapping.IpMap;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.intext.ip.map.ip.mapping.IpMapBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.intext.ip.map.ip.mapping.IpMapKey;
@@ -128,6 +129,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     private final NaptEventHandler naptEventHandler;
     private final NaptPacketInHandler naptPacketInHandler;
     private final IFibManager fibManager;
+    private final IVpnManager vpnManager;
     private static final BigInteger COOKIE_TUNNEL = new BigInteger("9000000", 16);
     static final BigInteger COOKIE_VM_LFIB_TABLE = new BigInteger("8000022", 16);
 
@@ -143,7 +145,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                                    final SNATDefaultRouteProgrammer snatDefaultRouteProgrammer,
                                    final NaptEventHandler naptEventHandler,
                                    final NaptPacketInHandler naptPacketInHandler,
-                                   final IFibManager fibManager) {
+                                   final IFibManager fibManager,
+                                   final IVpnManager vpnManager) {
         super(Routers.class, ExternalRoutersListener.class);
         this.dataBroker = dataBroker;
         this.mdsalManager = mdsalManager;
@@ -159,8 +162,10 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         this.naptEventHandler = naptEventHandler;
         this.naptPacketInHandler = naptPacketInHandler;
         this.fibManager = fibManager;
+        this.vpnManager = vpnManager;
     }
 
+    @Override
     public void init() {
         LOG.info("{} init", getClass().getSimpleName());
         registerListener(LogicalDatastoreType.CONFIGURATION, dataBroker);
@@ -179,9 +184,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
         // Populate the router-id-name container
         String routerName = routers.getRouterName();
-        Long routerId = NatUtil.getVpnId(dataBroker, routerName);
-        RouterIds rtrs = new RouterIdsBuilder().setKey(new RouterIdsKey(routerId)).setRouterId(routerId).setRouterName(routerName).build();
-        MDSALUtil.syncWrite( dataBroker, LogicalDatastoreType.CONFIGURATION, getRoutersIdentifier(routerId), rtrs);
+        NatUtil.createRouterIdsConfigDS(dataBroker, routerName);
 
         LOG.info("NAT Service : Installing NAT default route on all dpns part of router {}", routers.getRouterName());
         try {
@@ -190,35 +193,30 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             LOG.debug("NAT Service : Exception {} while Installing NAT default route on all dpns part of router {}",ex,routers.getRouterName());
         }
 
+        long segmentId = NatUtil.getVpnId(dataBroker, routerName);
+        // Allocate Primary Napt Switch for this router
+        BigInteger primarySwitchId = getPrimaryNaptSwitch(routerName, segmentId);
+        if(primarySwitchId == null || primarySwitchId.equals(BigInteger.ZERO)){
+            LOG.error("NAT Service: Failed to get or allocated NAPT switch");
+            return;
+        }
+
+        handleRouterGwFlows(routers, primarySwitchId, NwConstants.ADD_FLOW);
         if( !routers.isEnableSnat()) {
-            LOG.info("NAT Service : SNAT is disabled for external router {} ", routers.getRouterName());
+            LOG.info("NAT Service : SNAT is disabled for external router {} ", routerName);
             return;
         }
 
-        handleEnableSnat(routers);
+        handleEnableSnat(routers, segmentId, primarySwitchId);
     }
 
-    public void handleEnableSnat(Routers routers){
+    public void handleEnableSnat(Routers routers, long segmentId, BigInteger primarySwitchId) {
         String routerName = routers.getRouterName();
         LOG.info("NAT Service : Handling SNAT for router {}", routerName);
 
-        long segmentId = NatUtil.getVpnId(dataBroker, routerName);
         naptManager.initialiseExternalCounter(routers, segmentId);
-
         subnetRegisterMapping(routers,segmentId);
 
-        // Allocate Primary Napt Switch for this router
-        BigInteger primarySwitchId = NatUtil.getPrimaryNaptfromRouterId(dataBroker,segmentId);
-        if (primarySwitchId != null && !primarySwitchId.equals(BigInteger.ZERO)) {
-            LOG.debug("NAT Service : Primary NAPT switch with DPN ID {} is already elected for router",primarySwitchId,routerName);
-            return;
-        }
-        primarySwitchId = naptSwitchSelector.selectNewNAPTSwitch(routerName);
-        LOG.debug("NAT Service : Primary NAPT switch DPN ID {}", primarySwitchId);
-        if(primarySwitchId == null || primarySwitchId.equals(BigInteger.ZERO)){
-            LOG.error("NAT Service : Unable to to select the primary NAPT switch");
-            return;
-        }
         LOG.debug("NAT Service : About to create and install outbound miss entry in Primary Switch {} for router {}", primarySwitchId, routerName);
 
         long bgpVpnId = NatConstants.INVALID_ID;
@@ -242,7 +240,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     } else {
                         LOG.debug("NAT Service : Handle NAPT switch");
                         handlePrimaryNaptSwitch(dpnId, routerName);
-                        installNaptPfibExternalOutputFlow(routers, routerName, dpnId);
+                        installNaptPfibExternalOutputFlow(routers, dpnId);
                     }
                 }
             }
@@ -352,19 +350,57 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         LOG.info("NAT Service : handleEnableSnat() Exit");
     }
 
-    private void installNaptPfibExternalOutputFlow(Routers routers, String routerName, BigInteger dpnId) {
-        Long routerId = NatUtil.getVpnId(dataBroker, routerName);
+    private BigInteger getPrimaryNaptSwitch(String routerName, long segmentId) {
+        // Allocate Primary Napt Switch for this router
+        BigInteger primarySwitchId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
+        if (primarySwitchId != null && !primarySwitchId.equals(BigInteger.ZERO)) {
+            LOG.debug("NAT Service : Primary NAPT switch with DPN ID {} is already elected for router",primarySwitchId,routerName);
+            return primarySwitchId;
+        }
+
+        primarySwitchId = naptSwitchSelector.selectNewNAPTSwitch(routerName);
+        LOG.debug("NAT Service : Primary NAPT switch DPN ID {}", primarySwitchId);
+        if(primarySwitchId == null || primarySwitchId.equals(BigInteger.ZERO)){
+            LOG.error("NAT Service : Unable to to select the primary NAPT switch");
+        }
 
-        String ip = getExternalIpFromRouter(routers);
-        Uuid subnetId = getSubnetIdOfIp(ip);
+        return primarySwitchId;
+    }
 
-        if (subnetId != null) {
-            FlowEntity postNaptFlowEntity = buildNaptFibExternalOutputFlowEntity(dpnId, routerId, subnetId, ip);
-            mdsalManager.installFlow(postNaptFlowEntity);
+    private void installNaptPfibExternalOutputFlow(Routers routers, BigInteger dpnId) {
+        Long extVpnId = NatUtil.getVpnId(dataBroker, routers.getNetworkId().getValue());
+        List<String> extIps = routers.getExternalIps();
+        installNaptPfibExternalOutputFlow(dpnId, extVpnId, extIps);
+    }
+
+    protected void installNaptPfibExternalOutputFlow(String routerName, Long routerId, BigInteger dpnId) {
+        Long extVpnId = NatUtil.getVpnId(dataBroker, routerId);
+        if (extVpnId == null || extVpnId == NatConstants.INVALID_ID) {
+            LOG.debug("installNaptPfibExternalOutputFlow - not found extVpnId for router {}", routerId);
+            extVpnId = routerId;
         }
+        List<String> externalIps = NatUtil.getExternalIpsFromRouter(dataBroker, routerName);
+        installNaptPfibExternalOutputFlow(dpnId, extVpnId, externalIps);
     }
 
-    private Uuid getSubnetIdOfIp(String ip) {
+    private void installNaptPfibExternalOutputFlow(BigInteger dpnId, Long extVpnId, List<String> externalIps) {
+        if (externalIps == null || externalIps.isEmpty()) {
+            LOG.debug("installNaptPfibExternalOutputFlow - empty external Ips list for dpnId {} extVpnId {}",
+                    dpnId, extVpnId);
+            return;
+        }
+        for (String ip : externalIps) {
+            Uuid subnetId = getSubnetIdForFixedIp(ip);
+            if (subnetId != null) {
+                LOG.debug("installNaptPfibExternalOutputFlow - dpnId {} extVpnId {} subnetId {} ip {}",
+                        dpnId, extVpnId, subnetId, ip);
+                FlowEntity postNaptFlowEntity = buildNaptFibExternalOutputFlowEntity(dpnId, extVpnId, subnetId, ip);
+                mdsalManager.installFlow(postNaptFlowEntity);
+            }
+        }
+    }
+
+    private Uuid getSubnetIdForFixedIp(String ip) {
         if (ip != null) {
             IpAddress externalIpv4Address = new IpAddress(new Ipv4Address(ip));
             Port port = NatUtil.getNeutronPortForRouterGetewayIp(dataBroker, externalIpv4Address);
@@ -374,15 +410,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         return null;
     }
 
-    private String getExternalIpFromRouter(Routers routers) {
-        List<String> extIps = routers.getExternalIps();
-        if (extIps != null && !extIps.isEmpty()) {
-            return extIps.get(0);
-        }
-        return null;
-    }
-
-    private void subnetRegisterMapping(Routers routerEntry,Long segmentId) {
+    protected void subnetRegisterMapping(Routers routerEntry,Long segmentId) {
         List<Uuid> subnetList = null;
         List<String> externalIps = null;
         LOG.debug("NAT Service : Fetching values from extRouters model");
@@ -647,7 +675,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     }
 
     protected void installSnatMissEntryForPrimrySwch(BigInteger dpnId, String routerName) {
-        LOG.debug("NAT Service : installSnatMissEntry called for for the primary NAOT switch dpnId {} ", dpnId);
+        LOG.debug("NAT Service : installSnatMissEntry called for for the primary NAPT switch dpnId {} ", dpnId);
         // Install miss entry pointing to group
         FlowEntity flowEntity = buildSnatFlowEntityForPrmrySwtch(dpnId, routerName);
         mdsalManager.installFlow(flowEntity);
@@ -692,7 +720,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         actionsInfo.add(actionSetField);
         LOG.debug("NAT Service : Setting the tunnel to the list of action infos {}", actionsInfo);
         actionsInfo.add(new ActionInfo(ActionType.group, new String[] {String.valueOf(groupId)}));
-        instructions.add(new InstructionInfo(InstructionType.write_actions, actionsInfo));
+        instructions.add(new InstructionInfo(InstructionType.apply_actions, actionsInfo));
         String flowRef = getFlowRefSnat(dpId, NwConstants.PSNAT_TABLE, routerName);
         FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, NwConstants.PSNAT_TABLE, flowRef,
                 NatConstants.DEFAULT_PSNAT_FLOW_PRIORITY, flowRef, 0, 0,
@@ -888,6 +916,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
         ArrayList<ActionInfo> listActionInfo = new ArrayList<>();
         ArrayList<InstructionInfo> instructionInfo = new ArrayList<>();
+        listActionInfo.add(new ActionInfo(ActionType.nx_load_in_port, new BigInteger[]{BigInteger.ZERO}));
         listActionInfo.add(new ActionInfo(ActionType.nx_resubmit, new String[] { Integer.toString(NwConstants.L3_FIB_TABLE) }));
         instructionInfo.add(new InstructionInfo(InstructionType.apply_actions, listActionInfo));
 
@@ -942,7 +971,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                         for (IpMap dbIpMap : dbIpMaps) {
                             String dbExternalIp = dbIpMap.getExternalIp();
                             //Select the IPMap, whose external IP is the IP for which FIB is installed
-                            if (externalIp.equals(dbExternalIp)) {
+                            if (dbExternalIp.contains(externalIp)) {
                                 String dbInternalIp = dbIpMap.getInternalIp();
                                 IpMapKey dbIpMapKey = dbIpMap.getKey();
                                 LOG.debug("Setting label {} for internalIp {} and externalIp {}", label, dbInternalIp, externalIp);
@@ -951,9 +980,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                                 externalIpInDsFlag++;
                             }
                         }
-                        if (externalIpInDsFlag <=0) {
-                            LOG.debug("NAT Service : External Ip {} not found in DS,Failed to update label {} for routerId {} in DS", externalIp, label, routerId);
-                            String errMsg = String.format("Failed to update label %s due to external Ip %s not found in DS for router %s", externalIp, label, routerId);
+                        if (externalIpInDsFlag <= 0) {
+                            LOG.debug("NAT Service : External Ip {} not found in DS, Failed to update label {} for routerId {} in DS", externalIp, label, routerId);
+                            String errMsg = String.format("Failed to update label %s due to external Ip %s not found in DS for router %s", label, externalIp, routerId);
                             return Futures.immediateFailedFuture(new Exception(errMsg));
                         }
                     } else {
@@ -972,8 +1001,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     makeTunnelTableEntry(dpnId, label, customInstructions);
                     makeLFibTableEntry(dpnId, label, tableId);
 
+                    String fibExternalIp = externalIp.contains("/32") ? externalIp : (externalIp + "/32");
                     CreateFibEntryInput input = new CreateFibEntryInputBuilder().setVpnName(vpnName).setSourceDpid(dpnId)
-                            .setIpAddress(externalIp).setServiceId(label).setInstruction(customInstructions).build();
+                            .setIpAddress(fibExternalIp).setServiceId(label).setInstruction(customInstructions).build();
                     Future<RpcResult<Void>> future = fibService.createFibEntry(input);
                     return JdkFutureAdapters.listenInPoolThread(future);
                 } else {
@@ -1057,7 +1087,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     protected void update(InstanceIdentifier<Routers> identifier, Routers original, Routers update) {
         String routerName = original.getRouterName();
         Long routerId = NatUtil.getVpnId(dataBroker, routerName);
-        BigInteger dpnId = NatUtil.getPrimaryNaptfromRouterId(dataBroker, routerId);
+        BigInteger dpnId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
         Uuid networkId = original.getNetworkId();
 
         // Check if its update on SNAT flag
@@ -1074,13 +1104,24 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     return;
                 }
                 List<String> externalIps = NatUtil.getExternalIpsForRouter(dataBroker,routerId);
-                handleDisableSnat(routerName, networkUuid, externalIps, false, null);
+                handleDisableSnat(original, networkUuid, externalIps, false, null, dpnId);
             } else {
+                // Allocate Primary Napt Switch for existing router
+                BigInteger primarySwitchId = getPrimaryNaptSwitch(routerName, routerId);
+                if(primarySwitchId == null || primarySwitchId.equals(BigInteger.ZERO)){
+                    LOG.error("NAT Service: Failed to get or allocated NAPT switch in ExternalRouterListener.Update()");
+                    return;
+                }
                 LOG.info("NAT Service : SNAT enabled for Router {}", original.getRouterName());
-                handleEnableSnat(original);
+                handleEnableSnat(original, routerId, primarySwitchId);
             }
         }
 
+        if (!Objects.equals(original.getExtGwMacAddress(), update.getExtGwMacAddress())) {
+            handleRouterGwFlows(original, dpnId, NwConstants.DEL_FLOW);
+            handleRouterGwFlows(update, dpnId, NwConstants.ADD_FLOW);
+        }
+
         //Check if the Update is on External IPs
         LOG.debug("NAT Service : Checking if this is update on External IPs");
         List<String> originalExternalIpsList = original.getExternalIps();
@@ -1092,6 +1133,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         SetView<String> addedExternalIps = Sets.difference(updatedExternalIps, originalExternalIps);
         if(addedExternalIps.size() != 0) {
             LOG.debug("NAT Service : Start processing of the External IPs addition during the update operation");
+            vpnManager.setupArpResponderFlowsToExternalNetworkIps(routerName, addedExternalIps, update.getExtGwMacAddress(), dpnId,
+                    update.getNetworkId(), null, NwConstants.ADD_FLOW);
+
             for (String addedExternalIp : addedExternalIps) {
                 /*
                     1) Do nothing in the IntExtIp model.
@@ -1112,6 +1156,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         SetView<String> removedExternalIps = Sets.difference(originalExternalIps, updatedExternalIps);
         if(removedExternalIps.size() > 0) {
             LOG.debug("NAT Service : Start processing of the External IPs removal during the update operation");
+            vpnManager.setupArpResponderFlowsToExternalNetworkIps(routerName, removedExternalIps, original.getExtGwMacAddress(),
+                    dpnId, networkId, null, NwConstants.DEL_FLOW);
+
             List<String> removedExternalIpsAsList = new ArrayList<>();
             for (String removedExternalIp : removedExternalIps) {
              /*
@@ -1380,7 +1427,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         }
     }
 
-    private Long checkExternalIpLabel(long routerId, String externalIp){
+    protected Long checkExternalIpLabel(long routerId, String externalIp){
         List<IpMap> ipMaps = naptManager.getIpMapList(dataBroker, routerId);
         for(IpMap ipMap : ipMaps){
             if(ipMap.getExternalIp().equals(externalIp)){
@@ -1423,19 +1470,44 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 LOG.error("NAT Service : Invalid routerId returned for routerName {}", routerName);
                 return;
             }
+
+            BigInteger primarySwitchId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
+            handleRouterGwFlows(router, primarySwitchId, NwConstants.DEL_FLOW);
             List<String> externalIps = NatUtil.getExternalIpsForRouter(dataBroker, routerId);
-            handleDisableSnat(routerName, networkUuid, externalIps, true, null);
+            handleDisableSnat(router, networkUuid, externalIps, true, null, primarySwitchId);
         }
     }
 
-    public void handleDisableSnat(String routerName, Uuid networkUuid, List<String> externalIps, boolean routerFlag, String vpnId){
+    private void handleRouterGwFlows(Routers router, BigInteger primarySwitchId, int addOrRemove) {
+        WriteTransaction writeTx = dataBroker.newWriteOnlyTransaction();
+        vpnManager.setupRouterGwMacFlow(router.getRouterName(), router.getExtGwMacAddress(), primarySwitchId,
+                router.getNetworkId(), writeTx, addOrRemove);
+        vpnManager.setupArpResponderFlowsToExternalNetworkIps(router.getRouterName(), router.getExternalIps(),
+                router.getExtGwMacAddress(), primarySwitchId, router.getNetworkId(), writeTx, addOrRemove);
+        writeTx.submit();
+    }
+
+    public void handleDisableSnat(Routers router, Uuid networkUuid, List<String> externalIps, boolean routerFlag,
+            String vpnId, BigInteger naptSwitchDpnId) {
         LOG.info("NAT Service : handleDisableSnat() Entry");
+        String routerName = router.getRouterName();
         try {
             Long routerId = NatUtil.getVpnId(dataBroker, routerName);
+            //Use the NaptMananager removeMapping API to remove the entire list of IP addresses maintained for the router ID.
+            LOG.debug("NAT Service : Remove the Internal to external IP address maintained for the router ID {} in the DS", routerId);
+            naptManager.removeMapping(routerId);
 
-            BigInteger naptSwitchDpnId = NatUtil.getPrimaryNaptfromRouterId(dataBroker, routerId);
-            LOG.debug("NAT Service : got primarySwitch as dpnId{} ", naptSwitchDpnId);
-            if (naptSwitchDpnId == null || naptSwitchDpnId.equals(BigInteger.ZERO)){
+            if (routerFlag) {
+                removeNaptSwitch(routerName);
+            } else {
+                updateNaptSwitch(routerName, BigInteger.ZERO);
+            }
+
+            LOG.debug("NAT Service : Remove the ExternalCounter model for the router ID {}", routerId);
+            naptManager.removeExternalCounter(routerId);
+
+            LOG.debug("NAT Service : got primarySwitch as dpnId {}", naptSwitchDpnId);
+            if (naptSwitchDpnId == null || naptSwitchDpnId.equals(BigInteger.ZERO)) {
                 LOG.error("NAT Service : Unable to retrieve the primary NAPT switch for the router ID {} from RouterNaptSwitch model", routerId);
                 return;
             }
@@ -1447,18 +1519,6 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 LOG.debug("Failed to remove fib entries for routerId {} in naptSwitchDpnId {} : {}", routerId, naptSwitchDpnId,ex);
             }
 
-            //Use the NaptMananager removeMapping API to remove the entire list of IP addresses maintained for the router ID.
-            LOG.debug("NAT Service : Remove the Internal to external IP address maintained for the router ID {} in the DS", routerId);
-            naptManager.removeMapping(routerId);
-
-            if(routerFlag) {
-                removeNaptSwitch(routerName);
-            } else {
-                updateNaptSwitch(routerName, BigInteger.ZERO);
-            }
-
-            LOG.debug("NAT Service : Remove the ExternalCounter model for the router ID {}", routerId);
-            naptManager.removeExternalCounter(routerId);
         } catch (Exception ex) {
             LOG.error("Exception while handling disableSNAT : {}", ex);
         }
@@ -1538,7 +1598,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         LOG.info("NAT Service : Remove the flow in the " + NwConstants.OUTBOUND_NAPT_TABLE + " for the active switch with the DPN ID {} and router ID {}", dpnId, routerId);
         mdsalManager.removeFlow(outboundNatFlowEntity);
 
-        removeNaptFibExternalOutputFlows(routerId, dpnId, externalIps);
+        removeNaptFibExternalOutputFlows(routerId, dpnId, networkId, externalIps);
 
         //Remove the NAPT PFIB TABLE which forwards the incoming packet to FIB Table matching on the router ID.
         String natPfibFlowRef = getFlowRefTs(dpnId, NwConstants.NAPT_PFIB_TABLE, routerId);
@@ -1614,12 +1674,30 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         }
     }
 
-    private void removeNaptFibExternalOutputFlows(long routerId, BigInteger dpnId, List<String> externalIps) {
+    protected void removeNaptFibExternalOutputFlows(long routerId, BigInteger dpnId, Uuid networkId,
+            List<String> externalIps) {
+        Long extVpnId = null;
+        if (networkId != null) {
+            Uuid vpnUuid = NatUtil.getVpnIdfromNetworkId(dataBroker, networkId);
+            if (vpnUuid != null) {
+                extVpnId = NatUtil.getVpnId(dataBroker, vpnUuid.getValue());
+            } else {
+                LOG.debug("NAT Service: removeNaptFibExternalOutputFlows - vpnUuid is null");
+            }
+        } else {
+            LOG.debug("NAT Service: removeNaptFibExternalOutputFlows - networkId is null");
+            extVpnId = NatUtil.getVpnId(dataBroker, routerId);
+        }
+        if (extVpnId == null || extVpnId == NatConstants.INVALID_ID) {
+            LOG.debug("removeNaptFibExternalOutputFlows - extVpnId not found for routerId {}", routerId);
+            extVpnId = routerId;
+        }
         for (String ip : externalIps) {
             String extIp = removeMaskFromIp(ip);
-            String postNaptFlowRef = getFlowRefNaptFib(dpnId, NwConstants.NAPT_PFIB_TABLE, routerId, extIp);
-            LOG.info("NAT Service : Remove the flow in the " + NwConstants.NAPT_PFIB_TABLE + " for the active switch with the DPN ID {} and router ID {} and IP {}", dpnId, routerId, extIp);
-            mdsalManager.removeFlow(NatUtil.buildFlowEntity(dpnId,NwConstants.NAPT_PFIB_TABLE, postNaptFlowRef));
+            String naptFlowRef = getFlowRefNaptFib(dpnId, NwConstants.NAPT_PFIB_TABLE, extVpnId, extIp);
+            LOG.info("NAT Service: Remove the flow in the " + NwConstants.NAPT_PFIB_TABLE + " for the active switch"
+                    + " with the DPN ID {} and router ID {} and IP {} flowRef {}", dpnId, routerId, extIp, naptFlowRef);
+            mdsalManager.removeFlow(NatUtil.buildFlowEntity(dpnId,NwConstants.NAPT_PFIB_TABLE, naptFlowRef));
         }
     }
 
@@ -1969,7 +2047,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
     /**
      * router association to vpn
-     *
+     *@param routerName - Name of router
+     *@param bgpVpnName BGP VPN name
      */
     public void changeLocalVpnIdToBgpVpnId(String routerName, String bgpVpnName){
         LOG.debug("NAT Service : Router associated to BGP VPN");
@@ -2000,7 +2079,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
     /**
      * router disassociation from vpn
-     *
+     *@param routerName - Name of router
+     *@param bgpVpnName BGP VPN name
      */
     public void changeBgpVpnIdToLocalVpnId(String routerName, String bgpVpnName){
         LOG.debug("NAT Service : Router dissociated from BGP VPN");
@@ -2159,7 +2239,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         actionsInfo.add(actionSetField);
         LOG.debug("NAT Service : Setting the tunnel to the list of action infos {}", actionsInfo);
         actionsInfo.add(new ActionInfo(ActionType.group, new String[] {String.valueOf(groupId)}));
-        instructions.add(new InstructionInfo(InstructionType.write_actions, actionsInfo));
+        instructions.add(new InstructionInfo(InstructionType.apply_actions, actionsInfo));
         String flowRef = getFlowRefSnat(dpId, NwConstants.PSNAT_TABLE, routerName);
         FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, NwConstants.PSNAT_TABLE, flowRef,
                 NatConstants.DEFAULT_PSNAT_FLOW_PRIORITY, flowRef, 0, 0,
@@ -2250,27 +2330,24 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         return flowEntity;
     }
 
-    protected FlowEntity buildNaptFibExternalOutputFlowEntity(BigInteger dpId, long routerId, Uuid subnetId, String externalIp) {
-        LOG.debug("NAT Service : buildPostSnatFlowEntity called for dpId {}, routerId {}, srcIp {}", dpId, routerId,
+    protected FlowEntity buildNaptFibExternalOutputFlowEntity(BigInteger dpId, long extVpnId, Uuid subnetId, String externalIp) {
+        LOG.debug("NAT Service : buildNaptFibExternalOutputFlowEntity called for dpId {}, routerId {}, srcIp {}", dpId, extVpnId,
                 externalIp);
 
         List<MatchInfo> matches = new ArrayList<>();
         matches.add(new MatchInfo(MatchFieldType.eth_type, new long[] { NwConstants.ETHTYPE_IPV4 }));
         matches.add(new MatchInfo(MatchFieldType.metadata,
-                new BigInteger[] { MetaDataUtil.getVpnIdMetadata(routerId), MetaDataUtil.METADATA_MASK_VRFID }));
+                new BigInteger[] { MetaDataUtil.getVpnIdMetadata(extVpnId), MetaDataUtil.METADATA_MASK_VRFID }));
         matches.add(new MatchInfo(MatchFieldType.ipv4_source, new String[] { externalIp , "32" }));
 
         List<InstructionInfo> instructions = new ArrayList<>();
         List<ActionInfo> actionsInfos = new ArrayList<>();
         instructions.add(new InstructionInfo(InstructionType.apply_actions, actionsInfos));
-        instructions.add(new InstructionInfo(InstructionType.write_metadata,
-                new BigInteger[] { MetaDataUtil.getVpnIdMetadata(routerId), MetaDataUtil.METADATA_MASK_VRFID }));
-
         long groupId = NatUtil.createGroupId(NatUtil.getGroupIdKey(subnetId.getValue()), idManager);
         actionsInfos.add(new ActionInfo(ActionType.group, new String[] { String.valueOf(groupId) }));
 
-        String flowRef = getFlowRefNaptFib(dpId, NwConstants.NAPT_PFIB_TABLE, routerId, externalIp);
-        BigInteger cookie = getCookieOutboundFlow(routerId);
+        String flowRef = getFlowRefNaptFib(dpId, NwConstants.NAPT_PFIB_TABLE, extVpnId, externalIp);
+        BigInteger cookie = getCookieOutboundFlow(extVpnId);
         FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, NwConstants.NAPT_PFIB_TABLE, flowRef, 6, flowRef, 0,
                 0, cookie, matches, instructions);
         LOG.debug("NAT Service : returning flowEntity {}", flowEntity);
@@ -2294,6 +2371,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
         ArrayList<ActionInfo> listActionInfo = new ArrayList<>();
         ArrayList<InstructionInfo> instructionInfo = new ArrayList<>();
+        listActionInfo.add(new ActionInfo(ActionType.nx_load_in_port, new BigInteger[]{BigInteger.ZERO}));
         listActionInfo.add(new ActionInfo(ActionType.nx_resubmit, new String[] { Integer.toString(NwConstants.L3_FIB_TABLE) }));
         instructionInfo.add(new InstructionInfo(InstructionType.apply_actions, listActionInfo));