RE: [PATCH V3 3/7] net/mlx5: add new devargs to control probe optimization

[Slava Ovsiienko <viacheslavo@nvidia.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, October 31, 2024 9:31 PM
> To: Minggang(Gavin) Li <gavinl@nvidia.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Matan
> Azrad <matan@nvidia.com>; Ori Kam <orika@nvidia.com>; Dariusz
> Sosnowski <dsosnowski@nvidia.com>; Bing Zhao <bingz@nvidia.com>;
> Suanming Mou <suanmingm@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; rongwei liu <rongweil@nvidia.com>
> Subject: Re: [PATCH V3 3/7] net/mlx5: add new devargs to control probe
> optimization
> 
> 29/10/2024 17:20, Stephen Hemminger:
> > On Tue, 29 Oct 2024 15:42:52 +0200
> > "Minggang Li(Gavin)" <gavinl@nvidia.com> wrote:
> >
> > > From: Rongwei Liu <rongweil@nvidia.com>
> > >
> > > Add a new devarg probe_opt_en to control probe optimization in PMD.
> > >
> > > By default, the value is 0 and no behavior changed.
> > >
> > > Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> > > Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> >
> > Once again, every option you introduce expands the test space by 2X.
> > "Do or Do not. There is no try"
> > Either it works all the time or it is a bad idea.
> 
> I fully agree.
> We should not merge this series before providing a good answer, or making
> it automatic.
> 
> One more thing: a commit log should always explain "why".
> Here it should say why it is not automatic.
> Is there a good reason to disable this feature?

The feature is event-driven and depends on the system/DPDK environment.
Example:
- DPDK handles interrupts in the single dedicated EAL thread
- the failsafe PMD in "interrupt"  handler  performs device probe() action,
that might take a long time and DPDK event/interrupt handle experiences the
significant delays, sometime causing the malfunction.

Another concern - Netlink buffers have limited capacity, and with high message
rate might be overflown.

This is just the examples, sure, most of the time feature works reliably.
The feature is needed for few users only, that's why we follow conservative
approach. Do you think we should put all the stuff above in the commit log?

With best regards,
Slava
 

> 
> > Sorry if I sound like a broken record, the project I used to work on
> > had the same kind of "always add an option" policy. But every time an
> > option was changed, there was a 50/50 chance that it was broken
> > because that combination of options had not been tested since
> > originally added and was non functional due to bit rot.
> 
>