On Tue, Sep 22, 2020 at 4:33 PM Stephen Hemminger <step...@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+dp9gux...@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 <step...@networkplumber.org> > Acked-by: Luca Boccassi <bl...@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