udp: refactor port allocation and sharing 49/37649/16
authorFlorin Coras <fcoras@cisco.com>
Fri, 11 Nov 2022 19:37:36 +0000 (11:37 -0800)
committerDave Barach <vpp@barachs.net>
Tue, 29 Nov 2022 23:51:59 +0000 (23:51 +0000)
Type: improvement

Signed-off-by: Florin Coras <fcoras@cisco.com>
Change-Id: I4f7314ddf95d26f1939bd3772d29d011fb4cea47

src/vnet/udp/udp.c
src/vnet/udp/udp.h
src/vnet/udp/udp_input.c
src/vnet/udp/udp_local.c

index 36be18b..710d1ac 100644 (file)
@@ -26,67 +26,32 @@ static void
 udp_connection_register_port (u16 lcl_port, u8 is_ip4)
 {
   udp_main_t *um = &udp_main;
-  udp_dst_port_info_t *pi;
   u16 *n;
 
-  pi = udp_get_dst_port_info (um, lcl_port, is_ip4);
-  if (!pi)
-    {
-      udp_add_dst_port (um, lcl_port, 0, is_ip4);
-      pi = udp_get_dst_port_info (um, lcl_port, is_ip4);
-      pi->n_connections = 1;
-    }
-  else
-    {
-      pi->n_connections += 1;
-      /* Do not return. The fact that the pi is valid does not mean
-       * it's up to date */
-    }
-
-  pi->node_index = is_ip4 ? udp4_input_node.index : udp6_input_node.index;
-  pi->next_index = um->local_to_input_edge[is_ip4];
+  /* Setup udp protocol -> next index sparse vector mapping. Do not setup
+   * udp_dst_port_info_t as that is used to distinguish between external
+   * and transport consumed ports */
 
-  /* Setup udp protocol -> next index sparse vector mapping. */
   if (is_ip4)
-    n = sparse_vec_validate (um->next_by_dst_port4,
-                            clib_host_to_net_u16 (lcl_port));
+    n = sparse_vec_validate (um->next_by_dst_port4, lcl_port);
   else
-    n = sparse_vec_validate (um->next_by_dst_port6,
-                            clib_host_to_net_u16 (lcl_port));
+    n = sparse_vec_validate (um->next_by_dst_port6, lcl_port);
 
-  n[0] = pi->next_index;
+  n[0] = um->local_to_input_edge[is_ip4];
 }
 
 static void
 udp_connection_unregister_port (u16 lcl_port, u8 is_ip4)
 {
   udp_main_t *um = &udp_main;
-  udp_dst_port_info_t *pi;
-
-  pi = udp_get_dst_port_info (um, lcl_port, is_ip4);
-  if (!pi)
-    return;
-
-  if (!pi->n_connections)
-    {
-      clib_warning ("no connections using port %u", lcl_port);
-      return;
-    }
-
-  if (!clib_atomic_sub_fetch (&pi->n_connections, 1))
-    udp_unregister_dst_port (0, lcl_port, is_ip4);
-}
+  u16 *n;
 
-void
-udp_connection_share_port (u16 lcl_port, u8 is_ip4)
-{
-  udp_main_t *um = &udp_main;
-  udp_dst_port_info_t *pi;
+  if (is_ip4)
+    n = sparse_vec_validate (um->next_by_dst_port4, lcl_port);
+  else
+    n = sparse_vec_validate (um->next_by_dst_port6, lcl_port);
 
-  /* Done without a lock but the operation is atomic. Writers to pi hash
-   * table and vector should be guarded by a barrier sync */
-  pi = udp_get_dst_port_info (um, lcl_port, is_ip4);
-  clib_atomic_fetch_add_rel (&pi->n_connections, 1);
+  n[0] = UDP_NO_NODE_SET;
 }
 
 udp_connection_t *
@@ -118,10 +83,10 @@ udp_connection_free (udp_connection_t * uc)
 static void
 udp_connection_cleanup (udp_connection_t * uc)
 {
-  transport_release_local_endpoint (TRANSPORT_PROTO_UDP, &uc->c_lcl_ip,
-                                   uc->c_lcl_port);
-  udp_connection_unregister_port (clib_net_to_host_u16 (uc->c_lcl_port),
-                                 uc->c_is_ip4);
+  /* Unregister port from udp_local only if refcount went to zero */
+  if (!transport_release_local_endpoint (TRANSPORT_PROTO_UDP, &uc->c_lcl_ip,
+                                        uc->c_lcl_port))
+    udp_connection_unregister_port (uc->c_lcl_port, uc->c_is_ip4);
   udp_connection_free (uc);
 }
 
@@ -171,8 +136,7 @@ udp_connection_port_used_extern (u16 lcl_port, u8 is_ip4)
   udp_dst_port_info_t *pi;
 
   pi = udp_get_dst_port_info (um, lcl_port, is_ip4);
-  return (pi && !pi->n_connections
-         && udp_is_valid_dst_port (lcl_port, is_ip4));
+  return (pi && udp_is_valid_dst_port (lcl_port, is_ip4));
 }
 
 static u16
@@ -188,12 +152,10 @@ udp_session_bind (u32 session_index, transport_endpoint_cfg_t *lcl)
   udp_main_t *um = vnet_get_udp_main ();
   transport_endpoint_cfg_t *lcl_ext;
   udp_connection_t *listener;
-  u16 lcl_port_ho;
   void *iface_ip;
 
-  lcl_port_ho = clib_net_to_host_u16 (lcl->port);
-
-  if (udp_connection_port_used_extern (lcl_port_ho, lcl->is_ip4))
+  if (udp_connection_port_used_extern (clib_net_to_host_u16 (lcl->port),
+                                      lcl->is_ip4))
     {
       clib_warning ("port already used");
       return SESSION_E_PORTINUSE;
@@ -229,7 +191,7 @@ udp_session_bind (u32 session_index, transport_endpoint_cfg_t *lcl)
   if (!um->csum_offload)
     listener->cfg_flags |= UDP_CFG_F_NO_CSUM_OFFLOAD;
 
-  udp_connection_register_port (lcl_port_ho, lcl->is_ip4);
+  udp_connection_register_port (listener->c_lcl_port, lcl->is_ip4);
   return listener->c_c_index;
 }
 
@@ -240,8 +202,7 @@ udp_session_unbind (u32 listener_index)
   udp_connection_t *listener;
 
   listener = udp_listener_get (listener_index);
-  udp_connection_unregister_port (clib_net_to_host_u16 (listener->c_lcl_port),
-                                 listener->c_is_ip4);
+  udp_connection_unregister_port (listener->c_lcl_port, listener->c_is_ip4);
   clib_spinlock_free (&listener->rx_lock);
   pool_put (um->listener_pool, listener);
   return 0;
@@ -413,8 +374,6 @@ udp_open_connection (transport_endpoint_cfg_t * rmt)
 
 conn_alloc:
 
-  udp_connection_register_port (lcl_port, rmt->is_ip4);
-
   /* We don't poll main thread if we have workers */
   thread_index = transport_cl_thread ();
 
@@ -445,6 +404,8 @@ conn_alloc:
   uc->next_node_index = rmt->next_node_index;
   uc->next_node_opaque = rmt->next_node_opaque;
 
+  udp_connection_register_port (uc->c_lcl_port, rmt->is_ip4);
+
   return uc->c_c_index;
 }
 
@@ -524,7 +485,7 @@ udp_realloc_ports_sv (u16 **ports_nh_svp)
   vec_resize (ports_nh_sv, 65535);
 
   for (port = 1; port < 65535; port++)
-    ports_nh_sv[port] = ~0;
+    ports_nh_sv[port] = UDP_NO_NODE_SET;
 
   for (i = 0; i < vec_len (ports); i++)
     ports_nh_sv[ports[i]] = nh[i];
index 7103524..ab088bd 100644 (file)
@@ -25,6 +25,8 @@
 #include <vnet/ip/ip.h>
 #include <vnet/session/transport.h>
 
+#define UDP_NO_NODE_SET ((u16) ~0)
+
 typedef enum
 {
 #define udp_error(f, n, s, d) UDP_ERROR_##f,
@@ -103,9 +105,6 @@ typedef struct
   /* Next index for this type. */
   u32 next_index;
 
-  /* UDP sessions refcount (not tunnels) */
-  u32 n_connections;
-
   /* Parser for packet generator edits for this protocol */
   unformat_function_t *unformat_pg_edit;
 } udp_dst_port_info_t;
@@ -236,8 +235,6 @@ format_function_t format_udp_connection;
 unformat_function_t unformat_udp_header;
 unformat_function_t unformat_udp_port;
 
-void udp_connection_share_port (u16 lcl_port, u8 is_ip4);
-
 void udp_punt_unknown (vlib_main_t * vm, u8 is_ip4, u8 is_add);
 
 /*
index 1c1d9e8..cd70939 100644 (file)
@@ -123,8 +123,8 @@ udp_connection_accept (udp_connection_t * listener, session_dgram_hdr_t * hdr,
       udp_connection_free (uc);
       return 0;
     }
-  udp_connection_share_port (clib_net_to_host_u16
-                            (uc->c_lcl_port), uc->c_is_ip4);
+  transport_share_local_endpoint (TRANSPORT_PROTO_UDP, &uc->c_lcl_ip,
+                                 uc->c_lcl_port);
   return uc;
 }
 
index 06bafbb..90a43a2 100644 (file)
@@ -42,8 +42,6 @@ static vlib_error_desc_t udp_error_counters[] = {
 #undef udp_error
 };
 
-#define UDP_NO_NODE_SET ((u16) ~0)
-
 #ifndef CLIB_MARCH_VARIANT
 u8 *
 format_udp_rx_trace (u8 * s, va_list * args)