Re: [dpdk-dev] [PATCH v7 12/17] devargs: parse bus policies

[Thomas Monjalon <thomas@monjalon.net>]

Hi,

I think there is a real bug to fix in 17.08-rc2.
More details below. Proposals are welcome.

07/07/2017 02:04, Gaetan Rivet:
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -143,6 +143,21 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>  
>  		break;
>  	}
> +	if (devargs->type == RTE_DEVTYPE_WHITELISTED_PCI) {
> +		if (bus->conf.scan_mode == RTE_BUS_SCAN_UNDEFINED) {
> +			bus->conf.scan_mode = RTE_BUS_SCAN_WHITELIST;
> +		} else if (bus->conf.scan_mode == RTE_BUS_SCAN_BLACKLIST) {
> +			fprintf(stderr, "ERROR: incompatible device type and bus scan mode\n");
> +			goto fail;
> +		}
> +	} else if (devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> +		if (bus->conf.scan_mode == RTE_BUS_SCAN_UNDEFINED) {
> +			bus->conf.scan_mode = RTE_BUS_SCAN_BLACKLIST;
> +		} else if (bus->conf.scan_mode == RTE_BUS_SCAN_WHITELIST) {
> +			fprintf(stderr, "ERROR: incompatible device type and bus scan mode\n");
> +			goto fail;
> +		}
> +	}

After another look, there is something wrong here.
You are checking a probe policy (wrongly named scan_mode), in a function
which can be not related at all with bus probing.

Example with failsafe:
1/ We blacklist a device for the global EAL probe.
So the probing mode is set from "undefined" to "blacklist".
2/ We add the device as a failsafe slave.
3/ The device must be plugged, no matter of the probe policy.
But it is seen as a whitelist and rejected because the bus probing
is in blacklist mode.

I think it is a serious bug.

PS: thanks Matan for having checked it