From 555e1d96271ebfc646bc0455b7ce1cac96cc893f Mon Sep 17 00:00:00 2001 From: Kip Walker Date: Wed, 27 Nov 2024 15:09:54 -0800 Subject: [PATCH 1/2] [dv/dpi] tcp_server error out on port conflict When the tcp_server is unable to create its listening socket, the simulation now exits. Previously, an error would be printed but the simulation would continue. Clients like dmidpi and jtagdpi should only call tcp_server_create if there's a chance a client will connect, and if the listening port number is being selected to avoid conflict with other processes on the same machine. Signed-off-by: Kip Walker --- hw/dv/dpi/common/tcp_server/tcp_server.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/dv/dpi/common/tcp_server/tcp_server.c b/hw/dv/dpi/common/tcp_server/tcp_server.c index c54e380c5c93b..582d71d780b06 100644 --- a/hw/dv/dpi/common/tcp_server/tcp_server.c +++ b/hw/dv/dpi/common/tcp_server/tcp_server.c @@ -326,7 +326,10 @@ static void *server_create(void *ctx_void) { if (rv != 0) { fprintf(stderr, "%s: Unable to create TCP server on port %d\n", ctx->display_name, ctx->listen_port); - goto err_cleanup_return; + // Failing to create the listening socket is treated as a fatal + // error. If the creation of this socket is not important, it + // should not even be attempted. + assert(false); } // Initialise fd_set From 6b0a9e36b7233e7ff16bb4f73b59c7b709b59dc5 Mon Sep 17 00:00:00 2001 From: Kip Walker Date: Wed, 27 Nov 2024 16:08:06 -0800 Subject: [PATCH 2/2] [dv/dpi] tcp_server synchronous socket create The listening socket creation is made synchronous with the tcp_server_create call, while the processing of socket events is still handled on a dedicated thread. Signed-off-by: Kip Walker --- hw/dv/dpi/common/tcp_server/tcp_server.c | 33 ++++++++++++------------ 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/hw/dv/dpi/common/tcp_server/tcp_server.c b/hw/dv/dpi/common/tcp_server/tcp_server.c index 582d71d780b06..8101b05d4746b 100644 --- a/hw/dv/dpi/common/tcp_server/tcp_server.c +++ b/hw/dv/dpi/common/tcp_server/tcp_server.c @@ -311,29 +311,16 @@ static void ctx_free(struct tcp_server_ctx *ctx) { } /** - * Thread function to create a new server instance + * Thread function to process server events * * @param ctx_void context object * @return Always returns NULL */ -static void *server_create(void *ctx_void) { +static void *server_process(void *ctx_void) { // Cast to a server struct struct tcp_server_ctx *ctx = (struct tcp_server_ctx *)ctx_void; struct timeval timeout; - // Start the server - int rv = start(ctx); - if (rv != 0) { - fprintf(stderr, "%s: Unable to create TCP server on port %d\n", - ctx->display_name, ctx->listen_port); - // Failing to create the listening socket is treated as a fatal - // error. If the creation of this socket is not important, it - // should not even be attempted. - assert(false); - } - - // Initialise fd_set - // Start waiting for connection / data char xfer_data; while (ctx->socket_run) { @@ -355,7 +342,7 @@ static void *server_create(void *ctx_void) { timeout.tv_usec = 50; // Wait for socket activity or timeout - rv = select(mfd + 1, &read_fds, NULL, NULL, &timeout); + int rv = select(mfd + 1, &read_fds, NULL, NULL, &timeout); if (rv < 0) { if (errno == EINTR) { @@ -419,7 +406,19 @@ struct tcp_server_ctx *tcp_server_create(const char *display_name, ctx->display_name = strdup(display_name); assert(ctx->display_name); - if (pthread_create(&ctx->sock_thread, NULL, server_create, (void *)ctx) != + // Open the listening socket synchronously + int rv = start(ctx); + if (rv != 0) { + fprintf(stderr, "%s: Unable to create TCP server on port %d\n", + ctx->display_name, ctx->listen_port); + // Failing to create the listening socket is treated as a fatal + // error. If the creation of this socket is not important, it + // should not even be attempted. + assert(false); + } + + // Start a thread to accept connections and transfer data + if (pthread_create(&ctx->sock_thread, NULL, server_process, (void *)ctx) != 0) { fprintf(stderr, "%s: Unable to create TCP socket thread\n", ctx->display_name);