From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7AE76A034C; Wed, 9 Nov 2022 22:46:59 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6342E400EF; Wed, 9 Nov 2022 22:46:59 +0100 (CET) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 4964D400D4 for ; Wed, 9 Nov 2022 22:46:58 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id DA2E01A7B7 for ; Wed, 9 Nov 2022 22:46:57 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id D8EDC1A7B6; Wed, 9 Nov 2022 22:46:57 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.5 required=5.0 tests=ALL_TRUSTED, AWL, NICE_REPLY_A autolearn=disabled version=3.4.6 X-Spam-Score: -1.5 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 64ED11AA0F; Wed, 9 Nov 2022 22:46:56 +0100 (CET) Message-ID: <097a2759-f958-84ed-0ddf-4b23eb1eee04@lysator.liu.se> Date: Wed, 9 Nov 2022 22:46:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v4] testpmd: cleanup cleanly from signal To: Stephen Hemminger , dev@dpdk.org Cc: phil.yang@arm.com References: <20221014172328.185219-2-stephen@networkplumber.org> <20221109041046.199840-1-stephen@networkplumber.org> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= In-Reply-To: <20221109041046.199840-1-stephen@networkplumber.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2022-11-09 05:10, Stephen Hemminger wrote: > Do a clean shutdown of testpmd when a signal is received; > instead of having testpmd kill itself. > This fixes problem where a signal could be received > in the middle of a PMD and then the signal handler would call > PMD's close routine which could cause a deadlock. > > Added benefit is it gets rid of Windows specific code. > > Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container") > Signed-off-by: Stephen Hemminger > --- > v4 - use select() because that is available on Windows; and other > functions poll() and sigaction() are not. > > app/test-pmd/testpmd.c | 63 +++++++++++++++++++++++------------------- > 1 file changed, 34 insertions(+), 29 deletions(-) > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index cf5942d0c422..274e96cac2d4 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -12,6 +12,7 @@ > #ifndef RTE_EXEC_ENV_WINDOWS > #include > #endif > +#include > #include > #include > #include > @@ -4251,26 +4252,11 @@ print_stats(void) > static void > signal_handler(int signum) > { > - if (signum == SIGINT || signum == SIGTERM) { > - fprintf(stderr, "\nSignal %d received, preparing to exit...\n", > - signum); > -#ifdef RTE_LIB_PDUMP > - /* uninitialize packet capture framework */ > - rte_pdump_uninit(); > -#endif > -#ifdef RTE_LIB_LATENCYSTATS > - if (latencystats_enabled != 0) > - rte_latencystats_uninit(); > -#endif > - force_quit(); > - /* Set flag to indicate the force termination. */ > - f_quit = 1; > - /* exit with the expected status */ > -#ifndef RTE_EXEC_ENV_WINDOWS > - signal(signum, SIG_DFL); > - kill(getpid(), signum); > -#endif > - } > + fprintf(stderr, "\nSignal %d %s received, preparing to exit...\n", > + signum, strsignal(signum)); fprintf() is not async signal safe, and neither is strsignal(). This is not a regression introduced by this patch, but I thought it might be worth fixing. > + > + /* Set flag to indicate the force termination. */ > + f_quit = 1; > } > > int > @@ -4449,9 +4435,6 @@ main(int argc, char** argv) > } else > #endif > { > - char c; > - int rc; > - > f_quit = 0; > > printf("No commandline core given, start packet forwarding\n"); > @@ -4476,15 +4459,37 @@ main(int argc, char** argv) > prev_time = cur_time; > rte_delay_us_sleep(US_PER_S); > } > - } > + } else { > + char c; > + fd_set fds; > > - printf("Press enter to exit\n"); > - rc = read(0, &c, 1); > - pmd_test_exit(); > - if (rc < 0) > - return 1; > + printf("Press enter to exit\n"); > + > + FD_ZERO(&fds); > + FD_SET(0, &fds); > + > + if (select(1, &fds, NULL, NULL, NULL) <= 0) { > + fprintf(stderr, "Select failed: %s\n", > + strerror(errno)); Why is select() needed? Wouldn't a blocking read suffice? Or getchar(). > + } else if (read(0, &c, 1) <= 0) { > + fprintf(stderr, > + "Read stdin failed: %s\n", > + strerror(errno)); > + } > + } > + stop_packet_forwarding(); > + force_quit(); > } > > +#ifdef RTE_LIB_PDUMP > + /* uninitialize packet capture framework */ > + rte_pdump_uninit(); > +#endif > +#ifdef RTE_LIB_LATENCYSTATS > + if (latencystats_enabled != 0) > + rte_latencystats_uninit(); > +#endif > + > ret = rte_eal_cleanup(); > if (ret != 0) > rte_exit(EXIT_FAILURE,