From 1334761e1e558b2107e62ffe60ce81bd2ce5780c Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Wed, 3 Mar 2021 12:58:57 -0800 Subject: [PATCH] hsa: fix builtin echo apps with multiple workers Type: fix Signed-off-by: Florin Coras Change-Id: I9507b5a9755e938b4d1da657bed3a8681a056427 --- src/plugins/hs_apps/echo_client.c | 59 +++++++++++++++++++++++++++++++-------- src/plugins/hs_apps/echo_server.c | 54 ++++++++++++++++++++++++++--------- test/test_tcp.py | 2 -- 3 files changed, 88 insertions(+), 27 deletions(-) diff --git a/src/plugins/hs_apps/echo_client.c b/src/plugins/hs_apps/echo_client.c index c26329468ec..9da7bc5fd4b 100644 --- a/src/plugins/hs_apps/echo_client.c +++ b/src/plugins/hs_apps/echo_client.c @@ -767,18 +767,16 @@ echo_clients_command_fn (vlib_main_t * vm, echo_client_main_t *ecm = &echo_client_main; vlib_thread_main_t *thread_main = vlib_get_thread_main (); u64 tmp, total_bytes, appns_flags = 0, appns_secret = 0; + session_endpoint_cfg_t sep = SESSION_ENDPOINT_CFG_NULL; f64 test_timeout = 20.0, syn_timeout = 20.0, delta; char *default_uri = "tcp://6.0.1.1/1234"; + u8 *appns_id = 0, barrier_acq_needed = 0; + int preallocate_sessions = 0, i, rv; uword *event_data = 0, event_type; f64 time_before_connects; u32 n_clients = 1; - int preallocate_sessions = 0; char *transfer_type; clib_error_t *error = 0; - u8 *appns_id = 0; - int i; - session_endpoint_cfg_t sep = SESSION_ENDPOINT_CFG_NULL; - int rv; ecm->quic_streams = 1; ecm->bytes_to_send = 8192; @@ -795,6 +793,26 @@ echo_clients_command_fn (vlib_main_t * vm, ecm->no_copy = 0; ecm->run_test = ECHO_CLIENTS_STARTING; + if (vlib_num_workers ()) + { + /* The request came over the binary api and the inband cli handler + * is not mp_safe. Drop the barrier to make sure the workers are not + * blocked. + */ + if (vlib_thread_is_main_w_barrier ()) + { + barrier_acq_needed = 1; + vlib_worker_thread_barrier_release (vm); + } + /* + * There's a good chance that both the client and the server echo + * apps will be enabled so make sure the session queue node polls on + * the main thread as connections will probably be established on it. + */ + vlib_node_set_state (vm, session_queue_node.index, + VLIB_NODE_STATE_POLLING); + } + if (thread_main->n_vlib_mains > 1) clib_spinlock_init (&ecm->sessions_lock); vec_free (ecm->connect_uri); @@ -828,8 +846,11 @@ echo_clients_command_fn (vlib_main_t * vm, unformat_memory_size, &tmp)) { if (tmp >= 0x100000000ULL) - return clib_error_return - (0, "private segment size %lld (%llu) too large", tmp, tmp); + { + error = clib_error_return ( + 0, "private segment size %lld (%llu) too large", tmp, tmp); + goto cleanup; + } ecm->private_segment_size = tmp; } else if (unformat (input, "preallocate-fifos")) @@ -857,8 +878,11 @@ echo_clients_command_fn (vlib_main_t * vm, else if (unformat (input, "tls-engine %d", &ecm->tls_engine)) ; else - return clib_error_return (0, "failed: unknown input `%U'", - format_unformat_error, input); + { + error = clib_error_return (0, "failed: unknown input `%U'", + format_unformat_error, input); + goto cleanup; + } } /* Store cli process node index for signalling */ @@ -868,7 +892,10 @@ echo_clients_command_fn (vlib_main_t * vm, if (ecm->is_init == 0) { if (echo_clients_init (vm)) - return clib_error_return (0, "failed init"); + { + error = clib_error_return (0, "failed init"); + goto cleanup; + } } @@ -884,7 +911,10 @@ echo_clients_command_fn (vlib_main_t * vm, } if ((rv = parse_uri ((char *) ecm->connect_uri, &sep))) - return clib_error_return (0, "Uri parse error: %d", rv); + { + error = clib_error_return (0, "Uri parse error: %d", rv); + goto cleanup; + } ecm->transport_proto = sep.transport_proto; ecm->is_dgram = (sep.transport_proto == TRANSPORT_PROTO_UDP); @@ -902,7 +932,7 @@ echo_clients_command_fn (vlib_main_t * vm, { vec_free (appns_id); clib_error_report (error); - return error; + goto cleanup; } vec_free (appns_id); } @@ -1024,6 +1054,11 @@ cleanup: if (error) ec_cli_output ("test failed"); vec_free (ecm->connect_uri); + clib_spinlock_free (&ecm->sessions_lock); + + if (barrier_acq_needed) + vlib_worker_thread_barrier_sync (vm); + return error; } diff --git a/src/plugins/hs_apps/echo_server.c b/src/plugins/hs_apps/echo_server.c index da79cb40745..63150d5a8d8 100644 --- a/src/plugins/hs_apps/echo_server.c +++ b/src/plugins/hs_apps/echo_server.c @@ -449,12 +449,23 @@ static clib_error_t * echo_server_create_command_fn (vlib_main_t * vm, unformat_input_t * input, vlib_cli_command_t * cmd) { + session_endpoint_cfg_t sep = SESSION_ENDPOINT_CFG_NULL; echo_server_main_t *esm = &echo_server_main; u8 server_uri_set = 0, *appns_id = 0; u64 tmp, appns_flags = 0, appns_secret = 0; char *default_uri = "tcp://0.0.0.0/1234"; - int rv, is_stop = 0; - session_endpoint_cfg_t sep = SESSION_ENDPOINT_CFG_NULL; + int rv, is_stop = 0, barrier_acq_needed = 0; + clib_error_t *error = 0; + + /* The request came over the binary api and the inband cli handler + * is not mp_safe. Drop the barrier to make sure the workers are not + * blocked. + */ + if (vlib_num_workers () && vlib_thread_is_main_w_barrier ()) + { + barrier_acq_needed = 1; + vlib_worker_thread_barrier_release (vm); + } esm->no_echo = 0; esm->fifo_size = 64 << 10; @@ -484,8 +495,11 @@ echo_server_create_command_fn (vlib_main_t * vm, unformat_input_t * input, unformat_memory_size, &tmp)) { if (tmp >= 0x100000000ULL) - return clib_error_return - (0, "private segment size %lld (%llu) too large", tmp, tmp); + { + error = clib_error_return ( + 0, "private segment size %lld (%llu) too large", tmp, tmp); + goto cleanup; + } esm->private_segment_size = tmp; } else if (unformat (input, "appns %_%v%_", &appns_id)) @@ -504,8 +518,11 @@ echo_server_create_command_fn (vlib_main_t * vm, unformat_input_t * input, else if (unformat (input, "tls-engine %d", &esm->tls_engine)) ; else - return clib_error_return (0, "failed: unknown input `%U'", - format_unformat_error, input); + { + error = clib_error_return (0, "failed: unknown input `%U'", + format_unformat_error, input); + goto cleanup; + } } if (is_stop) @@ -513,15 +530,17 @@ echo_server_create_command_fn (vlib_main_t * vm, unformat_input_t * input, if (esm->app_index == (u32) ~ 0) { clib_warning ("server not running"); - return clib_error_return (0, "failed: server not running"); + error = clib_error_return (0, "failed: server not running"); + goto cleanup; } rv = echo_server_detach (); if (rv) { clib_warning ("failed: detach"); - return clib_error_return (0, "failed: server detach %d", rv); + error = clib_error_return (0, "failed: server detach %d", rv); + goto cleanup; } - return 0; + goto cleanup; } vnet_session_enable_disable (vm, 1 /* turn on TCP, etc. */ ); @@ -533,19 +552,28 @@ echo_server_create_command_fn (vlib_main_t * vm, unformat_input_t * input, } if ((rv = parse_uri ((char *) esm->server_uri, &sep))) - return clib_error_return (0, "Uri parse error: %d", rv); + { + error = clib_error_return (0, "Uri parse error: %d", rv); + goto cleanup; + } esm->transport_proto = sep.transport_proto; esm->is_dgram = (sep.transport_proto == TRANSPORT_PROTO_UDP); rv = echo_server_create (vm, appns_id, appns_flags, appns_secret); - vec_free (appns_id); if (rv) { vec_free (esm->server_uri); - return clib_error_return (0, "failed: server_create returned %d", rv); + error = clib_error_return (0, "failed: server_create returned %d", rv); + goto cleanup; } - return 0; +cleanup: + vec_free (appns_id); + + if (barrier_acq_needed) + vlib_worker_thread_barrier_sync (vm); + + return error; } /* *INDENT-OFF* */ diff --git a/test/test_tcp.py b/test/test_tcp.py index 4ceb46e666c..f8e51057d38 100644 --- a/test/test_tcp.py +++ b/test/test_tcp.py @@ -2,12 +2,10 @@ import unittest -from framework import tag_fixme_vpp_workers from framework import VppTestCase, VppTestRunner from vpp_ip_route import VppIpTable, VppIpRoute, VppRoutePath -@tag_fixme_vpp_workers class TestTCP(VppTestCase): """ TCP Test Case """ -- 2.16.6