[dpdk-dev] [PATCH v5 2/2] app/pdump: enhance to support multi-core capture

[Varghese, Vipin]

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 >= 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 ‘multi’ and ‘pdump’. 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 >= 256, 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.
Snipped
+       printf("usage: %s [EAL options] -- [--%s] "
+                       "--%s "
                        "'(port=<port id> | device_id=<pci id or vdev name>),"
                        "(queue=<queue_id>),"
                        "(rx-dev=<iface or pcap file> |"
@@ -152,7 +157,7 @@ pdump_usage(const char *prgname)
                        "[ring-size=<ring size>default:16384],"
                        "[mbuf-size=<mbuf data size>default:2176],"
                        "[total-num-mbufs=<number of 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