From 55c952ed5f56a1a478f03f8458e82530478c4359 Mon Sep 17 00:00:00 2001 From: liuyacan Date: Sun, 13 Jun 2021 14:54:55 +0800 Subject: [PATCH] vcl: improve shutdown() This commit does following: - Change the behavior of shutdown() with SHUT_RDWR flag. - Check SHUT_RD flag when read() - Change the errno when write() after SHUT_WR - Remove unused code All the above modification passed the packetdrill test. Type: improvement Signed-off-by: liuyacan Change-Id: I0c81f52e563562e58580d70976526b898e65e915 --- src/vcl/ldp.c | 21 ++----------------- src/vcl/vcl_locked.c | 4 ++-- src/vcl/vcl_locked.h | 2 +- src/vcl/vcl_private.h | 5 ++--- src/vcl/vppcom.c | 57 ++++++++++++++++++++++++++++----------------------- src/vcl/vppcom.h | 5 ++--- 6 files changed, 40 insertions(+), 54 deletions(-) diff --git a/src/vcl/ldp.c b/src/vcl/ldp.c index b51d2e0ebb4..54a8f66354f 100644 --- a/src/vcl/ldp.c +++ b/src/vcl/ldp.c @@ -2175,8 +2175,7 @@ int shutdown (int fd, int how) { vls_handle_t vlsh; - int rv = 0, flags; - u32 flags_len = sizeof (flags); + int rv = 0; ldp_init_check (); @@ -2184,23 +2183,7 @@ shutdown (int fd, int how) if (vlsh != VLS_INVALID_HANDLE) { LDBG (0, "called shutdown: fd %u vlsh %u how %d", fd, vlsh, how); - - if (vls_attr (vlsh, VPPCOM_ATTR_SET_SHUT, &how, &flags_len)) - { - close (fd); - return -1; - } - - if (vls_attr (vlsh, VPPCOM_ATTR_GET_SHUT, &flags, &flags_len)) - { - close (fd); - return -1; - } - - if (flags == SHUT_RDWR) - rv = close (fd); - else if (flags == SHUT_WR) - rv = vls_shutdown (vlsh); + rv = vls_shutdown (vlsh, how); } else { diff --git a/src/vcl/vcl_locked.c b/src/vcl/vcl_locked.c index 69f492b8694..ea22bcd22f5 100644 --- a/src/vcl/vcl_locked.c +++ b/src/vcl/vcl_locked.c @@ -1314,7 +1314,7 @@ vls_close (vls_handle_t vlsh) } int -vls_shutdown (vls_handle_t vlsh) +vls_shutdown (vls_handle_t vlsh, int how) { vcl_locked_session_t *vls; int rv; @@ -1324,7 +1324,7 @@ vls_shutdown (vls_handle_t vlsh) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_SPOOL); - rv = vppcom_session_shutdown (vls_to_sh (vls)); + rv = vppcom_session_shutdown (vls_to_sh (vls), how); vls_mt_unguard (); vls_get_and_unlock (vlsh); diff --git a/src/vcl/vcl_locked.h b/src/vcl/vcl_locked.h index 3adcf62bc77..fa3a2735eb7 100644 --- a/src/vcl/vcl_locked.h +++ b/src/vcl/vcl_locked.h @@ -26,7 +26,7 @@ typedef int vls_handle_t; vls_handle_t vls_create (uint8_t proto, uint8_t is_nonblocking); -int vls_shutdown (vls_handle_t vlsh); +int vls_shutdown (vls_handle_t vlsh, int how); int vls_close (vls_handle_t vlsh); int vls_bind (vls_handle_t vlsh, vppcom_endpt_t * ep); int vls_listen (vls_handle_t vlsh, int q_len); diff --git a/src/vcl/vcl_private.h b/src/vcl/vcl_private.h index 628b7310788..5b19f946fe7 100644 --- a/src/vcl/vcl_private.h +++ b/src/vcl/vcl_private.h @@ -126,8 +126,6 @@ typedef enum VCL_SESS_ATTR_TCP_NODELAY, // SOL_TCP,TCP_NODELAY VCL_SESS_ATTR_TCP_KEEPIDLE, // SOL_TCP,TCP_KEEPIDLE VCL_SESS_ATTR_TCP_KEEPINTVL, // SOL_TCP,TCP_KEEPINTVL - VCL_SESS_ATTR_SHUT_RD, - VCL_SESS_ATTR_SHUT_WR, VCL_SESS_ATTR_MAX } vppcom_session_attr_t; @@ -137,7 +135,8 @@ typedef enum vcl_session_flags_ VCL_SESSION_F_IS_VEP = 1 << 1, VCL_SESSION_F_IS_VEP_SESSION = 1 << 2, VCL_SESSION_F_HAS_RX_EVT = 1 << 3, - VCL_SESSION_F_SHUTDOWN = 1 << 4, + VCL_SESSION_F_RD_SHUTDOWN = 1 << 4, + VCL_SESSION_F_WR_SHUTDOWN = 1 << 5, } __clib_packed vcl_session_flags_t; typedef struct vcl_session_ diff --git a/src/vcl/vppcom.c b/src/vcl/vppcom.c index 330d590cfa0..d378f404381 100644 --- a/src/vcl/vppcom.c +++ b/src/vcl/vppcom.c @@ -807,7 +807,7 @@ vcl_session_disconnected_handler (vcl_worker_t * wrk, } int -vppcom_session_shutdown (uint32_t session_handle) +vppcom_session_shutdown (uint32_t session_handle, int how) { vcl_worker_t *wrk = vcl_worker_get_current (); vcl_session_t *session; @@ -830,13 +830,20 @@ vppcom_session_shutdown (uint32_t session_handle) return VPPCOM_EBADFD; } + if (how == SHUT_RD || how == SHUT_RDWR) + { + session->flags |= VCL_SESSION_F_RD_SHUTDOWN; + if (how == SHUT_RD) + return VPPCOM_OK; + } + session->flags |= VCL_SESSION_F_WR_SHUTDOWN; + if (PREDICT_TRUE (state == VCL_STATE_READY)) { VDBG (1, "session %u [0x%llx]: sending shutdown...", session->session_index, vpp_handle); vcl_send_session_shutdown (wrk, session); - session->flags |= VCL_SESSION_F_SHUTDOWN; } return VPPCOM_OK; @@ -1948,6 +1955,18 @@ vppcom_session_read_internal (uint32_t session_handle, void *buf, int n, return vcl_session_closed_error (s); } + if (PREDICT_FALSE (s->flags & VCL_SESSION_F_RD_SHUTDOWN)) + { + /* Vpp would ack the incoming data and enqueue it for reading. + * So even if SHUT_RD is set, we can still read() the data if + * the session is ready. + */ + if (!vcl_session_read_ready (s)) + { + return 0; + } + } + is_nonblocking = vcl_session_has_attr (s, VCL_SESS_ATTR_NONBLOCK); is_ct = vcl_session_is_ct (s); mq = wrk->app_event_queue; @@ -2166,8 +2185,7 @@ vppcom_session_write_inline (vcl_worker_t * wrk, vcl_session_t * s, void *buf, return VPPCOM_EBADFD; } - if (PREDICT_FALSE (!vcl_session_is_open (s) || - (s->flags & VCL_SESSION_F_SHUTDOWN))) + if (PREDICT_FALSE (!vcl_session_is_open (s))) { VDBG (1, "session %u [0x%llx]: is not open! state 0x%x (%s)", s->session_index, s->vpp_handle, s->session_state, @@ -2175,6 +2193,14 @@ vppcom_session_write_inline (vcl_worker_t * wrk, vcl_session_t * s, void *buf, return vcl_session_closed_error (s);; } + if (PREDICT_FALSE (s->flags & VCL_SESSION_F_WR_SHUTDOWN)) + { + VDBG (1, "session %u [0x%llx]: is shutdown! state 0x%x (%s)", + s->session_index, s->vpp_handle, s->session_state, + vppcom_session_state_str (s->session_state)); + return VPPCOM_EPIPE; + } + is_ct = vcl_session_is_ct (s); tx_fifo = is_ct ? s->ct_tx_fifo : s->tx_fifo; is_nonblocking = vcl_session_has_attr (s, VCL_SESS_ATTR_NONBLOCK); @@ -3209,7 +3235,7 @@ vppcom_session_attr (uint32_t session_handle, uint32_t op, void *buffer, uint32_t * buflen) { vcl_worker_t *wrk = vcl_worker_get_current (); - u32 *flags = buffer, tmp_flags = 0; + u32 *flags = buffer; vppcom_endpt_t *ep = buffer; transport_endpt_attr_t tea; vcl_session_t *session; @@ -3747,27 +3773,6 @@ vppcom_session_attr (uint32_t session_handle, uint32_t op, *buflen); break; - case VPPCOM_ATTR_SET_SHUT: - if (*flags == SHUT_RD || *flags == SHUT_RDWR) - vcl_session_set_attr (session, VCL_SESS_ATTR_SHUT_RD); - if (*flags == SHUT_WR || *flags == SHUT_RDWR) - vcl_session_set_attr (session, VCL_SESS_ATTR_SHUT_WR); - break; - - case VPPCOM_ATTR_GET_SHUT: - if (vcl_session_has_attr (session, VCL_SESS_ATTR_SHUT_RD)) - tmp_flags = 1; - if (vcl_session_has_attr (session, VCL_SESS_ATTR_SHUT_WR)) - tmp_flags |= 2; - if (tmp_flags == 1) - *(int *) buffer = SHUT_RD; - else if (tmp_flags == 2) - *(int *) buffer = SHUT_WR; - else if (tmp_flags == 3) - *(int *) buffer = SHUT_RDWR; - *buflen = sizeof (int); - break; - case VPPCOM_ATTR_SET_CONNECTED: session->flags |= VCL_SESSION_F_CONNECTED; break; diff --git a/src/vcl/vppcom.h b/src/vcl/vppcom.h index c2a625e974e..19a01c798b5 100644 --- a/src/vcl/vppcom.h +++ b/src/vcl/vppcom.h @@ -99,6 +99,7 @@ typedef enum VPPCOM_ETIMEDOUT = -ETIMEDOUT, VPPCOM_EEXIST = -EEXIST, VPPCOM_ENOPROTOOPT = -ENOPROTOOPT, + VPPCOM_EPIPE = -EPIPE, VPPCOM_ENOENT = -ENOENT, } vppcom_error_t; @@ -138,8 +139,6 @@ typedef enum VPPCOM_ATTR_SET_TCP_KEEPINTVL, VPPCOM_ATTR_GET_TCP_USER_MSS, VPPCOM_ATTR_SET_TCP_USER_MSS, - VPPCOM_ATTR_SET_SHUT, - VPPCOM_ATTR_GET_SHUT, VPPCOM_ATTR_SET_CONNECTED, VPPCOM_ATTR_SET_CKPAIR, VPPCOM_ATTR_SET_VRF, @@ -174,7 +173,7 @@ extern int vppcom_app_create (const char *app_name); extern void vppcom_app_destroy (void); extern int vppcom_session_create (uint8_t proto, uint8_t is_nonblocking); -extern int vppcom_session_shutdown (uint32_t session_handle); +extern int vppcom_session_shutdown (uint32_t session_handle, int how); extern int vppcom_session_close (uint32_t session_handle); extern int vppcom_session_bind (uint32_t session_handle, vppcom_endpt_t * ep); extern int vppcom_session_listen (uint32_t session_handle, uint32_t q_len); -- 2.16.6