ldp: fixes/improvements to select

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

2 files changed, 84 insertions, 94 deletions

diff --git a/src/vcl/ldp.c b/src/vcl/ldp.c
index e806dff7959..beaa988185d 100644
--- a/src/vcl/ldp.c
+++ b/src/vcl/ldp.c
@@ -612,36 +612,35 @@ fcntl (int fd, int cmd, ...)
 	{
 	case F_SETFL:
 	  func_str = "vppcom_session_attr[SET_FLAGS]";
-	  if (LDP_DEBUG > 2)
-	    clib_warning ("LDP<%d>: fd %d (0x%x): calling %s(): "
-			  "sid %u (0x%x) flags %d (0x%x), size %d",
-			  getpid (), fd, fd, func_str, sid, sid,
-			  flags, flags, size);
+	  LDBG (2, "LDP<%d>: fd %d (0x%x): calling %s(): sid %u (0x%x) "
+		"flags %d (0x%x), size %d", getpid (), fd, fd, func_str, sid,
+		sid, flags, flags, size);
 
-	  rv =
-	    vppcom_session_attr (sid, VPPCOM_ATTR_SET_FLAGS, &flags, &size);
+	  rv = vppcom_session_attr (sid, VPPCOM_ATTR_SET_FLAGS, &flags,
+				    &size);
 	  break;
 
 	case F_GETFL:
 	  func_str = "vppcom_session_attr[GET_FLAGS]";
-	  if (LDP_DEBUG > 2)
-	    clib_warning
-	      ("LDP<%d>: fd %d (0x%x): calling %s(): sid %u (0x%x), "
-	       "flags %d (0x%x), size %d", getpid (), fd, fd, func_str, sid,
-	       sid, flags, flags, size);
+	  LDBG (2, "LDP<%d>: fd %d (0x%x): calling %s(): sid %u (0x%x), "
+		"flags %d (0x%x), size %d", getpid (), fd, fd, func_str, sid,
+		sid, flags, flags, size);
 
-	  rv =
-	    vppcom_session_attr (sid, VPPCOM_ATTR_GET_FLAGS, &flags, &size);
+	  rv = vppcom_session_attr (sid, VPPCOM_ATTR_GET_FLAGS, &flags,
+				    &size);
 	  if (rv == VPPCOM_OK)
 	    {
-	      if (LDP_DEBUG > 2)
-		clib_warning ("LDP<%d>: fd %d (0x%x), cmd %d (F_GETFL): "
-			      "%s() returned flags %d (0x%x)",
-			      getpid (), fd, fd, cmd, func_str, flags, flags);
+	      LDBG (2, "LDP<%d>: fd %d (0x%x), cmd %d (F_GETFL): %s() "
+		    "returned flags %d (0x%x)", getpid (), fd, fd, cmd,
+		    func_str, flags, flags);
 	      rv = flags;
 	    }
 	  break;
-
+	case F_SETFD:
+	  /* TODO handle this */
+	  LDBG (0, "F_SETFD ignored flags %u", flags);
+	  rv = 0;
+	  break;
 	default:
 	  rv = -EOPNOTSUPP;
 	  break;
@@ -800,9 +799,8 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
       /* select as fine grained sleep */
       if (!nfds)
 	{
-	  if (LDP_DEBUG > 3)
-	    clib_warning ("LDP<%d>: sleeping for %.02f seconds",
-			  getpid (), time_out);
+	  LDBG (3, "LDP<%d>: sleeping for %.02f seconds", getpid (),
+		time_out);
 
 	  time_out += clib_time_now (&ldp->clib_time);
 	  while (clib_time_now (&ldp->clib_time) < time_out)
@@ -823,11 +821,9 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
     {
       func_str = "libc_pselect";
 
-      if (LDP_DEBUG > 3)
-	clib_warning
-	  ("LDP<%d>: calling %s(): nfds %d, readfds %p, writefds %p, "
-	   "exceptfds %p, timeout %p, sigmask %p", getpid (), func_str, nfds,
-	   readfds, writefds, exceptfds, timeout, sigmask);
+      LDBG (3, "LDP<%d>: calling %s(): nfds %d, readfds %p, writefds %p, "
+	    "exceptfds %p, timeout %p, sigmask %p", getpid (), func_str, nfds,
+	    readfds, writefds, exceptfds, timeout, sigmask);
 
       rv = libc_pselect (nfds, readfds, writefds, exceptfds,
 			 timeout, sigmask);
@@ -845,27 +841,27 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
     }
 
   sid_bits = libc_bits = 0;
+  u32 n_bytes = nfds / 8 + ((nfds % 8) ? 1 : 0);
   if (readfds)
     {
       clib_bitmap_validate (ldp->sid_rd_bitmap, minbits);
       clib_bitmap_validate (ldp->libc_rd_bitmap, minbits);
       clib_bitmap_validate (ldp->rd_bitmap, minbits);
-      clib_memcpy_fast (ldp->rd_bitmap, readfds,
-			vec_len (ldp->rd_bitmap) * sizeof (clib_bitmap_t));
-      FD_ZERO (readfds);
+      clib_memcpy_fast (ldp->rd_bitmap, readfds, n_bytes);
+      memset (readfds, 0, n_bytes);
 
       /* *INDENT-OFF* */
-      clib_bitmap_foreach (fd, ldp->rd_bitmap,
-        ({
-          sid = ldp_sid_from_fd (fd);
-          if (LDP_DEBUG > 3)
-            clib_warning ("LDP<%d>: readfds: fd %d (0x%x), sid %u (0x%x)",
-                          getpid (), fd, fd, sid, sid);
-          if (sid == INVALID_SESSION_ID)
-            clib_bitmap_set_no_check (ldp->libc_rd_bitmap, fd, 1);
-          else
-            clib_bitmap_set_no_check (ldp->sid_rd_bitmap, sid, 1);
-        }));
+      clib_bitmap_foreach (fd, ldp->rd_bitmap, ({
+	if (fd > nfds)
+	  break;
+        sid = ldp_sid_from_fd (fd);
+        LDBG (3, "LDP<%d>: readfds: fd %d (0x%x), sid %u (0x%x)",
+              getpid (), fd, fd, sid, sid);
+        if (sid == INVALID_SESSION_ID)
+          clib_bitmap_set_no_check (ldp->libc_rd_bitmap, fd, 1);
+        else
+          clib_bitmap_set_no_check (ldp->sid_rd_bitmap, sid, 1);
+      }));
       /* *INDENT-ON* */
 
       sid_bits_set = clib_bitmap_last_set (ldp->sid_rd_bitmap) + 1;
@@ -874,32 +870,30 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
       libc_bits_set = clib_bitmap_last_set (ldp->libc_rd_bitmap) + 1;
       libc_bits = (libc_bits_set > libc_bits) ? libc_bits_set : libc_bits;
 
-      if (LDP_DEBUG > 3)
-	clib_warning ("LDP<%d>: readfds: sid_bits_set %d, sid_bits %d, "
-		      "libc_bits_set %d, libc_bits %d", getpid (),
-		      sid_bits_set, sid_bits, libc_bits_set, libc_bits);
+      LDBG (3, "LDP<%d>: readfds: sid_bits_set %d, sid_bits %d, "
+	    "libc_bits_set %d, libc_bits %d", getpid (), sid_bits_set,
+	    sid_bits, libc_bits_set, libc_bits);
     }
   if (writefds)
     {
       clib_bitmap_validate (ldp->sid_wr_bitmap, minbits);
       clib_bitmap_validate (ldp->libc_wr_bitmap, minbits);
       clib_bitmap_validate (ldp->wr_bitmap, minbits);
-      clib_memcpy_fast (ldp->wr_bitmap, writefds,
-			vec_len (ldp->wr_bitmap) * sizeof (clib_bitmap_t));
-      FD_ZERO (writefds);
+      clib_memcpy_fast (ldp->wr_bitmap, writefds, n_bytes);
+      memset (writefds, 0, n_bytes);
 
       /* *INDENT-OFF* */
-      clib_bitmap_foreach (fd, ldp->wr_bitmap,
-        ({
-          sid = ldp_sid_from_fd (fd);
-          if (LDP_DEBUG > 3)
-            clib_warning ("LDP<%d>: writefds: fd %d (0x%x), sid %u (0x%x)",
-                          getpid (), fd, fd, sid, sid);
-          if (sid == INVALID_SESSION_ID)
-            clib_bitmap_set_no_check (ldp->libc_wr_bitmap, fd, 1);
-          else
-            clib_bitmap_set_no_check (ldp->sid_wr_bitmap, sid, 1);
-        }));
+      clib_bitmap_foreach (fd, ldp->wr_bitmap, ({
+	if (fd > nfds)
+	  break;
+        sid = ldp_sid_from_fd (fd);
+        LDBG (3, "LDP<%d>: writefds: fd %d (0x%x), sid %u (0x%x)",
+                        getpid (), fd, fd, sid, sid);
+        if (sid == INVALID_SESSION_ID)
+          clib_bitmap_set_no_check (ldp->libc_wr_bitmap, fd, 1);
+        else
+          clib_bitmap_set_no_check (ldp->sid_wr_bitmap, sid, 1);
+      }));
       /* *INDENT-ON* */
 
       sid_bits_set = clib_bitmap_last_set (ldp->sid_wr_bitmap) + 1;
@@ -908,32 +902,30 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
       libc_bits_set = clib_bitmap_last_set (ldp->libc_wr_bitmap) + 1;
       libc_bits = (libc_bits_set > libc_bits) ? libc_bits_set : libc_bits;
 
-      if (LDP_DEBUG > 3)
-	clib_warning ("LDP<%d>: writefds: sid_bits_set %d, sid_bits %d, "
-		      "libc_bits_set %d, libc_bits %d", getpid (),
-		      sid_bits_set, sid_bits, libc_bits_set, libc_bits);
+      LDBG (3, "LDP<%d>: writefds: sid_bits_set %d, sid_bits %d, "
+	    "libc_bits_set %d, libc_bits %d", getpid (),
+	    sid_bits_set, sid_bits, libc_bits_set, libc_bits);
     }
   if (exceptfds)
     {
       clib_bitmap_validate (ldp->sid_ex_bitmap, minbits);
       clib_bitmap_validate (ldp->libc_ex_bitmap, minbits);
       clib_bitmap_validate (ldp->ex_bitmap, minbits);
-      clib_memcpy_fast (ldp->ex_bitmap, exceptfds,
-			vec_len (ldp->ex_bitmap) * sizeof (clib_bitmap_t));
-      FD_ZERO (exceptfds);
+      clib_memcpy_fast (ldp->ex_bitmap, exceptfds, n_bytes);
+      memset (exceptfds, 0, n_bytes);
 
       /* *INDENT-OFF* */
-      clib_bitmap_foreach (fd, ldp->ex_bitmap,
-        ({
-          sid = ldp_sid_from_fd (fd);
-          if (LDP_DEBUG > 3)
-            clib_warning ("LDP<%d>: exceptfds: fd %d (0x%x), sid %u (0x%x)",
-                          getpid (), fd, fd, sid, sid);
-          if (sid == INVALID_SESSION_ID)
-            clib_bitmap_set_no_check (ldp->libc_ex_bitmap, fd, 1);
-          else
-            clib_bitmap_set_no_check (ldp->sid_ex_bitmap, sid, 1);
-        }));
+      clib_bitmap_foreach (fd, ldp->ex_bitmap, ({
+	if (fd > nfds)
+	  break;
+        sid = ldp_sid_from_fd (fd);
+        LDBG (3, "LDP<%d>: exceptfds: fd %d (0x%x), sid %u (0x%x)",
+                        getpid (), fd, fd, sid, sid);
+        if (sid == INVALID_SESSION_ID)
+          clib_bitmap_set_no_check (ldp->libc_ex_bitmap, fd, 1);
+        else
+          clib_bitmap_set_no_check (ldp->sid_ex_bitmap, sid, 1);
+      }));
       /* *INDENT-ON* */
 
       sid_bits_set = clib_bitmap_last_set (ldp->sid_ex_bitmap) + 1;
@@ -942,10 +934,9 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
       libc_bits_set = clib_bitmap_last_set (ldp->libc_ex_bitmap) + 1;
       libc_bits = (libc_bits_set > libc_bits) ? libc_bits_set : libc_bits;
 
-      if (LDP_DEBUG > 3)
-	clib_warning ("LDP<%d>: exceptfds: sid_bits_set %d, sid_bits %d, "
-		      "libc_bits_set %d, libc_bits %d", getpid (),
-		      sid_bits_set, sid_bits, libc_bits_set, libc_bits);
+      LDBG (3, "LDP<%d>: exceptfds: sid_bits_set %d, sid_bits %d, "
+	    "libc_bits_set %d, libc_bits %d", getpid (),
+	    sid_bits_set, sid_bits, libc_bits_set, libc_bits);
     }
 
   if (PREDICT_FALSE (!sid_bits && !libc_bits))
@@ -1074,15 +1065,15 @@ ldp_pselect (int nfds, fd_set * __restrict readfds,
 
 done:
   /* TBD: set timeout to amount of time left */
-  vec_reset_length (ldp->rd_bitmap);
-  vec_reset_length (ldp->sid_rd_bitmap);
-  vec_reset_length (ldp->libc_rd_bitmap);
-  vec_reset_length (ldp->wr_bitmap);
-  vec_reset_length (ldp->sid_wr_bitmap);
-  vec_reset_length (ldp->libc_wr_bitmap);
-  vec_reset_length (ldp->ex_bitmap);
-  vec_reset_length (ldp->sid_ex_bitmap);
-  vec_reset_length (ldp->libc_ex_bitmap);
+  clib_bitmap_zero (ldp->rd_bitmap);
+  clib_bitmap_zero (ldp->sid_rd_bitmap);
+  clib_bitmap_zero (ldp->libc_rd_bitmap);
+  clib_bitmap_zero (ldp->wr_bitmap);
+  clib_bitmap_zero (ldp->sid_wr_bitmap);
+  clib_bitmap_zero (ldp->libc_wr_bitmap);
+  clib_bitmap_zero (ldp->ex_bitmap);
+  clib_bitmap_zero (ldp->sid_ex_bitmap);
+  clib_bitmap_zero (ldp->libc_ex_bitmap);
 
   if (LDP_DEBUG > 3)
     {
@@ -1227,10 +1218,9 @@ socketpair (int domain, int type, int protocol, int fds[2])
     {
       func_str = "libc_socket";
 
-      if (LDP_DEBUG > 1)
-	clib_warning ("LDP<%d>: : calling %s()", getpid (), func_str);
+      LDBG (1, "LDP<%d>: : calling %s()", getpid (), func_str);
 
-      rv = libc_socket (domain, type, protocol);
+      rv = libc_socketpair (domain, type, protocol, fds);
     }
 
   if (LDP_DEBUG > 1)
diff --git a/src/vcl/ldp.h b/src/vcl/ldp.h
index f96219f753b..304b79c40fd 100644
--- a/src/vcl/ldp.h
+++ b/src/vcl/ldp.h
@@ -31,7 +31,7 @@
 #define LDP_ENV_APP_NAME  "LDP_APP_NAME"
 #define LDP_ENV_SID_BIT   "LDP_SID_BIT"
 
-#define LDP_SID_BIT_MIN   9
+#define LDP_SID_BIT_MIN   5
 #define LDP_SID_BIT_MAX   30
 
 #define LDP_APP_NAME_MAX  256