[PATCH 1/2] app/pdump: remove hard-coded memory channels EAL arg

[PATCH 1/2] app/pdump: remove hard-coded memory channels EAL arg

[Bruce Richardson]

The pdump main function modifies argc/argv before calling EAL init
function. It needs to do so in order to add the proc-type=secondary
flag, but also adds in "-n4" as parameter. This parameter is unnecessary
and also causes duplicate parameter errors if the user also passes in -n
flag on the commandline. Just remove this flag.

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/pdump/main.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index fa85859703..f3ac81cb80 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -982,9 +982,8 @@ main(int argc, char **argv)
 	int ret;
 	int i;
 
-	char n_flag[] = "-n4";
 	char mp_flag[] = "--proc-type=secondary";
-	char *argp[argc + 2];
+	char *argp[argc + 1];
 
 	/* catch ctrl-c so we can cleanup on exit */
 	sigemptyset(&action.sa_mask);
@@ -996,13 +995,12 @@ main(int argc, char **argv)
 		sigaction(SIGHUP, &action, NULL);
 
 	argp[0] = argv[0];
-	argp[1] = n_flag;
-	argp[2] = mp_flag;
+	argp[1] = mp_flag;
 
 	for (i = 1; i < argc; i++)
-		argp[i + 2] = argv[i];
+		argp[i + 1] = argv[i];
 
-	argc += 2;
+	argc += 1;
 
 	diag = rte_eal_init(argc, argp);
 	if (diag < 0)
@@ -1012,7 +1010,7 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
 
 	argc -= diag;
-	argv += (diag - 2);
+	argv += (diag - 1);
 
 	/* parse app arguments */
 	if (argc > 1) {
-- 
2.48.1

[PATCH 2/2] app/pdump: improve cmdline argument handling

[Bruce Richardson]

Rework the argc/argv handling in pdump a little to improve things.
Issues with the original implementation:
* assumed user would never pass proc-type parameter on the commandline
* did not null-terminate the argv array (generally harmless but not in
  spec)
* did not handle case where arg processing in eal_init would reorder
  non-flag args to the end to be handled by the app.

Fix these - all-be-it minor issues, by having a separate count value for
the number of arguments we put in the argp array, rather than assuming
that its argc + 1 (for proc-type flag). Properly set the last argv entry
to NULL, and when processing app args, reuse the argp array passed to
eal_init rather than reverting back to the original argv array.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/pdump/main.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f3ac81cb80..89347d70dc 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -983,7 +983,8 @@ main(int argc, char **argv)
 	int i;
 
 	char mp_flag[] = "--proc-type=secondary";
-	char *argp[argc + 1];
+	char *argp[argc + 2];  /* add proc-type, and final NULL entry */
+	int n_argp = 0;
 
 	/* catch ctrl-c so we can cleanup on exit */
 	sigemptyset(&action.sa_mask);
@@ -994,27 +995,27 @@ main(int argc, char **argv)
 	if (origaction.sa_handler == SIG_DFL)
 		sigaction(SIGHUP, &action, NULL);
 
-	argp[0] = argv[0];
-	argp[1] = mp_flag;
+	argp[n_argp++] = argv[0];
+	argp[n_argp++] = mp_flag;
 
-	for (i = 1; i < argc; i++)
-		argp[i + 1] = argv[i];
-
-	argc += 1;
+	for (i = 1; i < argc; i++) {
+		argp[n_argp] = argv[i];
+		/* drop any user-provided proc-type to avoid dup flags */
+		if (strncmp(argv[i], mp_flag, strlen("--proc-type")) != 0)
+			n_argp++;
+	}
+	argp[n_argp] = NULL;
 
-	diag = rte_eal_init(argc, argp);
+	diag = rte_eal_init(n_argp, argp);
 	if (diag < 0)
 		rte_panic("Cannot init EAL\n");
 
 	if (rte_eth_dev_count_avail() == 0)
 		rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
 
-	argc -= diag;
-	argv += (diag - 1);
-
 	/* parse app arguments */
-	if (argc > 1) {
-		ret = launch_args_parse(argc, argv, argp[0]);
+	if (n_argp - diag > 1) {
+		ret = launch_args_parse(n_argp - diag, argp + diag, argp[0]);
 		if (ret < 0)
 			rte_exit(EXIT_FAILURE, "Invalid argument\n");
 	}
-- 
2.48.1

Re: [PATCH 1/2] app/pdump: remove hard-coded memory channels EAL arg

[Burakov, Anatoly]

On 10/17/2025 1:06 PM, Bruce Richardson wrote:
> The pdump main function modifies argc/argv before calling EAL init
> function. It needs to do so in order to add the proc-type=secondary
> flag, but also adds in "-n4" as parameter. This parameter is unnecessary
> and also causes duplicate parameter errors if the user also passes in -n
> flag on the commandline. Just remove this flag.
> 
> Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [PATCH 2/2] app/pdump: improve cmdline argument handling

[Burakov, Anatoly]

On 10/17/2025 1:06 PM, Bruce Richardson wrote:
> Rework the argc/argv handling in pdump a little to improve things.
> Issues with the original implementation:
> * assumed user would never pass proc-type parameter on the commandline
> * did not null-terminate the argv array (generally harmless but not in
>    spec)
> * did not handle case where arg processing in eal_init would reorder
>    non-flag args to the end to be handled by the app.
> 
> Fix these - all-be-it minor issues, by having a separate count value for
> the number of arguments we put in the argp array, rather than assuming
> that its argc + 1 (for proc-type flag). Properly set the last argv entry
> to NULL, and when processing app args, reuse the argp array passed to
> eal_init rather than reverting back to the original argv array.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

<snip>

>   
> -	argp[0] = argv[0];
> -	argp[1] = mp_flag;
> +	argp[n_argp++] = argv[0];
> +	argp[n_argp++] = mp_flag;
>   
> -	for (i = 1; i < argc; i++)
> -		argp[i + 1] = argv[i];
> -
> -	argc += 1;
> +	for (i = 1; i < argc; i++) {
> +		argp[n_argp] = argv[i];
> +		/* drop any user-provided proc-type to avoid dup flags */
> +		if (strncmp(argv[i], mp_flag, strlen("--proc-type")) != 0)
> +			n_argp++;

Feels a little odd to assign the value and then overwrite it in next 
iteration, but same end result so no biggie

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>


-- 
Thanks,
Anatoly