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 <chrisy@flirble.org>
Signed-off-by: Dave Barach <dave@barachs.net>
diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c
index 2d5a22d..fa61c69 100644
--- a/src/vlib/unix/cli.c
+++ b/src/vlib/unix/cli.c
@@ -47,7 +47,6 @@
#include <vlib/vlib.h>
#include <vlib/unix/unix.h>
-#include <vppinfra/timer.h>
#include <ctype.h>
#include <fcntl.h>
@@ -61,6 +60,7 @@
#include <unistd.h>
#include <limits.h>
#include <netinet/tcp.h>
+#include <math.h>
/** ANSI escape code. */
#define ESC "\x1b"
@@ -451,6 +451,21 @@
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 @@
/** 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 @@
}
-/** @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);
- cf = pool_elt_at_index (cm->cli_file_pool, (uword) arg);
+ event_type = vlib_process_get_events (vm, &event_data);
- if (!cf->started)
- unix_cli_file_welcome (cm, cf);
+ 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;
+
+ 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;
+ }
+
+ 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_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 @@
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;
}