Re: [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API

[Thomas Monjalon <thomas@monjalon.net>]

06/08/2020 12:29, Maxime Coquelin:
> On 8/6/20 1:33 AM, Thomas Monjalon wrote:
> > 25/06/2020 10:04, Maxime Coquelin:
> >> This patch makes rte_dev_probe() to return the
> >> rte_device pointer on success and NULL on error
> >> instead of returning 0 on success and negative
> >> value on error.
> >>
> >> The goal is to avoid that the calling application
> >> iterates the devices list afterwards to retrieve
> >> the pointer. Retrieving the pointer is required
> >> for calling rte_dev_remove() later.
> >>
> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> ---
> >> --- a/lib/librte_eal/include/rte_dev.h
> >> +++ b/lib/librte_eal/include/rte_dev.h
> >> @@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
> >>   * @param devargs
> >>   *   Device arguments including bus, class and driver properties.
> >>   * @return
> >> - *   0 on success, negative on error.
> >> + *   Generic device pointer on success, NULL on error.
> >>   */
> >> -int rte_dev_probe(const char *devargs);
> >> +struct rte_device *rte_dev_probe(const char *devargs);
> > 
> > Sorry for not catching it earlier, I think this change is against
> > the idea of having a generic devargs syntax.
> > One string could identify multiple devices.
> 
> That sounds fragile to me. What if one of the multiple devices fails to
> probe? Do the other ones are going to be removed?

No, what is correctly probed should not be rolled back in my opinion.

> > And a successful probe does not mean there is a new rte_device
> > (can be an update, allowing more ports on the same device).
> 
> This should be done by a separate API in my opinion, ports may be seen
> as a sub-function of the device.
> 
> But anyway, I am fine with dropping it. It is not blocking vDPA probing,
> just making it more cumbersome.

Apps should not iterate on devices.
vDPA should have an event mechanism like in ethdev:
a callback with RTE_ETH_EVENT_NEW is called for each new port probed.