[dpdk-dev] [PATCH 4/8] eal: replace pci-whitelist/pci-blacklist options
David Marchand
david.marchand at redhat.com
Wed Oct 14 18:07:30 CEST 2020
On Tue, Sep 22, 2020 at 4:33 PM Stephen Hemminger
<stephen at networkplumber.org> wrote:
>
> Replace -w / --pci-whitelist with -a / --allow options
> and -b / --pci-blacklist with -b / --block.
Looks like there is a mix or -a/-b and -i/-x from the previous proposal.
This is what causes CI failure as I previously reported:
http://inbox.dpdk.org/dev/CAJFAV8yCWg9YXKBiKsoEq7dHGr3PjbTGzHL_d1j5+Dp9GuXbOw@mail.gmail.com/
I guess the intention of this patch is:
'-b' / '--pci-blacklist' becomes '-b' / '--block' + warning on '--pci-blacklist'
'-w' / '--pci-whitelist' becomes '-a' / '--allow' + warning on 'w' /
'--pci-whitelist'
Comments below.
>
> Allow the old options for now, but print a nag
> warning since old options are deprecated.
>
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
> Acked-by: Luca Boccassi <bluca at debian.org>
> ---
> lib/librte_eal/common/eal_common_options.c | 70 +++++++++++++---------
> lib/librte_eal/common/eal_options.h | 9 ++-
> 2 files changed, 51 insertions(+), 28 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index a5426e12346a..4e54512c874c 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -51,7 +51,8 @@
>
> const char
> eal_short_options[] =
> - "b:" /* pci-blacklist */
> + "a:" /* allow */
> + "b:" /* block */
> "c:" /* coremask */
> "s:" /* service coremask */
> "d:" /* driver */
> @@ -62,7 +63,7 @@ eal_short_options[] =
> "n:" /* memory channels */
> "r:" /* memory ranks */
> "v" /* version */
> - "w:" /* pci-whitelist */
> + "w:" /* whitelist (deprecated) */
> ;
>
> const struct option
> @@ -87,8 +88,8 @@ eal_long_options[] = {
> {OPT_NO_PCI, 0, NULL, OPT_NO_PCI_NUM },
> {OPT_NO_SHCONF, 0, NULL, OPT_NO_SHCONF_NUM },
> {OPT_IN_MEMORY, 0, NULL, OPT_IN_MEMORY_NUM },
> - {OPT_PCI_BLACKLIST, 1, NULL, OPT_PCI_BLACKLIST_NUM },
> - {OPT_PCI_WHITELIST, 1, NULL, OPT_PCI_WHITELIST_NUM },
> + {OPT_DEV_BLOCK, 1, NULL, OPT_DEV_BLOCK_NUM },
> + {OPT_DEV_ALLOW, 1, NULL, OPT_DEV_ALLOW_NUM },
> {OPT_PROC_TYPE, 1, NULL, OPT_PROC_TYPE_NUM },
> {OPT_SOCKET_MEM, 1, NULL, OPT_SOCKET_MEM_NUM },
> {OPT_SOCKET_LIMIT, 1, NULL, OPT_SOCKET_LIMIT_NUM },
> @@ -102,6 +103,11 @@ eal_long_options[] = {
> {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> +
> + /* legacy options that will be removed in next LTS */
> + {OPT_PCI_BLACKLIST, 1, NULL, OPT_PCI_BLACKLIST_NUM },
> + {OPT_PCI_WHITELIST, 1, NULL, OPT_PCI_WHITELIST_NUM },
> +
> {0, 0, NULL, 0 }
> };
>
> @@ -1414,29 +1420,38 @@ int
> eal_parse_common_option(int opt, const char *optarg,
> struct internal_config *conf)
> {
> - static int b_used;
> - static int w_used;
> + static bool x_used, i_used;
Previously the variables matched the short options.
It should be a_used and b_used.
>
> switch (opt) {
> - /* blacklist */
> + /* deprecated option */
> case 'b':
Should be
case OPT_PCI_BLACKLIST_NUM:
> - if (w_used)
> - goto bw_used;
> - if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
> + fprintf(stderr,
> + "Option -b, --blacklist is deprecated, use -x, --exclude option instead\n");
We keep the -b option as is, don't complain about it.
Plus, the previous option was not --blacklist but --pci-blacklist.
Please use "--"OPT_PCI_BLACKLIST.
> + /* fallthrough */
> + case 'x':
> + /* excluded list */
Should be
case 'b':
/* block list */
> + if (i_used)
> + goto include_exclude;
> + if (eal_option_device_add(RTE_DEVTYPE_BLOCKED,
> optarg) < 0) {
> return -1;
> }
> - b_used = 1;
> + x_used = true;
> break;
> - /* whitelist */
> +
> case 'w':
> - if (b_used)
> - goto bw_used;
> - if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
> + fprintf(stderr,
> + "Option -w, --whitelist is deprecated, use -i, --include option instead\n");
Plus, the previous option was not --whitelist but --pci-whitelist.
Please use "--"OPT_PCI_WHITELIST.
The new option is '-a' not '-i'.
> + /* fallthrough */
> + case 'i':
> + /* include device list */
Should be
case 'a':
/* allow list */
etc...
> + if (x_used)
> + goto include_exclude;
> + if (eal_option_device_add(RTE_DEVTYPE_ALLOWED,
> optarg) < 0) {
> return -1;
> }
> - w_used = 1;
> + i_used = true;
> break;
> /* coremask */
> case 'c': {
> @@ -1715,9 +1730,10 @@ eal_parse_common_option(int opt, const char *optarg,
> }
>
> return 0;
> -bw_used:
> - RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) "
> - "cannot be used at the same time\n");
> +
> +include_exclude:
> + RTE_LOG(ERR, EAL,
> + "Options include (-i) and exclude (-x) can't be used at the same time\n");
-a / -b
> return -1;
> }
>
> @@ -1926,14 +1942,14 @@ eal_common_usage(void)
> " -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"
> - " -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"
> - " -w, --"OPT_PCI_WHITELIST" Add a PCI device in white list.\n"
> - " Only use the specified PCI devices. The argument format\n"
> - " is <[domain:]bus:devid.func>. This option can be present\n"
> - " several times (once per device).\n"
> - " [NOTE: PCI whitelist cannot be used with -b option]\n"
> + " -b, --"OPT_DEV_BLOCK" Add a device to the excluded list.\n"
> + " Prevent EAL from using this device. The argument\n"
> + " format for PCI devices is <domain:bus:devid.func>.\n"
> + " -a, --"OPT_DEV_ALLOW" Add a device to the included list.\n"
> + " Only use the specified devices. The argument format\n"
> + " for PCI devices is <[domain:]bus:devid.func>.\n"
> + " This option can be present several times.\n"
> + " [NOTE: " OPT_DEV_ALLOW " cannot be used with "OPT_DEV_BLOCK" option]\n"
> " --"OPT_VDEV" Add a virtual device.\n"
> " The argument format is <driver><id>[,key=val,...]\n"
> " (ex: --vdev=net_pcap0,iface=eth2).\n"
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 89769d48b487..fc0fe2258a32 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -13,8 +13,15 @@ enum {
> /* long options mapped to a short option */
> #define OPT_HELP "help"
> OPT_HELP_NUM = 'h',
> +#define OPT_DEV_ALLOW "allow"
> + OPT_DEV_ALLOW_NUM = 'a',
> +#define OPT_DEV_BLOCK "block"
> + OPT_DEV_BLOCK_NUM = 'b',
Fix indent.
> +
> + /* legacy options that will be removed in next LTS */
> #define OPT_PCI_BLACKLIST "pci-blacklist"
> - OPT_PCI_BLACKLIST_NUM = 'b',
> +#define OPT_PCI_BLACKLIST_NUM OPT_DEV_BLOCK_NUM
Move OPT_PCI_BLACKLIST_NUM as a long-only option.
This way we can differentiate it from the 'b' short option that has
been preserved.
> +
> #define OPT_PCI_WHITELIST "pci-whitelist"
> OPT_PCI_WHITELIST_NUM = 'w',
>
> --
> 2.27.0
>
--
David Marchand
More information about the dev
mailing list