ELAN flows overlap with VPN flows in Table36
[netvirt.git] / natservice / impl / src / main / java / org / opendaylight / netvirt / natservice / internal / ExternalRoutersListener.java
index fc62293aea708ddef1d7b2f396ce41bd8e5ecf2b..1d978d65c19d29b3f8fe1c635f00717efe1623a7 100644 (file)
@@ -31,15 +31,16 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+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.binding.api.ReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase;
+import org.opendaylight.genius.datastoreutils.SingleTransactionDataBroker;
 import org.opendaylight.genius.infra.Datastore.Configuration;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunner;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl;
@@ -56,7 +57,6 @@ import org.opendaylight.genius.mdsalutil.MatchInfo;
 import org.opendaylight.genius.mdsalutil.MetaDataUtil;
 import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.genius.mdsalutil.NwConstants.NxmOfFieldType;
-import org.opendaylight.genius.mdsalutil.UpgradeState;
 import org.opendaylight.genius.mdsalutil.actions.ActionDrop;
 import org.opendaylight.genius.mdsalutil.actions.ActionGroup;
 import org.opendaylight.genius.mdsalutil.actions.ActionLearn;
@@ -81,17 +81,12 @@ import org.opendaylight.netvirt.bgpmanager.api.IBgpManager;
 import org.opendaylight.netvirt.elanmanager.api.IElanService;
 import org.opendaylight.netvirt.fibmanager.api.IFibManager;
 import org.opendaylight.netvirt.fibmanager.api.RouteOrigin;
-import org.opendaylight.netvirt.natservice.api.CentralizedSwitchScheduler;
-import org.opendaylight.netvirt.neutronvpn.interfaces.INeutronVpnManager;
 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;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.instruction.list.Instruction;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.AllocateIdInput;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.AllocateIdInputBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.AllocateIdOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.IdManagerService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.TunnelTypeBase;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.TunnelTypeGre;
@@ -147,12 +142,12 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.vpn.rpc.rev160201.R
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.vpn.rpc.rev160201.RemoveVpnLabelOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.vpn.rpc.rev160201.VpnRpcService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.Port;
-import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+
 @Singleton
 public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Routers, ExternalRoutersListener> {
     private static final Logger LOG = LoggerFactory.getLogger(ExternalRoutersListener.class);
@@ -177,13 +172,11 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     private final IFibManager fibManager;
     private final IVpnManager vpnManager;
     private final EvpnSnatFlowProgrammer evpnSnatFlowProgrammer;
-    private final CentralizedSwitchScheduler  centralizedSwitchScheduler;
     private final NatMode natMode;
-    private final INeutronVpnManager nvpnManager;
     private final IElanService elanManager;
     private final JobCoordinator coordinator;
-    private final UpgradeState upgradeState;
     private final IInterfaceManager interfaceManager;
+    private final NatOverVxlanUtil natOverVxlanUtil;
     private final int snatPuntTimeout;
 
     @Inject
@@ -202,12 +195,10 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                                    final IFibManager fibManager,
                                    final IVpnManager vpnManager,
                                    final EvpnSnatFlowProgrammer evpnSnatFlowProgrammer,
-                                   final INeutronVpnManager nvpnManager,
-                                   final CentralizedSwitchScheduler centralizedSwitchScheduler,
                                    final NatserviceConfig config,
                                    final IElanService elanManager,
                                    final JobCoordinator coordinator,
-                                   final UpgradeState upgradeState,
+                                   final NatOverVxlanUtil natOverVxlanUtil,
                                    final IInterfaceManager interfaceManager) {
         super(Routers.class, ExternalRoutersListener.class);
         this.dataBroker = dataBroker;
@@ -227,12 +218,10 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         this.fibManager = fibManager;
         this.vpnManager = vpnManager;
         this.evpnSnatFlowProgrammer = evpnSnatFlowProgrammer;
-        this.nvpnManager = nvpnManager;
         this.elanManager = elanManager;
-        this.centralizedSwitchScheduler = centralizedSwitchScheduler;
         this.coordinator = coordinator;
-        this.upgradeState = upgradeState;
         this.interfaceManager = interfaceManager;
+        this.natOverVxlanUtil = natOverVxlanUtil;
         if (config != null) {
             this.natMode = config.getNatMode();
             this.snatPuntTimeout = config.getSnatPuntTimeout().intValue();
@@ -320,6 +309,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         } else {
             // write metadata and punt
             installOutboundMissEntry(routerName, routerId, primarySwitchId, confTx);
+            handlePrimaryNaptSwitch(primarySwitchId, routerName, routerId, confTx);
             // Now install entries in SNAT tables to point to Primary for each router
             List<BigInteger> switches = naptSwitchSelector.getDpnsForVpn(routerName);
             for (BigInteger dpnId : switches) {
@@ -327,9 +317,6 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 if (!dpnId.equals(primarySwitchId)) {
                     LOG.debug("handleEnableSnat : Handle Ordinary switch");
                     handleSwitches(dpnId, routerName, routerId, primarySwitchId);
-                } else {
-                    LOG.debug("handleEnableSnat : Handle NAPT switch");
-                    handlePrimaryNaptSwitch(dpnId, routerName, routerId, confTx);
                 }
             }
         }
@@ -353,18 +340,14 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         // Allocate Primary Napt Switch for this router
         BigInteger primarySwitchId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
         if (primarySwitchId != null && !primarySwitchId.equals(BigInteger.ZERO)) {
-            LOG.debug("handleEnableSnat : Primary NAPT switch with DPN ID {} is already elected for router {}",
+            LOG.debug("getPrimaryNaptSwitch : Primary NAPT switch with DPN ID {} is already elected for router {}",
                 primarySwitchId, routerName);
             return primarySwitchId;
         }
-        // Validating and creating VNI pool during when NAPT switch is selected.
-        // With Assumption this might be the first NAT service comes up.
-        NatOverVxlanUtil.validateAndCreateVxlanVniPool(dataBroker, nvpnManager, idManager,
-                NatConstants.ODL_VNI_POOL_NAME);
         // Allocated an id from VNI pool for the Router.
-        NatOverVxlanUtil.getRouterVni(idManager, routerName, NatConstants.INVALID_ID);
+        natOverVxlanUtil.getRouterVni(routerName, NatConstants.INVALID_ID);
         primarySwitchId = naptSwitchSelector.selectNewNAPTSwitch(routerName);
-        LOG.debug("handleEnableSnat : Primary NAPT switch DPN ID {}", primarySwitchId);
+        LOG.debug("getPrimaryNaptSwitch : Primary NAPT switch DPN ID {}", primarySwitchId);
 
         return primarySwitchId;
     }
@@ -392,7 +375,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 LOG.debug("installNaptPfibExternalOutputFlow - dpnId {} extVpnId {} subnetId {}",
                     dpnId, extVpnId, subnetId);
                 FlowEntity postNaptFlowEntity = buildNaptPfibFlowEntity(dpnId, extVpnId);
-                mdsalManager.addFlow(confTx, postNaptFlowEntity);
+                if (postNaptFlowEntity != null) {
+                    mdsalManager.addFlow(confTx, postNaptFlowEntity);
+                }
             }
         }
     }
@@ -410,19 +395,29 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
     protected void subnetRegisterMapping(Routers routerEntry, Long segmentId) {
         LOG.debug("subnetRegisterMapping : Fetching values from extRouters model");
-        List<Uuid> subnetList = routerEntry.getSubnetIds();
         List<String> externalIps = NatUtil.getIpsListFromExternalIps(routerEntry.getExternalIps());
         int counter = 0;
         int extIpCounter = externalIps.size();
         LOG.debug("subnetRegisterMapping : counter values before looping counter {} and extIpCounter {}",
                 counter, extIpCounter);
-        for (Uuid subnet : subnetList) {
+        @Nullable List<Uuid> subnetIds = routerEntry.getSubnetIds();
+        if (subnetIds == null) {
+            return;
+        }
+        for (Uuid subnet : subnetIds) {
             LOG.debug("subnetRegisterMapping : Looping internal subnets for subnet {}", subnet);
             InstanceIdentifier<Subnetmap> subnetmapId = InstanceIdentifier
                 .builder(Subnetmaps.class)
                 .child(Subnetmap.class, new SubnetmapKey(subnet))
                 .build();
-            Optional<Subnetmap> sn = read(dataBroker, LogicalDatastoreType.CONFIGURATION, subnetmapId);
+            Optional<Subnetmap> sn;
+            try {
+                sn = SingleTransactionDataBroker.syncReadOptional(dataBroker,
+                                LogicalDatastoreType.CONFIGURATION, subnetmapId);
+            } catch (ReadFailedException e) {
+                LOG.error("Failed to read SubnetMap for  subnetmap Id {}", subnetmapId, e);
+                sn = Optional.absent();
+            }
             if (sn.isPresent()) {
                 // subnets
                 Subnetmap subnetmapEntry = sn.get();
@@ -583,19 +578,6 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         }
     }
 
-    // TODO Clean up the exception handling
-    @SuppressWarnings("checkstyle:IllegalCatch")
-    public static <T extends DataObject> Optional<T> read(DataBroker broker, LogicalDatastoreType datastoreType,
-                                                          InstanceIdentifier<T> path) {
-        ReadOnlyTransaction tx = broker.newReadOnlyTransaction();
-
-        try {
-            return tx.read(datastoreType, path).get();
-        } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
-    }
-
     protected void installOutboundMissEntry(String routerName, long routerId, BigInteger primarySwitchId,
         TypedWriteTransaction<Configuration> confTx) {
         LOG.debug("installOutboundMissEntry : Router ID from getVpnId {}", routerId);
@@ -715,6 +697,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         mdsalManager.addFlow(confTx, icmpDropFlow);
     }
 
+    @Nullable
     protected String getTunnelInterfaceName(BigInteger srcDpId, BigInteger dstDpId) {
         Class<? extends TunnelTypeBase> tunType = TunnelTypeVxlan.class;
         RpcResult<GetTunnelInterfaceNameOutput> rpcResult;
@@ -767,7 +750,12 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         LOG.debug("installSnatMissEntry : called for dpnId {} with primaryBucket {} ",
             dpnId, bucketInfo.get(0));
         // Install the select group
-        long groupId = createGroupId(getGroupIdKey(routerName));
+        long groupId = NatUtil.getUniqueId(idManager, NatConstants.SNAT_IDPOOL_NAME,
+            NatUtil.getGroupIdKey(routerName));
+        if (groupId == NatConstants.INVALID_ID) {
+            LOG.error("installSnatMissEntry: Unable to obtain group ID for Key: {}", routerName);
+            return;
+        }
         GroupEntity groupEntity =
             MDSALUtil.buildGroupEntity(dpnId, groupId, routerName, GroupTypes.GroupAll, bucketInfo);
         LOG.debug("installSnatMissEntry : installing the SNAT to NAPT GroupEntity:{}", groupEntity);
@@ -783,13 +771,11 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         mdsalManager.installFlow(flowEntity);
     }
 
-    long installGroup(BigInteger dpnId, String routerName, List<BucketInfo> bucketInfo) {
-        long groupId = createGroupId(getGroupIdKey(routerName));
+    void installGroup(BigInteger dpnId, String routerName, long groupId, List<BucketInfo> bucketInfo) {
         GroupEntity groupEntity =
             MDSALUtil.buildGroupEntity(dpnId, groupId, routerName, GroupTypes.GroupAll, bucketInfo);
         LOG.debug("installGroup : installing the SNAT to NAPT GroupEntity:{}", groupEntity);
         mdsalManager.syncInstallGroup(groupEntity);
-        return groupId;
     }
 
     private FlowEntity buildSnatFlowEntity(BigInteger dpId, String routerName, long routerId, long groupId) {
@@ -800,8 +786,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         matches.add(new MatchMetadata(MetaDataUtil.getVpnIdMetadata(routerId), MetaDataUtil.METADATA_MASK_VRFID));
 
         List<ActionInfo> actionsInfo = new ArrayList<>();
-        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, elanManager, idManager, routerId,
-                routerName);
+        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, natOverVxlanUtil, elanManager, idManager,
+                routerId, routerName);
         actionsInfo.add(new ActionSetFieldTunnelId(BigInteger.valueOf(tunnelId)));
         LOG.debug("buildSnatFlowEntity : Setting the tunnel to the list of action infos {}", actionsInfo);
         actionsInfo.add(new ActionGroup(groupId));
@@ -857,8 +843,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     private FlowEntity buildTsFlowEntity(BigInteger dpId, String routerName, long routerId) {
         List<MatchInfo> matches = new ArrayList<>();
         matches.add(MatchEthernetType.IPV4);
-        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, elanManager, idManager, routerId,
-                routerName);
+        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, natOverVxlanUtil, elanManager,
+                idManager, routerId, routerName);
         matches.add(new MatchTunnelId(BigInteger.valueOf(tunnelId)));
         String flowRef = getFlowRefTs(dpId, NwConstants.INTERNAL_TUNNEL_TABLE, tunnelId);
         List<InstructionInfo> instructions = new ArrayList<>();
@@ -881,24 +867,6 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 .FLOWID_SEPARATOR + routerID;
     }
 
-    private String getGroupIdKey(String routerName) {
-        return "snatmiss." + routerName;
-    }
-
-    protected long createGroupId(String groupIdKey) {
-        AllocateIdInput getIdInput = new AllocateIdInputBuilder()
-            .setPoolName(NatConstants.SNAT_IDPOOL_NAME).setIdKey(groupIdKey)
-            .build();
-        try {
-            Future<RpcResult<AllocateIdOutput>> result = idManager.allocateId(getIdInput);
-            RpcResult<AllocateIdOutput> rpcResult = result.get();
-            return rpcResult.getResult().getIdValue();
-        } catch (NullPointerException | InterruptedException | ExecutionException e) {
-            LOG.error("Exception While allocating id for group: {}", groupIdKey, e);
-        }
-        return 0;
-    }
-
     protected void handleSwitches(BigInteger dpnId, String routerName, long routerId, BigInteger primarySwitchId) {
         LOG.debug("handleSwitches : Installing SNAT miss entry in switch {}", dpnId);
         List<ActionInfo> listActionInfoPrimary = new ArrayList<>();
@@ -975,11 +943,11 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         if (networkId != null) {
             Uuid vpnUuid = NatUtil.getVpnIdfromNetworkId(dataBroker, networkId);
             if (vpnUuid != null) {
-                Long vpnId = NatUtil.getVpnId(dataBroker, vpnUuid.getValue());
+                long vpnId = NatUtil.getVpnId(dataBroker, vpnUuid.getValue());
                 coordinator.enqueueJob(NatConstants.NAT_DJC_PREFIX + networkId, () -> {
                     installNaptPfibEntriesForExternalSubnets(routerName, dpnId, null);
                     //Install the NAPT PFIB TABLE which forwards outgoing packet to FIB Table matching on the VPN ID.
-                    if (vpnId != null && vpnId != NatConstants.INVALID_ID) {
+                    if (vpnId != NatConstants.INVALID_ID) {
                         installNaptPfibEntry(dpnId, vpnId, null);
                     }
                     return Collections.emptyList();
@@ -992,16 +960,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         }
     }
 
-    List<BucketInfo> getBucketInfoForPrimaryNaptSwitch() {
-        List<BucketInfo> listBucketInfo = new ArrayList<>();
-        List<ActionInfo> listActionInfoPrimary = new ArrayList<>();
-        listActionInfoPrimary.add(new ActionNxResubmit(NwConstants.INTERNAL_TUNNEL_TABLE));
-        BucketInfo bucketPrimary = new BucketInfo(listActionInfoPrimary);
-        listBucketInfo.add(0, bucketPrimary);
-        return listBucketInfo;
-    }
-
-    public void installNaptPfibEntry(BigInteger dpnId, long segmentId, TypedWriteTransaction<Configuration> confTx) {
+    public void installNaptPfibEntry(BigInteger dpnId, long segmentId,
+            @Nullable TypedWriteTransaction<Configuration> confTx) {
         LOG.debug("installNaptPfibEntry : called for dpnId {} and segmentId {} ", dpnId, segmentId);
         FlowEntity naptPfibFlowEntity = buildNaptPfibFlowEntity(dpnId, segmentId);
         if (confTx != null) {
@@ -1055,7 +1015,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
     public void advToBgpAndInstallFibAndTsFlows(final BigInteger dpnId, final short tableId, final String vpnName,
                                                 final long routerId, final String routerName, final String externalIp,
-                                                final Uuid extNetworkId, final Routers router,
+                                                final Uuid extNetworkId, @Nullable final Routers router,
                                                 final TypedWriteTransaction<Configuration> confTx) {
         LOG.debug("advToBgpAndInstallFibAndTsFlows : entry for DPN ID {}, tableId {}, vpnname {} "
                 + "and externalIp {}", dpnId, tableId, vpnName, externalIp);
@@ -1090,44 +1050,37 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 int externalIpInDsFlag = 0;
                 //Get IPMaps from the DB for the router ID
                 List<IpMap> dbIpMaps = NaptManager.getIpMapList(dataBroker, routerId);
-                if (dbIpMaps != null) {
-                    for (IpMap dbIpMap : dbIpMaps) {
-                        String dbExternalIp = dbIpMap.getExternalIp();
-                        //Select the IPMap, whose external IP is the IP for which FIB is installed
-                        if (dbExternalIp.contains(externalIp)) {
-                            String dbInternalIp = dbIpMap.getInternalIp();
-                            IpMapKey dbIpMapKey = dbIpMap.key();
-                            LOG.debug("advToBgpAndInstallFibAndTsFlows : Setting label {} for internalIp {} "
-                                    + "and externalIp {}", label, dbInternalIp, externalIp);
-                            IpMap newIpm = new IpMapBuilder().withKey(dbIpMapKey).setInternalIp(dbInternalIp)
-                                .setExternalIp(dbExternalIp).setLabel(label).build();
-                            MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.OPERATIONAL,
-                                naptManager.getIpMapIdentifier(routerId, dbInternalIp), newIpm);
-                            externalIpInDsFlag++;
-                        }
-                    }
-                    if (externalIpInDsFlag <= 0) {
-                        LOG.debug("advToBgpAndInstallFibAndTsFlows : 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));
+                for (IpMap dbIpMap : dbIpMaps) {
+                    String dbExternalIp = dbIpMap.getExternalIp();
+                    //Select the IPMap, whose external IP is the IP for which FIB is installed
+                    if (dbExternalIp.contains(externalIp)) {
+                        String dbInternalIp = dbIpMap.getInternalIp();
+                        IpMapKey dbIpMapKey = dbIpMap.key();
+                        LOG.debug("advToBgpAndInstallFibAndTsFlows : Setting label {} for internalIp {} "
+                                + "and externalIp {}", label, dbInternalIp, externalIp);
+                        IpMap newIpm = new IpMapBuilder().withKey(dbIpMapKey).setInternalIp(dbInternalIp)
+                            .setExternalIp(dbExternalIp).setLabel(label).build();
+                        MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.OPERATIONAL,
+                            naptManager.getIpMapIdentifier(routerId, dbInternalIp), newIpm);
+                        externalIpInDsFlag++;
                     }
-                } else {
-                    LOG.error("advToBgpAndInstallFibAndTsFlows : Failed to write label {} for externalIp {} for"
-                            + " routerId {} in DS", label, externalIp, routerId);
+                }
+                if (externalIpInDsFlag <= 0) {
+                    LOG.debug("advToBgpAndInstallFibAndTsFlows : 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));
                 }
                 //Inform BGP
                 long l3vni = 0;
                 if (NatUtil.isOpenStackVniSemanticsEnforcedForGreAndVxlan(elanManager, extNwProvType)) {
-                    l3vni = NatOverVxlanUtil.getInternetVpnVni(idManager, vpnName, l3vni).longValue();
+                    l3vni = natOverVxlanUtil.getInternetVpnVni(vpnName, l3vni).longValue();
                 }
                 Routers extRouter = router != null ? router :
                     NatUtil.getRoutersFromConfigDS(dataBroker, routerName);
-                Uuid externalSubnetId = NatUtil.getExternalSubnetForRouterExternalIp(externalIp,
-                        extRouter);
-                NatUtil.addPrefixToBGP(dataBroker, bgpManager, fibManager, vpnName, rd, externalSubnetId,
+                NatUtil.addPrefixToBGP(dataBroker, bgpManager, fibManager, vpnName, rd,
                     externalIp, nextHopIp, extRouter.getNetworkId().getValue(), null, label, l3vni,
                     RouteOrigin.STATIC, dpnId);
 
@@ -1145,13 +1098,16 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     //Install the flow table 25->44 If there is no FIP Match on table 25 (PDNAT_TABLE)
                     NatUtil.makePreDnatToSnatTableEntry(mdsalManager, dpnId, NwConstants.INBOUND_NAPT_TABLE, confTx);
                 }
-                String fibExternalIp = NatUtil.validateAndAddNetworkMask(externalIp);
-                Optional<Subnets> externalSubnet = NatUtil.getOptionalExternalSubnets(dataBroker,
-                        externalSubnetId);
                 String externalVpn = vpnName;
-                if (externalSubnet.isPresent()) {
-                    externalVpn =  externalSubnetId.getValue();
+                Uuid externalSubnetId = NatUtil.getExternalSubnetForRouterExternalIp(externalIp, extRouter);
+                if (extNwProvType == ProviderTypes.VLAN || extNwProvType == ProviderTypes.FLAT) {
+                    Optional<Subnets> externalSubnet = NatUtil.getOptionalExternalSubnets(dataBroker,
+                            externalSubnetId);
+                    if (externalSubnet.isPresent()) {
+                        externalVpn =  externalSubnetId.getValue();
+                    }
                 }
+                String fibExternalIp = NatUtil.validateAndAddNetworkMask(externalIp);
                 CreateFibEntryInput input = new CreateFibEntryInputBuilder()
                     .setVpnName(externalVpn)
                     .setSourceDpid(dpnId).setIpAddress(fibExternalIp).setServiceId(label)
@@ -1169,12 +1125,12 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         Futures.addCallback(future, new FutureCallback<RpcResult<CreateFibEntryOutput>>() {
 
             @Override
-            public void onFailure(@Nonnull Throwable error) {
+            public void onFailure(@NonNull Throwable error) {
                 LOG.error("advToBgpAndInstallFibAndTsFlows : Error in generate label or fib install process", error);
             }
 
             @Override
-            public void onSuccess(@Nonnull RpcResult<CreateFibEntryOutput> result) {
+            public void onSuccess(@NonNull RpcResult<CreateFibEntryOutput> result) {
                 if (result.isSuccessful()) {
                     LOG.info("advToBgpAndInstallFibAndTsFlows : Successfully installed custom FIB routes for prefix {}",
                             externalIp);
@@ -1246,9 +1202,10 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         }
 
         Flow terminatingServiceTableFlowEntity = MDSALUtil.buildFlowNew(NwConstants.INTERNAL_TUNNEL_TABLE,
-            getFlowRef(dpnId, NwConstants.INTERNAL_TUNNEL_TABLE, serviceId, ""), 5,
-            String.format("%s:%d", "TST Flow Entry ", serviceId),
-            0, 0, COOKIE_TUNNEL.add(BigInteger.valueOf(serviceId)), mkMatches, customInstructions);
+            getFlowRef(dpnId, NwConstants.INTERNAL_TUNNEL_TABLE, serviceId, ""),
+                NatConstants.DEFAULT_VPN_INTERNAL_TUNNEL_TABLE_PRIORITY,
+                String.format("%s:%d", "TST Flow Entry ", serviceId), 0, 0,
+                COOKIE_TUNNEL.add(BigInteger.valueOf(serviceId)), mkMatches, customInstructions);
 
         mdsalManager.addFlow(confTx, dpnId, terminatingServiceTableFlowEntity);
     }
@@ -1285,10 +1242,6 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             bgpVpnId = NatUtil.getVpnId(dataBroker, bgpVpnUuid.getValue());
         }
         BigInteger dpnId = getPrimaryNaptSwitch(routerName);
-        if (dpnId == null || dpnId.equals(BigInteger.ZERO)) {
-            // Router has no interface attached
-            return;
-        }
         final long finalBgpVpnId = bgpVpnId;
         coordinator.enqueueJob(NatConstants.NAT_DJC_PREFIX + update.key(), () -> {
             List<ListenableFuture<Void>> futures = new ArrayList<>();
@@ -1297,6 +1250,10 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     Uuid networkId = original.getNetworkId();
                     if (originalSNATEnabled != updatedSNATEnabled) {
                         if (originalSNATEnabled) {
+                            if (dpnId == null || dpnId.equals(BigInteger.ZERO)) {
+                                // Router has no interface attached
+                                return;
+                            }
                             //SNAT disabled for the router
                             Uuid networkUuid = original.getNetworkId();
                             LOG.info("update : SNAT disabled for Router {}", routerName);
@@ -1307,7 +1264,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                             LOG.info("update : SNAT enabled for Router {}", original.getRouterName());
                             addOrDelDefFibRouteToSNAT(routerName, routerId, finalBgpVpnId, bgpVpnUuid,
                                     true, writeFlowInvTx);
-                            handleEnableSnat(original, routerId, dpnId, finalBgpVpnId, removeFlowInvTx);
+                            handleEnableSnat(update, routerId, dpnId, finalBgpVpnId, writeFlowInvTx);
                         }
                     }
                     if (!Objects.equals(original.getExtGwMacAddress(), update.getExtGwMacAddress())) {
@@ -1315,39 +1272,16 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                         NatUtil.installRouterGwFlows(txRunner, vpnManager, update, dpnId, NwConstants.ADD_FLOW);
                     }
 
+                    if (updatedSNATEnabled != originalSNATEnabled) {
+                        LOG.info("update : no need to process external/subnet changes as it's will taken care in "
+                                + "handleDisableSnat/handleEnableSnat");
+                        return;
+                    }
                     //Check if the Update is on External IPs
                     LOG.debug("update : Checking if this is update on External IPs");
                     List<String> originalExternalIps = NatUtil.getIpsListFromExternalIps(original.getExternalIps());
                     List<String> updatedExternalIps = NatUtil.getIpsListFromExternalIps(update.getExternalIps());
 
-                    //Check if the External IPs are added during the update.
-                    Set<String> addedExternalIps = new HashSet<>(updatedExternalIps);
-                    addedExternalIps.removeAll(originalExternalIps);
-                    if (addedExternalIps.size() != 0) {
-                        LOG.debug("update : Start processing of the External IPs addition during the update "
-                                + "operation");
-                        vpnManager.addArpResponderFlowsToExternalNetworkIps(routerName, addedExternalIps,
-                                update.getExtGwMacAddress(), dpnId,
-                                update.getNetworkId());
-
-                        for (String addedExternalIp : addedExternalIps) {
-                /*
-                    1) Do nothing in the IntExtIp model.
-                    2) Initialise the count of the added external IP to 0 in the ExternalCounter model.
-                 */
-                            String[] externalIpParts = NatUtil.getExternalIpAndPrefix(addedExternalIp);
-                            String externalIp = externalIpParts[0];
-                            String externalIpPrefix = externalIpParts[1];
-                            String externalpStr = externalIp + "/" + externalIpPrefix;
-                            LOG.debug("update : Initialise the count mapping of the external IP {} for the "
-                                            + "router ID {} in the ExternalIpsCounter model.",
-                                    externalpStr, routerId);
-                            naptManager.initialiseNewExternalIpCounter(routerId, externalpStr);
-                        }
-                        LOG.debug(
-                                "update : End processing of the External IPs addition during the update operation");
-                    }
-
                     //Check if the External IPs are removed during the update.
                     Set<String> removedExternalIps = new HashSet<>(originalExternalIps);
                     removedExternalIps.removeAll(updatedExternalIps);
@@ -1417,27 +1351,30 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                                 allocateExternalIp(dpnId, update, routerId, routerName, networkId,
                                         removedInternalIp, writeFlowInvTx);
                             }
-
                             LOG.debug("update : Remove the NAPT translation entries from "
                                     + "Inbound and Outbound NAPT tables for the removed external IPs.");
                             //Get the internalIP and internal Port which were associated to the removed external IP.
-                            List<Integer> externalPorts = new ArrayList<>();
                             Map<ProtocolTypes, List<String>> protoTypesIntIpPortsMap = new HashMap<>();
                             InstanceIdentifier<IpPortMapping> ipPortMappingId = InstanceIdentifier
                                     .builder(IntextIpPortMap.class)
                                     .child(IpPortMapping.class, new IpPortMappingKey(routerId)).build();
-                            Optional<IpPortMapping> ipPortMapping =
-                                    MDSALUtil.read(dataBroker, LogicalDatastoreType.CONFIGURATION, ipPortMappingId);
+                            Optional<IpPortMapping> ipPortMapping;
+                            try {
+                                ipPortMapping = SingleTransactionDataBroker
+                                            .syncReadOptional(dataBroker,
+                                                    LogicalDatastoreType.CONFIGURATION, ipPortMappingId);
+                            } catch (ReadFailedException e) {
+                                LOG.error("Failed to read ipPortMapping for router id {}", routerId, e);
+                                ipPortMapping = Optional.absent();
+                            }
+
                             if (ipPortMapping.isPresent()) {
-                                List<IntextIpProtocolType> intextIpProtocolTypes = ipPortMapping.get()
-                                        .getIntextIpProtocolType();
-                                for (IntextIpProtocolType intextIpProtocolType : intextIpProtocolTypes) {
+                                for (IntextIpProtocolType intextIpProtocolType :
+                                        ipPortMapping.get().nonnullIntextIpProtocolType()) {
                                     ProtocolTypes protoType = intextIpProtocolType.getProtocol();
-                                    List<IpPortMap> ipPortMaps = intextIpProtocolType.getIpPortMap();
-                                    for (IpPortMap ipPortMap : ipPortMaps) {
+                                    for (IpPortMap ipPortMap : intextIpProtocolType.nonnullIpPortMap()) {
                                         IpPortExternal ipPortExternal = ipPortMap.getIpPortExternal();
                                         if (ipPortExternal.getIpAddress().equals(externalIp)) {
-                                            externalPorts.add(ipPortExternal.getPortNum());
                                             List<String> removedInternalIpPorts =
                                                     protoTypesIntIpPortsMap.get(protoType);
                                             if (removedInternalIpPorts != null) {
@@ -1479,10 +1416,37 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                                         if (removedInternalPortsList != null) {
                                             removedInternalPortsList.add(removedInternalPort);
                                             internalIpPortMap.put(removedInternalIp, removedInternalPortsList);
+                                            naptPacketInHandler.removeIncomingPacketMap(routerId
+                                                + NatConstants.COLON_SEPARATOR + removedInternalIp
+                                                + NatConstants.COLON_SEPARATOR + removedInternalPort);
+                                            //Remove the NAPT translation entries from Outbound NAPT table
+                                            naptEventHandler.removeNatFlows(dpnId,
+                                                NwConstants.OUTBOUND_NAPT_TABLE,
+                                                routerId, removedInternalIp,
+                                                Integer.parseInt(removedInternalPort),
+                                                protocolType.getName());
+                                            naptEventHandler.removeNatFlows(dpnId,
+                                                NwConstants.INBOUND_NAPT_TABLE,
+                                                routerId, removedInternalIp,
+                                                Integer.parseInt(removedInternalPort),
+                                                protocolType.getName());
                                         } else {
                                             removedInternalPortsList = new ArrayList<>();
                                             removedInternalPortsList.add(removedInternalPort);
                                             internalIpPortMap.put(removedInternalIp, removedInternalPortsList);
+                                            naptPacketInHandler.removeIncomingPacketMap(routerId
+                                                + NatConstants.COLON_SEPARATOR + removedInternalIp
+                                                + NatConstants.COLON_SEPARATOR + removedInternalPort);
+                                            //Remove the NAPT translation entries from Outbound NAPT table
+                                            naptEventHandler.removeNatFlows(dpnId,
+                                                NwConstants.OUTBOUND_NAPT_TABLE,
+                                                routerId, removedInternalIp,
+                                                Integer.parseInt(removedInternalPort),
+                                                protocolType.getName());
+                                            naptEventHandler.removeNatFlows(dpnId,
+                                                NwConstants.INBOUND_NAPT_TABLE, routerId, removedInternalIp,
+                                                Integer.parseInt(removedInternalPort),
+                                                protocolType.getName());
                                         }
                                     }
                                 }
@@ -1497,41 +1461,51 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                             }
 
                             naptManager.removeNaptPortPool(externalIp);
+                        }
+                        LOG.debug(
+                                "update : End processing of the External IPs removal during the update operation");
+                    }
 
-                            LOG.debug("update : Remove the NAPT translation entries from Inbound NAPT tables for "
-                                    + "the removed external IP {}", externalIp);
-                            for (Integer externalPort : externalPorts) {
-                                //Remove the NAPT translation entries from Inbound NAPT table
-                                naptEventHandler.removeNatFlows(dpnId, NwConstants.INBOUND_NAPT_TABLE,
-                                        routerId, externalIp, externalPort);
-                            }
+                    //Check if the External IPs are added during the update.
+                    Set<String> addedExternalIps = new HashSet<>(updatedExternalIps);
+                    addedExternalIps.removeAll(originalExternalIps);
+                    if (addedExternalIps.size() != 0) {
+                        LOG.debug("update : Start processing of the External IPs addition during the update "
+                                + "operation");
+                        vpnManager.addArpResponderFlowsToExternalNetworkIps(routerName, addedExternalIps,
+                                update.getExtGwMacAddress(), dpnId,
+                                update.getNetworkId());
 
-                            Set<Map.Entry<String, List<String>>> internalIpPorts = internalIpPortMap.entrySet();
-                            for (Map.Entry<String, List<String>> internalIpPort : internalIpPorts) {
-                                String internalIp = internalIpPort.getKey();
-                                LOG.debug("update : Remove the NAPT translation entries from Outbound NAPT tables "
-                                        + "for the removed internal IP {}", internalIp);
-                                List<String> internalPorts = internalIpPort.getValue();
-                                for (String internalPort : internalPorts) {
-                                    //Remove the NAPT translation entries from Outbound NAPT table
-                                    naptPacketInHandler.removeIncomingPacketMap(
-                                            routerId + NatConstants.COLON_SEPARATOR + internalIp
-                                                    + NatConstants.COLON_SEPARATOR + internalPort);
-                                    naptEventHandler.removeNatFlows(dpnId, NwConstants.OUTBOUND_NAPT_TABLE,
-                                            routerId, internalIp, Integer.parseInt(internalPort));
-                                }
-                            }
+                        for (String addedExternalIp : addedExternalIps) {
+                /*
+                    1) Do nothing in the IntExtIp model.
+                    2) Initialise the count of the added external IP to 0 in the ExternalCounter model.
+                 */
+                            String[] externalIpParts = NatUtil.getExternalIpAndPrefix(addedExternalIp);
+                            String externalIp = externalIpParts[0];
+                            String externalIpPrefix = externalIpParts[1];
+                            String externalpStr = externalIp + "/" + externalIpPrefix;
+                            LOG.debug("update : Initialise the count mapping of the external IP {} for the "
+                                            + "router ID {} in the ExternalIpsCounter model.",
+                                    externalpStr, routerId);
+                            naptManager.initialiseNewExternalIpCounter(routerId, externalpStr);
+                            subnetRegisterMapping(update, routerId);
+                            LOG.info("update : Installing fib flow fo newly added Ips");
+                            handleSnatReverseTraffic(writeFlowInvTx, dpnId, update, routerId, routerName, externalpStr);
                         }
                         LOG.debug(
-                                "update : End processing of the External IPs removal during the update operation");
+                                "update : End processing of the External IPs addition during the update operation");
                     }
 
                     //Check if its Update on subnets
                     LOG.debug("update : Checking if this is update on subnets");
                     List<Uuid> originalSubnetIds = original.getSubnetIds();
                     List<Uuid> updatedSubnetIds = update.getSubnetIds();
-                    Set<Uuid> addedSubnetIds = new HashSet<>(updatedSubnetIds);
-                    addedSubnetIds.removeAll(originalSubnetIds);
+                    Set<Uuid> addedSubnetIds =
+                        updatedSubnetIds != null ? new HashSet<>(updatedSubnetIds) : new HashSet<>();
+                    if (originalSubnetIds != null) {
+                        addedSubnetIds.removeAll(originalSubnetIds);
+                    }
 
                     //Check if the Subnet IDs are added during the update.
                     if (addedSubnetIds.size() != 0) {
@@ -1607,13 +1581,18 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
     private boolean isExternalIpAllocated(String externalIp) {
         InstanceIdentifier<ExternalIpsCounter> id = InstanceIdentifier.builder(ExternalIpsCounter.class).build();
-        Optional<ExternalIpsCounter> externalCountersData =
-            MDSALUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, id);
+        Optional<ExternalIpsCounter> externalCountersData;
+        try {
+            externalCountersData = SingleTransactionDataBroker.syncReadOptional(dataBroker,
+                        LogicalDatastoreType.OPERATIONAL, id);
+        } catch (ReadFailedException e) {
+            LOG.error("Failed to read external counters data for ExternalIp {}", externalIp, e);
+            externalCountersData = Optional.absent();
+        }
         if (externalCountersData.isPresent()) {
             ExternalIpsCounter externalIpsCounters = externalCountersData.get();
-            List<ExternalCounters> externalCounters = externalIpsCounters.getExternalCounters();
-            for (ExternalCounters ext : externalCounters) {
-                for (ExternalIpCounter externalIpCount : ext.getExternalIpCounter()) {
+            for (ExternalCounters ext : externalIpsCounters.nonnullExternalCounters()) {
+                for (ExternalIpCounter externalIpCount : ext.nonnullExternalIpCounter()) {
                     if (externalIpCount.getExternalIp().equals(externalIp)) {
                         if (externalIpCount.getCounter() != 0) {
                             return true;
@@ -1694,6 +1673,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         }
     }
 
+    @Nullable
     protected Long checkExternalIpLabel(long routerId, String externalIp) {
         List<IpMap> ipMaps = naptManager.getIpMapList(dataBroker, routerId);
         for (IpMap ipMap : ipMaps) {
@@ -1764,12 +1744,15 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     handleDisableSnat(router, networkUuid, externalIps, true, null, primarySwitchId,
                             routerId, tx);
                 }
-                NatOverVxlanUtil.releaseVNI(routerName, idManager);
+                if (NatUtil.releaseId(idManager, NatConstants.ODL_VNI_POOL_NAME, routerName)
+                    == NatConstants.INVALID_ID) {
+                    LOG.error("remove: Unable to release VNI for router - {}", routerName);
+                }
             })), NatConstants.NAT_DJC_MAX_RETRIES);
     }
 
-    public void handleDisableSnat(Routers router, Uuid networkUuid, @Nonnull Collection<String> externalIps,
-                                  boolean routerFlag, String vpnName, BigInteger naptSwitchDpnId,
+    public void handleDisableSnat(Routers router, Uuid networkUuid, @NonNull Collection<String> externalIps,
+                                  boolean routerFlag, @Nullable String vpnName, BigInteger naptSwitchDpnId,
                                   long routerId, TypedReadWriteTransaction<Configuration> removeFlowInvTx) {
         LOG.info("handleDisableSnat : Entry");
         String routerName = router.getRouterName();
@@ -1827,7 +1810,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     // TODO Clean up the exception handling
     @SuppressWarnings("checkstyle:IllegalCatch")
     public void handleDisableSnatInternetVpn(String routerName, long routerId, Uuid networkUuid,
-                                             @Nonnull Collection<String> externalIps,
+                                             @NonNull Collection<String> externalIps,
                                              String vpnId, TypedReadWriteTransaction<Configuration> writeFlowInvTx) {
         LOG.debug("handleDisableSnatInternetVpn: Started to process handle disable snat for router {} "
                 + "with internet vpn {}", routerName, vpnId);
@@ -1835,8 +1818,14 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             BigInteger naptSwitchDpnId = null;
             InstanceIdentifier<RouterToNaptSwitch> routerToNaptSwitch =
                 NatUtil.buildNaptSwitchRouterIdentifier(routerName);
-            Optional<RouterToNaptSwitch> rtrToNapt =
-                read(dataBroker, LogicalDatastoreType.CONFIGURATION, routerToNaptSwitch);
+            Optional<RouterToNaptSwitch> rtrToNapt;
+            try {
+                rtrToNapt = SingleTransactionDataBroker.syncReadOptional(dataBroker,
+                                LogicalDatastoreType.CONFIGURATION, routerToNaptSwitch);
+            } catch (ReadFailedException e) {
+                LOG.error("Failed to read NAPT switch for router {}", routerName, e);
+                rtrToNapt = Optional.absent();
+            }
             if (rtrToNapt.isPresent()) {
                 naptSwitchDpnId = rtrToNapt.get().getPrimarySwitchId();
             }
@@ -1860,7 +1849,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 LOG.error("handleDisableSnatInternetVpn : Failed to remove fib entries for routerId {} "
                         + "in naptSwitchDpnId {}", routerId, naptSwitchDpnId, ex);
             }
-            NatOverVxlanUtil.releaseVNI(vpnId, idManager);
+            if (NatUtil.releaseId(idManager, NatConstants.ODL_VNI_POOL_NAME, vpnId) == NatConstants.INVALID_ID) {
+                LOG.error("handleDisableSnatInternetVpn : Unable to release VNI for vpnId {} ", vpnId);
+            }
         } catch (InterruptedException | ExecutionException e) {
             LOG.error("handleDisableSnatInternetVpn: Exception while handling disableSNATInternetVpn for router {} "
                     + "with internet vpn {}", routerName, vpnId, e);
@@ -1898,7 +1889,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
     public void removeNaptFlowsFromActiveSwitch(long routerId, String routerName,
                                                 BigInteger dpnId, Uuid networkId, String vpnName,
-                                                @Nonnull Collection<String> externalIps,
+                                                @NonNull Collection<String> externalIps,
                                                 Collection<Uuid> externalSubnetList,
                                                 TypedReadWriteTransaction<Configuration> confTx,
                                                 ProviderTypes extNwProvType)
@@ -1919,8 +1910,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
         //Remove the Terminating Service table entry which forwards the packet to Outbound NAPT Table (For the
         // traffic which comes from the VMs of the non NAPT switches)
-        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, elanManager, idManager, routerId,
-            routerName);
+        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, natOverVxlanUtil,
+                elanManager, idManager, routerId, routerName);
         String tsFlowRef = getFlowRefTs(dpnId, NwConstants.INTERNAL_TUNNEL_TABLE, tunnelId);
         FlowEntity tsNatFlowEntity = NatUtil.buildFlowEntity(dpnId, NwConstants.INTERNAL_TUNNEL_TABLE, tsFlowRef);
         LOG.info(
@@ -2026,10 +2017,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             return;
         }
 
-        List<IntextIpProtocolType> intextIpProtocolTypes = ipPortMapping.getIntextIpProtocolType();
-        for (IntextIpProtocolType intextIpProtocolType : intextIpProtocolTypes) {
-            List<IpPortMap> ipPortMaps = intextIpProtocolType.getIpPortMap();
-            for (IpPortMap ipPortMap : ipPortMaps) {
+        for (IntextIpProtocolType intextIpProtocolType : ipPortMapping.nonnullIntextIpProtocolType()) {
+            String protocol = intextIpProtocolType.getProtocol().name();
+            for (IpPortMap ipPortMap : intextIpProtocolType.nonnullIpPortMap()) {
                 String ipPortInternal = ipPortMap.getIpPortInternal();
                 String[] ipPortParts = ipPortInternal.split(":");
                 if (ipPortParts.length != 2) {
@@ -2043,7 +2033,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 naptPacketInHandler.removeIncomingPacketMap(routerId + NatConstants.COLON_SEPARATOR + internalIp
                     + NatConstants.COLON_SEPARATOR + internalPort);
                 String switchFlowRef = NatUtil.getNaptFlowRef(dpnId, NwConstants.OUTBOUND_NAPT_TABLE,
-                    String.valueOf(routerId), internalIp, Integer.parseInt(internalPort));
+                    String.valueOf(routerId), internalIp, Integer.parseInt(internalPort), protocol);
                 FlowEntity outboundNaptFlowEntity =
                     NatUtil.buildFlowEntity(dpnId, NwConstants.OUTBOUND_NAPT_TABLE, cookieSnatFlow, switchFlowRef);
 
@@ -2051,13 +2041,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     + "with the DPN ID {} and router ID {}", NwConstants.OUTBOUND_NAPT_TABLE, dpnId, routerId);
                 mdsalManager.removeFlow(confTx, outboundNaptFlowEntity);
 
-                IpPortExternal ipPortExternal = ipPortMap.getIpPortExternal();
-                String externalIp = ipPortExternal.getIpAddress();
-                int externalPort = ipPortExternal.getPortNum();
-
-                //Build the flow for the inbound NAPT table
+                 //Build the flow for the inbound NAPT table
                 switchFlowRef = NatUtil.getNaptFlowRef(dpnId, NwConstants.INBOUND_NAPT_TABLE,
-                    String.valueOf(routerId), externalIp, externalPort);
+                    String.valueOf(routerId), internalIp, Integer.parseInt(internalPort), protocol);
                 FlowEntity inboundNaptFlowEntity =
                     NatUtil.buildFlowEntity(dpnId, NwConstants.INBOUND_NAPT_TABLE, cookieSnatFlow, switchFlowRef);
 
@@ -2069,7 +2055,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     }
 
     protected void removeNaptFibExternalOutputFlows(long routerId, BigInteger dpnId, Uuid networkId,
-                                                    @Nonnull Collection<String> externalIps,
+                                                    @NonNull Collection<String> externalIps,
                                                     TypedReadWriteTransaction<Configuration> writeFlowInvTx)
             throws ExecutionException, InterruptedException {
         long extVpnId = NatConstants.INVALID_ID;
@@ -2142,10 +2128,9 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 LOG.error("removeNaptFlowsFromActiveSwitchInternetVpn : Unable to retrieve the IpPortMapping");
                 return;
             }
-            List<IntextIpProtocolType> intextIpProtocolTypes = ipPortMapping.getIntextIpProtocolType();
-            for (IntextIpProtocolType intextIpProtocolType : intextIpProtocolTypes) {
-                List<IpPortMap> ipPortMaps = intextIpProtocolType.getIpPortMap();
-                for (IpPortMap ipPortMap : ipPortMaps) {
+            for (IntextIpProtocolType intextIpProtocolType : ipPortMapping.nonnullIntextIpProtocolType()) {
+                String protocol = intextIpProtocolType.getProtocol().name();
+                for (IpPortMap ipPortMap : intextIpProtocolType.nonnullIpPortMap()) {
                     String ipPortInternal = ipPortMap.getIpPortInternal();
                     String[] ipPortParts = ipPortInternal.split(":");
                     if (ipPortParts.length != 2) {
@@ -2160,7 +2145,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     naptPacketInHandler.removeIncomingPacketMap(routerId + NatConstants.COLON_SEPARATOR + internalIp
                             + NatConstants.COLON_SEPARATOR + internalPort);
                     String switchFlowRef = NatUtil.getNaptFlowRef(dpnId, NwConstants.OUTBOUND_NAPT_TABLE,
-                        String.valueOf(routerId), internalIp, Integer.parseInt(internalPort));
+                        String.valueOf(routerId), internalIp, Integer.parseInt(internalPort), protocol);
                     FlowEntity outboundNaptFlowEntity =
                         NatUtil.buildFlowEntity(dpnId, NwConstants.OUTBOUND_NAPT_TABLE, cookieSnatFlow, switchFlowRef);
 
@@ -2170,12 +2155,11 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     mdsalManager.removeFlow(writeFlowInvTx, outboundNaptFlowEntity);
 
                     IpPortExternal ipPortExternal = ipPortMap.getIpPortExternal();
-                    String externalIp = ipPortExternal.getIpAddress();
-                    int externalPort = ipPortExternal.getPortNum();
+                    final String externalIp = ipPortExternal.getIpAddress();
 
                     //Build the flow for the inbound NAPT table
                     switchFlowRef = NatUtil.getNaptFlowRef(dpnId, NwConstants.INBOUND_NAPT_TABLE,
-                        String.valueOf(routerId), externalIp, externalPort);
+                        String.valueOf(routerId), internalIp, Integer.parseInt(internalPort), protocol);
                     FlowEntity inboundNaptFlowEntity =
                         NatUtil.buildFlowEntity(dpnId, NwConstants.INBOUND_NAPT_TABLE, cookieSnatFlow, switchFlowRef);
 
@@ -2224,17 +2208,22 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 mdsalManager.removeFlow(removeFlowInvTx, preSnatFlowEntity);
 
                 //Remove the group entry which forwards the traffic to the out port (VXLAN tunnel).
-                long groupId = createGroupId(getGroupIdKey(routerName));
-
-                LOG.info("removeFlowsFromNonActiveSwitches : Remove the group {} for the non active switch with "
-                    + "the DPN ID {} and router ID {}", groupId, dpnId, routerId);
-                mdsalManager.removeGroup(removeFlowInvTx, dpnId, groupId);
+                long groupId = NatUtil.getUniqueId(idManager, NatConstants.SNAT_IDPOOL_NAME,
+                    NatUtil.getGroupIdKey(routerName));
+                if (groupId != NatConstants.INVALID_ID) {
+                    LOG.info(
+                        "removeFlowsFromNonActiveSwitches : Remove the group {} for the non active switch with "
+                            + "the DPN ID {} and router ID {}", groupId, dpnId, routerId);
+                    mdsalManager.removeGroup(removeFlowInvTx, dpnId, groupId);
+                } else {
+                    LOG.error("removeFlowsFromNonActiveSwitches: Unable to obtained groupID for router:{}", routerName);
+                }
             }
         }
     }
 
-    public void clrRtsFromBgpAndDelFibTs(final BigInteger dpnId, Long routerId, Uuid networkUuid,
-                                         @Nonnull Collection<String> externalIps, String vpnName,
+    public void clrRtsFromBgpAndDelFibTs(final BigInteger dpnId, Long routerId, @Nullable Uuid networkUuid,
+                                         @NonNull Collection<String> externalIps, @Nullable String vpnName,
                                          String extGwMacAddress, TypedReadWriteTransaction<Configuration> confTx)
             throws ExecutionException, InterruptedException {
         //Withdraw the corresponding routes from the BGP.
@@ -2278,13 +2267,13 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 removeFlowInvTx);
     }
 
-    protected void delFibTsAndReverseTraffic(final BigInteger dpnId, long routerId, String extIp,
-                                             final String vpnName, Uuid extNetworkId, long tempLabel,
+    protected void delFibTsAndReverseTraffic(final BigInteger dpnId, String routerName, long routerId, String extIp,
+                                             String vpnName, Uuid extNetworkId, long tempLabel,
                                              String gwMacAddress, boolean switchOver,
                                              TypedReadWriteTransaction<Configuration> removeFlowInvTx)
             throws ExecutionException, InterruptedException {
         LOG.debug("delFibTsAndReverseTraffic : Removing fib entry for externalIp {} in routerId {}", extIp, routerId);
-        String routerName = NatUtil.getRouterName(dataBroker,routerId);
+        //String routerName = NatUtil.getRouterName(dataBroker,routerId);
         if (routerName == null) {
             LOG.error("delFibTsAndReverseTraffic : Could not retrieve Router Name from Router ID {} ", routerId);
             return;
@@ -2307,10 +2296,23 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     extIp, routerId);
             return;
         }
-
         final long label = tempLabel;
         final String externalIp = NatUtil.validateAndAddNetworkMask(extIp);
-        RemoveFibEntryInput input = new RemoveFibEntryInputBuilder().setVpnName(vpnName)
+        if (extNwProvType == ProviderTypes.FLAT || extNwProvType == ProviderTypes.VLAN) {
+            LOG.debug("delFibTsAndReverseTraffic : Using extSubnetId as vpnName for FLAT/VLAN use-cases");
+            Routers extRouter = NatUtil.getRoutersFromConfigDS(dataBroker, routerName);
+            Uuid externalSubnetId = NatUtil.getExternalSubnetForRouterExternalIp(externalIp,
+                    extRouter);
+
+            Optional<Subnets> externalSubnet = NatUtil.getOptionalExternalSubnets(dataBroker,
+                    externalSubnetId);
+
+            if (externalSubnet.isPresent()) {
+                vpnName =  externalSubnetId.getValue();
+            }
+        }
+        final String externalVpn = vpnName;
+        RemoveFibEntryInput input = new RemoveFibEntryInputBuilder().setVpnName(externalVpn)
                 .setSourceDpid(dpnId).setIpAddress(externalIp).setServiceId(label)
                 .setIpAddressSource(RemoveFibEntryInput.IpAddressSource.ExternalFixedIP).build();
         ListenableFuture<RpcResult<RemoveFibEntryOutput>> future = fibService.removeFibEntry(input);
@@ -2328,7 +2330,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     if (result.isSuccessful()) {
                         NatUtil.removePreDnatToSnatTableEntry(removeFlowInvTx, mdsalManager, dpnId);
                         RemoveVpnLabelInput labelInput = new RemoveVpnLabelInputBuilder()
-                            .setVpnName(vpnName).setIpPrefix(externalIp).build();
+                            .setVpnName(externalVpn).setIpPrefix(externalIp).build();
                         return vpnService.removeVpnLabel(labelInput);
                     } else {
                         String errMsg =
@@ -2342,19 +2344,19 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             Futures.addCallback(labelFuture, new FutureCallback<RpcResult<RemoveVpnLabelOutput>>() {
 
                 @Override
-                public void onFailure(@Nonnull Throwable error) {
+                public void onFailure(@NonNull Throwable error) {
                     LOG.error("delFibTsAndReverseTraffic : Error in removing the label:{} or custom fib entries"
                         + "got external ip {}", label, extIp, error);
                 }
 
                 @Override
-                public void onSuccess(@Nonnull RpcResult<RemoveVpnLabelOutput> result) {
+                public void onSuccess(@NonNull RpcResult<RemoveVpnLabelOutput> result) {
                     if (result.isSuccessful()) {
                         LOG.debug("delFibTsAndReverseTraffic : Successfully removed the label for the prefix {} "
-                            + "from VPN {}", externalIp, vpnName);
+                            + "from VPN {}", externalIp, externalVpn);
                     } else {
                         LOG.error("delFibTsAndReverseTraffic : Error in removing the label for prefix {} "
-                            + " from VPN {}, {}", externalIp, vpnName, result.getErrors());
+                            + " from VPN {}, {}", externalIp, externalVpn, result.getErrors());
                     }
                 }
             }, MoreExecutors.directExecutor());
@@ -2447,12 +2449,12 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             Futures.addCallback(labelFuture, new FutureCallback<RpcResult<RemoveVpnLabelOutput>>() {
 
                 @Override
-                public void onFailure(@Nonnull Throwable error) {
+                public void onFailure(@NonNull Throwable error) {
                     LOG.error("delFibTsAndReverseTraffic : Error in removing the label or custom fib entries", error);
                 }
 
                 @Override
-                public void onSuccess(@Nonnull RpcResult<RemoveVpnLabelOutput> result) {
+                public void onSuccess(@NonNull RpcResult<RemoveVpnLabelOutput> result) {
                     if (result.isSuccessful()) {
                         LOG.debug("delFibTsAndReverseTraffic : Successfully removed the label for the prefix {} "
                             + "from VPN {}", externalIp, vpnName);
@@ -2469,9 +2471,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     }
 
     protected void clearFibTsAndReverseTraffic(final BigInteger dpnId, Long routerId, Uuid networkUuid,
-                                               List<String> externalIps, String vpnName, String extGwMacAddress,
-                                               TypedReadWriteTransaction<Configuration> writeFlowInvTx)
-            throws ExecutionException, InterruptedException {
+            List<String> externalIps, @Nullable String vpnName, String extGwMacAddress,
+            TypedReadWriteTransaction<Configuration> writeFlowInvTx) throws ExecutionException, InterruptedException {
         //Withdraw the corresponding routes from the BGP.
         //Get the network ID using the router ID.
         LOG.debug("clearFibTsAndReverseTraffic : for externalIps {} with routerId {},"
@@ -2552,15 +2553,12 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     getRoutersIdentifier(bgpVpnId), rtrs);
 
                 // Get the allocated Primary NAPT Switch for this router
-                LOG.debug("changeLocalVpnIdToBgpVpnId : Router ID value {} ", routerId);
-
                 LOG.debug("changeLocalVpnIdToBgpVpnId : Update the Router ID {} to the BGP VPN ID {} ",
                         routerId, bgpVpnId);
                 addDefaultFibRouteForSnatWithBgpVpn(routerName, routerId, bgpVpnId, writeFlowInvTx);
 
                 // Get the group ID
                 BigInteger primarySwitchId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
-                createGroupId(getGroupIdKey(routerName));
                 installFlowsWithUpdatedVpnId(primarySwitchId, routerName, bgpVpnId, routerId, true, writeFlowInvTx,
                         extNwProvType);
             }
@@ -2588,7 +2586,6 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             addDefaultFibRouteForSnatWithBgpVpn(routerName, routerId, NatConstants.INVALID_ID, writeFlowInvTx);
 
             // Get the group ID
-            createGroupId(getGroupIdKey(routerName));
             BigInteger primarySwitchId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
             installFlowsWithUpdatedVpnId(primarySwitchId, routerName, NatConstants.INVALID_ID, routerId, true,
                     writeFlowInvTx, extNwProvType);
@@ -2599,8 +2596,15 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         InstanceIdentifier<Routers> routerInstanceIndentifier =
             InstanceIdentifier.builder(ExtRouters.class)
                 .child(Routers.class, new RoutersKey(routerUuid.getValue())).build();
-        Optional<Routers> routerData = read(dataBroker, LogicalDatastoreType.CONFIGURATION, routerInstanceIndentifier);
-        return routerData.isPresent() && routerData.get().isEnableSnat();
+        try {
+            Optional<Routers> routerData = SingleTransactionDataBroker
+                    .syncReadOptional(dataBroker,
+                            LogicalDatastoreType.CONFIGURATION, routerInstanceIndentifier);
+            return routerData.isPresent() && routerData.get().isEnableSnat();
+        } catch (ReadFailedException e) {
+            LOG.error("Failed to read data for router id {}", routerUuid, e);
+            return false;
+        }
     }
 
     public void installFlowsWithUpdatedVpnId(BigInteger primarySwitchId, String routerName, long bgpVpnId,
@@ -2624,16 +2628,21 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                 LOG.debug("installFlowsWithUpdatedVpnId : Install group in non NAPT switch {}", dpnId);
                 List<BucketInfo> bucketInfoForNonNaptSwitches =
                     getBucketInfoForNonNaptSwitches(dpnId, primarySwitchId, routerName, routerId);
-                long groupId = createGroupId(getGroupIdKey(routerName));
-                if (!isSnatCfgd) {
-                    groupId = installGroup(dpnId, routerName, bucketInfoForNonNaptSwitches);
-                }
-
-                LOG.debug(
+                long groupId = NatUtil.getUniqueId(idManager, NatConstants.SNAT_IDPOOL_NAME,
+                    NatUtil.getGroupIdKey(routerName));
+                if (groupId != NatConstants.INVALID_ID) {
+                    if (!isSnatCfgd) {
+                        installGroup(dpnId, routerName, groupId, bucketInfoForNonNaptSwitches);
+                    }
+                    LOG.debug(
                         "installFlowsWithUpdatedVpnId : Update the {} ID {} in the SNAT miss entry pointing to group "
-                                + "{} in the non NAPT switch {}", idType, changedVpnId, groupId, dpnId);
-                FlowEntity flowEntity = buildSnatFlowEntityWithUpdatedVpnId(dpnId, routerName, groupId, changedVpnId);
-                mdsalManager.addFlow(confTx, flowEntity);
+                            + "{} in the non NAPT switch {}", idType, changedVpnId, groupId, dpnId);
+                    FlowEntity flowEntity = buildSnatFlowEntityWithUpdatedVpnId(dpnId, routerName,
+                        groupId, changedVpnId);
+                    mdsalManager.addFlow(confTx, flowEntity);
+                } else {
+                    LOG.error("installFlowsWithUpdatedVpnId: Unable to get groupId for router:{}", routerName);
+                }
             } else {
                 LOG.debug(
                         "installFlowsWithUpdatedVpnId : Update the {} ID {} in the SNAT miss entry pointing to group "
@@ -2695,10 +2704,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     routerId);
             return;
         }
-        List<IntextIpProtocolType> intextIpProtocolTypes = ipPortMapping.getIntextIpProtocolType();
-        for (IntextIpProtocolType intextIpProtocolType : intextIpProtocolTypes) {
-            List<IpPortMap> ipPortMaps = intextIpProtocolType.getIpPortMap();
-            for (IpPortMap ipPortMap : ipPortMaps) {
+        for (IntextIpProtocolType intextIpProtocolType : ipPortMapping.nonnullIntextIpProtocolType()) {
+            for (IpPortMap ipPortMap : intextIpProtocolType.nonnullIpPortMap()) {
                 String ipPortInternal = ipPortMap.getIpPortInternal();
                 String[] ipPortParts = ipPortInternal.split(":");
                 if (ipPortParts.length != 2) {
@@ -2743,8 +2750,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
         matches.add(new MatchMetadata(MetaDataUtil.getVpnIdMetadata(changedVpnId), MetaDataUtil.METADATA_MASK_VRFID));
 
         List<ActionInfo> actionsInfo = new ArrayList<>();
-        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, elanManager, idManager, changedVpnId,
-                routerName);
+        long tunnelId = NatUtil.getTunnelIdForNonNaptToNaptFlow(dataBroker, natOverVxlanUtil,
+                elanManager, idManager, changedVpnId, routerName);
         actionsInfo.add(new ActionSetFieldTunnelId(BigInteger.valueOf(tunnelId)));
         LOG.debug("buildSnatFlowEntityWithUpdatedVpnId : Setting the tunnel to the list of action infos {}",
                 actionsInfo);
@@ -2801,7 +2808,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
         BigInteger tunnelId = BigInteger.valueOf(changedVpnId);
         if (NatUtil.isOpenStackVniSemanticsEnforcedForGreAndVxlan(elanManager, extNwProvType)) {
-            tunnelId = NatOverVxlanUtil.getRouterVni(idManager, routerName, changedVpnId);
+            tunnelId = natOverVxlanUtil.getRouterVni(routerName, changedVpnId);
         }
         matches.add(new MatchTunnelId(tunnelId));
 
@@ -2897,7 +2904,7 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
     }
 
     protected void installNaptPfibEntriesForExternalSubnets(String routerName, BigInteger dpnId,
-                                                            TypedWriteTransaction<Configuration> writeFlowInvTx) {
+                                                        @Nullable TypedWriteTransaction<Configuration> writeFlowInvTx) {
         Collection<Uuid> externalSubnetIdsForRouter = NatUtil.getExternalSubnetIdsForRouter(dataBroker,
                 routerName);
         for (Uuid externalSubnetId : externalSubnetIdsForRouter) {