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 556F141B83; Mon, 30 Jan 2023 19:48:37 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EE7D440DF6; Mon, 30 Jan 2023 19:48:36 +0100 (CET) Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2066.outbound.protection.outlook.com [40.107.93.66]) by mails.dpdk.org (Postfix) with ESMTP id EB46340C35 for ; Mon, 30 Jan 2023 19:48:34 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=F81mSWIHmpkOqzEC0Ugn/g8mAAySigjXW54LC0+TJsvfYBFpZ/XTaHrOqPRNJFnKASeFiaDxCv1TeIrx0eTGlDFK7+qaY8jdtXUJsKAgBoGguJWqCGwuv2x7O6QqXwIwSu2QEERp046FMVVmqTS2Peg44dTkeAAJVpAzbrQXux10FS6AtXrQNuEEZ0pOIHgq6lh5dm7WAfX8UG6GyxXkRFDRTBwK7JzK3072jKX4eL/EtufBrsBa5UjOCdP1uD9QkToHVg54OYEYEEBRJ8r8quFnnfC83L2QKnN0PDEvc6pg5khCSjkP3gnbiDyuVPIwZJk8rhuZUKHcO6Mj9rgFTg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ME8L5sqENJGYpxWKor9VSnESrNBCLl//4PtxfXgMTUk=; b=MLv4OfGpIFPa8BlAkwxPGGcGrsSjRx0zfXlOHV1Z5WJJ/YUXKYQMGFhHoTdvxwfTq9+KFK+hEX0oSgzqJNE4NeoBJF5+jiVQoCYAqMxu0mbiWphIhr1a6cWb63+cBfnaJ9wzObCxPPJlNnCp/ra/eqhcKDG1KZs5dSIShIFuEuMZkaZK4Er/tT4ewZGYPdhpWo4puyj7RhK8qoz9Hkjk8YDHzfS01XVh5w7Tj3DDSwXspOVDTP+t5BBtdcvGo7gxPrlRpqsnxBlz79ox1I6Qdq8elqnMbBl0WwZGReURBslZKYaldEz+0bFhTxYxLmch2sU1LJqtDZJl9Ink2Wtwdw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ME8L5sqENJGYpxWKor9VSnESrNBCLl//4PtxfXgMTUk=; b=NQg5pkJ/V4JbugVundC3SINT68t7tNr3iavOIFduPDQ0Cy/EzRitYoyWTeaUSGFZeK+3IbiFrGu+Aa630Z8q/szemnEzGn83mj7kRoM9m//ZdLELZI59LX3heWYealOOuZCb/fzTa/pUxCSZN3etTkRkhQkfBw5G8ehU5d/5Gjs= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by DM4PR12MB5915.namprd12.prod.outlook.com (2603:10b6:8:68::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.33; Mon, 30 Jan 2023 18:48:31 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::4807:1f44:5e04:e05a]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::4807:1f44:5e04:e05a%8]) with mapi id 15.20.6043.036; Mon, 30 Jan 2023 18:48:31 +0000 Message-ID: <851ff1c2-ebb4-fd34-b950-06b0f6fd6ef3@amd.com> Date: Mon, 30 Jan 2023 18:48:25 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Content-Language: en-US To: Stephen Hemminger , dev@dpdk.org, Olivier Matz , Thomas Monjalon Cc: Aman Singh , Yuying Zhang , Phil Yang , Jianbo Liu References: <20221112172839.70087-1-stephen@networkplumber.org> <20230125183205.96554-1-stephen@networkplumber.org> From: Ferruh Yigit Subject: Re: [PATCH v9] testpmd: cleanup cleanly from signal In-Reply-To: <20230125183205.96554-1-stephen@networkplumber.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0404.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:f::32) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM4PR12MB5915:EE_ X-MS-Office365-Filtering-Correlation-Id: cc8e4668-876b-47b8-cb43-08db02f2955d X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: AtHMD9zmii4OTs9KunywNSxkC4YQeohYFq7Mvb3FI53CvdguhNe+sPQ379D/2y4TG0UbpCCEtG55FVkiuL40pVEaVPe4a3l57yq5ej8YqJyZgxUBtkzjZYVEANQlDRDgL8l3t+Rp810PTcgMu7WZWgRTbYclaXGLVt+7vOa5Uj1WqMhY2diCuPOsnfeB/ashLpYxwi8asQ5k7C/pD7R0a9gwEnLbC14wx66da9jomCQ0j4LVECB4x45gsVo+zUHv7kaay+bgWiAw3g/qKQKhOS6c7PFwiSs6znpFwIywS2T8sBYoT5gro5XlO+ifMpHZku0UVze1sbQNrAc5ws7kXj0ajoUzkh2pOVFJfF7kdutuyrYj+HJVv4jrImFS3jL7zprfjoUIbUyWQJkSiqT2RJdg0dZax9+oredMAi82IR+W08ds/1aBWOHzAZUwQVlpbiY4mtNItko5HvTtLXRJkG+FNlQID+RVQTF8b7MVo5hLKw4z3iIFk7dRAF8/62Su9MMEbqIn4G5EHGuSwBdbbHRicZyroqu4L8JDRgpSgvwsbLI3DWxJf4ENDdBOolAd7XGEMeYuU4pQPN7XPc0EhKWB6V8XFueRfy8Br1mrmzmY/yNYyRckZ/PK9JftTgF8jtl50iwRI0yabQd83z11iiQyRCECfqjMwVKqeWLbXweOuc2K7AQ/mVsaqzmQLXlLxy0U5ZQMtnUY7USJmAAuKaMzZS8pDDTKkE9myixf8+4= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(366004)(39860400002)(136003)(346002)(396003)(376002)(451199018)(66946007)(66476007)(66556008)(2616005)(4326008)(38100700002)(36756003)(54906003)(86362001)(110136005)(31696002)(5660300002)(316002)(2906002)(44832011)(83380400001)(8936002)(41300700001)(6512007)(26005)(478600001)(6486002)(8676002)(31686004)(53546011)(186003)(6506007)(6666004)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TXU0cFRuS04wOW0xQjNMWVUxd1k5MjdRWWw5ZE5DTmZ1NldoTDdRSWtqYUJF?= =?utf-8?B?SmNnQ0dubnJoUC9tRTJtb3NvejlqMXc4S2lBZXBkMTVpSGVCWlEyYmMzR2tQ?= =?utf-8?B?MUpteitlZHdVVE5xSWF1SVdpNFJFQkhMY0NSZE1pSk0wK3RhRWlMVUtkQUdh?= =?utf-8?B?dHlZbUNsSUNPZzRCWmVQM3BRU01zVkZjNkVhanU5RUNXNXAyam5WYnlSMjFP?= =?utf-8?B?UkpVb1lUUys5dWxCa01YalpxWWdyamhPenRVQUpIN3E2SDY3MFpPQUt5RDJB?= =?utf-8?B?ZDZFWHMzanViOFkwdk5uWE5nZGwrUGY2N2NiVVpCWi9HaExnWEs1dytaeWcz?= =?utf-8?B?eXdwTDkzZjU3V3d1OGRVNDBLZE9qSmZ1T1B4TzZ6TCtiLzhqNFdYd0tpOTVp?= =?utf-8?B?cTBVVndmTFVkc3ptN2RtZ0tqY0N0amRJT2F3cHN5d2pocUJVbEhoOVR0L3hp?= =?utf-8?B?L3dMNEh0NnlEdWdpU0E0K05uUW1iU3FPbG5XZ29qdFEvWTNWOXZnVlNHb0pL?= =?utf-8?B?ZWlxUXJGRVAyRGpiWU0vdWhMcE5NVUZvdkU2cmNPM09abUp6amVlMlYrVWNY?= =?utf-8?B?dUxMdDFIS0U0TjFFNmdVZXVnU2tUS1hUdTlCVGMxOXB4djc5UEZ2NFhHZ25t?= =?utf-8?B?VG1NMFZOWGk3WGhpaW8xMmVDVjUvQ1hsakQwT0t0Y1IzVlcySkw0MVgyNXZr?= =?utf-8?B?WGxyUlU5TTFGY2hROU0vVWdXWndRZDU3MEtuRjV5RVY3NVUzTmZFSTRLN2hW?= =?utf-8?B?MUxMWGJySCt6MDY3b1dKVVpHbENvc1B3VjEzOUZJOWd6UkJVUWg4djN2K0NU?= =?utf-8?B?UVU5VTczTngwTzFxWDNYVVJpWE1EYjFxY256ZitmNy9iZjV0amRkRE8zb0Zq?= =?utf-8?B?MGtma3lNVE5qN1E2RHdCTWYyZU1ITGYxMThKWGFCUGtZcXhyYStEWko0Q1po?= =?utf-8?B?bU4wTFdibDA4NkxYOEN5MEROMWFPVGw4K0NUM09YMGhBeWVDRnZEeTZrZVlM?= =?utf-8?B?SWNMeE5aQUJ4cmdYOGtQclV5YTV3V2tPWk9IWk9GRkJydGVoS1dDWWVFcGdR?= =?utf-8?B?citHR2ZCUUVGOWpyQVNCU2tUem5ITHYzelRDRFlEazhUM0ZQbGZNeTY0Mkp5?= =?utf-8?B?OWkyaUlTZFJBK3dlZkNYZzZCemZVRS9zV29xUVova0I5U0NEa25Wektrb29u?= =?utf-8?B?QXhjR3Z6UE9pT1FYWitmckFwbjh4b1cyck1KaG9aN2VPZlFaYjUrOVhpR29M?= =?utf-8?B?YStjTkp0bUxoSEh2dkU1c2dDNDdKeFo4VW40S1EvVGlFdEZkNTVhUWNDTE1X?= =?utf-8?B?NGxRRDBXMkpTaXR2VGlTaDVhOGtLRzFMSFkweFkrdW13eis3ajJmVkZwYW9r?= =?utf-8?B?Z09XckZLUzlDejdZWmZVdVpvVWFmTDhIRnFSOTFMWXpINjY1T3ptVWp2Wkgv?= =?utf-8?B?M0NRdlhmN3htWHA3YVF5eFN1YkxLRDRkdkZqOThlMHZldXM5WDBxemFwZXUx?= =?utf-8?B?bFFlajE3WkFPZUhnejdxVUJrWUFpaWpTeW51M2pCMlZpWXRiSmxnRk1YdzZK?= =?utf-8?B?bU96V2VldlNNRFRYQmVOQlhYb25kQ1hVK25EeW51TktmSEcydEZ6RWt1Zkkr?= =?utf-8?B?RVJXWTRRaWFiYng2alUxbGtxVkJxWFVpdEtSOGdyN1FpYzI4UEliRTJNSUpJ?= =?utf-8?B?cVZEKzA1MTA4alJWdy95dHlCb0cxcVJPem9tVDdEQWcxY0tNWDV3bzYzOWZ0?= =?utf-8?B?KzVlckdFUnJYbitZVEFQazdOYWtzd1liTmRmR24xK0FSZmpMWmd2TnBLV0Ir?= =?utf-8?B?UmxqTS9MQjFySXZ1a3dnWUhnQnlYY2llNEszMFhYWEtzajZWNldpak9WMTJo?= =?utf-8?B?RW1GMmhzTjZDbnkxR2R5cXErdmxzRjRXVjBvWUw1YXp2WitYdEhJNUxBWnpZ?= =?utf-8?B?NHB5WEllS2Z5eWpnVnZNRjRYZktzNnhqdXdQQU02dGUzSThDc1RNcnNqMUpa?= =?utf-8?B?RWhKK05CQ05temRoTFRnWmQ5YWhyZWdDcmtGZk5NSld2a3kwNUVzaXhBc3dB?= =?utf-8?B?VXdqYXlYUVZUcytOUUZmNnc2NCtyZGtDazlzR2F4Rlk3MXJXbmFvRGZFZkdQ?= =?utf-8?Q?qYmKXGcfJ4Ngpn+YCWwq2wBnm?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: cc8e4668-876b-47b8-cb43-08db02f2955d X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2023 18:48:31.7005 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: XU+I4zLtQE15CFXFS1t9VGKDwAsN92RzV6cQfOw6m+GVrcRAZMFEZbsxo956l2M4 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5915 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 1/25/2023 6:32 PM, Stephen Hemminger wrote: > Do a clean shutdown of testpmd when a signal is received; instead of > having testpmd kill itself. This fixes the problem where a signal could > be received in the middle of a PMD and then the signal handler would > call PMD's close routine leading to locking problems. > > An added benefit is it gets rid of some Windows specific code. > > Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container") > Signed-off-by: Stephen Hemminger Patch looks good to me, but './devtools/test-null.sh' hangs with change. It is possible the fix by updating './devtools/test-null.sh', but my concern is if this behavior change hit more consumers other than this internal tool, and I am for keeping previous behavior. './devtools/test-null.sh' sends 'stop' command to testpmd but that seems not really what makes testpmd quit, because following change still works with original testpmd code: -(sleep 1 && echo stop) | +(sleep 1 && echo help) | Somehow testpmd gets Ctrl-D or equivalent with above command. And it seems because of 'cmdline_interact()' and 'cmdline_poll()' difference behavior changes with above command. It is possible to add something like 'cmdline_interact_one()' (non loop version of 'cmdline_interact()'), but not really sure that is correct way to fix the issue. > --- > v9 - also handle the interactive case. > use cmdine_poll() rather than signals and atexit > > app/test-pmd/cmdline.c | 28 +++++---------- > app/test-pmd/testpmd.c | 77 ++++++++++++++++++++---------------------- > app/test-pmd/testpmd.h | 1 + > 3 files changed, 46 insertions(+), 60 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index b32dc8bfd445..fbe9ad694dee 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -65,7 +65,6 @@ > #include "cmdline_tm.h" > #include "bpf_cmd.h" > > -static struct cmdline *testpmd_cl; > static cmdline_parse_ctx_t *main_ctx; > static TAILQ_HEAD(, testpmd_driver_commands) driver_commands_head = > TAILQ_HEAD_INITIALIZER(driver_commands_head); > @@ -12921,28 +12920,19 @@ cmdline_read_from_file(const char *filename) > void > prompt(void) > { > - int ret; > + struct cmdline *cl; > > - testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> "); > - if (testpmd_cl == NULL) > + cl = cmdline_stdin_new(main_ctx, "testpmd> "); > + if (cl == NULL) > return; > > - ret = atexit(prompt_exit); > - if (ret != 0) > - fprintf(stderr, "Cannot set exit function for cmdline\n"); > - > - cmdline_interact(testpmd_cl); > - if (ret != 0) > - cmdline_stdin_exit(testpmd_cl); > -} > - > -void > -prompt_exit(void) > -{ > - if (testpmd_cl != NULL) { > - cmdline_quit(testpmd_cl); > - cmdline_stdin_exit(testpmd_cl); > + while (f_quit == 0 && cl_quit == 0) { > + if (cmdline_poll(cl) < 0) > + break; > } > + > + cmdline_quit(cl); > + cmdline_stdin_exit(cl); > } > > void > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index 134d79a55547..d01b6105b7de 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -11,6 +11,7 @@ > #include > #ifndef RTE_EXEC_ENV_WINDOWS > #include > +#include > #endif > #include > #include > @@ -231,7 +232,7 @@ unsigned int xstats_display_num; /**< Size of extended statistics to show */ > * In container, it cannot terminate the process which running with 'stats-period' > * option. Set flag to exit stats period loop after received SIGINT/SIGTERM. > */ > -static volatile uint8_t f_quit; > +volatile uint8_t f_quit; > uint8_t cl_quit; /* Quit testpmd from cmdline. */ > > /* > @@ -4315,13 +4316,6 @@ init_port(void) > memset(txring_numa, NUMA_NO_CONFIG, RTE_MAX_ETHPORTS); > } > > -static void > -force_quit(void) > -{ > - pmd_test_exit(); > - prompt_exit(); > -} > - > static void > print_stats(void) > { > @@ -4340,28 +4334,9 @@ print_stats(void) > } > > static void > -signal_handler(int signum) > +signal_handler(int signum __rte_unused) > { > - 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 > - } > + f_quit = 1; > } > > int > @@ -4536,15 +4511,9 @@ main(int argc, char** argv) > start_packet_forwarding(0); > } > prompt(); > - pmd_test_exit(); > } else > #endif > { > - char c; > - int rc; > - > - f_quit = 0; > - > printf("No commandline core given, start packet forwarding\n"); > start_packet_forwarding(tx_first); > if (stats_period != 0) { > @@ -4567,15 +4536,41 @@ 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"); > > - printf("Press enter to exit\n"); > - rc = read(0, &c, 1); > - pmd_test_exit(); > - if (rc < 0) > - return 1; > + FD_ZERO(&fds); > + FD_SET(0, &fds); > + > + /* wait for signal or enter */ > + ret = select(1, &fds, NULL, NULL, NULL); > + if (ret < 0 && errno != EINTR) > + rte_exit(EXIT_FAILURE, > + "Select failed: %s\n", > + strerror(errno)); > + > + /* if got enter then consume it */ > + if (ret == 1 && read(0, &c, 1) < 0) > + rte_exit(EXIT_FAILURE, > + "Read failed: %s\n", > + strerror(errno)); > + } > } > > + pmd_test_exit(); > + > +#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, > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > index 7d24d25970d2..022210a7a964 100644 > --- a/app/test-pmd/testpmd.h > +++ b/app/test-pmd/testpmd.h > @@ -34,6 +34,7 @@ > #define RTE_PORT_HANDLING (uint16_t)3 > > extern uint8_t cl_quit; > +extern volatile uint8_t f_quit; > > /* > * It is used to allocate the memory for hash key.