sock api: fix registrations and client reads

- 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>

5 files changed, 69 insertions, 28 deletions

diff --git a/src/vlibmemory/api.h b/src/vlibmemory/api.h
index d66c4398764..dc1e75e484d 100644
--- a/src/vlibmemory/api.h
+++ b/src/vlibmemory/api.h
@@ -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);
 }
 
diff --git a/src/vlibmemory/socket_api.c b/src/vlibmemory/socket_api.c
index 4787069daa3..f3cd169d2c0 100644
--- a/src/vlibmemory/socket_api.c
+++ b/src/vlibmemory/socket_api.c
@@ -47,6 +47,27 @@
 
 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);
diff --git a/src/vlibmemory/socket_api.h b/src/vlibmemory/socket_api.h
index 00985fe7591..f3af300238c 100644
--- a/src/vlibmemory/socket_api.h
+++ b/src/vlibmemory/socket_api.h
@@ -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_ */
 
diff --git a/src/vlibmemory/socket_client.c b/src/vlibmemory/socket_client.c
index 3d933deecb6..c04b804eb21 100644
--- a/src/vlibmemory/socket_client.c
+++ b/src/vlibmemory/socket_client.c
@@ -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)
diff --git a/src/vlibmemory/socket_client.h b/src/vlibmemory/socket_client.h
index 5612a9809a5..46e2c865250 100644
--- a/src/vlibmemory/socket_client.h
+++ b/src/vlibmemory/socket_client.h
@@ -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;