From 6a3a4f7340bdc687814d7905ef1e4ca1a3b02d57 Mon Sep 17 00:00:00 2001 From: Chris Luke Date: Tue, 9 Jul 2019 23:33:30 -0400 Subject: vlib: Replace timer in CLI with an event process The CLI code, when it accepts a socket connection, ran a timer for each session that would ensure the CLI session was started should the TELNET negotiation stage fail to complete. It has since transpired that this is unsafe; the timer is capable of firing in critical sections, during a spinlock, and since we peform non-trivial things in the handler it can cause a deadlock. This was reported recently in VPP-1711 but a search of history suggests this may also be (one of) the causes in VPP-1413. This change replaces that method with an event-driven process. The process is created when the first socket connection is accepted. When new connections are created the process is sent an event to register the new session in a list. That event process has a loop that evaluates the list of oustanding sessions and if a deadline expires, their session is started if it has not been already, and then removed from the list. If we have pending sessions then the loop waits on a timer or an event; if there are no sessions it waits on events only. Type: fix Ticket: VPP-1711 Change-Id: I8c6093b7d0fc1bea0eb790032ed282a0ca169194 Signed-off-by: Chris Luke Signed-off-by: Dave Barach --- src/vlib/unix/cli.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 140 insertions(+), 16 deletions(-) diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c index 2d5a22dc66a..fa61c6964be 100644 --- a/src/vlib/unix/cli.c +++ b/src/vlib/unix/cli.c @@ -47,7 +47,6 @@ #include #include -#include #include #include @@ -61,6 +60,7 @@ #include #include #include +#include /** ANSI escape code. */ #define ESC "\x1b" @@ -451,6 +451,21 @@ typedef enum UNIX_CLI_PROCESS_EVENT_QUIT, /**< A CLI session wants to close. */ } unix_cli_process_event_type_t; +/** CLI session telnet negotiation timer events. */ +typedef enum +{ + UNIX_CLI_NEW_SESSION_EVENT_ADD, /**< Add a CLI session to the new session list */ +} unix_cli_timeout_event_type_t; + +/** Each new session is stored on a list with a deadline after which + * a prompt is issued, in case the session TELNET negotiation fails to + * complete. */ +typedef struct +{ + uword cf_index; /**< Session index of the new session. */ + f64 deadline; /**< Deadline after which the new session must have a prompt. */ +} unix_cli_new_session_t; + /** CLI global state. */ typedef struct { @@ -468,6 +483,12 @@ typedef struct /** File pool index of current input. */ u32 current_input_file_index; + + /** New session process node identifier */ + u32 new_session_process_node_index; + + /** List of new sessions */ + unix_cli_new_session_t *new_sessions; } unix_cli_main_t; /** CLI global state */ @@ -1240,25 +1261,109 @@ unix_cli_file_welcome (unix_cli_main_t * cm, unix_cli_file_t * cf) } -/** @brief A failsafe triggered on a timer to ensure we send the prompt - * to telnet sessions that fail to negotiate the terminal type. */ -static void -unix_cli_file_welcome_timer (any arg, f64 delay) +/** + * @brief A failsafe manager that ensures CLI sessions issue an initial + * prompt if TELNET negotiation fails. + */ +static uword +unix_cli_new_session_process (vlib_main_t * vm, vlib_node_runtime_t * rt, + vlib_frame_t * f) { unix_cli_main_t *cm = &unix_cli_main; - unix_cli_file_t *cf; - (void) delay; + uword event_type, *event_data = 0; + f64 wait = 10.0; - /* Check the connection didn't close already */ - if (pool_is_free_index (cm->cli_file_pool, (uword) arg)) - return; + while (1) + { + if (vec_len (cm->new_sessions) > 0) + wait = vlib_process_wait_for_event_or_clock (vm, wait); + else + vlib_process_wait_for_event (vm); + + event_type = vlib_process_get_events (vm, &event_data); + + switch (event_type) + { + case ~0: /* no events => timeout */ + break; + + case UNIX_CLI_NEW_SESSION_EVENT_ADD: + { + /* Add an identifier to the new session list */ + unix_cli_new_session_t ns; - cf = pool_elt_at_index (cm->cli_file_pool, (uword) arg); + ns.cf_index = event_data[0]; + ns.deadline = vlib_time_now (vm) + 1.0; + + vec_add1 (cm->new_sessions, ns); + + if (wait > 0.1) + wait = 0.1; /* force a re-evaluation soon, but not too soon */ + } + break; + + default: + clib_warning ("BUG: unknown event type 0x%wx", event_type); + break; + } - if (!cf->started) - unix_cli_file_welcome (cm, cf); + vec_reset_length (event_data); + + if (vlib_process_suspend_time_is_zero (wait)) + { + /* Scan the list looking for expired deadlines. + * Emit needed prompts and remove from the list. + * While scanning, look for the nearest new deadline + * for the next iteration. + * Since the list is ordered with newest sessions first + * we can make assumptions about expired sessions being + * contiguous at the beginning and the next deadline is the + * next entry on the list, if any. + */ + f64 now = vlib_time_now (vm); + unix_cli_new_session_t *nsp; + word index = 0; + + wait = INFINITY; + + vec_foreach (nsp, cm->new_sessions) + { + if (vlib_process_suspend_time_is_zero (nsp->deadline - now)) + { + /* Deadline reached */ + unix_cli_file_t *cf; + + /* Mark the highwater */ + index++; + + /* Check the connection didn't close already */ + if (pool_is_free_index (cm->cli_file_pool, nsp->cf_index)) + continue; + + cf = pool_elt_at_index (cm->cli_file_pool, nsp->cf_index); + + if (!cf->started) + unix_cli_file_welcome (cm, cf); + } + else + { + wait = nsp->deadline - now; + break; + } + } + + if (index) + { + /* We have sessions to remove */ + vec_delete (cm->new_sessions, index, 0); + } + } + } + + return 0; } + /** @brief A mostly no-op Telnet state machine. * Process Telnet command bytes in a way that ensures we're mostly * transparent to the Telnet protocol. That is, it's mostly a no-op. @@ -2832,9 +2937,22 @@ unix_cli_listen_read_ready (clib_file_t * uf) unix_vlib_cli_output_raw (cf, uf, charmode_option, ARRAY_LEN (charmode_option)); - /* In case the client doesn't negotiate terminal type, use - * a timer to kick off the initial prompt. */ - timer_call (unix_cli_file_welcome_timer, cf_index, 1); + if (cm->new_session_process_node_index == ~0) + { + /* Create thw new session deadline process */ + cm->new_session_process_node_index = + vlib_process_create (um->vlib_main, "unix-cli-new-session", + unix_cli_new_session_process, + 16 /* log2_n_stack_bytes */ ); + } + + /* In case the client doesn't negotiate terminal type, register + * our session with a process that will emit the prompt if + * a deadline passes */ + vlib_process_signal_event (um->vlib_main, + cm->new_session_process_node_index, + UNIX_CLI_NEW_SESSION_EVENT_ADD, cf_index); + } return error; @@ -3685,6 +3803,12 @@ VLIB_CLI_COMMAND (cli_unix_cli_set_terminal_ansi, static) = { static clib_error_t * unix_cli_init (vlib_main_t * vm) { + unix_cli_main_t *cm = &unix_cli_main; + + /* Breadcrumb to indicate the new session process + * has not been started */ + cm->new_session_process_node_index = ~0; + return 0; } -- cgit 1.2.3-korg