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 BB127A0093; Wed, 9 Nov 2022 23:53:28 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A404F400EF; Wed, 9 Nov 2022 23:53:28 +0100 (CET) Received: from mail-pg1-f181.google.com (mail-pg1-f181.google.com [209.85.215.181]) by mails.dpdk.org (Postfix) with ESMTP id B0DC8400D4 for ; Wed, 9 Nov 2022 23:53:27 +0100 (CET) Received: by mail-pg1-f181.google.com with SMTP id 6so10762pgm.6 for ; Wed, 09 Nov 2022 14:53:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=kPVlpkcRPk3ONFcOMC+WcKko4V84LgnzaRXnamgMrhc=; b=OvwYF3Ufx2Wsp4mvtpEzqmSY+ygF7C76j/OkB9QalOr028qYE2dkhKn+jBV7oxf3ry wnE0veclizU8aRy05PakyD5rpfooPx5rEsxLOTMSMl3I3tCQ9/NIspZMhT/myD46LbsX fg5t/Y3s1Ejwq4t+6CJw9USZdMKa7ZTn6I6yTJdMi1eS5NgujUcMVLB8KcZgxow5ECrk lsUg7X6bigKXpD4wNbPpm3qGNELy1ZALnjLzmv62ogeJZdUxWgLJ7TvIVRaoOEGWfsuD nH1WF/SEZxmMonAd1ReB15A0+gm7K2CYb0H/fxEcetImM+Tjy6g113N9Qsv98JHLWymP fCAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kPVlpkcRPk3ONFcOMC+WcKko4V84LgnzaRXnamgMrhc=; b=kbNQUNKyid0vOzckRR7fMLNOB0I4CfE90OSxZsJLAnHpneNRxRr0zGg+DgmqsKMQWB m+ydXhbscFBydqqBg1pcRuC5/4Hklf3sqGia6pfqd+JO/DHanAuNU801OJc1UOrv2fwG 8M7p+jmv+/pN3/omxU/k7eVsSFNBbO7rCIir9NCQuSMgtafz0Pk4Dcy38/Zcez4TsH0N sUq1r7L1HnS9O0VLdHNhcbV0zDe3rj6rv8kHZVvR4px5fPL/3LP12ZI1AG7e9QrIHOFe teOtdCwNk9R+XPP5pW4rm6VFGYfONOKjEw90hlRYuQPfOM0iM49EnEFtD8mIkyNvgkJf KCrQ== X-Gm-Message-State: ACrzQf17v8aP0wITdx/kunLzE23/gz6YBl7aGdjROwJbtqcN6Q7mscaw yl0134rHwnucnGZDxjb2vlAfTmdKUIN/LQ== X-Google-Smtp-Source: AMsMyM70UhB6ZIkjCbbFPt8Y/8EJOM7JEg27J9ufDhj/2OdhljBY4txgO8bZmtF/M9dCbyMMuwrEhQ== X-Received: by 2002:a63:d64b:0:b0:46f:7941:a8ea with SMTP id d11-20020a63d64b000000b0046f7941a8eamr53049906pgj.331.1668034406813; Wed, 09 Nov 2022 14:53:26 -0800 (PST) Received: from hermes.local (204-195-120-218.wavecable.com. [204.195.120.218]) by smtp.gmail.com with ESMTPSA id p2-20020a170902e74200b001869b988d93sm9629244plf.187.2022.11.09.14.53.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Nov 2022 14:53:26 -0800 (PST) Date: Wed, 9 Nov 2022 14:53:23 -0800 From: Stephen Hemminger To: Mattias =?UTF-8?B?UsO2bm5ibG9t?= Cc: dev@dpdk.org, phil.yang@arm.com Subject: Re: [PATCH v4] testpmd: cleanup cleanly from signal Message-ID: <20221109145323.7ac2a1b4@hermes.local> In-Reply-To: <097a2759-f958-84ed-0ddf-4b23eb1eee04@lysator.liu.se> References: <20221014172328.185219-2-stephen@networkplumber.org> <20221109041046.199840-1-stephen@networkplumber.org> <097a2759-f958-84ed-0ddf-4b23eb1eee04@lysator.liu.se> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 Wed, 9 Nov 2022 22:46:55 +0100 Mattias R=C3=B6nnblom wrote: > 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. > >=20 > > Added benefit is it gets rid of Windows specific code. > >=20 > > 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. > >=20 > > app/test-pmd/testpmd.c | 63 +++++++++++++++++++++++------------------- > > 1 file changed, 34 insertions(+), 29 deletions(-) > >=20 > > 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 =3D=3D SIGINT || signum =3D=3D 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 !=3D 0) > > - rte_latencystats_uninit(); > > -#endif > > - force_quit(); > > - /* Set flag to indicate the force termination. */ > > - f_quit =3D 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)); =20 >=20 > fprintf() is not async signal safe, and neither is strsignal(). >=20 > This is not a regression introduced by this patch, but I thought it=20 > might be worth fixing. >=20 > > + > > + /* Set flag to indicate the force termination. */ > > + f_quit =3D 1; > > } > > =20 > > int > > @@ -4449,9 +4435,6 @@ main(int argc, char** argv) > > } else > > #endif > > { > > - char c; > > - int rc; > > - > > f_quit =3D 0; > > =20 > > printf("No commandline core given, start packet forwarding\n"); > > @@ -4476,15 +4459,37 @@ main(int argc, char** argv) > > prev_time =3D cur_time; > > rte_delay_us_sleep(US_PER_S); > > } > > - } > > + } else { > > + char c; > > + fd_set fds; > > =20 > > - printf("Press enter to exit\n"); > > - rc =3D 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) <=3D 0) { > > + fprintf(stderr, "Select failed: %s\n", > > + strerror(errno)); =20 >=20 > Why is select() needed? Wouldn't a blocking read suffice? Or getchar(). On Linux, signal set SA_RESTART so a simple read is not interrupted. One option was to use sigaction() which allows controlling flags, but that won't work on Windows. Using select() works on both.