NETVIRT-1173 : Prevent SNAT packets unnecessary looping

Problem Description:
======================
We noticed lot of SNAT packets are
getting punted to ODL which is causing delayed installation of certain
flows.

On analysis, it's been obseved that under one more scenario, packets
unnecessarily getting looped for 4 sec between ODL and OVS (threshold time we wait to flow
install before making drop decision)

1) N1-> subnet1 (10.0.0.0/24)
2) R1 and subnet1 added to R1.
3) VPN1(100:1) and R1 added to this VPN.
4) Ext-NET1 created and associated with Ext-BGPVPN.
5) Router-gw-set R1 with Ext-Net1

With this set-up, if following configuration is done,
certain packets are looped due to existing NAT bug

6) create N2(20.0.0.0/24) and associated this with VPN1.
7) Initiate TCP/UDP traffic from VMs of N2.

When first packet punted to ODL Controller, we make an
entry(<routerid>:<VM-IP>:<TCP/UDP Port>) in a map and try to find if
internal-to-external mapping available which will not be available for
N2's subnet(as this is not associated with R1) and we just drop that
packet. But, at this point we not taking care of removing of earlier added
entry. Hence, when the 2nd packet is punt to ODL Controller, this packet simply loops
between ODL<->OVS for 4 sec(same will happened with subsequent packets
too).

Solution Proposal:
==================
Changes done to prevent such packet being send back to OVS which is
causing congestion between ODL<->OVS.

Change-Id: I3215a5230f49e05e820b057d6fba415acad084a8
Signed-off-by: Shaik Zakir <shaikzakir119@gmail.com>

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptEventHandler.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptEventHandler.java
index bf76536e0e1578ae922979a69612bb4293595319..42db349e4c5afff3f62a5da3e6a2fc378beed28f 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptEventHandler.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptEventHandler.java
@@ -160,29 +160,38 @@ public class NaptEventHandler {
     */
         try {
             Long routerId = naptEntryEvent.getRouterId();
-            LOG.trace("handleEvent : Time Elapsed before procesing snat ({}:{}) packet is {} ms,"
-                    + "routerId: {},isPktProcessed:{}", naptEntryEvent.getIpAddress(), naptEntryEvent.getPortNumber(),
-                    System.currentTimeMillis() - naptEntryEvent.getObjectCreationTime(),
-                    routerId, naptEntryEvent.isPktProcessed());
+            String internalIpAddress = naptEntryEvent.getIpAddress();
+            int internalPort = naptEntryEvent.getPortNumber();
+            String sourceIPPortKey = routerId + NatConstants.COLON_SEPARATOR
+                + internalIpAddress + NatConstants.COLON_SEPARATOR + internalPort;
+            LOG.trace("handleEvent : Time Elapsed before procesing snat ({}:{}) packet is {} ms,routerId: {},"
+                    + "isPktProcessed:{}",
+                              internalIpAddress, internalPort,
+                              (System.currentTimeMillis() - naptEntryEvent.getObjectCreationTime()), routerId,
+                              naptEntryEvent.isPktProcessed());
             //Get the DPN ID
             BigInteger dpnId = NatUtil.getPrimaryNaptfromRouterId(dataBroker, routerId);
             long bgpVpnId = NatConstants.INVALID_ID;
             if (dpnId == null) {
-                LOG.warn("handleEvent : dpnId is null. Assuming the router ID {} as the BGP VPN ID and proceeding....",
-                    routerId);
+                LOG.warn("handleEvent : dpnId is null. Assuming the router ID {} as the BGP VPN ID and "
+                    + "proceeding....", routerId);
                 bgpVpnId = routerId;
                 LOG.debug("handleEvent : BGP VPN ID {}", bgpVpnId);
                 String vpnName = NatUtil.getRouterName(dataBroker, bgpVpnId);
                 String routerName = NatUtil.getRouterIdfromVpnInstance(dataBroker, vpnName);
                 if (routerName == null) {
-                    LOG.error("handleEvent : Unable to find router for VpnName {}", vpnName);
+                    NaptPacketInHandler.removeIncomingPacketMap(sourceIPPortKey);
+                    LOG.error("handleEvent: Unable to find router for VpnName {}. Droping packet for SNAT ({})"
+                        + "session", vpnName, sourceIPPortKey);
                     return;
                 }
                 routerId = NatUtil.getVpnId(dataBroker, routerName);
                 LOG.debug("handleEvent : Router ID {}", routerId);
                 dpnId = NatUtil.getPrimaryNaptfromRouterId(dataBroker, routerId);
                 if (dpnId == null) {
-                    LOG.error("handleEvent : dpnId is null for the router {}", routerId);
+                    NaptPacketInHandler.removeIncomingPacketMap(sourceIPPortKey);
+                    LOG.error("handleEvent: Unable to find router for VpnName {}. Droping packet for SNAT ({})"
+                        + "session", vpnName, sourceIPPortKey);
                     return;
                 }
             }
@@ -196,30 +205,33 @@ public class NaptEventHandler {
                     String extGwMacAddress = NatUtil.getExtGwMacAddFromRouterId(dataBroker, routerId);
                     if (extGwMacAddress != null) {
                         LOG.debug("handleEvent : External Gateway MAC address {} found for External Router ID {}",
-                                extGwMacAddress, routerId);
+                                  extGwMacAddress, routerId);
                     } else {
-                        LOG.error("handleEvent : No External Gateway MAC address found for External Router ID {}",
-                                routerId);
+                        NaptPacketInHandler.removeIncomingPacketMap(sourceIPPortKey);
+                        LOG.error("handleEvent: No External Gateway MAC address found for External Router ID {}."
+                            + "Droping packet for SNAT ({}) session", routerId, sourceIPPortKey);
                         return;
                     }
+
                     //Get the external network ID from the ExternalRouter model
                     Uuid networkId = NatUtil.getNetworkIdFromRouterId(dataBroker, routerId);
                     if (networkId == null) {
-                        LOG.error("handleEvent : networkId is null");
+                        NaptPacketInHandler.removeIncomingPacketMap(sourceIPPortKey);
+                        LOG.error("handleEvent: networkId is null. Droping packet for SNAT ({}) session",
+                                 sourceIPPortKey);
                         return;
                     }
 
                     //Get the VPN ID from the ExternalNetworks model
                     Uuid vpnUuid = NatUtil.getVpnIdfromNetworkId(dataBroker, networkId);
                     if (vpnUuid == null) {
-                        LOG.error("handleEvent : vpnUuid is null");
+                        NaptPacketInHandler.removeIncomingPacketMap(sourceIPPortKey);
+                        LOG.error("handleEvent: vpnUuid is null. Droping packet for SNAT ({}) session",
+                                 sourceIPPortKey);
                         return;
                     }
                     Long vpnId = NatUtil.getVpnId(dataBroker, vpnUuid.getValue());
 
-                    //Get the internal IpAddress, internal port number from the event
-                    String internalIpAddress = naptEntryEvent.getIpAddress();
-                    int internalPort = naptEntryEvent.getPortNumber();
                     SessionAddress internalAddress = new SessionAddress(internalIpAddress, internalPort);
                     NAPTEntryEvent.Protocol protocol = naptEntryEvent.getProtocol();
 
@@ -228,7 +240,9 @@ public class NaptEventHandler {
                             naptManager.getExternalAddressMapping(routerId, internalAddress,
                                     naptEntryEvent.getProtocol());
                     if (externalAddress == null) {
-                        LOG.error("handleEvent : externalAddress is null");
+                        NaptPacketInHandler.removeIncomingPacketMap(sourceIPPortKey);
+                        LOG.error("handleEvent: externalAddress is null. Droping packet for SNAT ({}) session",
+                                  sourceIPPortKey);
                         return;
                     }
 

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptPacketInHandler.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptPacketInHandler.java
index 9b7ad6807f76e27c0693912a45259345867c2ede..9f7856207bd4bcd88414366b1955cf4fea716e15 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptPacketInHandler.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptPacketInHandler.java
@@ -34,7 +34,7 @@ import org.slf4j.LoggerFactory;
 public class NaptPacketInHandler implements PacketProcessingListener {
 
     private static final Logger LOG = LoggerFactory.getLogger(NaptPacketInHandler.class);
-    private final ConcurrentMap<String,NatPacketProcessingState> incomingPacketMap = new ConcurrentHashMap<>();
+    private static final ConcurrentMap<String,NatPacketProcessingState> INCOMING_PKT_MAP = new ConcurrentHashMap<>();
     private final NaptEventHandler naptEventHandler;
     private final ExecutorService firstPacketExecutorService = SpecialExecutors.newBlockingBoundedFastThreadPool(
             NatConstants.SNAT_PACKET_THEADPOOL_SIZE, Integer.MAX_VALUE, "Napt-firstPacket", NaptPacketInHandler.class);
@@ -121,10 +121,10 @@ public class NaptPacketInHandler implements PacketProcessingListener {
                     String sourceIPPortKey = routerId + NatConstants.COLON_SEPARATOR
                             + internalIPAddress + NatConstants.COLON_SEPARATOR + portNumber;
 
-                    NatPacketProcessingState state = incomingPacketMap.get(sourceIPPortKey);
+                    NatPacketProcessingState state = INCOMING_PKT_MAP.get(sourceIPPortKey);
                     if (state == null) {
                         state = new NatPacketProcessingState(System.currentTimeMillis(), -1);
-                        incomingPacketMap.put(sourceIPPortKey, state);
+                        INCOMING_PKT_MAP.put(sourceIPPortKey, state);
                         LOG.trace("onPacketReceived : Processing new SNAT({}) Packet", sourceIPPortKey);
 
                         //send to Event Queue
@@ -160,8 +160,8 @@ public class NaptPacketInHandler implements PacketProcessingListener {
         }
     }
 
-    public void removeIncomingPacketMap(String sourceIPPortKey) {
-        incomingPacketMap.remove(sourceIPPortKey);
+    public static void removeIncomingPacketMap(String sourceIPPortKey) {
+        INCOMING_PKT_MAP.remove(sourceIPPortKey);
         LOG.debug("removeIncomingPacketMap : sourceIPPortKey {} mapping is removed from map", sourceIPPortKey);
     }