From: Florin Coras Date: Tue, 15 Mar 2022 04:17:25 +0000 (-0700) Subject: session: use safe realloc for pools X-Git-Tag: v22.10-rc0~237 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=6bd8d3fbba74f8f80a0c09f87c6cbfddd054042f;p=vpp.git session: use safe realloc for pools Type: improvement Signed-off-by: Florin Coras Change-Id: I313c916d268c4b2b448b93e90bc67da341b803e3 --- diff --git a/src/plugins/hsi/hsi.c b/src/plugins/hsi/hsi.c index bfdb3a07fc2..9382a94de56 100644 --- a/src/plugins/hsi/hsi.c +++ b/src/plugins/hsi/hsi.c @@ -85,13 +85,7 @@ hsi_udp_lookup (vlib_buffer_t *b, void *ip_hdr, u8 is_ip4) hdr->dst_port, hdr->src_port, TRANSPORT_PROTO_UDP); } - if (s) - { - session_pool_remove_peeker (s->thread_index); - return 1; - } - - return 0; + return s ? 1 : 0; } always_inline transport_connection_t * diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c index 5d070240152..f1d1a4e2cfe 100644 --- a/src/vnet/session/session.c +++ b/src/vnet/session/session.c @@ -197,28 +197,31 @@ session_program_transport_ctrl_evt (session_t * s, session_evt_type_t evt) session_send_ctrl_evt_to_thread (s, evt); } +static void +session_pool_realloc_rpc (void *rpc_args) +{ + session_worker_t *wrk; + u32 thread_index; + + thread_index = pointer_to_uword (rpc_args); + wrk = &session_main.wrk[thread_index]; + + pool_realloc_safe_aligned (wrk->sessions, CLIB_CACHE_LINE_BYTES); +} + session_t * session_alloc (u32 thread_index) { session_worker_t *wrk = &session_main.wrk[thread_index]; session_t *s; - u8 will_expand = pool_get_will_expand (wrk->sessions); - /* If we have peekers, let them finish */ - if (PREDICT_FALSE (will_expand && vlib_num_workers ())) - { - clib_rwlock_writer_lock (&wrk->peekers_rw_locks); - pool_get_aligned (wrk->sessions, s, CLIB_CACHE_LINE_BYTES); - clib_rwlock_writer_unlock (&wrk->peekers_rw_locks); - } - else - { - pool_get_aligned (wrk->sessions, s, CLIB_CACHE_LINE_BYTES); - } + pool_get_aligned_safe (wrk->sessions, s, thread_index, + session_pool_realloc_rpc, CLIB_CACHE_LINE_BYTES); clib_memset (s, 0, sizeof (*s)); s->session_index = s - wrk->sessions; s->thread_index = thread_index; s->app_index = APP_INVALID_INDEX; + return s; } @@ -1904,9 +1907,6 @@ session_manager_main_enable (vlib_main_t * vm) wrk->timerfd = -1; vec_validate (wrk->session_to_enqueue, smm->last_transport_proto_type); - if (num_threads > 1) - clib_rwlock_init (&smm->wrk[i].peekers_rw_locks); - if (!smm->no_adaptive && smm->use_private_rx_mqs) session_wrk_enable_adaptive_mode (wrk); } diff --git a/src/vnet/session/session.h b/src/vnet/session/session.h index 71f6d35d43b..215588edae1 100644 --- a/src/vnet/session/session.h +++ b/src/vnet/session/session.h @@ -134,9 +134,6 @@ typedef struct session_worker_ /** Head of list of pending events */ clib_llist_index_t old_head; - /** Peekers rw lock */ - clib_rwlock_t peekers_rw_locks; - /** Vector of buffers to be sent */ u32 *pending_tx_buffers; @@ -386,37 +383,9 @@ session_get_from_handle_if_valid (session_handle_t handle) u64 session_segment_handle (session_t * s); /** - * Acquires a lock that blocks a session pool from expanding. - * - * This is typically used for safely peeking into other threads' - * pools in order to clone elements. Lock should be dropped as soon - * as possible by calling @ref session_pool_remove_peeker. - * - * NOTE: Avoid using pool_elt_at_index while the lock is held because - * it may lead to free elt bitmap expansion/contraction! - */ -always_inline void -session_pool_add_peeker (u32 thread_index) -{ - session_worker_t *wrk = &session_main.wrk[thread_index]; - if (thread_index == vlib_get_thread_index ()) - return; - clib_rwlock_reader_lock (&wrk->peekers_rw_locks); -} - -always_inline void -session_pool_remove_peeker (u32 thread_index) -{ - session_worker_t *wrk = &session_main.wrk[thread_index]; - if (thread_index == vlib_get_thread_index ()) - return; - clib_rwlock_reader_unlock (&wrk->peekers_rw_locks); -} - -/** - * Get session from handle and 'lock' pool resize if not in same thread + * Get session from handle and avoid pool validation if no same thread * - * Caller should drop the peek 'lock' as soon as possible. + * Peekers are fine because pool grows with barrier (see @ref session_alloc) */ always_inline session_t * session_get_from_handle_safe (u64 handle) @@ -431,36 +400,24 @@ session_get_from_handle_safe (u64 handle) } else { - session_pool_add_peeker (thread_index); - /* Don't use pool_elt_at index. See @ref session_pool_add_peeker */ + /* Don't use pool_elt_at index to avoid pool bitmap reallocs */ return wrk->sessions + session_index_from_handle (handle); } } -always_inline u32 -session_get_index (session_t * s) -{ - return (s - session_main.wrk[s->thread_index].sessions); -} - always_inline session_t * session_clone_safe (u32 session_index, u32 thread_index) { + u32 current_thread_index = vlib_get_thread_index (), new_index; session_t *old_s, *new_s; - u32 current_thread_index = vlib_get_thread_index (); - /* If during the memcpy pool is reallocated AND the memory allocator - * decides to give the old chunk of memory to somebody in a hurry to - * scribble something on it, we have a problem. So add this thread as - * a session pool peeker. - */ - session_pool_add_peeker (thread_index); new_s = session_alloc (current_thread_index); + new_index = new_s->session_index; + /* Session pools are reallocated with barrier (see @ref session_alloc) */ old_s = session_main.wrk[thread_index].sessions + session_index; clib_memcpy_fast (new_s, old_s, sizeof (*new_s)); - session_pool_remove_peeker (thread_index); new_s->thread_index = current_thread_index; - new_s->session_index = session_get_index (new_s); + new_s->session_index = new_index; return new_s; } diff --git a/src/vnet/session/session_cli.c b/src/vnet/session/session_cli.c index 24d8cfb1e24..2003f0a7788 100644 --- a/src/vnet/session/session_cli.c +++ b/src/vnet/session/session_cli.c @@ -259,7 +259,6 @@ unformat_session (unformat_input_t * input, va_list * args) if (s) { *result = s; - session_pool_remove_peeker (s->thread_index); return 1; } return 0; diff --git a/src/vnet/session/session_lookup.c b/src/vnet/session/session_lookup.c index 5cd1712f195..68f98d0f046 100644 --- a/src/vnet/session/session_lookup.c +++ b/src/vnet/session/session_lookup.c @@ -1046,9 +1046,7 @@ session_lookup_connection4 (u32 fib_index, ip4_address_t * lcl, /** * Lookup session with ip4 and transport layer information * - * Important note: this may look into another thread's pool table and - * register as 'peeker'. Caller should call @ref session_pool_remove_peeker as - * if needed as soon as possible. + * Important note: this may look into another thread's pool table * * Lookup logic is similar to that of @ref session_lookup_connection_wt4 but * this returns a session as opposed to a transport connection and it does not diff --git a/src/vnet/udp/udp_input.c b/src/vnet/udp/udp_input.c index d14bdb8a298..e701ca5d12e 100644 --- a/src/vnet/udp/udp_input.c +++ b/src/vnet/udp/udp_input.c @@ -273,10 +273,8 @@ udp46_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node, ASSERT (s0->session_index == uc0->c_s_index); /* - * Drop the peeker lock on pool resize and ask session - * layer for a new session. + * Ask session layer for a new session. */ - session_pool_remove_peeker (s0->thread_index); session_dgram_connect_notify (&uc0->connection, s0->thread_index, &s0); queue_event = 0; @@ -286,7 +284,6 @@ udp46_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node, } udp_connection_enqueue (uc0, s0, &hdr0, thread_index, b[0], queue_event, &error0); - session_pool_remove_peeker (s0->thread_index); } else if (s0->session_state == SESSION_STATE_READY) { @@ -314,7 +311,6 @@ udp46_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node, else { error0 = UDP_ERROR_NOT_READY; - session_pool_remove_peeker (s0->thread_index); } done: