Re: [PATCH v1 4/8] bus: add probe mode setter

[Shreyansh Jain <shreyansh.jain@nxp.com>]

On Monday 11 December 2017 06:09 PM, Shreyansh Jain wrote:
> On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:
>> Introduce new rte_bus operation to configure the probe policy.
>>
>> Implementation is required from buses interested in supporting
>> this configuration element.
>>
> 
> [...]
> 
>> diff --git a/lib/librte_eal/common/eal_common_options.c 
>> b/lib/librte_eal/common/eal_common_options.c
>> index e40c049..630c9d2 100644
>> --- a/lib/librte_eal/common/eal_common_options.c
>> +++ b/lib/librte_eal/common/eal_common_options.c
>> @@ -997,29 +997,24 @@ int
>>   eal_parse_common_option(int opt, const char *optarg,
>>               struct internal_config *conf)
>>   {
>> -    static int b_used;
>> -    static int w_used;
>> -
>>       switch (opt) {
>>       /* blacklist */
>>       case 'b':
>> -        if (w_used)
>> -            goto bw_used;
>> +        if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0)
>> +            return -1;
> 
> Generic layer shouldn't be concerned about "pci" or other bus.
> Problem would be to find which bus this option needs to be set.
> 
> What I can think of as options is:
> 1. Storing this configuration until we can parse the argument for which 
> <bus> the argument has been created. That would mean changing the way 
> the "-b" and "-w" are passed and to allow non-PCI device identifier to 
> be passed.
> 2. Call each bus bus->ctrl and let it decide what to do based on the args.
>   - so, have a wrapper over rte_bus_probe_mode_set for all buses rather 
> than taking any one bus as option.
> 
> (2) sounds most plausible for now as the application will not send the 
> bus name as argument.
> And if brute force is not required, we need to split the argument to 
> know the bus - after making the device naming standardized (<bus>:<...>)
> 
> Even before that, we need to agree that "-w' and "-b" are not more valid 
> only for PCIs.
> 
> Above is more of loud thinging - I don't have concrete thought on this 
> for now. I'll revisit this after reviewing the patches in this series.

Apologies. I see that some work has been done for this in devargs series 
- I was too quick to reply on this.
Let me come back to you on this after reading through that series.

> 
>>           if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
>>                   optarg) < 0) {
>>               return -1;
>>           }
>> -        b_used = 1;
>>           break;
>>       /* whitelist */
>>       case 'w':
>> -        if (b_used)
>> -            goto bw_used;
>> +        if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0)
>> +            return -1;
>>           if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
>>                   optarg) < 0) {
>>               return -1;
>>           }
>> -        w_used = 1;
>>           break;
>>       /* coremask */
>>       case 'c':
>> @@ -1165,10 +1160,6 @@ eal_parse_common_option(int opt, const char 
>> *optarg,
> 
> [...]
>