From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-f66.google.com (mail-vs1-f66.google.com [209.85.217.66]) by dpdk.org (Postfix) with ESMTP id 927DB4F91 for ; Thu, 4 Apr 2019 09:39:44 +0200 (CEST) Received: by mail-vs1-f66.google.com with SMTP id g127so779599vsd.6 for ; Thu, 04 Apr 2019 00:39:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GAPdsoiHN87VBVF+iBBEPDVszcIdHDLpV2Dc5uddMDc=; b=QKImzUHQFOwHuDMB/3x9qPNSzDrR8HqBUcg5exazzJAnBJ1rcuNe8d49wLMtnWxgcw BUkPIN3oy4oEJahWz6XgdXT9hJ6lTygMcq6xN4Q6CCrM0J7wOORonR3mEdFTAd9foEOa VhGgscBGq5WdvR94xYBXxYTCT4rk4VZLrEve8J1QA3+hoKqZOP+NjZbGB0wawx7jPu/i WP3cUtjopOLi5r6SOoFIprLBoCS4qY5PPADjaJNGxHutBJzkwkwU2pGJopmtHOZ/hMt/ ubZD5QEwP+NwSShzEiDFQBi1qdvtP+2cdRbzIKm1xIMnLQZY7lhB7catqz+lyLSN+oDp 6M5w== X-Gm-Message-State: APjAAAUk3Jx04gp60zkT5PMSNjQW4QHyAsNPB1LMoXldBysmy09one4Y jGTJphERMwl2lT6lsrGO8MeRkagXJMGACko6hqi+1g== X-Google-Smtp-Source: APXvYqzZnLs9YM/R6YNcfmxEMd/az0n+fr+xAh92bPtZwxSXfSizYFwi2k6MoO+SBgeZJy3qp0wZUG8WIM03Q0n387g= X-Received: by 2002:a67:f588:: with SMTP id i8mr3080851vso.91.1554363583951; Thu, 04 Apr 2019 00:39:43 -0700 (PDT) MIME-Version: 1.0 References: <20190402043318.20382-3-vipin.varghese@intel.com> <20190402091836.35779-1-vipin.varghese@intel.com> <20190402091836.35779-3-vipin.varghese@intel.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D328FFD@BGSMSX101.gar.corp.intel.com> In-Reply-To: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D328FFD@BGSMSX101.gar.corp.intel.com> From: David Marchand Date: Thu, 4 Apr 2019 09:39:32 +0200 Message-ID: To: "Varghese, Vipin" Cc: dev , "Kovacevic, Marko" , "Pattan, Reshma" , "Wiles, Keith" , "Mcnamara, John" , "Byrne, Stephen1" , "Tamboli, Amit S" , "Padubidri, Sanjay A" , "Patel, Amol" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v5 2/2] app/pdump: enhance to support multi-core capture 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: Thu, 04 Apr 2019 07:39:44 -0000 On Tue, Apr 2, 2019 at 5:30 PM Varghese, Vipin wrote: > Hi David, > > > > snipped > #define CMD_LINE_OPT_PDUMP "pdump" > +#define CMD_LINE_OPT_PDUMP_NUM 1 > +#define CMD_LINE_OPT_MULTI "multi" > +#define CMD_LINE_OPT_MULTI_NUM 2 > #define PDUMP_PORT_ARG "port" > #define PDUMP_PCI_ARG "device_id" > #define PDUMP_QUEUE_ARG "queue" > > > > You'd better map to integers that do not collide with ascii characters > (even if non printable). > > So values >=3D 256 are better. > > This is the reason for the comment here: > > https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/eal_options.h#n19 > > > > In case of dpdk-pdump; there are 2 options =E2=80=98multi=E2=80=99 and = =E2=80=98pdump=E2=80=99. But in > case of eal_option there are multiple options which has same fist > character. Hence the comment 'first long only option value must be >=3D 2= 56, > so that we won't conflict with short options' is true. > > > > In my humble opinion, I think it need not change. But please let me know > otherwise. > The convention I had proposed is to just leave the whole [0-255] range to potential short options. This marks a demarcation between long options that map to short options and long "only" options. I don't care, just prefer to have something systematic. Snipped > > + printf("usage: %s [EAL options] -- [--%s] " > + "--%s " > "'(port=3D | device_id=3D name>)," > "(queue=3D)," > "(rx-dev=3D |" > @@ -152,7 +157,7 @@ pdump_usage(const char *prgname) > "[ring-size=3Ddefault:16384]," > "[mbuf-size=3Ddefault:2176]," > "[total-num-mbufs=3D mbufs>default:65535]'\n", > - prgname); > + prgname, CMD_LINE_OPT_MULTI, CMD_LINE_OPT_PDUMP); > } > > static int > > > > You can concatenate the macro. > > > > Thank you for the suggestion, > > > > #define OPT(X) #X > > #define STR_REPLACE "\n[--"OPT(multi)"]\n --"OPT(pdump) > > > prgname, STR_REPLACE); > > > > should we change to this or skip this as we are using this once? > > > > snipped > I don't understand what you propose. My suggestion is simple: + printf("usage: %s [EAL options] -- [--"CMD_LINE_OPT_MULTI"] " + "--"CMD_LINE_OPT_PDUMP" " --=20 David Marchand 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 633C9A0679 for ; Thu, 4 Apr 2019 09:39:46 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BF6714F98; Thu, 4 Apr 2019 09:39:45 +0200 (CEST) Received: from mail-vs1-f66.google.com (mail-vs1-f66.google.com [209.85.217.66]) by dpdk.org (Postfix) with ESMTP id 927DB4F91 for ; Thu, 4 Apr 2019 09:39:44 +0200 (CEST) Received: by mail-vs1-f66.google.com with SMTP id g127so779599vsd.6 for ; Thu, 04 Apr 2019 00:39:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GAPdsoiHN87VBVF+iBBEPDVszcIdHDLpV2Dc5uddMDc=; b=QKImzUHQFOwHuDMB/3x9qPNSzDrR8HqBUcg5exazzJAnBJ1rcuNe8d49wLMtnWxgcw BUkPIN3oy4oEJahWz6XgdXT9hJ6lTygMcq6xN4Q6CCrM0J7wOORonR3mEdFTAd9foEOa VhGgscBGq5WdvR94xYBXxYTCT4rk4VZLrEve8J1QA3+hoKqZOP+NjZbGB0wawx7jPu/i WP3cUtjopOLi5r6SOoFIprLBoCS4qY5PPADjaJNGxHutBJzkwkwU2pGJopmtHOZ/hMt/ ubZD5QEwP+NwSShzEiDFQBi1qdvtP+2cdRbzIKm1xIMnLQZY7lhB7catqz+lyLSN+oDp 6M5w== X-Gm-Message-State: APjAAAUk3Jx04gp60zkT5PMSNjQW4QHyAsNPB1LMoXldBysmy09one4Y jGTJphERMwl2lT6lsrGO8MeRkagXJMGACko6hqi+1g== X-Google-Smtp-Source: APXvYqzZnLs9YM/R6YNcfmxEMd/az0n+fr+xAh92bPtZwxSXfSizYFwi2k6MoO+SBgeZJy3qp0wZUG8WIM03Q0n387g= X-Received: by 2002:a67:f588:: with SMTP id i8mr3080851vso.91.1554363583951; Thu, 04 Apr 2019 00:39:43 -0700 (PDT) MIME-Version: 1.0 References: <20190402043318.20382-3-vipin.varghese@intel.com> <20190402091836.35779-1-vipin.varghese@intel.com> <20190402091836.35779-3-vipin.varghese@intel.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D328FFD@BGSMSX101.gar.corp.intel.com> In-Reply-To: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D328FFD@BGSMSX101.gar.corp.intel.com> From: David Marchand Date: Thu, 4 Apr 2019 09:39:32 +0200 Message-ID: To: "Varghese, Vipin" Cc: dev , "Kovacevic, Marko" , "Pattan, Reshma" , "Wiles, Keith" , "Mcnamara, John" , "Byrne, Stephen1" , "Tamboli, Amit S" , "Padubidri, Sanjay A" , "Patel, Amol" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v5 2/2] app/pdump: enhance to support multi-core capture 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: <20190404073932.Sg1cliYmYeut0J_lpRc-TtdglN2YNGtXWdVh0E8RKMI@z> On Tue, Apr 2, 2019 at 5:30 PM Varghese, Vipin wrote: > Hi David, > > > > snipped > #define CMD_LINE_OPT_PDUMP "pdump" > +#define CMD_LINE_OPT_PDUMP_NUM 1 > +#define CMD_LINE_OPT_MULTI "multi" > +#define CMD_LINE_OPT_MULTI_NUM 2 > #define PDUMP_PORT_ARG "port" > #define PDUMP_PCI_ARG "device_id" > #define PDUMP_QUEUE_ARG "queue" > > > > You'd better map to integers that do not collide with ascii characters > (even if non printable). > > So values >=3D 256 are better. > > This is the reason for the comment here: > > https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/eal_options.h#n19 > > > > In case of dpdk-pdump; there are 2 options =E2=80=98multi=E2=80=99 and = =E2=80=98pdump=E2=80=99. But in > case of eal_option there are multiple options which has same fist > character. Hence the comment 'first long only option value must be >=3D 2= 56, > so that we won't conflict with short options' is true. > > > > In my humble opinion, I think it need not change. But please let me know > otherwise. > The convention I had proposed is to just leave the whole [0-255] range to potential short options. This marks a demarcation between long options that map to short options and long "only" options. I don't care, just prefer to have something systematic. Snipped > > + printf("usage: %s [EAL options] -- [--%s] " > + "--%s " > "'(port=3D | device_id=3D name>)," > "(queue=3D)," > "(rx-dev=3D |" > @@ -152,7 +157,7 @@ pdump_usage(const char *prgname) > "[ring-size=3Ddefault:16384]," > "[mbuf-size=3Ddefault:2176]," > "[total-num-mbufs=3D mbufs>default:65535]'\n", > - prgname); > + prgname, CMD_LINE_OPT_MULTI, CMD_LINE_OPT_PDUMP); > } > > static int > > > > You can concatenate the macro. > > > > Thank you for the suggestion, > > > > #define OPT(X) #X > > #define STR_REPLACE "\n[--"OPT(multi)"]\n --"OPT(pdump) > > > prgname, STR_REPLACE); > > > > should we change to this or skip this as we are using this once? > > > > snipped > I don't understand what you propose. My suggestion is simple: + printf("usage: %s [EAL options] -- [--"CMD_LINE_OPT_MULTI"] " + "--"CMD_LINE_OPT_PDUMP" " --=20 David Marchand