vcl session: enforce full dgram reads/writes

Type: improvement

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

diff --git a/src/plugins/hs_apps/echo_client.c b/src/plugins/hs_apps/echo_client.c
index 3c55c6d..e10ee59 100644 (file)
--- a/src/plugins/hs_apps/echo_client.c
+++ b/src/plugins/hs_apps/echo_client.c
@@ -75,17 +75,19 @@ send_data_chunk (echo_client_main_t * ecm, eclient_session_t * s)
     }
   else
     {
+      svm_fifo_t *f = s->data.tx_fifo;
+      u32 max_enqueue = svm_fifo_max_enqueue_prod (f);
+
+      if (max_enqueue < sizeof (session_dgram_hdr_t))
+       return;
+
+      max_enqueue -= sizeof (session_dgram_hdr_t);
+
       if (ecm->no_copy)
        {
          session_dgram_hdr_t hdr;
-         svm_fifo_t *f = s->data.tx_fifo;
          app_session_transport_t *at = &s->data.transport;
-         u32 max_enqueue = svm_fifo_max_enqueue_prod (f);
 
-         if (max_enqueue <= sizeof (session_dgram_hdr_t))
-           return;
-
-         max_enqueue -= sizeof (session_dgram_hdr_t);
          rv = clib_min (max_enqueue, bytes_this_chunk);
 
          hdr.data_length = rv;
@@ -104,8 +106,11 @@ send_data_chunk (echo_client_main_t * ecm, eclient_session_t * s)
                                                SESSION_IO_EVT_TX);
        }
       else
-       rv = app_send_dgram (&s->data, test_data + test_buf_offset,
-                            bytes_this_chunk, 0);
+       {
+         bytes_this_chunk = clib_min (bytes_this_chunk, max_enqueue);
+         rv = app_send_dgram (&s->data, test_data + test_buf_offset,
+                              bytes_this_chunk, 0);
+       }
     }
 
   /* If we managed to enqueue data... */

diff --git a/src/vcl/vcl_private.c b/src/vcl/vcl_private.c
index 14582ce..300b82c 100644 (file)
--- a/src/vcl/vcl_private.c
+++ b/src/vcl/vcl_private.c
@@ -390,6 +390,15 @@ vcl_session_write_ready (vcl_session_t * session)
   if (vcl_session_is_ct (session))
     return svm_fifo_max_enqueue_prod (session->ct_tx_fifo);
 
+  if (session->is_dgram)
+    {
+      u32 max_enq = svm_fifo_max_enqueue_prod (session->tx_fifo);
+
+      if (max_enq <= sizeof (session_dgram_hdr_t))
+       return 0;
+      return max_enq - sizeof (session_dgram_hdr_t);
+    }
+
   return svm_fifo_max_enqueue_prod (session->tx_fifo);
 }
 

diff --git a/src/vcl/vppcom.c b/src/vcl/vppcom.c
index cb450d9..dd37460 100644 (file)
--- a/src/vcl/vppcom.c
+++ b/src/vcl/vppcom.c
@@ -2004,13 +2004,22 @@ vcl_is_tx_evt_for_session (session_event_t * e, u32 sid, u8 is_ct)
   return (e->event_type == SESSION_IO_EVT_TX && e->session_index == sid);
 }
 
-static inline int
-vppcom_session_write_inline (uint32_t session_handle, void *buf, size_t n,
-                            u8 is_flush)
+always_inline u8
+vcl_fifo_is_writeable (svm_fifo_t * f, u32 len, u8 is_dgram)
+{
+  u32 max_enq = svm_fifo_max_enqueue_prod (f);
+  if (is_dgram)
+    return max_enq >= (sizeof (session_dgram_hdr_t) + len);
+  else
+    return max_enq > 0;
+
+}
+
+always_inline int
+vppcom_session_write_inline (vcl_worker_t * wrk, vcl_session_t * s, void *buf,
+                            size_t n, u8 is_flush, u8 is_dgram)
 {
-  vcl_worker_t *wrk = vcl_worker_get_current ();
   int n_write, is_nonblocking;
-  vcl_session_t *s = 0;
   session_evt_type_t et;
   svm_msg_q_msg_t msg;
   svm_fifo_t *tx_fifo;
@@ -2021,10 +2030,6 @@ vppcom_session_write_inline (uint32_t session_handle, void *buf, size_t n,
   if (PREDICT_FALSE (!buf || n == 0))
     return VPPCOM_EINVAL;
 
-  s = vcl_session_get_w_handle (wrk, session_handle);
-  if (PREDICT_FALSE (!s))
-    return VPPCOM_EBADFD;
-
   if (PREDICT_FALSE (s->is_vep))
     {
       VDBG (0, "ERROR: session %u [0x%llx]: cannot write to an epoll"
@@ -2045,13 +2050,13 @@ vppcom_session_write_inline (uint32_t session_handle, void *buf, size_t n,
   is_nonblocking = VCL_SESS_ATTR_TEST (s->attr, VCL_SESS_ATTR_NONBLOCK);
 
   mq = wrk->app_event_queue;
-  if (svm_fifo_is_full_prod (tx_fifo))
+  if (!vcl_fifo_is_writeable (tx_fifo, n, is_dgram))
     {
       if (is_nonblocking)
        {
          return VPPCOM_EWOULDBLOCK;
        }
-      while (svm_fifo_is_full_prod (tx_fifo))
+      while (!vcl_fifo_is_writeable (tx_fifo, n, is_dgram))
        {
          svm_fifo_add_want_deq_ntf (tx_fifo, SVM_FIFO_WANT_DEQ_NOTIF);
          if (vcl_session_is_closing (s))
@@ -2074,7 +2079,7 @@ vppcom_session_write_inline (uint32_t session_handle, void *buf, size_t n,
   if (is_flush && !is_ct)
     et = SESSION_IO_EVT_TX_FLUSH;
 
-  if (s->is_dgram)
+  if (is_dgram)
     n_write = app_send_dgram_raw (tx_fifo, &s->transport,
                                  s->vpp_evt_q, buf, n, et,
                                  0 /* do_evt */ , SVM_Q_WAIT);
@@ -2097,15 +2102,29 @@ vppcom_session_write_inline (uint32_t session_handle, void *buf, size_t n,
 int
 vppcom_session_write (uint32_t session_handle, void *buf, size_t n)
 {
-  return vppcom_session_write_inline (session_handle, buf, n,
-                                     0 /* is_flush */ );
+  vcl_worker_t *wrk = vcl_worker_get_current ();
+  vcl_session_t *s;
+
+  s = vcl_session_get_w_handle (wrk, session_handle);
+  if (PREDICT_FALSE (!s))
+    return VPPCOM_EBADFD;
+
+  return vppcom_session_write_inline (wrk, s, buf, n,
+                                     0 /* is_flush */ , s->is_dgram ? 1 : 0);
 }
 
 int
 vppcom_session_write_msg (uint32_t session_handle, void *buf, size_t n)
 {
-  return vppcom_session_write_inline (session_handle, buf, n,
-                                     1 /* is_flush */ );
+  vcl_worker_t *wrk = vcl_worker_get_current ();
+  vcl_session_t *s;
+
+  s = vcl_session_get_w_handle (wrk, session_handle);
+  if (PREDICT_FALSE (!s))
+    return VPPCOM_EBADFD;
+
+  return vppcom_session_write_inline (wrk, s, buf, n,
+                                     1 /* is_flush */ , s->is_dgram ? 1 : 0);
 }
 
 #define vcl_fifo_rx_evt_valid_or_break(_s)                             \
@@ -3608,18 +3627,18 @@ int
 vppcom_session_sendto (uint32_t session_handle, void *buffer,
                       uint32_t buflen, int flags, vppcom_endpt_t * ep)
 {
+  vcl_worker_t *wrk = vcl_worker_get_current ();
+  vcl_session_t *s;
+
+  s = vcl_session_get_w_handle (wrk, session_handle);
+  if (!s)
+    return VPPCOM_EBADFD;
+
   if (!buffer)
     return VPPCOM_EINVAL;
 
   if (ep)
     {
-      vcl_worker_t *wrk = vcl_worker_get_current ();
-      vcl_session_t *s;
-
-      s = vcl_session_get_w_handle (wrk, session_handle);
-      if (!s)
-       return VPPCOM_EBADFD;
-
       if (s->session_type != VPPCOM_PROTO_UDP
          || (s->flags & VCL_SESSION_F_CONNECTED))
        return VPPCOM_EINVAL;
@@ -3643,7 +3662,8 @@ vppcom_session_sendto (uint32_t session_handle, void *buffer,
       VDBG (2, "handling flags 0x%u (%d) not implemented yet.", flags, flags);
     }
 
-  return (vppcom_session_write_inline (session_handle, buffer, buflen, 1));
+  return (vppcom_session_write_inline (wrk, s, buffer, buflen, 1,
+                                      s->is_dgram ? 1 : 0));
 }
 
 int

diff --git a/src/vnet/session/application_interface.h b/src/vnet/session/application_interface.h
index 7d5d044..b410c63 100644 (file)
--- a/src/vnet/session/application_interface.h
+++ b/src/vnet/session/application_interface.h
@@ -608,7 +608,7 @@ app_send_dgram_raw (svm_fifo_t * f, app_session_transport_t * at,
   int rv;
 
   max_enqueue = svm_fifo_max_enqueue_prod (f);
-  if (max_enqueue <= sizeof (session_dgram_hdr_t))
+  if (max_enqueue < (sizeof (session_dgram_hdr_t) + len))
     return 0;
 
   max_enqueue -= sizeof (session_dgram_hdr_t);
@@ -694,18 +694,16 @@ app_recv_dgram_raw (svm_fifo_t * f, u8 * buf, u32 len,
 
   svm_fifo_peek (f, 0, sizeof (ph), (u8 *) & ph);
   ASSERT (ph.data_length >= ph.data_offset);
-  if (!ph.data_offset)
-    svm_fifo_peek (f, sizeof (ph), sizeof (*at), (u8 *) at);
+  svm_fifo_peek (f, sizeof (ph), sizeof (*at), (u8 *) at);
+
   len = clib_min (len, ph.data_length - ph.data_offset);
   rv = svm_fifo_peek (f, ph.data_offset + SESSION_CONN_HDR_LEN, len, buf);
   if (peek)
     return rv;
-  ASSERT (rv > 0);
-  ph.data_offset += rv;
-  if (ph.data_offset == ph.data_length)
-    svm_fifo_dequeue_drop (f, ph.data_length + SESSION_CONN_HDR_LEN);
-  else
-    svm_fifo_overwrite_head (f, (u8 *) & ph, sizeof (ph));
+
+  /* Discards data that did not fit in buffer */
+  svm_fifo_dequeue_drop (f, ph.data_length + SESSION_CONN_HDR_LEN);
+
   return rv;
 }
 

diff --git a/test/test_udp.py b/test/test_udp.py
index 5336675..0f1c5a4 100644 (file)
--- a/test/test_udp.py
+++ b/test/test_udp.py
@@ -320,6 +320,8 @@ class TestUDP(VppTestCase):
             self.logger.critical(error)
             self.assertNotIn("failed", error)
 
+        self.logger.debug(self.vapi.cli("show session verbose 2"))
+
         # Delete inter-table routes
         ip_t01.remove_vpp_config()
         ip_t10.remove_vpp_config()