Re: [dpdk-dev] [PATCH v2] eal/devargs: add option to supply PCI dev args

[Shahaf Shuler <shahafs@mellanox.com>]

Hi Pavan,

Friday, June 15, 2018 7:44 AM, Pavan Nikhilesh:
> Subject: [dpdk-dev] [PATCH v2] eal/devargs: add option to supply PCI dev
> args
> 
> Currently, the only way of supplying device argument to a pci device is to
> whitelist it i.e. -w 000X:00:0X.0,self_test=1. This is not a very feasible method
> as whitelisting a device has its own side effects i.e only the whitelisted pci
> devices are probed.
> 
> Add a new eal command line option --pci-args to pass device args without the
> need to whitelist the devices.
> 		--pci-args 000X:00:0X.0,self_test=1
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>

Tested-by: Shahaf Shuler <shahafs@mellanox.com>

It seems to work. 
Please see small comments below

> ---
>  v2 Changes:
>  - Document the option usage in eal_common_usage.
>  - Update commit log to be more informative.
> 
>  lib/librte_eal/common/eal_common_devargs.c  | 3 +++
> lib/librte_eal/common/eal_common_options.c  | 9 +++++++++
>  lib/librte_eal/common/eal_options.h         | 2 ++
>  lib/librte_eal/common/include/rte_dev.h     | 1 +
>  lib/librte_eal/common/include/rte_devargs.h | 1 +
>  5 files changed, 16 insertions(+)

Should we also update the manual of testpmd (doc/guides/testpmd_app_ug/run_app.rst ) for the new eal arg?

> 
> diff --git a/lib/librte_eal/common/eal_common_devargs.c
> b/lib/librte_eal/common/eal_common_devargs.c
> index b0434158b..a56bfeea0 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -156,6 +156,9 @@ rte_devargs_add(enum rte_devtype devtype, const
> char *devargs_str)
>  	bus = devargs->bus;
>  	if (devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)
>  		devargs->policy = RTE_DEV_BLACKLISTED;
> +	else if (devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
> +		devargs->policy = RTE_DEV_WHITELISTED;
> +
>  	if (bus->conf.scan_mode == RTE_BUS_SCAN_UNDEFINED) {
>  		if (devargs->policy == RTE_DEV_WHITELISTED)
>  			bus->conf.scan_mode = RTE_BUS_SCAN_WHITELIST;
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index ecebb2923..31eebaa53 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -76,6 +76,7 @@ eal_long_options[] = {
>  	{OPT_VMWARE_TSC_MAP,    0, NULL,
> OPT_VMWARE_TSC_MAP_NUM   },
>  	{OPT_LEGACY_MEM,        0, NULL, OPT_LEGACY_MEM_NUM       },
>  	{OPT_SINGLE_FILE_SEGMENTS, 0, NULL,
> OPT_SINGLE_FILE_SEGMENTS_NUM},
> +	{OPT_PCI_DEVARGS,       1, NULL, OPT_PCI_DEVARGS_NUM},
>  	{0,                     0, NULL, 0                        }
>  };
> 
> @@ -1224,6 +1225,12 @@ eal_parse_common_option(int opt, const char
> *optarg,
>  	case OPT_SINGLE_FILE_SEGMENTS_NUM:
>  		conf->single_file_segments = 1;
>  		break;
> +	case OPT_PCI_DEVARGS_NUM:
> +		if (eal_option_device_add(RTE_DEVTYPE_NORMAL,
> +				optarg) < 0) {
> +			return -1;
> +		}
> +		break;
> 
>  	/* don't know what to do, leave this to caller */
>  	default:
> @@ -1360,6 +1367,8 @@ eal_common_usage(void)
>  	       "  --"OPT_VDEV"              Add a virtual device.\n"
>  	       "                      The argument format is <driver><id>[,key=val,...]\n"
>  	       "                      (ex: --vdev=net_pcap0,iface=eth2).\n"
> +	       "  --"OPT_PCI_DEVARGS"          Pass key-value arguments to a pci
> device\n"
> +	       "                      The argument format is
> <domain:bus:devid.func>[,key=val,...]\n"
>  	       "  -d 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"
> diff --git a/lib/librte_eal/common/eal_options.h
> b/lib/librte_eal/common/eal_options.h
> index 211ae06ae..d52d38e32 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -59,6 +59,8 @@ enum {
>  	OPT_LEGACY_MEM_NUM,
>  #define OPT_SINGLE_FILE_SEGMENTS    "single-file-segments"
>  	OPT_SINGLE_FILE_SEGMENTS_NUM,
> +#define OPT_PCI_DEVARGS       "pci-args"
> +	OPT_PCI_DEVARGS_NUM,
>  	OPT_LONG_MAX_NUM
>  };
> 
> diff --git a/lib/librte_eal/common/include/rte_dev.h
> b/lib/librte_eal/common/include/rte_dev.h
> index 3879ff3ca..fb3e5033f 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -124,6 +124,7 @@ enum rte_kernel_driver {
>   * Device policies.
>   */
>  enum rte_dev_policy {
> +	RTE_DEV_NORMAL,
>  	RTE_DEV_WHITELISTED,
>  	RTE_DEV_BLACKLISTED,
>  };
> diff --git a/lib/librte_eal/common/include/rte_devargs.h
> b/lib/librte_eal/common/include/rte_devargs.h
> index 58fbd90a2..78c600bf2 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -29,6 +29,7 @@ extern "C" {
>   * Type of generic device
>   */
>  enum rte_devtype {
> +	RTE_DEVTYPE_NORMAL, /* Normal dev with special pci args */

What is "Normal" device? Can we find a better name? 

>  	RTE_DEVTYPE_WHITELISTED_PCI,
>  	RTE_DEVTYPE_BLACKLISTED_PCI,
>  	RTE_DEVTYPE_VIRTUAL,
> --
> 2.17.1