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 8437FA0032; Fri, 16 Sep 2022 14:52:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1D7624021D; Fri, 16 Sep 2022 14:52:13 +0200 (CEST) Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com [209.85.219.174]) by mails.dpdk.org (Postfix) with ESMTP id CBC7D40156 for ; Fri, 16 Sep 2022 14:52:11 +0200 (CEST) Received: by mail-yb1-f174.google.com with SMTP id g5so32369566ybg.11 for ; Fri, 16 Sep 2022 05:52:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=ZouNsTOAdUj/UBPiBu4rvSbK4iF2W6Y/wzYJ6B4t9og=; b=GDWVJjdHykPObesWfO09ALcJZlJ4KXeTFhs7qtvlCrOLr4k9s/yF8yzNW4V0QRaHvJ D9VZsY9axoHHVux4vZ2EEv15zeea0pxyOYq0qLK17jVxReF5oOSqvglgMfqN6lyiciW1 M9MiBnGJqedBOeo9lgcNcJBohTgd2H5HC9J4r0spd1IyDOpwSIk/pW1CNgHjCyU/GK5g 0JUMWUoOkWZZsg2JYaCx6uvoQhN9yjmBtuTjUGtkMK3fumgcrhUCfjwHWujAvOUzJjzx /stMqc8V3DkiGaqO7zH/MBnGP9jplGcXVzvcmpv1LIfor3wTAOFbxuXXuEebF6b421ee 3MDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=ZouNsTOAdUj/UBPiBu4rvSbK4iF2W6Y/wzYJ6B4t9og=; b=qJWcXxa+fZ/JsoChSWD2HiOLdJTliVbYmsQXjFGoEq7OyJ/Arh42GQpL/77Y81Q1WB fhpop1wAyvSG6lc3MT6P9/THpmgRxxMKtXlLKZf3fg7J4qYIPdYCTGBOtdfnQEAVGRJm yQHPAHJgrxvtJGEupzlTfs3HaF0otTLxahJbdjW4KeuL86DJOIrrZfcFJssNSHo+NWqv D2loCQk7faJuekjS6yaqxI4dWb4DgTE775yv3hYvnp6WeRIk9/qTo46vb/LelLy8JbzP iDzKZu+y91j/oWm7yEp/FZ8jv0rXAjgrMdW7UAHrDFMjcwN3rZ4FZaTcn28DnvIMetcJ 1kOg== X-Gm-Message-State: ACrzQf2eIn0EVv6uYLFZesaxRqXB/lk2u5iLSe/iknqSYobJaRSin3gk qpA3AYqYR8AUUQmdZyj3OsCdQh9tcfRIdMM3FWk= X-Google-Smtp-Source: AMsMyM5pefBfR2xomb2NDe01D6hnz5wFvTfUbVLrfByTJ8qQZWMzq6LIksi2QpQXltP1EptXoK94DfhaXsW9GRRN9Ws= X-Received: by 2002:a25:94b:0:b0:68f:4e05:e8f0 with SMTP id u11-20020a25094b000000b0068f4e05e8f0mr4207502ybm.115.1663332730930; Fri, 16 Sep 2022 05:52:10 -0700 (PDT) MIME-Version: 1.0 References: <20220912124309.233363-1-arshdeep.kaur@intel.com> <20220912190330.73159-1-stephen@networkplumber.org> In-Reply-To: From: Ben Magistro Date: Fri, 16 Sep 2022 08:51:59 -0400 Message-ID: Subject: Re: [RFT] dumpcap: add file-prefix option To: "Kaur, Arshdeep" Cc: Stephen Hemminger , "dev@dpdk.org" Content-Type: multipart/alternative; boundary="00000000000092097a05e8cad429" 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 --00000000000092097a05e8cad429 Content-Type: text/plain; charset="UTF-8" Kaur, I believe parse_opts() should be called before dpdk_init() now see https://patches.dpdk.org/project/dpdk/patch/20220125032545.7704-1-koncept1@gmail.com/ On Fri, Sep 16, 2022 at 4:19 AM Kaur, Arshdeep wrote: > > > > -----Original Message----- > > From: Stephen Hemminger > > Sent: Tuesday, September 13, 2022 12:34 AM > > To: dev@dpdk.org > > Cc: Stephen Hemminger ; Kaur, Arshdeep > > > > Subject: [RFT] dumpcap: add file-prefix option > > > > When using dumpcap in container environment or with multiple DPDK > > processes, it is useful to be able to specify file prefix. > > > > This version only accepts the long format option used by other commands. > > If no prefix is specified then the default is used. > > > > Suggested-by: Arshdeep Kaur > > Signed-off-by: Stephen Hemminger > > --- > > Did basic command line test, but still needs testing with a prefix being > used > > (ie multiple apps). > > > > app/dumpcap/main.c | 24 ++++++++++++++++++------ > > 1 file changed, 18 insertions(+), 6 deletions(-) > > > > diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c index > > a6041d4ff495..bdeef96d9c0b 100644 > > --- a/app/dumpcap/main.c > > +++ b/app/dumpcap/main.c > > @@ -61,6 +61,7 @@ static char *output_name; static const char > > *filter_str; static unsigned int ring_size = 2048; static const char > > *capture_comment; > > +static const char *file_prefix; > > static uint32_t snaplen = RTE_MBUF_DEFAULT_BUF_SIZE; static bool > > dump_bpf; static struct { @@ -122,6 +123,7 @@ static void usage(void) > > " add a capture comment to the > output file\n" > > "\n" > > "Miscellaneous:\n" > > + " --file-prefix= prefix to use for > multi-process\n" > > " -q don't report packet capture > counts\n" > > " -v, --version print version information and > exit\n" > > " -h, --help display this help and exit\n" > > @@ -310,6 +312,7 @@ static void parse_opts(int argc, char **argv) > > static const struct option long_options[] = { > > { "autostop", required_argument, NULL, 'a' }, > > { "capture-comment", required_argument, NULL, 0 }, > > + { "file-prefix", required_argument, NULL, 0 }, > > { "help", no_argument, NULL, 'h' }, > > { "interface", required_argument, NULL, 'i' }, > > { "list-interfaces", no_argument, NULL, 'D' }, > > @@ -330,11 +333,13 @@ static void parse_opts(int argc, char **argv) > > > > switch (c) { > > case 0: > > - switch (option_index) { > > - case 0: > > + if (!strcmp(long_options[option_index].name, > > + "capture-comment")) { > > capture_comment = optarg; > > - break; > > - default: > > + } else if (!strcmp(long_options[option_index].name, > > + "file-prefix")) { > > + file_prefix = optarg; > > + } else { > > usage(); > > exit(1); > > } > > parse_opts() is called after dpdk_init(). So whatever file-prefix we > provide, for eal init, it remains NULL. > Please let me know your thoughts about it. > > > @@ -512,12 +517,14 @@ static void dpdk_init(void) > > static const char * const args[] = { > > "dumpcap", "--proc-type", "secondary", > > "--log-level", "notice" > > - > > }; > > - const int eal_argc = RTE_DIM(args); > > + int eal_argc = RTE_DIM(args); > > char **eal_argv; > > unsigned int i; > > > > + if (file_prefix != NULL) > > + eal_argc += 2; > > + > > /* DPDK API requires mutable versions of command line arguments. > > */ > > eal_argv = calloc(eal_argc + 1, sizeof(char *)); > > if (eal_argv == NULL) > > @@ -527,6 +534,11 @@ static void dpdk_init(void) > > for (i = 1; i < RTE_DIM(args); i++) > > eal_argv[i] = strdup(args[i]); > > > > + if (file_prefix != NULL) { > > + eal_argv[i++] = strdup("--file-prefix"); > > + eal_argv[i++] = strdup(file_prefix); > > + } > > + > > if (rte_eal_init(eal_argc, eal_argv) < 0) > > rte_exit(EXIT_FAILURE, "EAL init failed: is primary process > > running?\n"); > > > > -- > > 2.35.1 > > --00000000000092097a05e8cad429 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Kaur,

I believe parse_opts() should be = called before dpdk_init() now see=C2=A0https://patch= es.dpdk.org/project/dpdk/patch/20220125032545.7704-1-koncept1@gmail.com/

On Fri, Sep 16, 2022 at 4:19 AM Kaur, Arshdeep <arshdeep.kaur@intel.com> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, September 13, 2022 12:34 AM
> To: dev@dpdk.org=
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Kaur, Arshdeep
> <arshd= eep.kaur@intel.com>
> Subject: [RFT] dumpcap: add file-prefix option
>
> When using dumpcap in container environment or with multiple DPDK
> processes, it is useful to be able to specify file prefix.
>
> This version only accepts the long format option used by other command= s.
> If no prefix is specified then the default is used.
>
> Suggested-by: Arshdeep Kaur <arshdeep.kaur@intel.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> Did basic command line test, but still needs testing with a prefix bei= ng used
> (ie multiple apps).
>
>=C2=A0 app/dumpcap/main.c | 24 ++++++++++++++++++------
>=C2=A0 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c index
> a6041d4ff495..bdeef96d9c0b 100644
> --- a/app/dumpcap/main.c
> +++ b/app/dumpcap/main.c
> @@ -61,6 +61,7 @@ static char *output_name;=C2=A0 static const char > *filter_str;=C2=A0 static unsigned int ring_size =3D 2048;=C2=A0 stati= c const char
> *capture_comment;
> +static const char *file_prefix;
>=C2=A0 static uint32_t snaplen =3D RTE_MBUF_DEFAULT_BUF_SIZE;=C2=A0 sta= tic bool
> dump_bpf;=C2=A0 static struct { @@ -122,6 +123,7 @@ static void usage(= void)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0add a capture comment to the output file\n"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "\n"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "Miscellaneous:\n= "
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "=C2=A0 --file-prefix= =3D<prefix>=C2=A0 =C2=A0prefix to use for multi-process\n"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "=C2=A0 -q=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0don't report packet capture counts\n"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "=C2=A0 -v, --ver= sion=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 print version information and= exit\n"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "=C2=A0 -h, --hel= p=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0display this help a= nd exit\n"
> @@ -310,6 +312,7 @@ static void parse_opts(int argc, char **argv)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0static const struct option long_options[] = =3D {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ "autostop= ",=C2=A0 =C2=A0 =C2=A0 =C2=A0 required_argument, NULL, 'a' },<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ "capture-= comment", required_argument, NULL, 0 },
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ "file-prefix&q= uot;,=C2=A0 =C2=A0 =C2=A0required_argument, NULL, 0 },
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ "help&quo= t;,=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 no_argument,=C2=A0 =C2=A0 =C2= =A0 =C2=A0NULL, 'h' },
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ "interfac= e",=C2=A0 =C2=A0 =C2=A0 =C2=A0required_argument, NULL, 'i' },<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ "list-int= erfaces", no_argument,=C2=A0 =C2=A0 =C2=A0 =C2=A0NULL, 'D' },<= br> > @@ -330,11 +333,13 @@ static void parse_opts(int argc, char **argv) >
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0switch (c) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case 0:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0switch (option_index) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0case 0:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (!strcmp(long_options[option_index].name,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"capture-comment"= ;)) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0capture_comment =3D optarg;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0} else if (!strcmp(long_options[option_index].name,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &quo= t;file-prefix")) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0file_prefix =3D optarg;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0} else {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0usage();
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0exit(1);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0}

parse_opts() is called after dpdk_init(). So whatever file-prefix we provid= e, for eal init, it remains NULL.
Please let me know your thoughts about it.

> @@ -512,12 +517,14 @@ static void dpdk_init(void)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0static const char * const args[] =3D {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"dumpcap&qu= ot;, "--proc-type", "secondary",
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"--log-leve= l", "notice"
> -
>=C2=A0 =C2=A0 =C2=A0 =C2=A0};
> -=C2=A0 =C2=A0 =C2=A0const int eal_argc =3D RTE_DIM(args);
> +=C2=A0 =C2=A0 =C2=A0int eal_argc =3D RTE_DIM(args);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0char **eal_argv;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int i;
>
> +=C2=A0 =C2=A0 =C2=A0if (file_prefix !=3D NULL)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0eal_argc +=3D 2;
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/* DPDK API requires mutable versions of com= mand line arguments.
> */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0eal_argv =3D calloc(eal_argc + 1, sizeof(cha= r *));
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (eal_argv =3D=3D NULL)
> @@ -527,6 +534,11 @@ static void dpdk_init(void)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0for (i =3D 1; i < RTE_DIM(args); i++)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0eal_argv[i] =3D = strdup(args[i]);
>
> +=C2=A0 =C2=A0 =C2=A0if (file_prefix !=3D NULL) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0eal_argv[i++] =3D str= dup("--file-prefix");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0eal_argv[i++] =3D str= dup(file_prefix);
> +=C2=A0 =C2=A0 =C2=A0}
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (rte_eal_init(eal_argc, eal_argv) < 0)=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rte_exit(EXIT_FA= ILURE, "EAL init failed: is primary process
> running?\n");
>
> --
> 2.35.1

--00000000000092097a05e8cad429--