From 7a2abce4c9fc2191b3d3989c84516391036b6882 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Sun, 5 Apr 2020 19:25:44 +0000 Subject: [PATCH] vcl session: enforce full dgram reads/writes Type: improvement Signed-off-by: Florin Coras Change-Id: I4a3861e31ca42faf0b59f8f09393fb10413bf3af --- src/plugins/hs_apps/echo_client.c | 21 ++++++---- src/vcl/vcl_private.c | 9 +++++ src/vcl/vppcom.c | 68 +++++++++++++++++++++----------- src/vnet/session/application_interface.h | 16 ++++---- test/test_udp.py | 2 + 5 files changed, 75 insertions(+), 41 deletions(-) diff --git a/src/plugins/hs_apps/echo_client.c b/src/plugins/hs_apps/echo_client.c index 3c55c6dc720..e10ee59a2b7 100644 --- 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 14582ce025d..300b82c4f21 100644 --- 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 cb450d99055..dd37460a322 100644 --- 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 7d5d044cc5a..b410c638155 100644 --- 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 5336675c79d..0f1c5a47255 100644 --- 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() -- 2.16.6