From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by dpdk.org (Postfix) with ESMTP id 470D21B122 for ; Mon, 29 Apr 2019 11:44:10 +0200 (CEST) Received: from DGGEMS411-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 8B07429D52A4F8AE859C for ; Mon, 29 Apr 2019 17:44:08 +0800 (CST) Received: from [127.0.0.1] (10.177.131.206) by DGGEMS411-HUB.china.huawei.com (10.3.19.211) with Microsoft SMTP Server id 14.3.439.0; Mon, 29 Apr 2019 17:44:04 +0800 To: "Burakov, Anatoly" , CC: References: <1556210141-43153-1-git-send-email-mousuanming@huawei.com> <1556427506-49150-1-git-send-email-mousuanming@huawei.com> From: Suanming.Mou Message-ID: <58514852-12be-5ba5-60df-b7b01b586072@huawei.com> Date: Mon, 29 Apr 2019 17:43:48 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.177.131.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v2] app/pdump: add exit_with_primary option support. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Apr 2019 09:44:10 -0000 On 2019/4/29 17:14, Burakov, Anatoly wrote: > On 28-Apr-19 5:58 AM, Suanming.Mou wrote: >> When primary app exits, the residual running pdump will stop >> the primary app to restart. Add an exit_with_primary option >> to make pdump exit with primary. >> >> Suggested-by: Varghese, Vipin >> Suggested-by: Burakov, Anatoly >> Signed-off-by: Suanming.Mou >> --- >>   app/pdump/main.c | 26 ++++++++++++++++++++++++++ >>   1 file changed, 26 insertions(+) >> >> diff --git a/app/pdump/main.c b/app/pdump/main.c >> index 3d20854..3909f15 100644 >> --- a/app/pdump/main.c >> +++ b/app/pdump/main.c >> @@ -26,11 +26,14 @@ >>   #include >>   #include >>   #include >> +#include >>     #define CMD_LINE_OPT_PDUMP "pdump" >>   #define CMD_LINE_OPT_PDUMP_NUM 256 >>   #define CMD_LINE_OPT_MULTI "multi" >>   #define CMD_LINE_OPT_MULTI_NUM 257 >> +#define CMD_LINE_OPT_EXIT_WP "exit_with_primary" >> +#define CMD_LINE_OPT_EXIT_WP_NUM 258 > > Unrelated to this patch, but seems very flaky and prone to error. How > about replacing this stuff with enum-based automatic value assignment, > like in lib/librte_eal/common/eal_options.h ? :) > >>   #define PDUMP_PORT_ARG "port" >>   #define PDUMP_PCI_ARG "device_id" >>   #define PDUMP_QUEUE_ARG "queue" >> @@ -65,6 +68,7 @@ >>   #define SIZE 256 >>   #define BURST_SIZE 32 >>   #define NUM_VDEVS 2 >> +#define MONITOR_INTERVEL (500 * 1000) > > I believe it should be INTERVAL Ah, yes, sorry for the typo. > >>     /* true if x is a power of 2 */ >>   #define POWEROF2(x) ((((x)-1) & (x)) == 0) >> @@ -143,12 +147,14 @@ struct parse_val { >>   static struct rte_eth_conf port_conf_default; >>   static volatile uint8_t quit_signal; >>   static uint8_t multiple_core_capture; >> +static uint8_t exit_with_primary; > > Could you please help to confirm that the 'snip' here mean we should delete the 'exit_with_primary' code? > >>   @@ -403,6 +410,9 @@ struct parse_val { >>           case CMD_LINE_OPT_MULTI_NUM: >>               multiple_core_capture = 1; >>               break; >> +        case CMD_LINE_OPT_EXIT_WP_NUM: >> +            exit_with_primary = 1; >> +            break; > > Any particular reason why it is not made the default? It's OK to make it default.  How about Varghese ? Thank you for the review. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 83F29A0679 for ; Mon, 29 Apr 2019 11:44:13 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A59701B13F; Mon, 29 Apr 2019 11:44:11 +0200 (CEST) Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by dpdk.org (Postfix) with ESMTP id 470D21B122 for ; Mon, 29 Apr 2019 11:44:10 +0200 (CEST) Received: from DGGEMS411-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 8B07429D52A4F8AE859C for ; Mon, 29 Apr 2019 17:44:08 +0800 (CST) Received: from [127.0.0.1] (10.177.131.206) by DGGEMS411-HUB.china.huawei.com (10.3.19.211) with Microsoft SMTP Server id 14.3.439.0; Mon, 29 Apr 2019 17:44:04 +0800 To: "Burakov, Anatoly" , CC: References: <1556210141-43153-1-git-send-email-mousuanming@huawei.com> <1556427506-49150-1-git-send-email-mousuanming@huawei.com> From: Suanming.Mou Message-ID: <58514852-12be-5ba5-60df-b7b01b586072@huawei.com> Date: Mon, 29 Apr 2019 17:43:48 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.177.131.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v2] app/pdump: add exit_with_primary option support. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190429094348.GuCU5WyC29NQ_K-QeTNF74oUqAen8StU7SqFQIqHiXY@z> On 2019/4/29 17:14, Burakov, Anatoly wrote: > On 28-Apr-19 5:58 AM, Suanming.Mou wrote: >> When primary app exits, the residual running pdump will stop >> the primary app to restart. Add an exit_with_primary option >> to make pdump exit with primary. >> >> Suggested-by: Varghese, Vipin >> Suggested-by: Burakov, Anatoly >> Signed-off-by: Suanming.Mou >> --- >>   app/pdump/main.c | 26 ++++++++++++++++++++++++++ >>   1 file changed, 26 insertions(+) >> >> diff --git a/app/pdump/main.c b/app/pdump/main.c >> index 3d20854..3909f15 100644 >> --- a/app/pdump/main.c >> +++ b/app/pdump/main.c >> @@ -26,11 +26,14 @@ >>   #include >>   #include >>   #include >> +#include >>     #define CMD_LINE_OPT_PDUMP "pdump" >>   #define CMD_LINE_OPT_PDUMP_NUM 256 >>   #define CMD_LINE_OPT_MULTI "multi" >>   #define CMD_LINE_OPT_MULTI_NUM 257 >> +#define CMD_LINE_OPT_EXIT_WP "exit_with_primary" >> +#define CMD_LINE_OPT_EXIT_WP_NUM 258 > > Unrelated to this patch, but seems very flaky and prone to error. How > about replacing this stuff with enum-based automatic value assignment, > like in lib/librte_eal/common/eal_options.h ? :) > >>   #define PDUMP_PORT_ARG "port" >>   #define PDUMP_PCI_ARG "device_id" >>   #define PDUMP_QUEUE_ARG "queue" >> @@ -65,6 +68,7 @@ >>   #define SIZE 256 >>   #define BURST_SIZE 32 >>   #define NUM_VDEVS 2 >> +#define MONITOR_INTERVEL (500 * 1000) > > I believe it should be INTERVAL Ah, yes, sorry for the typo. > >>     /* true if x is a power of 2 */ >>   #define POWEROF2(x) ((((x)-1) & (x)) == 0) >> @@ -143,12 +147,14 @@ struct parse_val { >>   static struct rte_eth_conf port_conf_default; >>   static volatile uint8_t quit_signal; >>   static uint8_t multiple_core_capture; >> +static uint8_t exit_with_primary; > > Could you please help to confirm that the 'snip' here mean we should delete the 'exit_with_primary' code? > >>   @@ -403,6 +410,9 @@ struct parse_val { >>           case CMD_LINE_OPT_MULTI_NUM: >>               multiple_core_capture = 1; >>               break; >> +        case CMD_LINE_OPT_EXIT_WP_NUM: >> +            exit_with_primary = 1; >> +            break; > > Any particular reason why it is not made the default? It's OK to make it default.  How about Varghese ? Thank you for the review.