[UPG-NAT] Minor code refactor

Changes:
- Remove redundant vrf_id from "upf nat pool" cli from code and
  upf scapy tests.
- Free nat addresses vector on nat pool deletion.
- Code optimization and cleanup.

upf/test/test_upf.py

@@ -100,8 +100,8 @@ def tdf_setup_cmds(cls):
             "upf application TST rule 3001 add ipfilter " +
             "permit out ip from %s to assigned" % APP_RULE_IP_V4,
             "nat44 enable sessions 1024 endpoint-dependent",
-            "upf nat pool nwi sgi 78.32.0.2 - 78.32.0.25 block_size 512 vrf 200 name testing",
-            "upf nat pool nwi sgi 78.32.20.2 - 78.32.20.25 block_size 512 vrf 200 name not-testing",
+            "upf nat pool 78.32.0.2 - 78.32.0.25 block_size 512 nwi sgi name testing",
+            "upf nat pool 78.32.20.2 - 78.32.20.25 block_size 512 nwi sgi name not-testing",
             "upf ueip pool nwi sgi id mypool",
         ]
 
@@ -119,8 +119,8 @@ def pgw_setup_cmds(cls):
             "upf gtpu endpoint ip %s nwi cp teid 0x80000000/2" % cls.if_cp.local_ip4,
             "upf gtpu endpoint ip %s nwi epc teid 0x80000000/2" % cls.if_grx.local_ip4,
             "nat44 enable sessions 1024 endpoint-dependent",
-            "upf nat pool nwi sgi 78.32.0.2 - 78.32.0.250 block_size 512 vrf 200 name testing",
-            "upf nat pool nwi sgi 78.32.20.2 - 78.32.20.25 block_size 512 vrf 200 name not-testing",
+            "upf nat pool 78.32.0.2 - 78.32.0.25 block_size 512 nwi sgi name testing",
+            "upf nat pool 78.32.20.2 - 78.32.20.25 block_size 512 nwi sgi name not-testing",
             "upf ueip pool nwi sgi id mypool",
         ]
 

upf/upf.c

@@ -98,16 +98,13 @@ upf_nat_pool_t *
 get_nat_pool_by_name (u8 * name)
 {
   upf_main_t *gtm = &upf_main;
-  upf_nat_pool_t *np = NULL;
   uword *p;
 
   p = hash_get_mem (gtm->nat_pool_index_by_name, name);
   if (!p)
     return NULL;
 
-  np = pool_elt_at_index (gtm->nat_pools, p[0]);
-
-  return np;
+  return pool_elt_at_index (gtm->nat_pools, p[0]);
 }
 
 int
@@ -116,10 +113,18 @@ upf_init_nat_addresses (upf_nat_pool_t * np, ip4_address_t start_addr,
 {
   u32 i = 0;
 
-  vec_reset_length (np->addresses);
+  u32 start;
+  u32 end;
+
+  start = clib_net_to_host_u32 (start_addr.as_u32);
+  end = clib_net_to_host_u32 (end_addr.as_u32);
+
+  if (start > end)
+    return -1;
+
+  vec_alloc (np->addresses, end - start + 1);
 
-  for (i = clib_net_to_host_u32 (start_addr.as_u32);
-       i <= clib_net_to_host_u32 (end_addr.as_u32); i++)
+  for (i = start; i <= end; i++)
     {
       upf_nat_addr_t *ap;
 
@@ -135,7 +140,7 @@ int
 vnet_upf_nat_pool_add_del (u8 * nwi_name, ip4_address_t start_addr,
 			   ip4_address_t end_addr, u8 * name,
 			   u16 port_block_size, u16 min_port, u16 max_port,
-			   u32 vrf_id, u8 is_add)
+			   u8 is_add)
 {
   upf_main_t *gtm = &upf_main;
   upf_nat_pool_t *nat_pool = NULL;
@@ -149,17 +154,21 @@ vnet_upf_nat_pool_add_del (u8 * nwi_name, ip4_address_t start_addr,
 	return VNET_API_ERROR_VALUE_EXIST;
 
       pool_get (gtm->nat_pools, nat_pool);
+
+      if (upf_init_nat_addresses (nat_pool, start_addr, end_addr))
+	{
+	  pool_put (gtm->nat_pools, nat_pool);
+	  return -1;
+	}
+
       nat_pool->name = vec_dup (name);
       nat_pool->network_instance = vec_dup (nwi_name);
       nat_pool->port_block_size = port_block_size;
       nat_pool->min_port = UPF_NAT_MIN_PORT;
       nat_pool->max_port = UPF_NAT_MAX_PORT;
-      nat_pool->vrf_id = vrf_id;
       nat_pool->max_blocks_per_addr =
 	(u16) ((nat_pool->max_port - nat_pool->min_port) / port_block_size);
 
-      upf_init_nat_addresses (nat_pool, start_addr, end_addr);
-
       hash_set_mem (gtm->nat_pool_index_by_name, name,
 		    nat_pool - gtm->nat_pools);
 
@@ -170,7 +179,7 @@ vnet_upf_nat_pool_add_del (u8 * nwi_name, ip4_address_t start_addr,
 	return VNET_API_ERROR_NO_SUCH_ENTRY;
 
       nat_pool = pool_elt_at_index (gtm->nat_pools, p[0]);
-      //TBD: nat pool cleanup upf_delete_nat_addresses(nat_pool);
+      vec_free (nat_pool->addresses);
       hash_unset_mem (gtm->nat_pool_index_by_name, name);
       vec_free (nat_pool->name);
       vec_free (nat_pool->network_instance);

upf/upf.h

@@ -353,7 +353,6 @@ typedef struct
   u16 port_block_size;
   u16 min_port;
   u16 max_port;
-  u32 vrf_id;
   u16 max_blocks_per_addr;
   u8 *network_instance;
 } upf_nat_pool_t;
@@ -972,7 +971,7 @@ int
 vnet_upf_nat_pool_add_del (u8 * nwi_name, ip4_address_t start_addr,
 			   ip4_address_t end_addr, u8 * name,
 			   u16 port_block_size, u16 min_port, u16 max_port,
-			   u32 vrf_id, u8 is_add);
+			   u8 is_add);
 
 int vnet_upf_ue_ip_pool_add_del (u8 * identity, u8 * nwi_name, int is_add);
 
@@ -994,7 +993,7 @@ static int (*upf_nat_del_binding) (ip4_address_t user_addr);
 
 static u16
   (*upf_nat_create_binding) (ip4_address_t user_addr, ip4_address_t ext_addr,
-			     u16 min_port, u16 block_size, u32 vrf);
+			     u16 min_port, u16 block_size);
 
 static inline void
 increment_v4_address (ip4_address_t * a)

upf/upf_cli.c

@@ -217,8 +217,8 @@ upf_ueip_pool_add_del_command_fn (vlib_main_t * vm,
 {
   unformat_input_t _line_input, *line_input = &_line_input;
   clib_error_t *error = NULL;
-  u8 *name;
-  u8 *nwi_s;
+  u8 *name = 0;
+  u8 *nwi_s = 0;
   u8 *nwi_name;
   int rc = 0;
   int is_add = 1;
@@ -262,15 +262,13 @@ upf_nat_pool_add_del_command_fn (vlib_main_t * vm,
 {
   unformat_input_t _line_input, *line_input = &_line_input;
   clib_error_t *error = NULL;
-  u8 *name;
+  u8 *name = 0;
   u8 *nwi_name;
   u8 *nwi_s = 0;
-  u32 vrf_id = 0;
   ip4_address_t start, end;
   u16 min_port;
   u16 max_port;
   u16 port_block_size;
-  //u32 vrf_id;
   u8 is_add = 1;
   int rv;
 
@@ -287,27 +285,23 @@ upf_nat_pool_add_del_command_fn (vlib_main_t * vm,
 	;
       else if (unformat (line_input, "block_size %u", &port_block_size))
 	;
-      else if (unformat (line_input, "vrf %u", &vrf_id))
+      else if (unformat (line_input, "nwi %_%v%_", &nwi_s))
 	;
       else if (unformat (line_input, "name %_%v%_", &name))
 	;
       else if (unformat (line_input, "del"))
 	is_add = 0;
-      else if (unformat (line_input, "nwi %_%v%_", &nwi_s))
-	;
     }
 
   nwi_name = upf_name_to_labels (nwi_s);
   vec_free (nwi_s);
 
-  upf_debug
-    ("POOL\n  START %U END %U\n PORTSTART %u PORTEND %u PORTBLOCK %u VRF %u",
-     format_ip4_address, &start, format_ip4_address, &end, min_port, max_port,
-     port_block_size, vrf_id);
-
   rv =
     vnet_upf_nat_pool_add_del (nwi_name, start, end, name, port_block_size,
-			       min_port, max_port, vrf_id, is_add);
+			       min_port, max_port, is_add);
+
+  if (rv)
+    error = clib_error_return (0, "Unable to create NAT Pool");
 
   return error;
 }
@@ -317,7 +311,7 @@ VLIB_CLI_COMMAND (upf_nat_pool_add_del_command, static) =
 {
   .path = "upf nat pool",
   .short_help =
-  "upf nat pool nwi <nwi-name> start <ip4-addr> end <ip4-addr> min_port <min-port> max_port <max-port> block_size <port-block-size> vrf <vrf-id> name <name> [del]",
+  "upf nat pool nwi <nwi-name> start <ip4-addr> end <ip4-addr> min_port <min-port> max_port <max-port> block_size <port-block-size> name <name> [del]",
   .function = upf_nat_pool_add_del_command_fn,
 };
 /* *INDENT-ON* */

upf/upf_pfcp_api.c

@@ -175,6 +175,7 @@ build_ue_ip_address_information (pfcp_ue_ip_address_pool_information_t **
     {
       if (!(vec_is_equal (np->network_instance, ue_p->nwi_name)))
 	continue;
+
       pfcp_bbf_nat_port_block_t *block;
 
       vec_add2 (ueif->port_blocks, block, 1);
@@ -763,7 +764,7 @@ upf_alloc_and_assign_nat_binding (upf_nat_pool_t * np, upf_nat_addr_t * addr,
 
   port_start =
     upf_nat_create_binding (user_ip, addr->ext_addr, np->min_port,
-			    np->port_block_size, np->vrf_id);
+			    np->port_block_size);
   if (port_start)
     {
       port_end = port_start + np->port_block_size;

vpp-patches/0015-Controlled-NAT-function.patch

@@ -1,4 +1,4 @@
-From ed9246f02b1fe76ee1fd5c55b17bb01b697ad777 Mon Sep 17 00:00:00 2001
+From 5619620e46927fc45ef4da5d128ba2301d6d08e3 Mon Sep 17 00:00:00 2001
 From: Sergey Matov <sergey.matov@travelping.com>
 Date: Wed, 31 Mar 2021 15:19:02 +0400
 Subject: [PATCH] Controlled NAT function
@@ -209,7 +209,7 @@ index 776efdf13..fc91d6f13 100644
        /* Add to lookup tables */
        init_ed_kv (&s_kv, s->in2out.addr, 0, ip->dst_address, 0, rx_fib_index,
 diff --git a/src/plugins/nat/nat.c b/src/plugins/nat/nat.c
-index eeaa443bf..fea0f5f8b 100644
+index eeaa443bf..c396f89ce 100644
 --- a/src/plugins/nat/nat.c
 +++ b/src/plugins/nat/nat.c
 @@ -36,6 +36,13 @@
@@ -424,7 +424,7 @@ index eeaa443bf..fea0f5f8b 100644
 +
 +u16
 +nat_create_binding (ip4_address_t user_addr, ip4_address_t ext_addr,
-+		    u16 min_port, u16 block_size, u32 vrf)
++		    u16 min_port, u16 block_size)
 +{
 +  snat_main_t *sm = &snat_main;
 +  snat_main_per_thread_data_t *tsm = &sm->per_thread_data[0];
@@ -493,7 +493,7 @@ index eeaa443bf..fea0f5f8b 100644
 
  void
 diff --git a/src/plugins/nat/nat.h b/src/plugins/nat/nat.h
-index daebe451f..080632048 100644
+index daebe451f..c5dc852d7 100644
 --- a/src/plugins/nat/nat.h
 +++ b/src/plugins/nat/nat.h
 @@ -226,6 +226,16 @@ typedef enum
@@ -582,7 +582,7 @@ index daebe451f..080632048 100644
 +
 +__clib_export u16
 +nat_create_binding (ip4_address_t user_addr, ip4_address_t ext_addr,
-+		    u16 min_port, u16 block_size, u32 vrf);
++		    u16 min_port, u16 block_size);
 +__clib_export u16 nat_calc_block (ip4_address_t ext_addr, u16 start_port,
 +				  u16 block_size);
 +