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

[Jerin Jacob <jerinjacobk@gmail.com>]

> > >  rte_eal_check_module(const char *module_name)
> > >  {
> > > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > > index 7cbf82d37..ccc4ffb21 100644
> > > --- a/lib/librte_eal/rte_eal_version.map
> > > +++ b/lib/librte_eal/rte_eal_version.map
> > > @@ -312,6 +312,14 @@ DPDK_19.08 {
> > >
> > >  } DPDK_19.05;
> > >
> > > +DPDK_19.11 {
> > > +       global:
> > > +
> > > +       rte_eal_manual_probe;
> > > +       rte_eal_manual_probe_set;
> >
> > Do we need public API for this? it is only used by eal lib.
> >
>
> I think some PMDs and some applications would need to issue warnings to
> the user if manual probing is not enabled (i.e. a port representor
> parameter was used in a PMD, but probing is automatic --> on some
> platform it would work and others it would fail with the same config).
>
> Some applications would also prefer forcing it enabled without having to
> use the option on the command line.
>
> This motivates the public API. Given that there is a public API, I think
 it's better to use it even from within the EAL.

No issue from my side to expose through public API as well.
Please rte_eal_manual_probe_set() when setting in flags in eal as well.
Currently rte_eal_manual_probe_set() not used anywhere.

>
> However I agree that it does not need two separate implementations. I
> could move it to some eal_common_* part probably, maybe bus or dev?

eal should be fine. No strong opinion on the place.

>
> > checkpatch complains the following too.
> > ERROR: symbol rte_eal_manual_probe_set is added in the DPDK_19.11
> > section, but is expected to be added in the EXPERIMENTAL section of
> > the version map
>
> The API is simple enough IMO to make it immediately stable: it will
> allow applications forbidding the use of experimental API to make use of
> it.
>
> This is only for the sake of simplicity for some users. I'm not
> opposed to making it experimental first if it's expected for all new
> APIs.

All new APIs are treated as experimental. I don't think, we need to
make an exception here.

Other these nits, This patch looks good.

>
> --
> Gaėtan Rivet
> 6WIND