NAT44: make 1:1NAT for DHCP addressed interface persistent

Static mapping is not deleted from resolution vector after address is set on
interface.

Change-Id: Ib7c45ca2e307123d101248c5a1b17d130ac32cd0
Signed-off-by: Matus Fabian <matfabia@cisco.com>

diff --git a/src/plugins/nat/nat.c b/src/plugins/nat/nat.c
index 8f4050e..e3f7fba 100644 (file)
--- a/src/plugins/nat/nat.c
+++ b/src/plugins/nat/nat.c
@@ -687,25 +687,67 @@ int snat_add_static_mapping(ip4_address_t l_addr, ip4_address_t e_addr,
   if (sw_if_index != ~0)
     {
       ip4_address_t * first_int_addr;
+      snat_static_map_resolve_t *rp, *rp_match = 0;
+
+      for (i = 0; i < vec_len (sm->to_resolve); i++)
+        {
+          rp = sm->to_resolve + i;
+          if (rp->sw_if_index != sw_if_index &&
+              rp->l_addr.as_u32 != l_addr.as_u32 &&
+              rp->vrf_id != vrf_id && rp->addr_only != addr_only)
+            continue;
+
+          if (!addr_only)
+            {
+              if (rp->l_port != l_port && rp->e_port != e_port && rp->proto != proto)
+                continue;
+            }
+
+          rp_match = rp;
+          break;
+        }
 
       /* Might be already set... */
       first_int_addr = ip4_interface_first_address
         (sm->ip4_main, sw_if_index, 0 /* just want the address*/);
 
-      /* DHCP resolution required? */
-      if (first_int_addr == 0)
+      if (is_add)
         {
-          snat_add_static_mapping_when_resolved
-            (sm, l_addr, l_port, sw_if_index, e_port, vrf_id, proto,
-             addr_only,  is_add, tag);
-          return 0;
+          if (rp_match)
+            return VNET_API_ERROR_VALUE_EXIST;
+
+          /* DHCP resolution required? */
+          if (first_int_addr == 0)
+            {
+              snat_add_static_mapping_when_resolved
+                (sm, l_addr, l_port, sw_if_index, e_port, vrf_id, proto,
+                 addr_only,  is_add, tag);
+              return 0;
+            }
+            else
+            {
+              e_addr.as_u32 = first_int_addr->as_u32;
+              /* Identity mapping? */
+              if (l_addr.as_u32 == 0)
+                l_addr.as_u32 = e_addr.as_u32;
+            }
         }
-        else
+      else
         {
-          e_addr.as_u32 = first_int_addr->as_u32;
-          /* Identity mapping? */
-          if (l_addr.as_u32 == 0)
-            l_addr.as_u32 = e_addr.as_u32;
+          if (!rp_match)
+            return VNET_API_ERROR_NO_SUCH_ENTRY;
+
+          vec_del1 (sm->to_resolve, i);
+
+          if (first_int_addr)
+            {
+              e_addr.as_u32 = first_int_addr->as_u32;
+              /* Identity mapping? */
+              if (l_addr.as_u32 == 0)
+                l_addr.as_u32 = e_addr.as_u32;
+            }
+          else
+            return 0;
         }
     }
 
@@ -2701,7 +2743,6 @@ snat_ip4_add_del_interface_address_cb (ip4_main_t * im,
 {
   snat_main_t *sm = &snat_main;
   snat_static_map_resolve_t *rp;
-  u32 *indices_to_delete = 0;
   ip4_address_t l_addr;
   int i, j;
   int rv;
@@ -2759,18 +2800,8 @@ match:
               if (rv)
                 clib_warning ("snat_add_static_mapping returned %d",
                               rv);
-              vec_free (rp->tag);
-              vec_add1 (indices_to_delete, j);
             }
         }
-      /* If we resolved any of the outstanding static mappings */
-      if (vec_len(indices_to_delete))
-        {
-          /* Delete them */
-          for (j = vec_len(indices_to_delete)-1; j >= 0; j--)
-            vec_delete(sm->to_resolve, 1, j);
-          vec_free(indices_to_delete);
-        }
       return;
     }
   else

diff --git a/test/test_nat.py b/test/test_nat.py
index 695014f..4470a05 100644 (file)
--- a/test/test_nat.py
+++ b/test/test_nat.py
@@ -1031,6 +1031,7 @@ class TestNAT44(MethodHolder):
                 twice_nat=sm.twice_nat,
                 out2in_only=sm.out2in_only,
                 tag=sm.tag,
+                external_sw_if_index=sm.external_sw_if_index,
                 is_add=0)
 
         lb_static_mappings = self.vapi.nat44_lb_static_mapping_dump()
@@ -2463,15 +2464,44 @@ class TestNAT44(MethodHolder):
         # configure interface address and check static mappings
         self.pg7.config_ip4()
         static_mappings = self.vapi.nat44_static_mapping_dump()
-        self.assertEqual(1, len(static_mappings))
-        self.assertEqual(static_mappings[0].external_ip_address[0:4],
-                         self.pg7.local_ip4n)
-        self.assertEqual(0xFFFFFFFF, static_mappings[0].external_sw_if_index)
-        self.assertEqual((static_mappings[0].tag).split('\0', 1)[0], tag)
+        self.assertEqual(2, len(static_mappings))
+        resolved = False
+        for sm in static_mappings:
+            if sm.external_sw_if_index == 0xFFFFFFFF:
+                self.assertEqual(sm.external_ip_address[0:4],
+                                 self.pg7.local_ip4n)
+                self.assertEqual((sm.tag).split('\0', 1)[0], tag)
+                resolved = True
+        self.assertTrue(resolved)
 
         # remove interface address and check static mappings
         self.pg7.unconfig_ip4()
         static_mappings = self.vapi.nat44_static_mapping_dump()
+        self.assertEqual(1, len(static_mappings))
+        self.assertEqual(self.pg7.sw_if_index,
+                         static_mappings[0].external_sw_if_index)
+        self.assertEqual((static_mappings[0].tag).split('\0', 1)[0], tag)
+
+        # configure interface address again and check static mappings
+        self.pg7.config_ip4()
+        static_mappings = self.vapi.nat44_static_mapping_dump()
+        self.assertEqual(2, len(static_mappings))
+        resolved = False
+        for sm in static_mappings:
+            if sm.external_sw_if_index == 0xFFFFFFFF:
+                self.assertEqual(sm.external_ip_address[0:4],
+                                 self.pg7.local_ip4n)
+                self.assertEqual((sm.tag).split('\0', 1)[0], tag)
+                resolved = True
+        self.assertTrue(resolved)
+
+        # remove static mapping
+        self.nat44_add_static_mapping(
+            '1.2.3.4',
+            external_sw_if_index=self.pg7.sw_if_index,
+            tag=tag,
+            is_add=0)
+        static_mappings = self.vapi.nat44_static_mapping_dump()
         self.assertEqual(0, len(static_mappings))
 
     def test_interface_addr_identity_nat(self):
@@ -2494,17 +2524,23 @@ class TestNAT44(MethodHolder):
         # configure interface address and check identity mappings
         self.pg7.config_ip4()
         identity_mappings = self.vapi.nat44_identity_mapping_dump()
-        self.assertEqual(1, len(identity_mappings))
-        self.assertEqual(identity_mappings[0].ip_address,
-                         self.pg7.local_ip4n)
-        self.assertEqual(0xFFFFFFFF, identity_mappings[0].sw_if_index)
-        self.assertEqual(port, identity_mappings[0].port)
-        self.assertEqual(IP_PROTOS.tcp, identity_mappings[0].protocol)
+        resolved = False
+        self.assertEqual(2, len(identity_mappings))
+        for sm in identity_mappings:
+            if sm.sw_if_index == 0xFFFFFFFF:
+                self.assertEqual(identity_mappings[0].ip_address,
+                                 self.pg7.local_ip4n)
+                self.assertEqual(port, identity_mappings[0].port)
+                self.assertEqual(IP_PROTOS.tcp, identity_mappings[0].protocol)
+                resolved = True
+        self.assertTrue(resolved)
 
         # remove interface address and check identity mappings
         self.pg7.unconfig_ip4()
         identity_mappings = self.vapi.nat44_identity_mapping_dump()
-        self.assertEqual(0, len(identity_mappings))
+        self.assertEqual(1, len(identity_mappings))
+        self.assertEqual(self.pg7.sw_if_index,
+                         identity_mappings[0].sw_if_index)
 
     def test_ipfix_nat44_sess(self):
         """ IPFIX logging NAT44 session created/delted """