session: update due to clib_socket refactoring 39/38739/3
authorNathan Skrzypczak <nathan.skrzypczak@gmail.com>
Thu, 27 Apr 2023 10:43:46 +0000 (12:43 +0200)
committerFlorin Coras <florin.coras@gmail.com>
Fri, 28 Apr 2023 03:00:59 +0000 (03:00 +0000)
After the clib_socket_init syntax changed, the behavior of VCL
socket creation was broken. This patch introduces app_namespace_add_del_v4
to address the behavioral change.

Type: refactor

Change-Id: Ice016bdb372233fd3317f166d45625e086e9b4df
Signed-off-by: Nathan Skrzypczak <nathan.skrzypczak@gmail.com>
13 files changed:
src/vnet/session/application_namespace.c
src/vnet/session/application_namespace.h
src/vnet/session/session.api
src/vnet/session/session_api.c
src/vnet/session/session_test.c
src/vppinfra/socket.c
src/vppinfra/socket.h
test/asf/test_quic.py
test/asf/test_session.py
test/asf/test_tcp.py
test/asf/test_tls.py
test/asf/test_vcl.py
test/test_udp.py

index cd2636c..13b540c 100644 (file)
@@ -119,11 +119,6 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t * a)
          st->is_local = 1;
          st->appns_index = app_namespace_index (app_ns);
          app_ns->local_table_index = session_table_index (st);
-         if (a->netns)
-           {
-             app_ns->netns = vec_dup (a->netns);
-             vec_terminate_c_string (app_ns->netns);
-           }
          if (a->sock_name)
            {
              app_ns->sock_name = vec_dup (a->sock_name);
@@ -167,8 +162,6 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t * a)
       st = session_table_get (app_ns->local_table_index);
 
       session_table_free (st, FIB_PROTOCOL_MAX);
-      if (app_ns->netns)
-       vec_free (app_ns->netns);
       if (app_ns->sock_name)
        vec_free (app_ns->sock_name);
 
@@ -255,7 +248,6 @@ app_namespaces_init (void)
   /* clang-format off */
   vnet_app_namespace_add_del_args_t a = {
     .ns_id = ns_id,
-    .netns = 0,
     .sock_name = 0,
     .secret = 0,
     .sw_if_index = APP_NAMESPACE_INVALID_INDEX,
@@ -272,7 +264,7 @@ app_ns_fn (vlib_main_t * vm, unformat_input_t * input,
           vlib_cli_command_t * cmd)
 {
   u8 is_add = 0, *ns_id = 0, secret_set = 0, sw_if_index_set = 0;
-  u8 *netns = 0, *sock_name = 0;
+  u8 *sock_name = 0;
   unformat_input_t _line_input, *line_input = &_line_input;
   u32 sw_if_index, fib_id = APP_NAMESPACE_INVALID_INDEX;
   vnet_main_t *vnm = vnet_get_main ();
@@ -302,8 +294,6 @@ app_ns_fn (vlib_main_t * vm, unformat_input_t * input,
        sw_if_index_set = 1;
       else if (unformat (line_input, "fib_id", &fib_id))
        ;
-      else if (unformat (line_input, "netns %_%v%_", &netns))
-       ;
       else if (unformat (line_input, "sock-name %_%v%_", &sock_name))
        ;
       else
@@ -329,7 +319,6 @@ app_ns_fn (vlib_main_t * vm, unformat_input_t * input,
   /* clang-format off */
   vnet_app_namespace_add_del_args_t args = {
     .ns_id = ns_id,
-    .netns = netns,
     .secret = secret,
     .sw_if_index = sw_if_index,
     .sock_name = sock_name,
@@ -344,7 +333,6 @@ app_ns_fn (vlib_main_t * vm, unformat_input_t * input,
 done:
 
   vec_free (ns_id);
-  vec_free (netns);
   vec_free (sock_name);
   unformat_free (line_input);
 
@@ -355,7 +343,7 @@ done:
 VLIB_CLI_COMMAND (app_ns_command, static) = {
   .path = "app ns",
   .short_help = "app ns [add|del] id <namespace-id> secret <secret> "
-               "sw_if_index <sw_if_index> if <interface> [netns <ns>]",
+               "sw_if_index <sw_if_index> if <interface>",
   .function = app_ns_fn,
 };
 /* *INDENT-ON* */
@@ -371,8 +359,6 @@ format_app_namespace (u8 * s, va_list * args)
   if (app_ns->sw_if_index != (u32) ~0)
     s = format (s, "\nInterface: %U", format_vnet_sw_if_index_name, vnm,
                app_ns->sw_if_index);
-  if (app_ns->netns)
-    s = format (s, "\nNetns:     %s", app_ns->netns);
   if (app_ns->sock_name)
     s = format (s, "\nSocket:    %s", app_ns->sock_name);
 
@@ -482,8 +468,7 @@ show_app_ns_fn (vlib_main_t * vm, unformat_input_t * main_input,
     }
 
 do_ns_list:
-  table_add_header_col (t, 6, "Index", "Secret", "Interface", "Id", "Netns",
-                       "Socket");
+  table_add_header_col (t, 5, "Index", "Secret", "Interface", "Id", "Socket");
   int i = 0;
   pool_foreach (app_ns, app_namespace_pool)
     {
@@ -493,7 +478,6 @@ do_ns_list:
       table_format_cell (t, i, j++, "%U", format_vnet_sw_if_index_name, vnm,
                         app_ns->sw_if_index);
       table_format_cell (t, i, j++, "%s", app_ns->ns_id);
-      table_format_cell (t, i, j++, "%s", app_ns->netns);
       table_format_cell (t, i++, j++, "%s", app_ns->sock_name);
     }
 
index 1750d41..02a4a07 100644 (file)
@@ -50,11 +50,6 @@ typedef struct _app_namespace
    */
   u8 *ns_id;
 
-  /**
-   * Linux netns if one was provided
-   */
-  u8 *netns;
-
   /**
    * Name of socket applications can use to attach to session layer
    */
@@ -69,7 +64,6 @@ typedef struct _app_namespace
 typedef struct _vnet_app_namespace_add_del_args
 {
   u8 *ns_id;
-  u8 *netns;
   u8 *sock_name;
   u64 secret;
   u32 sw_if_index;
index 9a7bb01..6affae4 100644 (file)
@@ -207,6 +207,45 @@ define app_namespace_add_del {
   string namespace_id[];
 };
 
+/** \brief add/del application namespace
+    @param client_index - opaque cookie to identify the sender
+                          client to vpp direction only
+    @param context - sender context, to match reply w/ request
+    @param secret - secret shared between app and vpp
+    @param sw_if_index - local interface that "supports" namespace. Set to
+                         ~0 if no preference
+    @param ip4_fib_id - id of ip4 fib that "supports" the namespace. Ignored
+                        if sw_if_index set.
+    @param ip6_fib_id - id of ip6 fib that "supports" the namespace. Ignored
+                        if sw_if_index set.
+    @param namespace_id - namespace id
+    @param sock_name - socket name (path, abstract socket name)
+*/
+define app_namespace_add_del_v4 {
+  option deprecated;
+  u32 client_index;
+  u32 context;
+  u64 secret;
+  bool is_add [default=true];
+  vl_api_interface_index_t sw_if_index [default=0xffffffff];
+  u32 ip4_fib_id;
+  u32 ip6_fib_id;
+  string namespace_id[64];
+  string sock_name[];
+};
+
+/** \brief Reply for app namespace add/del
+    @param context - returned sender context, to match reply w/ request
+    @param retval - return code
+    @param appns_index - app namespace index
+*/
+define app_namespace_add_del_v4_reply
+{
+  u32 context;
+  i32 retval;
+  u32 appns_index;
+};
+
 /** \brief add/del application namespace
     @param client_index - opaque cookie to identify the sender
                           client to vpp direction only
@@ -222,6 +261,7 @@ define app_namespace_add_del {
     @param netns - linux net namespace
 */
 define app_namespace_add_del_v2 {
+  option deprecated;
   u32 client_index;
   u32 context;
   u64 secret;
@@ -248,6 +288,7 @@ define app_namespace_add_del_v2 {
     @param sock_name - socket name (path, abstract socket name)
 */
 define app_namespace_add_del_v3 {
+  option deprecated;
   u32 client_index;
   u32 context;
   u64 secret;
@@ -280,6 +321,7 @@ define app_namespace_add_del_reply
 */
 define app_namespace_add_del_v2_reply
 {
+  option deprecated;
   u32 context;
   i32 retval;
   u32 appns_index;
@@ -287,6 +329,7 @@ define app_namespace_add_del_v2_reply
 
 define app_namespace_add_del_v3_reply
 {
+  option deprecated;
   u32 context;
   i32 retval;
   u32 appns_index;
index eb35f19..3e99938 100644 (file)
@@ -725,7 +725,6 @@ vl_api_app_namespace_add_del_t_handler (vl_api_app_namespace_add_del_t * mp)
 
   vnet_app_namespace_add_del_args_t args = {
     .ns_id = ns_id,
-    .netns = 0,
     .sock_name = 0,
     .secret = clib_net_to_host_u64 (mp->secret),
     .sw_if_index = clib_net_to_host_u32 (mp->sw_if_index),
@@ -759,7 +758,7 @@ vl_api_app_namespace_add_del_v2_t_handler (
   vl_api_app_namespace_add_del_v2_t *mp)
 {
   vl_api_app_namespace_add_del_v2_reply_t *rmp;
-  u8 *ns_id = 0, *netns = 0;
+  u8 *ns_id = 0;
   u32 appns_index = 0;
   int rv = 0;
 
@@ -770,13 +769,10 @@ vl_api_app_namespace_add_del_v2_t_handler (
     }
 
   mp->namespace_id[sizeof (mp->namespace_id) - 1] = 0;
-  mp->netns[sizeof (mp->netns) - 1] = 0;
   ns_id = format (0, "%s", &mp->namespace_id);
-  netns = format (0, "%s", &mp->netns);
 
   vnet_app_namespace_add_del_args_t args = {
     .ns_id = ns_id,
-    .netns = netns,
     .sock_name = 0,
     .secret = clib_net_to_host_u64 (mp->secret),
     .sw_if_index = clib_net_to_host_u32 (mp->sw_if_index),
@@ -795,7 +791,6 @@ vl_api_app_namespace_add_del_v2_t_handler (
        }
     }
   vec_free (ns_id);
-  vec_free (netns);
 
 done:
   REPLY_MACRO2 (VL_API_APP_NAMESPACE_ADD_DEL_V2_REPLY, ({
@@ -804,12 +799,56 @@ done:
                }));
 }
 
+static void
+vl_api_app_namespace_add_del_v4_t_handler (
+  vl_api_app_namespace_add_del_v4_t *mp)
+{
+  vl_api_app_namespace_add_del_v4_reply_t *rmp;
+  u8 *ns_id = 0, *sock_name = 0;
+  u32 appns_index = 0;
+  int rv = 0;
+  if (session_main_is_enabled () == 0)
+    {
+      rv = VNET_API_ERROR_FEATURE_DISABLED;
+      goto done;
+    }
+  mp->namespace_id[sizeof (mp->namespace_id) - 1] = 0;
+  ns_id = format (0, "%s", &mp->namespace_id);
+  sock_name = vl_api_from_api_to_new_vec (mp, &mp->sock_name);
+  vnet_app_namespace_add_del_args_t args = {
+    .ns_id = ns_id,
+    .sock_name = sock_name,
+    .secret = clib_net_to_host_u64 (mp->secret),
+    .sw_if_index = clib_net_to_host_u32 (mp->sw_if_index),
+    .ip4_fib_id = clib_net_to_host_u32 (mp->ip4_fib_id),
+    .ip6_fib_id = clib_net_to_host_u32 (mp->ip6_fib_id),
+    .is_add = mp->is_add,
+  };
+  rv = vnet_app_namespace_add_del (&args);
+  if (!rv && mp->is_add)
+    {
+      appns_index = app_namespace_index_from_id (ns_id);
+      if (appns_index == APP_NAMESPACE_INVALID_INDEX)
+       {
+         clib_warning ("app ns lookup failed id:%s", ns_id);
+         rv = VNET_API_ERROR_UNSPECIFIED;
+       }
+    }
+  vec_free (ns_id);
+  vec_free (sock_name);
+done:
+  REPLY_MACRO2 (VL_API_APP_NAMESPACE_ADD_DEL_V4_REPLY, ({
+                 if (!rv)
+                   rmp->appns_index = clib_host_to_net_u32 (appns_index);
+               }));
+}
+
 static void
 vl_api_app_namespace_add_del_v3_t_handler (
   vl_api_app_namespace_add_del_v3_t *mp)
 {
   vl_api_app_namespace_add_del_v3_reply_t *rmp;
-  u8 *ns_id = 0, *netns = 0, *sock_name = 0;
+  u8 *ns_id = 0, *sock_name = 0, *api_sock_name = 0;
   u32 appns_index = 0;
   int rv = 0;
   if (session_main_is_enabled () == 0)
@@ -818,13 +857,22 @@ vl_api_app_namespace_add_del_v3_t_handler (
       goto done;
     }
   mp->namespace_id[sizeof (mp->namespace_id) - 1] = 0;
-  mp->netns[sizeof (mp->netns) - 1] = 0;
   ns_id = format (0, "%s", &mp->namespace_id);
-  netns = format (0, "%s", &mp->netns);
-  sock_name = vl_api_from_api_to_new_vec (mp, &mp->sock_name);
+  api_sock_name = vl_api_from_api_to_new_vec (mp, &mp->sock_name);
+  mp->netns[sizeof (mp->netns) - 1] = 0;
+  if (strlen ((char *) mp->netns) != 0)
+    {
+      sock_name =
+       format (0, "abstract:%v,netns_name=%s", api_sock_name, &mp->netns);
+    }
+  else
+    {
+      sock_name = api_sock_name;
+      api_sock_name = 0; // for vec_free
+    }
+
   vnet_app_namespace_add_del_args_t args = {
     .ns_id = ns_id,
-    .netns = netns,
     .sock_name = sock_name,
     .secret = clib_net_to_host_u64 (mp->secret),
     .sw_if_index = clib_net_to_host_u32 (mp->sw_if_index),
@@ -843,8 +891,8 @@ vl_api_app_namespace_add_del_v3_t_handler (
        }
     }
   vec_free (ns_id);
-  vec_free (netns);
   vec_free (sock_name);
+  vec_free (api_sock_name);
 done:
   REPLY_MACRO2 (VL_API_APP_NAMESPACE_ADD_DEL_V3_REPLY, ({
                  if (!rv)
@@ -1655,27 +1703,10 @@ appns_sapi_add_ns_socket (app_namespace_t * app_ns)
   clib_socket_t *cs;
   char dir[4096];
 
-  if (app_ns->netns)
-    {
-      if (!app_ns->sock_name)
-       app_ns->sock_name = format (0, "@vpp/session/%v%c", app_ns->ns_id, 0);
-      if (app_ns->sock_name[0] != '@')
-       return VNET_API_ERROR_INVALID_VALUE;
-    }
-  else
-    {
-      snprintf (dir, sizeof (dir), "%s%s", vlib_unix_get_runtime_dir (),
-               subdir);
-      err = vlib_unix_recursive_mkdir ((char *) dir);
-      if (err)
-       {
-         clib_error_report (err);
-         return VNET_API_ERROR_SYSCALL_ERROR_1;
-       }
+  snprintf (dir, sizeof (dir), "%s%s", vlib_unix_get_runtime_dir (), subdir);
 
-      if (!app_ns->sock_name)
-       app_ns->sock_name = format (0, "%s%v%c", dir, app_ns->ns_id, 0);
-    }
+  if (!app_ns->sock_name)
+    app_ns->sock_name = format (0, "%s%v%c", dir, app_ns->ns_id, 0);
 
   /*
    * Create and initialize socket to listen on
@@ -1686,13 +1717,24 @@ appns_sapi_add_ns_socket (app_namespace_t * app_ns)
     CLIB_SOCKET_F_ALLOW_GROUP_WRITE |
     CLIB_SOCKET_F_SEQPACKET | CLIB_SOCKET_F_PASSCRED;
 
-  if ((err = clib_socket_init_netns (cs, app_ns->netns)))
+  if (clib_socket_prefix_get_type (cs->config) == CLIB_SOCKET_TYPE_UNIX)
+    {
+      err = vlib_unix_recursive_mkdir ((char *) dir);
+      if (err)
+       {
+         clib_error_report (err);
+         return VNET_API_ERROR_SYSCALL_ERROR_1;
+       }
+    }
+
+  if ((err = clib_socket_init (cs)))
     {
       clib_error_report (err);
       return -1;
     }
 
-  if (!app_ns->netns && stat ((char *) app_ns->sock_name, &file_stat) == -1)
+  if (clib_socket_prefix_get_type (cs->config) == CLIB_SOCKET_TYPE_UNIX &&
+      stat ((char *) app_ns->sock_name, &file_stat) == -1)
     return -1;
 
   /*
index 13970d5..770e726 100644 (file)
@@ -324,6 +324,18 @@ api_app_namespace_add_del (vat_main_t *vam)
   return ret;
 }
 
+static void
+vl_api_app_namespace_add_del_v4_reply_t_handler (
+  vl_api_app_namespace_add_del_v4_reply_t *mp)
+{
+}
+
+static int
+api_app_namespace_add_del_v4 (vat_main_t *vat)
+{
+  return -1;
+}
+
 static void
 vl_api_app_namespace_add_del_v3_reply_t_handler (
   vl_api_app_namespace_add_del_v3_reply_t *mp)
index 40374d7..ef0eaae 100644 (file)
@@ -374,6 +374,16 @@ clib_socket_prefix_is_valid (char *s)
   return 0;
 }
 
+__clib_export int
+clib_socket_prefix_get_type (char *s)
+{
+  for (typeof (clib_socket_type_data[0]) *d = clib_socket_type_data;
+       d - clib_socket_type_data < ARRAY_LEN (clib_socket_type_data); d++)
+    if (strncmp (s, d->prefix, strlen (d->prefix)) == 0)
+      return d->type;
+  return 0;
+}
+
 __clib_export clib_error_t *
 clib_socket_init (clib_socket_t *s)
 {
@@ -730,45 +740,6 @@ done:
   return err;
 }
 
-__clib_export clib_error_t *
-clib_socket_init_netns (clib_socket_t *s, u8 *namespace)
-{
-  if (namespace == NULL || namespace[0] == 0)
-    return clib_socket_init (s);
-
-  clib_error_t *error;
-  int old_netns_fd, nfd = -1;
-
-  old_netns_fd = clib_netns_open (NULL /* self */);
-  if (old_netns_fd < 0)
-    return clib_error_return_unix (0, "get current netns failed");
-
-  if ((nfd = clib_netns_open (namespace)) == -1)
-    {
-      error = clib_error_return_unix (0, "clib_netns_open '%s'", namespace);
-      goto done;
-    }
-
-  if (clib_setns (nfd) == -1)
-    {
-      error = clib_error_return_unix (0, "setns '%s'", namespace);
-      goto done;
-    }
-
-  error = clib_socket_init (s);
-
-done:
-  if (clib_setns (old_netns_fd) == -1)
-    clib_warning ("Cannot set old ns");
-
-  close (old_netns_fd);
-
-  if (-1 != nfd)
-    close (nfd);
-
-  return error;
-}
-
 __clib_export clib_error_t *
 clib_socket_accept (clib_socket_t * server, clib_socket_t * client)
 {
index 132e89f..13e09e6 100644 (file)
@@ -118,12 +118,11 @@ typedef struct _socket_t
    from IPPORT_USERRESERVED (5000). */
 clib_error_t *clib_socket_init (clib_socket_t * socket);
 
-clib_error_t *clib_socket_init_netns (clib_socket_t *socket, u8 *namespace);
-
 clib_error_t *clib_socket_accept (clib_socket_t * server,
                                  clib_socket_t * client);
 
 int clib_socket_prefix_is_valid (char *s);
+int clib_socket_prefix_get_type (char *s);
 
 always_inline uword
 clib_socket_is_server (clib_socket_t * sock)
index 31e9a81..681385a 100644 (file)
@@ -87,12 +87,12 @@ class QUICTestCase(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id=self.server_appns,
             secret=self.server_appns_secret,
             sw_if_index=self.loop0.sw_if_index,
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id=self.client_appns,
             secret=self.client_appns_secret,
             sw_if_index=self.loop1.sw_if_index,
index cbca98d..d013815 100644 (file)
@@ -40,10 +40,10 @@ class TestSession(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="0", sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="1", sw_if_index=self.loop1.sw_if_index
         )
 
index 678d2cc..184e570 100644 (file)
@@ -36,10 +36,10 @@ class TestTCP(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="0", sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="1", sw_if_index=self.loop1.sw_if_index
         )
 
index 68107f7..89ee450 100644 (file)
@@ -83,10 +83,10 @@ class TestTLS(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="0", sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="1", sw_if_index=self.loop1.sw_if_index
         )
 
index cfa9777..fbc8249 100644 (file)
@@ -162,10 +162,10 @@ class VCLTestCase(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="1", secret=1234, sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="2", secret=5678, sw_if_index=self.loop1.sw_if_index
         )
 
@@ -212,10 +212,10 @@ class VCLTestCase(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="1", secret=1234, sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="2", secret=5678, sw_if_index=self.loop1.sw_if_index
         )
 
index ebc99e8..a026b04 100644 (file)
@@ -709,10 +709,10 @@ class TestUDP(VppTestCase):
             table_id += 1
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="0", sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del(
+        self.vapi.app_namespace_add_del_v4(
             namespace_id="1", sw_if_index=self.loop1.sw_if_index
         )