diff mbox

[dpdk-dev] eal: add missing long-options for short option arguments

Message ID 1456427356-67147-1-git-send-email-keith.wiles@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Wiles, Keith Feb. 25, 2016, 7:09 p.m. UTC 
  A number of short options for EAL are missing long options
and this patch adds those missing options.

The missing long options are for:
-c add --coremask
-d add --driver
-l add --corelist
-m add --memsize
-n add --mem-channels
-r add --mem-ranks
-v add --version
Add an alias for --lcores using --lcore-map

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 doc/guides/testpmd_app_ug/run_app.rst      | 16 +++++++--------
 lib/librte_eal/common/eal_common_options.c | 31 ++++++++++++++++++++----------
 lib/librte_eal/common/eal_options.h        | 16 +++++++++++++++
 3 files changed, 45 insertions(+), 18 deletions(-)

Comments

Bruce Richardson Feb. 25, 2016, 9:32 p.m. UTC | #1 
On Thu, Feb 25, 2016 at 01:09:16PM -0600, Keith Wiles wrote:
> A number of short options for EAL are missing long options
> and this patch adds those missing options.
> 
> The missing long options are for:
> -c add --coremask
> -d add --driver
> -l add --corelist
> -m add --memsize
> -n add --mem-channels
> -r add --mem-ranks
> -v add --version
> Add an alias for --lcores using --lcore-map
> 
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>

Why do we need long options for all the short options?

/Bruce
Wiles, Keith Feb. 25, 2016, 10:12 p.m. UTC | #2 
>On Thu, Feb 25, 2016 at 01:09:16PM -0600, Keith Wiles wrote:

>> A number of short options for EAL are missing long options

>> and this patch adds those missing options.

>> 

>> The missing long options are for:

>> -c add --coremask

>> -d add --driver

>> -l add --corelist

>> -m add --memsize

>> -n add --mem-channels

>> -r add --mem-ranks

>> -v add --version

>> Add an alias for --lcores using --lcore-map

>> 

>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>

>

>Why do we need long options for all the short options?


I think we need the long options to match the short options just because it makes sense to me to have long options for all short options. Take the case of -v, just about everyone else has a —version long-option, but we do not.

The real reason is to allow for DPDK configuration via a configuration file and I wanted to use the same strings for the config file variables as the command line options. I figured I would add the long options now as they do not effect the configuration file patch.
>

>/Bruce

>

>



Regards,
Keith
Wiles, Keith March 3, 2016, 2:52 p.m. UTC | #3 
>>On Thu, Feb 25, 2016 at 01:09:16PM -0600, Keith Wiles wrote:

>>> A number of short options for EAL are missing long options

>>> and this patch adds those missing options.

>>> 

>>> The missing long options are for:

>>> -c add --coremask

>>> -d add --driver

>>> -l add --corelist

>>> -m add --memsize

>>> -n add --mem-channels

>>> -r add --mem-ranks

>>> -v add --version

>>> Add an alias for --lcores using --lcore-map

>>> 

>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>

>>

>>Why do we need long options for all the short options?

>

>I think we need the long options to match the short options just because it makes sense to me to have long options for all short options. Take the case of -v, just about everyone else has a —version long-option, but we do not.

>

>The real reason is to allow for DPDK configuration via a configuration file and I wanted to use the same strings for the config file variables as the command line options. I figured I would add the long options now as they do not effect the configuration file patch.


Ping. I really want to have long options for the short option to allow me to use those same options for the config file support I would like to use for DPDK. A config file support is much more reasonable for live or production systems IMHO. Plus it could be very nice for the examples to have a config file on how that example could be configured.

I can create the config file support without the long option names for the short ones, but it would be a lot cleaner to have the same names for config file and command line.

Thanks
++Keith

>>

>>/Bruce

>>

>>

>

>

>Regards,

>Keith

>

>

>

>

>



Regards,
Keith
David Marchand March 3, 2016, 2:55 p.m. UTC | #4 
On Thu, Feb 25, 2016 at 11:12 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>On Thu, Feb 25, 2016 at 01:09:16PM -0600, Keith Wiles wrote:
>>> A number of short options for EAL are missing long options
>>> and this patch adds those missing options.
>>>
>>> The missing long options are for:
>>> -c add --coremask
>>> -d add --driver
>>> -l add --corelist
>>> -m add --memsize
>>> -n add --mem-channels
>>> -r add --mem-ranks
>>> -v add --version
>>> Add an alias for --lcores using --lcore-map
>>>
>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>
>>Why do we need long options for all the short options?
>
> I think we need the long options to match the short options just because it makes sense to me to have long options for all short options. Take the case of -v, just about everyone else has a —version long-option, but we do not.
>
> The real reason is to allow for DPDK configuration via a configuration file and I wanted to use the same strings for the config file variables as the command line options. I figured I would add the long options now as they do not effect the configuration file patch.

No strong opinion on this.

Just, why "memsize" with no -  but "mem-channels" ?
And why cut down to mem rather than memory ?
Wiles, Keith March 3, 2016, 3:02 p.m. UTC | #5 
>On Thu, Feb 25, 2016 at 11:12 PM, Wiles, Keith <keith.wiles@intel.com> wrote:

>>>On Thu, Feb 25, 2016 at 01:09:16PM -0600, Keith Wiles wrote:

>>>> A number of short options for EAL are missing long options

>>>> and this patch adds those missing options.

>>>>

>>>> The missing long options are for:

>>>> -c add --coremask

>>>> -d add --driver

>>>> -l add --corelist

>>>> -m add --memsize

>>>> -n add --mem-channels

>>>> -r add --mem-ranks

>>>> -v add --version

>>>> Add an alias for --lcores using --lcore-map

>>>>

>>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>

>>>

>>>Why do we need long options for all the short options?

>>

>> I think we need the long options to match the short options just because it makes sense to me to have long options for all short options. Take the case of -v, just about everyone else has a —version long-option, but we do not.

>>

>> The real reason is to allow for DPDK configuration via a configuration file and I wanted to use the same strings for the config file variables as the command line options. I figured I would add the long options now as they do not effect the configuration file patch.

>

>No strong opinion on this.

>

>Just, why "memsize" with no -  but "mem-channels" ?

>And why cut down to mem rather than memory ?


I debated on mem-size, but I noticed in a couple places some used memsize. I can change them to any thing someone wants. If you want memory-channels and memory-ranks I am good with that too.

>

>

>-- 

>David Marchand

>



Regards,
Keith
David Marchand March 18, 2016, 10:50 a.m. UTC | #6 
On Thu, Mar 3, 2016 at 4:02 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>On Thu, Feb 25, 2016 at 11:12 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>>>On Thu, Feb 25, 2016 at 01:09:16PM -0600, Keith Wiles wrote:
>>>>> A number of short options for EAL are missing long options
>>>>> and this patch adds those missing options.
>>>>>
>>>>> The missing long options are for:
>>>>> -c add --coremask
>>>>> -d add --driver
>>>>> -l add --corelist
>>>>> -m add --memsize
>>>>> -n add --mem-channels
>>>>> -r add --mem-ranks
>>>>> -v add --version
>>>>> Add an alias for --lcores using --lcore-map
[snip]
>>No strong opinion on this.
>>
>>Just, why "memsize" with no -  but "mem-channels" ?
>>And why cut down to mem rather than memory ?
>
> I debated on mem-size, but I noticed in a couple places some used memsize. I can change them to any thing someone wants. If you want memory-channels and memory-ranks I am good with that too.

Ok, since I can see no real argument against, let's go with explicit options :
--memory-size
--memory-channels
--memory-ranks

The best would be then to align all other existing long options (but
keeping compat with existing ones).

Deal ?
diff mbox

Patch

diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index f605564..753a013 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -38,18 +38,18 @@  The following are the EAL command-line options that can be used in conjunction w
 or any other DPDK application.
 See the DPDK Getting Started Guides for more information on these options.
 
-*   ``-c COREMASK``
+*   ``-c, --coremask COREMASK``
 
     Set the hexadecimal bitmask of the cores to run on.
 
-*   ``-l CORELIST``
+*   ``-l, --corelist CORELIST``
 
     List of cores to run on
 
     The argument format is ``<c1>[-c2][,c3[-c4],...]``
     where ``c1``, ``c2``, etc are core indexes between 0 and 128.
 
-*   ``--lcores COREMAP``
+*   ``--lcores COREMAP or --lcore-map COREMAP``
 
     Map lcore set to physical cpu set
 
@@ -66,7 +66,7 @@  See the DPDK Getting Started Guides for more information on these options.
 
     Core ID that is used as master.
 
-*   ``-n NUM``
+*   ``-n, --mem-channels NUM``
 
     Set the number of memory channels to use.
 
@@ -74,7 +74,7 @@  See the DPDK Getting Started Guides for more information on these options.
 
     Blacklist a PCI devise to prevent EAL from using it. Multiple -b options are allowed.
 
-*   ``-d LIB.so``
+*   ``-d, --driver LIB.so|DIR``
 
     Load an external driver. Multiple -d options are allowed.
 
@@ -82,15 +82,15 @@  See the DPDK Getting Started Guides for more information on these options.
 
     Add a PCI device in white list.
 
-*   ``-m MB``
+*   ``-m, --memsize MB``
 
     Memory to allocate. See also ``--socket-mem``.
 
-*   ``-r NUM``
+*   ``-r, --mem-ranks NUM``
 
     Set the number of memory ranks (auto-detected by default).
 
-*   ``-v``
+*   ``-v, --version``
 
     Display the version information on startup.
 
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 29942ea..cf9801d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -95,6 +95,14 @@  eal_long_options[] = {
 	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
 	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
 	{OPT_XEN_DOM0,          0, NULL, OPT_XEN_DOM0_NUM         },
+	{OPT_COREMASK,          1, NULL, OPT_COREMASK_NUM         },
+	{OPT_DRIVER,            1, NULL, OPT_DRIVER_NUM           },
+	{OPT_CORELIST,          1, NULL, OPT_CORELIST_NUM         },
+	{OPT_MEM_SIZE,          1, NULL, OPT_MEM_SIZE_NUM         },
+	{OPT_MEM_CHANNELS,      1, NULL, OPT_MEM_CHANNELS_NUM     },
+	{OPT_MEM_RANKS,         1, NULL, OPT_MEM_RANKS_NUM        },
+	{OPT_VERSION,           0, NULL, OPT_VERSION_NUM          },
+	{OPT_LCORE_MAP,         1, NULL, OPT_LCORE_MAP_NUM        },
 	{0,                     0, NULL, 0                        }
 };
 
@@ -889,6 +897,7 @@  eal_parse_common_option(int opt, const char *optarg,
 		conf->log_level = log;
 		break;
 	}
+	case OPT_LCORE_MAP_NUM:
 	case OPT_LCORES_NUM:
 		if (eal_parse_lcores(optarg) < 0) {
 			RTE_LOG(ERR, EAL, "invalid parameter for --"
@@ -978,11 +987,13 @@  eal_common_usage(void)
 {
 	printf("[options]\n\n"
 	       "EAL common options:\n"
-	       "  -c COREMASK         Hexadecimal bitmask of cores to run on\n"
-	       "  -l CORELIST         List of cores to run on\n"
+	       "  -c, --"OPT_COREMASK"      Hexadecimal bitmask of cores to run on\n"
+	       "  -l, --"OPT_CORELIST"      List of cores to run on\n"
 	       "                      The argument format is <c1>[-c2][,c3[-c4],...]\n"
 	       "                      where c1, c2, etc are core indexes between 0 and %d\n"
-	       "  --"OPT_LCORES" COREMAP    Map lcore set to physical cpu set\n"
+	       "                        (ex: 1-3,7,9-10) skipping 4,5,6 and 8 cores.\n"
+	       "  --"OPT_LCORES" COREMAP\n"
+	       "  --"OPT_LCORE_MAP" COREMAP Map lcore set to physical cpu set\n"
 	       "                      The argument format is\n"
 	       "                            '<lcores[@cpus]>[<,lcores[@cpus]>...]'\n"
 	       "                      lcores and cpus list are grouped by '(' and ')'\n"
@@ -991,9 +1002,9 @@  eal_common_usage(void)
 	       "                      '( )' can be omitted for single element group,\n"
 	       "                      '@' can be omitted if cpus and lcores have the same value\n"
 	       "  --"OPT_MASTER_LCORE" ID   Core ID that is used as master\n"
-	       "  -n CHANNELS         Number of memory channels\n"
-	       "  -m MB               Memory to allocate (see also --"OPT_SOCKET_MEM")\n"
-	       "  -r RANKS            Force number of memory ranks (don't detect)\n"
+	       "  -n, --"OPT_MEM_CHANNELS"  Number of memory channels\n"
+	       "  -m, --"OPT_MEM_SIZE"       Memory to allocate (MB) (see also --"OPT_SOCKET_MEM")\n"
+	       "  -r, --"OPT_MEM_RANKS"     Force number of memory ranks (don't detect)\n"
 	       "  -b, --"OPT_PCI_BLACKLIST" Add a PCI device in black list.\n"
 	       "                      Prevent EAL from using this PCI device. The argument\n"
 	       "                      format is <domain:bus:devid.func>.\n"
@@ -1005,14 +1016,14 @@  eal_common_usage(void)
 	       "  --"OPT_VDEV"              Add a virtual device.\n"
 	       "                      The argument format is <driver><id>[,key=val,...]\n"
 	       "                      (ex: --vdev=eth_pcap0,iface=eth2).\n"
-	       "  -d LIB.so|DIR       Add a driver or driver directory\n"
+	       "  -d, --"OPT_DRIVER" LIB.so|DIR Add a driver or driver directory\n"
 	       "                      (can be used multiple times)\n"
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
-	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
-	       "  -v                  Display version information on startup\n"
-	       "  -h, --help          This help\n"
+	       "  --"OPT_LOG_LEVEL"         Set default log level 0(no output) to 9(verbose)\n"
+	       "  -v, --"OPT_VERSION"       Display version information on startup\n"
+	       "  -h, --"OPT_HELP"          This help\n"
 	       "\nEAL options for DEBUG use only:\n"
 	       "  --"OPT_HUGE_UNLINK"       Unlink hugepage files after init\n"
 	       "  --"OPT_NO_HUGE"           Use malloc instead of hugetlbfs\n"
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index a881c62..ee4a0eb 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -41,6 +41,20 @@  enum {
 	OPT_PCI_BLACKLIST_NUM   = 'b',
 #define OPT_PCI_WHITELIST     "pci-whitelist"
 	OPT_PCI_WHITELIST_NUM   = 'w',
+#define OPT_COREMASK          "coremask"
+	OPT_COREMASK_NUM        = 'c',
+#define OPT_DRIVER            "driver"
+	OPT_DRIVER_NUM          = 'd',
+#define OPT_CORELIST          "corelist"
+	OPT_CORELIST_NUM        = 'l',
+#define OPT_MEM_SIZE          "memsize"
+	OPT_MEM_SIZE_NUM        = 'm',
+#define OPT_MEM_CHANNELS      "mem-channels"
+	OPT_MEM_CHANNELS_NUM    = 'n',
+#define OPT_MEM_RANKS         "mem-ranks"
+	OPT_MEM_RANKS_NUM       = 'r',
+#define OPT_VERSION           "version"
+	OPT_VERSION_NUM         = 'v',
 
 	/* first long only option value must be >= 256, so that we won't
 	 * conflict with short options */
@@ -57,6 +71,8 @@  enum {
 	OPT_HUGE_UNLINK_NUM,
 #define OPT_LCORES            "lcores"
 	OPT_LCORES_NUM,
+#define OPT_LCORE_MAP         "lcore-map"	/* Alias for --lcores */
+	OPT_LCORE_MAP_NUM,
 #define OPT_LOG_LEVEL         "log-level"
 	OPT_LOG_LEVEL_NUM,
 #define OPT_MASTER_LCORE      "master-lcore"