VCL-LDPRELOAD: Refactor vcom_socket* and fix crash in vppcom_select

- filter verbose debug output with VCOM_DEBUG > 2
- clean up nomenclature, renaming vppcom_*() functions to
  vcom_session_*()
- fix vppcom_select crash with NULL maps.

Change-Id: I6e416a096d6fd800aa26991c2439e24e8fc38cc5
Signed-off-by: Dave Wallace <dwallacelf@gmail.com>

diff --git a/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom.c b/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom.c
index 35dbf57..6ddb245 100644 (file)
--- a/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom.c
+++ b/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom.c
@@ -237,14 +237,14 @@ read (int __fd, void *__buf, size_t __nbytes)
 
   if (is_vcom_socket_fd (__fd))
     {
-      if (VCOM_DEBUG > 0)
+      if (VCOM_DEBUG > 2)
        fprintf (stderr,
                 "[%d][%lu (0x%lx)] read:1 "
                 "'%04d'='%04d', '%p', '%04d'\n",
                 pid, (unsigned long) tid, (unsigned long) tid,
                 (int) size, __fd, __buf, (int) __nbytes);
       size = vcom_read (__fd, __buf, __nbytes);
-      if (VCOM_DEBUG > 0)
+      if (VCOM_DEBUG > 2)
        fprintf (stderr,
                 "[%d][%lu (0x%lx)] read:2 "
                 "'%04d'='%04d', '%p', '%04d'\n",
@@ -314,14 +314,14 @@ write (int __fd, const void *__buf, size_t __n)
 
   if (is_vcom_socket_fd (__fd))
     {
-      if (VCOM_DEBUG > 0)
+      if (VCOM_DEBUG > 2)
        fprintf (stderr,
                 "[%d][%lu (0x%lx)] write:1 "
                 "'%04d'='%04d', '%p', '%04d'\n",
                 pid, (unsigned long) tid, (unsigned long) tid,
                 (int) size, __fd, __buf, (int) __n);
       size = vcom_write (__fd, __buf, __n);
-      if (VCOM_DEBUG > 0)
+      if (VCOM_DEBUG > 2)
        fprintf (stderr,
                 "[%d][%lu (0x%lx)] write:2 "
                 "'%04d'='%04d', '%p', '%04d'\n",
@@ -1219,7 +1219,7 @@ vcom_select (int __nfds, fd_set * __restrict __readfds,
                                       __writefds ? &vcom_writefds : NULL,
                                       __exceptfds ? &vcom_exceptfds : NULL,
                                       &tv);
-         if (VCOM_DEBUG > 0)
+         if (VCOM_DEBUG > 2)
            fprintf (stderr,
                     "[%d] select vcom: "
                     "'%04d'='%04d'\n", pid, vcom_nfd, vcom_nfds);
@@ -1237,7 +1237,7 @@ vcom_select (int __nfds, fd_set * __restrict __readfds,
                                  __readfds ? &libc_readfds : NULL,
                                  __writefds ? &libc_writefds : NULL,
                                  __exceptfds ? &libc_exceptfds : NULL, &tv);
-         if (VCOM_DEBUG > 0)
+         if (VCOM_DEBUG > 2)
            fprintf (stderr,
                     "[%d] select libc: "
                     "'%04d'='%04d'\n", pid, libc_nfd, libc_nfds);
@@ -1328,7 +1328,7 @@ vcom_select (int __nfds, fd_set * __restrict __readfds,
   rv = 0;
 
 select_done:
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] vselect1: " "'%04d'='%04d'\n", pid, rv, __nfds);
   /*
    * modify timeout parameter to reflect the amount of time not slept
@@ -1358,7 +1358,7 @@ select_done:
            }
        }
     }
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] vselect2: " "'%04d',='%04d'\n", pid, rv, __nfds);
 
   return rv;
@@ -1486,10 +1486,10 @@ select (int __nfds, fd_set * __restrict __readfds,
   int rv = 0;
   pid_t pid = getpid ();
 
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] select1: " "'%04d'='%04d'\n", pid, rv, __nfds);
   rv = vcom_select (__nfds, __readfds, __writefds, __exceptfds, __timeout);
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] select2: " "'%04d'='%04d'\n", pid, rv, __nfds);
   if (rv < 0)
     {
@@ -1656,7 +1656,7 @@ pselect (int __nfds, fd_set * __restrict __readfds,
       rv = nfd;
     }
 
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] pselect: " "'%04d'='%04d'\n", pid, rv, __nfds);
   return rv;
 }
@@ -2413,7 +2413,7 @@ getsockopt (int __fd, int __level, int __optname,
   if (is_vcom_socket_fd (__fd))
     {
       rv = vcom_getsockopt (__fd, __level, __optname, __optval, __optlen);
-      if (VCOM_DEBUG > 0)
+      if (VCOM_DEBUG > 2)
        fprintf (stderr,
                 "[%d] getsockopt: "
                 "'%04d'='%04d', '%04d', '%04d', "
@@ -3130,7 +3130,7 @@ vcom_poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
           * to return immediately
           * */
          rlibc_nfds = libc_poll (__fds, __nfds, 0);
-         if (VCOM_DEBUG > 0)
+         if (VCOM_DEBUG > 2)
            fprintf (stderr,
                     "[%d] poll libc: "
                     "'%04d'='%08lu'\n", pid, rlibc_nfds, __nfds);
@@ -3155,7 +3155,7 @@ vcom_poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
           * to return immediately
           * */
          rvcom_nfds = vcom_socket_poll (vcom_fds, __nfds, 0);
-         if (VCOM_DEBUG > 0)
+         if (VCOM_DEBUG > 2)
            fprintf (stderr,
                     "[%d] poll vcom: "
                     "'%04d'='%08lu'\n", pid, rvcom_nfds, __nfds);
@@ -3214,7 +3214,7 @@ poll_done_update_nfds:
     }
 
 poll_done:
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] vpoll: " "'%04d'='%08lu'\n", pid, rv, __nfds);
   return rv;
 }
@@ -3242,11 +3242,11 @@ poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
   pid_t pid = getpid ();
 
 
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] poll1: " "'%04d'='%08lu, %d, 0x%x'\n",
             pid, rv, __nfds, __fds[0].fd, __fds[0].events);
   rv = vcom_poll (__fds, __nfds, __timeout);
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr, "[%d] poll2: " "'%04d'='%08lu, %d, 0x%x'\n",
             pid, rv, __nfds, __fds[0].fd, __fds[0].revents);
   if (rv < 0)

diff --git a/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom_socket.c b/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom_socket.c
index 4a5f285..7b5f6c8 100644 (file)
--- a/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom_socket.c
+++ b/extras/vcl-ldpreload/src/libvcl-ldpreload/vcom_socket.c
@@ -673,8 +673,8 @@ vcom_socket_check_fcntl_cmd (int __cmd)
   return 0;
 }
 
-static int
-vppcom_session_fcntl_va (int __sid, int __cmd, va_list __ap)
+static inline int
+vcom_session_fcntl_va (int __sid, int __cmd, va_list __ap)
 {
   int flags = va_arg (__ap, int);
   int rv = -EOPNOTSUPP;
@@ -730,7 +730,7 @@ vcom_socket_fcntl_va (int __fd, int __cmd, va_list __ap)
       break;
       /* cmd handled by vppcom */
     case 3:
-      rv = vppcom_session_fcntl_va (vsock->sid, __cmd, __ap);
+      rv = vcom_session_fcntl_va (vsock->sid, __cmd, __ap);
       break;
 
     default:
@@ -767,8 +767,8 @@ vcom_socket_check_ioctl_cmd (unsigned long int __cmd)
   return rc;
 }
 
-static int
-vppcom_session_ioctl_va (int __sid, int __cmd, va_list __ap)
+static inline int
+vcom_session_ioctl_va (int __sid, int __cmd, va_list __ap)
 {
   int rv;
 
@@ -817,7 +817,7 @@ vcom_socket_ioctl_va (int __fd, unsigned long int __cmd, va_list __ap)
 
       /* cmd handled by vppcom */
     case 3:
-      rv = vppcom_session_ioctl_va (vsock->sid, __cmd, __ap);
+      rv = vcom_session_ioctl_va (vsock->sid, __cmd, __ap);
       break;
 
     default:
@@ -1005,6 +1005,8 @@ vcom_socket_select (int vcom_nfds, fd_set * __restrict vcom_readfds,
                    fd_set * __restrict vcom_exceptfds,
                    struct timeval *__restrict timeout)
 {
+  static unsigned long vcom_nsid_fds = 0;
+  int vcom_nsid = 0;
   int rv = -EBADF;
   pid_t pid = getpid ();
 
@@ -1015,8 +1017,6 @@ vcom_socket_select (int vcom_nfds, fd_set * __restrict vcom_readfds,
   fd_set vcom_rd_sid_fds;
   fd_set vcom_wr_sid_fds;
   fd_set vcom_ex_sid_fds;
-  unsigned long vcom_nsid_fds = 0;
-  int vcom_nsid = 0;
 
   /* in seconds eg. 3.123456789 seconds */
   double time_to_wait = (double) 0;
@@ -1070,6 +1070,24 @@ vcom_socket_select (int vcom_nfds, fd_set * __restrict vcom_readfds,
   _(&vcom_ex_sid_fds, vcom_exceptfds);
 #undef _
 
+  if (vcom_nfds == 0)
+    {
+      if (time_to_wait > 0)
+       {
+         if (VCOM_DEBUG > 0)
+           fprintf (stderr,
+                    "[%d] vcom_socket_select called to "
+                    "emulate delay_ns()!\n", pid);
+         rv = vppcom_select (0, NULL, NULL, NULL, time_to_wait);
+       }
+      else
+       {
+         fprintf (stderr, "[%d] vcom_socket_select called vcom_nfds = 0 "
+                  "and invalid time_to_wait (%f)!\n", pid, time_to_wait);
+       }
+      return 0;
+    }
+
   /* populate read, write and except sid_sets */
   vcom_nsid = vcom_socket_fds_2_sid_fds (
                                          /* dest */
@@ -1102,8 +1120,8 @@ vcom_socket_select (int vcom_nfds, fd_set * __restrict vcom_readfds,
                      NULL,
                      vcom_exceptfds ? (unsigned long *) &vcom_ex_sid_fds :
                      NULL, time_to_wait);
-  if (VCOM_DEBUG > 0)
-    fprintf (stderr, "[%d] vppcom_select: "
+  if (VCOM_DEBUG > 2)
+    fprintf (stderr, "[%d] called vppcom_select(): "
             "'%04d'='%04d'\n", pid, rv, (int) vcom_nsid_fds);
 
   /* check if any file descriptors changed status */
@@ -1259,8 +1277,8 @@ vcom_socket_bind (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len)
   return rv;
 }
 
-int
-vppcom_session_getsockname (int sid, vppcom_endpt_t * ep)
+static inline int
+vcom_session_getsockname (int sid, vppcom_endpt_t * ep)
 {
   int rv;
   uint32_t size = sizeof (*ep);
@@ -1300,7 +1318,7 @@ vcom_socket_getsockname (int __fd, __SOCKADDR_ARG __addr,
 
   vppcom_endpt_t ep;
   ep.ip = (u8 *) & ((const struct sockaddr_in *) __addr)->sin_addr;
-  rv = vppcom_session_getsockname (vsock->sid, &ep);
+  rv = vcom_session_getsockname (vsock->sid, &ep);
   if (rv == 0)
     {
       if (ep.vrf == VPPCOM_VRF_DEFAULT)
@@ -1371,8 +1389,8 @@ vcom_socket_connect (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len)
   return rv;
 }
 
-int
-vppcom_session_getpeername (int sid, vppcom_endpt_t * ep)
+static inline int
+vcom_session_getpeername (int sid, vppcom_endpt_t * ep)
 {
   int rv;
   uint32_t size = sizeof (*ep);
@@ -1412,7 +1430,7 @@ vcom_socket_getpeername (int __fd, __SOCKADDR_ARG __addr,
 
   vppcom_endpt_t ep;
   ep.ip = (u8 *) & ((const struct sockaddr_in *) __addr)->sin_addr;
-  rv = vppcom_session_getpeername (vsock->sid, &ep);
+  rv = vcom_session_getpeername (vsock->sid, &ep);
   if (rv == 0)
     {
       if (ep.vrf == VPPCOM_VRF_DEFAULT)
@@ -1499,10 +1517,10 @@ vcom_socket_is_connection_mode_socket (int __fd)
   return 0;
 }
 
-ssize_t
-vvppcom_session_sendto (int __sid, const void *__buf, size_t __n,
-                       int __flags, __CONST_SOCKADDR_ARG __addr,
-                       socklen_t __addr_len)
+static inline ssize_t
+vcom_session_sendto (int __sid, void *__buf, size_t __n,
+                    int __flags, __CONST_SOCKADDR_ARG __addr,
+                    socklen_t __addr_len)
 {
   int rv = -1;
   /* TBD add new vpp api  */
@@ -1529,10 +1547,7 @@ vcom_socket_sendto (int __fd, const void *__buf, size_t __n,
   if (!vsock)
     return -ENOTSOCK;
 
-  if (vsock->type != SOCKET_TYPE_VPPCOM_BOUND)
-    return -EINVAL;
-
-  if (!__buf || __n < 0)
+  if ((vsock->type != SOCKET_TYPE_VPPCOM_BOUND) || !__buf || __n < 0)
     {
       return -EINVAL;
     }
@@ -1559,16 +1574,15 @@ vcom_socket_sendto (int __fd, const void *__buf, size_t __n,
        }
     }
 
-  rv = vvppcom_session_sendto (vsock->sid, (void *) __buf, (int) __n,
-                              __flags, __addr, __addr_len);
+  rv = vcom_session_sendto (vsock->sid, (void *) __buf, (int) __n,
+                           __flags, __addr, __addr_len);
   return rv;
 }
 
-/* TBD: move it to vppcom */
-static ssize_t
-vppcom_session_recvfrom (int __sid, void *__restrict __buf, size_t __n,
-                        int __flags, __SOCKADDR_ARG __addr,
-                        socklen_t * __restrict __addr_len)
+static inline ssize_t
+vcom_session_recvfrom (int __sid, void *__restrict __buf, size_t __n,
+                      int __flags, __SOCKADDR_ARG __addr,
+                      socklen_t * __restrict __addr_len)
 {
   int rv = -1;
 
@@ -1595,27 +1609,20 @@ vcom_socket_recvfrom (int __fd, void *__restrict __buf, size_t __n,
   if (!vsock)
     return -ENOTSOCK;
 
-  if (vsock->type != SOCKET_TYPE_VPPCOM_BOUND)
-    return -EINVAL;
-
-  if (!__buf || __n < 0)
+  if ((vsock->type != SOCKET_TYPE_VPPCOM_BOUND) ||
+      !__buf || __n < 0 || !__addr || !__addr_len || (__addr_len < 0))
     {
       return -EINVAL;
     }
 
-  if (__addr || __addr_len < 0)
-    {
-      return -EINVAL;
-    }
-
-  rv = vppcom_session_recvfrom (vsock->sid, __buf, __n,
-                               __flags, __addr, __addr_len);
+  rv = vcom_session_recvfrom (vsock->sid, __buf, __n,
+                             __flags, __addr, __addr_len);
   return rv;
 }
 
 /* TBD: move it to vppcom */
-static ssize_t
-vppcom_sendmsg (int __sid, const struct msghdr *__message, int __flags)
+static inline ssize_t
+vcom_session_sendmsg (int __sid, const struct msghdr *__message, int __flags)
 {
   int rv = -1;
   /* rv = vppcom_session_write (__sid, (void *) __message->__buf,
@@ -1656,7 +1663,7 @@ vcom_socket_sendmsg (int __fd, const struct msghdr * __message, int __flags)
       ;
     }
 
-  rv = vppcom_sendmsg (vsock->sid, __message, __flags);
+  rv = vcom_session_sendmsg (vsock->sid, __message, __flags);
 
   return rv;
 }
@@ -1673,8 +1680,8 @@ vcom_socket_sendmmsg (int __fd, struct mmsghdr *__vmessages,
 #endif
 
 /* TBD: move it to vppcom */
-static ssize_t
-vppcom_recvmsg (int __sid, struct msghdr *__message, int __flags)
+static inline ssize_t
+vcom_session_recvmsg (int __sid, struct msghdr *__message, int __flags)
 {
   int rv = -1;
   /* rv = vppcom_session_read (__sid, (void *) __message->__buf,
@@ -1709,7 +1716,7 @@ vcom_socket_recvmsg (int __fd, struct msghdr * __message, int __flags)
 
   /* validate __flags */
 
-  rv = vppcom_recvmsg (vsock->sid, __message, __flags);
+  rv = vcom_session_recvmsg (vsock->sid, __message, __flags);
   return rv;
 }
 
@@ -1725,9 +1732,10 @@ vcom_socket_recvmmsg (int __fd, struct mmsghdr *__vmessages,
 #endif
 
 /* TBD: move it to vppcom */
-static int
-vppcom_getsockopt (int __sid, int __level, int __optname,
-                  void *__restrict __optval, socklen_t * __restrict __optlen)
+static inline int
+vcom_session_get_sockopt (int __sid, int __level, int __optname,
+                         void *__restrict __optval,
+                         socklen_t * __restrict __optlen)
 {
   /* 1. for socket level options that are NOT socket attributes
    *    and that has corresponding vpp options get from vppcom */
@@ -1842,23 +1850,18 @@ vcom_socket_getsockopt (int __fd, int __level, int __optname,
     default:
       /* 1. handle options that are NOT socket level options,
        *    but have corresponding vpp otions. */
-      rv = vppcom_getsockopt (vsock->sid, __level, __optname,
-                             __optval, __optlen);
-
-      return rv;
-#if 0
-      /* 2. unhandled options */
-      return -ENOPROTOOPT;
-#endif
+      rv = vcom_session_get_sockopt (vsock->sid, __level, __optname,
+                                    __optval, __optlen);
+      break;
     }
 
   return rv;
 }
 
 /* TBD: move it to vppcom */
-int
-vppcom_session_setsockopt (int __sid, int __level, int __optname,
-                          const void *__optval, socklen_t __optlen)
+static inline int
+vcom_session_setsockopt (int __sid, int __level, int __optname,
+                        const void *__optval, socklen_t __optlen)
 {
   int rv = -EOPNOTSUPP;
 
@@ -1958,8 +1961,8 @@ vcom_socket_setsockopt (int __fd, int __level, int __optname,
       switch (__optname)
        {
        case IPV6_V6ONLY:
-         rv = vppcom_session_setsockopt (vsock->sid, __level, __optname,
-                                         __optval, __optlen);
+         rv = vcom_session_setsockopt (vsock->sid, __level, __optname,
+                                       __optval, __optlen);
          break;
        default:
          return -EOPNOTSUPP;
@@ -1972,8 +1975,8 @@ vcom_socket_setsockopt (int __fd, int __level, int __optname,
          return 0;
        case TCP_KEEPIDLE:
        case TCP_KEEPINTVL:
-         rv = vppcom_session_setsockopt (vsock->sid, __level, __optname,
-                                         __optval, __optlen);
+         rv = vcom_session_setsockopt (vsock->sid, __level, __optname,
+                                       __optval, __optlen);
          break;
        default:
          return -EOPNOTSUPP;
@@ -1986,8 +1989,8 @@ vcom_socket_setsockopt (int __fd, int __level, int __optname,
        case SO_REUSEADDR:
        case SO_BROADCAST:
        case SO_KEEPALIVE:
-         rv = vppcom_session_setsockopt (vsock->sid, __level, __optname,
-                                         __optval, __optlen);
+         rv = vcom_session_setsockopt (vsock->sid, __level, __optname,
+                                       __optval, __optlen);
          break;
 
          /*
@@ -2371,8 +2374,8 @@ vcom_socket_accept4 (int __fd, __SOCKADDR_ARG __addr,
 #endif
 
 /* TBD: move it to vppcom */
-int
-vppcom_session_shutdown (int __fd, int __how)
+static inline int
+vcom_session_shutdown (int __fd, int __how)
 {
   return 0;
 }
@@ -2394,7 +2397,7 @@ vcom_socket_shutdown (int __fd, int __how)
        case SHUT_RD:
        case SHUT_WR:
        case SHUT_RDWR:
-         rv = vppcom_session_shutdown (vsock->sid, __how);
+         rv = vcom_session_shutdown (vsock->sid, __how);
          return rv;
          break;
 
@@ -3065,7 +3068,7 @@ vcom_socket_poll_select_impl (struct pollfd *__fds, nfds_t __nfds,
   vcom_nfd = vcom_socket_select (vcom_nfds,
                                 &vcom_readfds,
                                 &vcom_writefds, &vcom_exceptfds, &tv);
-  if (VCOM_DEBUG > 0)
+  if (VCOM_DEBUG > 2)
     fprintf (stderr,
             "[%d] vcom_socket_select: "
             "'%04d'='%04d'\n", pid, vcom_nfd, vcom_nfds);

diff --git a/src/uri/vppcom.c b/src/uri/vppcom.c
index 8d254cc..24475b4 100644 (file)
--- a/src/uri/vppcom.c
+++ b/src/uri/vppcom.c
@@ -2513,91 +2513,100 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
   do
     {
       /* *INDENT-OFF* */
-      clib_bitmap_foreach (session_index, vcm->rd_bitmap,
-        ({
-          clib_spinlock_lock (&vcm->sessions_lockp);
-          rv = vppcom_session_at_index (session_index, &session);
-          if (rv < 0)
+      if (n_bits)
+        {
+          if (read_map)
             {
-              clib_spinlock_unlock (&vcm->sessions_lockp);
-              if (VPPCOM_DEBUG > 1)
-                clib_warning ("[%d] session %d specified in "
-                              "read_map is closed.", vcm->my_pid,
-                              session_index);
-              bits_set = VPPCOM_EBADFD;
-              goto select_done;
+              clib_bitmap_foreach (session_index, vcm->rd_bitmap,
+                ({
+                  clib_spinlock_lock (&vcm->sessions_lockp);
+                  rv = vppcom_session_at_index (session_index, &session);
+                  if (rv < 0)
+                    {
+                      clib_spinlock_unlock (&vcm->sessions_lockp);
+                      if (VPPCOM_DEBUG > 1)
+                        clib_warning ("[%d] session %d specified in "
+                                      "read_map is closed.", vcm->my_pid,
+                                      session_index);
+                      bits_set = VPPCOM_EBADFD;
+                      goto select_done;
+                    }
+
+                  rv = vppcom_session_read_ready (session, session_index);
+                  clib_spinlock_unlock (&vcm->sessions_lockp);
+                  if (except_map && vcm->ex_bitmap &&
+                      clib_bitmap_get (vcm->ex_bitmap, session_index) &&
+                      (rv < 0))
+                    {
+                      // TBD: clib_warning
+                      clib_bitmap_set_no_check (except_map, session_index, 1);
+                      bits_set++;
+                    }
+                  else if (rv > 0)
+                    {
+                      // TBD: clib_warning
+                      clib_bitmap_set_no_check (read_map, session_index, 1);
+                      bits_set++;
+                    }
+                }));
             }
 
-          rv = vppcom_session_read_ready (session, session_index);
-          clib_spinlock_unlock (&vcm->sessions_lockp);
-          if (vcm->ex_bitmap &&
-              clib_bitmap_get (vcm->ex_bitmap, session_index) && (rv < 0))
+          if (write_map)
             {
-              // TBD: clib_warning
-              /* coverity[FORWARD_NULL] */
-              clib_bitmap_set_no_check (except_map, session_index, 1);
-              bits_set++;
-            }
-          else if (rv > 0)
-            {
-              // TBD: clib_warning
-              /* coverity[FORWARD_NULL] */
-              clib_bitmap_set_no_check (read_map, session_index, 1);
-              bits_set++;
-            }
-        }));
-
-      clib_bitmap_foreach (session_index, vcm->wr_bitmap,
-        ({
-          clib_spinlock_lock (&vcm->sessions_lockp);
-          rv = vppcom_session_at_index (session_index, &session);
-          if (rv < 0)
-            {
-              clib_spinlock_unlock (&vcm->sessions_lockp);
-              if (VPPCOM_DEBUG > 0)
-                clib_warning ("[%d] session %d specified in "
-                              "write_map is closed.", vcm->my_pid,
-                              session_index);
-              bits_set = VPPCOM_EBADFD;
-              goto select_done;
-            }
-
-          rv = vppcom_session_write_ready (session, session_index);
-          clib_spinlock_unlock (&vcm->sessions_lockp);
-          if (rv > 0 )
-            {
-              // TBD: clib_warning
-              /* coverity[FORWARD_NULL] */
-              clib_bitmap_set_no_check (write_map, session_index, 1);
-              bits_set++;
-            }
-        }));
-
-      clib_bitmap_foreach (session_index, vcm->ex_bitmap,
-        ({
-          clib_spinlock_lock (&vcm->sessions_lockp);
-          rv = vppcom_session_at_index (session_index, &session);
-          if (rv < 0)
-            {
-              clib_spinlock_unlock (&vcm->sessions_lockp);
-              if (VPPCOM_DEBUG > 1)
-                clib_warning ("[%d] session %d specified in "
-                              "except_map is closed.", vcm->my_pid,
-                              session_index);
-              bits_set = VPPCOM_EBADFD;
-              goto select_done;
+              clib_bitmap_foreach (session_index, vcm->wr_bitmap,
+                ({
+                  clib_spinlock_lock (&vcm->sessions_lockp);
+                  rv = vppcom_session_at_index (session_index, &session);
+                  if (rv < 0)
+                    {
+                      clib_spinlock_unlock (&vcm->sessions_lockp);
+                      if (VPPCOM_DEBUG > 0)
+                        clib_warning ("[%d] session %d specified in "
+                                      "write_map is closed.", vcm->my_pid,
+                                      session_index);
+                      bits_set = VPPCOM_EBADFD;
+                      goto select_done;
+                    }
+
+                  rv = vppcom_session_write_ready (session, session_index);
+                  clib_spinlock_unlock (&vcm->sessions_lockp);
+                  if (write_map && (rv > 0))
+                    {
+                      // TBD: clib_warning
+                      clib_bitmap_set_no_check (write_map, session_index, 1);
+                      bits_set++;
+                    }
+                }));
             }
 
-          rv = vppcom_session_read_ready (session, session_index);
-          clib_spinlock_unlock (&vcm->sessions_lockp);
-          if (rv < 0)
+          if (except_map)
             {
-              // TBD: clib_warning
-              /* coverity[FORWARD_NULL] */
-              clib_bitmap_set_no_check (except_map, session_index, 1);
-              bits_set++;
+              clib_bitmap_foreach (session_index, vcm->ex_bitmap,
+                ({
+                  clib_spinlock_lock (&vcm->sessions_lockp);
+                  rv = vppcom_session_at_index (session_index, &session);
+                  if (rv < 0)
+                    {
+                      clib_spinlock_unlock (&vcm->sessions_lockp);
+                      if (VPPCOM_DEBUG > 1)
+                        clib_warning ("[%d] session %d specified in "
+                                      "except_map is closed.", vcm->my_pid,
+                                      session_index);
+                      bits_set = VPPCOM_EBADFD;
+                      goto select_done;
+                    }
+
+                  rv = vppcom_session_read_ready (session, session_index);
+                  clib_spinlock_unlock (&vcm->sessions_lockp);
+                  if (rv < 0)
+                    {
+                      // TBD: clib_warning
+                      clib_bitmap_set_no_check (except_map, session_index, 1);
+                      bits_set++;
+                    }
+                }));
             }
-        }));
+        }
       /* *INDENT-ON* */
     }
   while (clib_time_now (&vcm->clib_time) < timeout);
@@ -2641,6 +2650,7 @@ vep_verify_epoll_chain (u32 vep_idx)
   do
     {
       vep = &session->vep;
+      sid = vep->next_sid;
       if (session->is_vep_session)
        {
          if (VPPCOM_DEBUG > 1)
@@ -2659,7 +2669,6 @@ vep_verify_epoll_chain (u32 vep_idx)
                          vep->vep_idx, vep->vep_idx,
                          vep->ev.events, vep->ev.data.u64, vep->et_mask);
        }
-      sid = vep->next_sid;
       if (sid != ~0)
        {
          rv = vppcom_session_at_index (sid, &session);
@@ -3229,6 +3238,10 @@ vppcom_session_attr (uint32_t session_index, uint32_t op,
 
     case VPPCOM_ATTR_SET_TCP_KEEPINTVL:
       break;
+
+    default:
+      rv = VPPCOM_EINVAL;
+      break;
     }
 
 done:
@@ -3236,10 +3249,10 @@ done:
   return rv;
 }
 
-  /*
-   * fd.io coding-style-patch-verification: ON
-   *
-   * Local Variables:
-   * eval: (c-set-style "gnu")
-   * End:
-   */
+/*
+ * fd.io coding-style-patch-verification: ON
+ *
+ * Local Variables:
+ * eval: (c-set-style "gnu")
+ * End:
+ */