Re: [dpdk-dev] [PATCH v7] eal: add manual probing option

[Gaetan Rivet <grive@u256.net>]

On 04/02/2020 12:07, Thomas Monjalon wrote:
> 04/02/2020 11:03, Gaetan Rivet:
>> On 03/02/2020 23:21, Thomas Monjalon wrote:
>>> 03/02/2020 06:16, Pavan Nikhilesh Bhagavatula:
>>>> @David Marchand @thomas@monjalon.net
>>>>
>>>> Ping?
>>>>
>>>> Are there any more changes required for this patch? It's been in queue since last October.
>>>
>>> Sorry we have not decided whether it is a good idea or not.
>>>
>>> All changes related to probing are very sensitive,
>>> and we know a big refactoring would be better than stacking
>>> more and more options and corner cases.
>>>
>>> As we are busy with ABI stability stuff, we did not allocate
>>> enough time to properly think about this feature.
>>> Please accept our apologies, and let's consider it as
>>> a high priority for 20.05 cycle.
>>>
>>
>> Hello Thomas,
>>
>> This is unfortunate. I pushed Pavan to accept an alternative implementation of this functionality that was less obtrusive, to make the integration smoother. I took care to alleviate those risks from the common path.
>>
>> The big refactoring is needed yes, but considering the current path I'm not seeing it happen in 20.05. If that means taking this patch as-is in 20.05 for Marvell users, I'm not sure much is gained from waiting 3 months, except minimal risk avoidance.
> 
> 
> Yes, life is full of bad decisions and consequences.


Ah, yes, but I stand by my initial opinion, the first implementation [1] was riskier and less useful.

> 
> I still think there is a risk in adding new user expectations,
> and maintaining some code to workaround unknown issues.
> 
> The real question here is to know why this patch?
> Is it to workaround a broken driver?
> Or to workaround a broken design in EAL and bus drivers?
> 
> 

Two birds - one stone here: OVS needed a way to disable automatic probing cleanly (current workaround seen in multiple deployment is to add a dummy whitelisted device, which will be ignored by the PCI bus --> it sets the bus in whitelist mode but avoid probing anything), and as a bonus this option allows using devices that depends on other devices being probed already (LAG, representors, failsafe, etc).

I'm not sure having a dependent-probe by default is good, and that would be a big change.

If we are doing the genesis of this patch, the initial motivation should be asked for more details from Marvell people and David for the OVS side.

[1]: First proposal:
        http://mails.dpdk.org/archives/dev/2019-September/144166.html
      My arguments:
        http://mails.dpdk.org/archives/dev/2019-September/144564.html