sock api: fix registrations and client reads 14/15114/5
authorFlorin Coras <fcoras@cisco.com>
Wed, 3 Oct 2018 01:29:25 +0000 (18:29 -0700)
committerDave Barach <openvpp@barachs.net>
Wed, 3 Oct 2018 16:42:38 +0000 (16:42 +0000)
- When clients connect, instead of returing registration indicies return
handles. By convention socket registrations will have the MSB set to 1.
This makes it easy to distinguish them from shm registrations.
- Fix client reads to allow for messages larger than 4kB (needed for the
creat reply wherein the message table is provided).

Change-Id: I7bc0a072d066dffbf2e3ad9ba3ed50291231af9d
Signed-off-by: Florin Coras <fcoras@cisco.com>
src/vlibmemory/api.h
src/vlibmemory/socket_api.c
src/vlibmemory/socket_api.h
src/vlibmemory/socket_client.c
src/vlibmemory/socket_client.h

index d66c439..dc1e75e 100644 (file)
@@ -55,10 +55,8 @@ vl_api_can_send_msg (vl_api_registration_t * rp)
 always_inline vl_api_registration_t *
 vl_api_client_index_to_registration (u32 index)
 {
-  vl_api_registration_t *reg =
-    vl_socket_api_client_index_to_registration (index);
-  if (reg && reg->registration_type != REGISTRATION_TYPE_FREE)
-    return reg;
+  if (vl_socket_api_registration_handle_is_valid (ntohl (index)))
+    return vl_socket_api_client_handle_to_registration (ntohl (index));
   return vl_mem_api_client_index_to_registration (index);
 }
 
index 4787069..f3cd169 100644 (file)
 
 socket_main_t socket_main;
 
+#define SOCK_API_REG_HANDLE_BIT (1<<31)
+
+static u32
+sock_api_registration_handle (vl_api_registration_t * regp)
+{
+  ASSERT (regp->vl_api_registration_pool_index < SOCK_API_REG_HANDLE_BIT);
+  return regp->vl_api_registration_pool_index | SOCK_API_REG_HANDLE_BIT;
+}
+
+static u32
+socket_api_registration_handle_to_index (u32 reg_index)
+{
+  return (reg_index & ~SOCK_API_REG_HANDLE_BIT);
+}
+
+u8
+vl_socket_api_registration_handle_is_valid (u32 reg_handle)
+{
+  return ((reg_handle & SOCK_API_REG_HANDLE_BIT) != 0);
+}
+
 void
 vl_sock_api_dump_clients (vlib_main_t * vm, api_main_t * am)
 {
@@ -75,17 +96,18 @@ vl_sock_api_dump_clients (vlib_main_t * vm, api_main_t * am)
 }
 
 vl_api_registration_t *
-vl_socket_api_client_index_to_registration (u32 index)
+vl_socket_api_client_handle_to_registration (u32 handle)
 {
   socket_main_t *sm = &socket_main;
-  if (pool_is_free_index (sm->registration_pool, ntohl (index)))
+  u32 index = socket_api_registration_handle_to_index (handle);
+  if (pool_is_free_index (sm->registration_pool, index))
     {
 #if DEBUG > 2
-      clib_warning ("Invalid index %d\n", ntohl (index));
+      clib_warning ("Invalid index %d\n", index);
 #endif
       return 0;
     }
-  return pool_elt_at_index (sm->registration_pool, ntohl (index));
+  return pool_elt_at_index (sm->registration_pool, index);
 }
 
 void
@@ -419,7 +441,7 @@ vl_api_sockclnt_create_t_handler (vl_api_sockclnt_create_t * mp)
   u32 size = sizeof (*rp) + (nmsg * sizeof (vl_api_message_table_entry_t));
   rp = vl_msg_api_alloc (size);
   rp->_vl_msg_id = htons (VL_API_SOCKCLNT_CREATE_REPLY);
-  rp->index = htonl (regp->vl_api_registration_pool_index);
+  rp->index = htonl (sock_api_registration_handle (regp));
   rp->context = mp->context;
   rp->response = htonl (rv);
   rp->count = htons (nmsg);
index 00985fe..f3af300 100644 (file)
@@ -75,7 +75,8 @@ clib_error_t *vl_sock_api_send_fd_msg (int socket_fd, int fds[], int n_fds);
 clib_error_t *vl_sock_api_recv_fd_msg (int socket_fd, int fds[], int n_fds,
                                       u32 wait);
 
-vl_api_registration_t *vl_socket_api_client_index_to_registration (u32 index);
+vl_api_registration_t *vl_socket_api_client_handle_to_registration (u32 idx);
+u8 vl_socket_api_registration_handle_is_valid (u32 reg_index);
 
 #endif /* SRC_VLIBMEMORY_SOCKET_API_H_ */
 
index 3d933de..c04b804 100644 (file)
@@ -56,6 +56,7 @@ int
 vl_socket_client_read (int wait)
 {
   socket_client_main_t *scm = &socket_client_main;
+  u32 data_len = 0, msg_size;
   int n, current_rx_index;
   msgbuf_t *mbp = 0;
   f64 timeout;
@@ -68,10 +69,9 @@ vl_socket_client_read (int wait)
 
   while (1)
     {
-      current_rx_index = vec_len (scm->socket_rx_buffer);
-      while (vec_len (scm->socket_rx_buffer) <
-            sizeof (*mbp) + 2 /* msg id */ )
+      while (vec_len (scm->socket_rx_buffer) < sizeof (*mbp))
        {
+         current_rx_index = vec_len (scm->socket_rx_buffer);
          vec_validate (scm->socket_rx_buffer, current_rx_index
                        + scm->socket_buffer_size - 1);
          _vec_len (scm->socket_rx_buffer) = current_rx_index;
@@ -90,20 +90,35 @@ vl_socket_client_read (int wait)
        clib_warning ("read %d bytes", n);
 #endif
 
-      if (mbp == 0)
-       mbp = (msgbuf_t *) (scm->socket_rx_buffer);
+      mbp = (msgbuf_t *) (scm->socket_rx_buffer);
+      data_len = ntohl (mbp->data_len);
+      current_rx_index = vec_len (scm->socket_rx_buffer);
+      vec_validate (scm->socket_rx_buffer, current_rx_index + data_len);
+      _vec_len (scm->socket_rx_buffer) = current_rx_index;
+      mbp = (msgbuf_t *) (scm->socket_rx_buffer);
+      msg_size = data_len + sizeof (*mbp);
+
+      while (vec_len (scm->socket_rx_buffer) < msg_size)
+       {
+         n = read (scm->socket_fd,
+                   scm->socket_rx_buffer + vec_len (scm->socket_rx_buffer),
+                   msg_size - vec_len (scm->socket_rx_buffer));
+         if (n < 0 && errno != EAGAIN)
+           {
+             clib_unix_warning ("socket_read");
+             return -1;
+           }
+         _vec_len (scm->socket_rx_buffer) += n;
+       }
 
-      if (vec_len (scm->socket_rx_buffer) >= ntohl (mbp->data_len)
-         + sizeof (*mbp))
+      if (vec_len (scm->socket_rx_buffer) >= data_len + sizeof (*mbp))
        {
          vl_msg_api_socket_handler ((void *) (mbp->data));
 
-         if (vec_len (scm->socket_rx_buffer) == ntohl (mbp->data_len)
-             + sizeof (*mbp))
+         if (vec_len (scm->socket_rx_buffer) == data_len + sizeof (*mbp))
            _vec_len (scm->socket_rx_buffer) = 0;
          else
-           vec_delete (scm->socket_rx_buffer, ntohl (mbp->data_len)
-                       + sizeof (*mbp), 0);
+           vec_delete (scm->socket_rx_buffer, data_len + sizeof (*mbp), 0);
          mbp = 0;
 
          /* Quit if we're out of data, and not expecting a ping reply */
@@ -111,7 +126,6 @@ vl_socket_client_read (int wait)
              && scm->control_pings_outstanding == 0)
            break;
        }
-
       if (wait && clib_time_now (&scm->clib_time) >= timeout)
        return -1;
     }
@@ -295,7 +309,10 @@ vl_api_sockclnt_create_reply_t_handler (vl_api_sockclnt_create_reply_t * mp)
 {
   socket_client_main_t *scm = &socket_client_main;
   if (!mp->response)
-    scm->socket_enable = 1;
+    {
+      scm->socket_enable = 1;
+      scm->client_index = clib_net_to_host_u32 (mp->index);
+    }
 }
 
 #define foreach_sock_client_api_msg                            \
@@ -366,19 +383,21 @@ vl_socket_client_connect (char *socket_path, char *client_name,
   mp->name[sizeof (mp->name) - 1] = 0;
   mp->context = 0xfeedface;
 
+  clib_time_init (&scm->clib_time);
+
   if (vl_socket_client_write () <= 0)
     return (-1);
 
-  if (vl_socket_client_read (1))
+  if (vl_socket_client_read (5))
     return (-1);
 
-  clib_time_init (&scm->clib_time);
   return (0);
 }
 
 int
 vl_socket_client_init_shm (vl_api_shm_elem_config_t * config)
 {
+  socket_client_main_t *scm = &socket_client_main;
   vl_api_sock_init_shm_t *mp;
   int rv, i;
   u64 *cfg;
@@ -387,7 +406,7 @@ vl_socket_client_init_shm (vl_api_shm_elem_config_t * config)
                                   vec_len (config) * sizeof (u64));
   memset (mp, 0, sizeof (*mp));
   mp->_vl_msg_id = clib_host_to_net_u16 (VL_API_SOCK_INIT_SHM);
-  mp->client_index = ~0;
+  mp->client_index = clib_host_to_net_u32 (scm->client_index);
   mp->requested_size = 64 << 20;
 
   if (config)
index 5612a98..46e2c86 100644 (file)
@@ -25,8 +25,9 @@
 typedef struct
 {
   int socket_fd;
-  /* Temporarily disable the connection, so we can keep it around... */
-  int socket_enable;
+  int socket_enable;           /**< Can temporarily disable the connection
+                                    but still can keep it around... */
+  u32 client_index;            /**< Client index allocated by VPP */
 
   clib_socket_t client_socket;