[dpdk-dev,v5,5/7] bus: add helper to find a bus from a device name

  Find which bus should be able to parse this device name into an internal
device representation.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 15 +++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 12 ++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 29 insertions(+)
  

On Wed, Jun 21, 2017 at 01:30:34AM +0200, Gaetan Rivet wrote:
> Find which bus should be able to parse this device name into an internal
> device representation.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> Find which bus should be able to parse this device name into an internal
> device representation.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>  lib/librte_eal/common/eal_common_bus.c          | 15 +++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 12 ++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>  4 files changed, 29 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 3517d74..04fa882 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -202,5 +202,6 @@ DPDK_17.08 {
>         global:
>
>         rte_bus_from_name;
> +       rte_bus_from_dev;
>
>  } DPDK_17.05;
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index e69ce38..e9fbc03 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -231,3 +231,18 @@ rte_bus_from_name(const char *str)
>  {
>         return rte_bus_find(bus_cmp_name, str, NULL);
>  }
> +
> +static int
> +bus_can_parse(const struct rte_bus *bus, const void *_name)
> +{
> +       const char *name = _name;
> +
> +       return !(bus->parse && bus->parse(name, NULL) == 0);
> +}
> +
> +/* find a bus capable of parsing a device description */
> +struct rte_bus *
> +rte_bus_from_dev(const char *str)
> +{
> +       return rte_bus_find(bus_can_parse, str, NULL);
> +}
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index da8b8a1..61d9e6e 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -283,6 +283,18 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
>  struct rte_bus *rte_bus_from_name(const char *str);
>
>  /**
> + * Find a bus capable of identifying a device.
> + *
> + * @param str
> + *   A device identifier (PCI address, virtual PMD name, ...).
> + *
> + * @return
> + *   A valid bus handle if found.
> + *   NULL if no bus is able to parse this device.
> + */
> +struct rte_bus *rte_bus_from_dev(const char *str);

I still don't agree with this. The bus name should be passed
explicitly by the user of the API.

NAK.


> +
> +/**
>   * Helper for Bus registration.
>   * The constructor has higher priority than PMD constructors.
>   */
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 6607acc..a5127d6 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -206,5 +206,6 @@ DPDK_17.08 {
>         global:
>
>         rte_bus_from_name;
> +       rte_bus_from_dev;
>
>  } DPDK_17.05;
> --
> 2.1.4
>
  

27/06/2017 20:55, Jan Blunck:
> On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> >  /**
> > + * Find a bus capable of identifying a device.
> > + *
> > + * @param str
> > + *   A device identifier (PCI address, virtual PMD name, ...).
> > + *
> > + * @return
> > + *   A valid bus handle if found.
> > + *   NULL if no bus is able to parse this device.
> > + */
> > +struct rte_bus *rte_bus_from_dev(const char *str);
> 
> I still don't agree with this. The bus name should be passed
> explicitly by the user of the API.
> 
> NAK.

Please explain why you think the bus name should be explicit.
If the bus is ambiguous, it can be explicited by the user.

I see some good benefits in being tolerant with the bus/device
representation. It provides a smooth transition to the bus model.
  

On Wed, Jun 28, 2017 at 7:03 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 27/06/2017 20:55, Jan Blunck:
>> On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
>> >  /**
>> > + * Find a bus capable of identifying a device.
>> > + *
>> > + * @param str
>> > + *   A device identifier (PCI address, virtual PMD name, ...).
>> > + *
>> > + * @return
>> > + *   A valid bus handle if found.
>> > + *   NULL if no bus is able to parse this device.
>> > + */
>> > +struct rte_bus *rte_bus_from_dev(const char *str);
>>
>> I still don't agree with this. The bus name should be passed
>> explicitly by the user of the API.
>>
>> NAK.
>
> Please explain why you think the bus name should be explicit.
> If the bus is ambiguous, it can be explicited by the user.
>
> I see some good benefits in being tolerant with the bus/device
> representation. It provides a smooth transition to the bus model.
>

We build libraries. The applications we build with the help of those
libraries get notified by the OS about device events. Those devices
are chields of their parent bus. At the time the event is fired the OS
already knows about:

- the bus name (parent)
- the device name (child)
- additional event parameters (environment)

Blame me that I probably spent too much time with Kay Sievers and
GregKH to understand that device naming is easy to get wrong. Just
look at the hyperv device names and how they switched to the UUID
scheme. I don't think that hyperv is the only bus that uses UUID as
device identification. We should not codify a policy of how to deduce
a bus name from a given device name if that is knowledge that is
already present externally. Otherwise I fear this part of the EAL will
be subject to constant churn.
  

29/06/2017 09:56, Jan Blunck:
> On Wed, Jun 28, 2017 at 7:03 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 27/06/2017 20:55, Jan Blunck:
> >> On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> >> >  /**
> >> > + * Find a bus capable of identifying a device.
> >> > + *
> >> > + * @param str
> >> > + *   A device identifier (PCI address, virtual PMD name, ...).
> >> > + *
> >> > + * @return
> >> > + *   A valid bus handle if found.
> >> > + *   NULL if no bus is able to parse this device.
> >> > + */
> >> > +struct rte_bus *rte_bus_from_dev(const char *str);
> >>
> >> I still don't agree with this. The bus name should be passed
> >> explicitly by the user of the API.
> >>
> >> NAK.
> >
> > Please explain why you think the bus name should be explicit.
> > If the bus is ambiguous, it can be explicited by the user.
> >
> > I see some good benefits in being tolerant with the bus/device
> > representation. It provides a smooth transition to the bus model.
> >
> 
> We build libraries. The applications we build with the help of those
> libraries get notified by the OS about device events. Those devices
> are chields of their parent bus. At the time the event is fired the OS
> already knows about:
> 
> - the bus name (parent)
> - the device name (child)
> - additional event parameters (environment)
> 
> Blame me that I probably spent too much time with Kay Sievers and
> GregKH to understand that device naming is easy to get wrong. Just
> look at the hyperv device names and how they switched to the UUID
> scheme. I don't think that hyperv is the only bus that uses UUID as
> device identification. We should not codify a policy of how to deduce
> a bus name from a given device name if that is knowledge that is
> already present externally. Otherwise I fear this part of the EAL will
> be subject to constant churn.

OK I understand your point that it is a weak identification.
It is as weak as what we have currently.
It works at least when we have only PCI and VDEV.
However it does not prevent to use a strong identification with
bus and parent names.
I see rte_bus_from_dev() as a helper to transition to the new strong
identification model. So we could remove it in few releases.
Does it make sense?
  

On Thu, Jun 29, 2017 at 09:56:30AM +0200, Jan Blunck wrote:
> On Wed, Jun 28, 2017 at 7:03 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 27/06/2017 20:55, Jan Blunck:
> >> On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> >> >  /**
> >> > + * Find a bus capable of identifying a device.
> >> > + *
> >> > + * @param str
> >> > + *   A device identifier (PCI address, virtual PMD name, ...).
> >> > + *
> >> > + * @return
> >> > + *   A valid bus handle if found.
> >> > + *   NULL if no bus is able to parse this device.
> >> > + */
> >> > +struct rte_bus *rte_bus_from_dev(const char *str);
> >>
> >> I still don't agree with this. The bus name should be passed
> >> explicitly by the user of the API.
> >>
> >> NAK.
> >
> > Please explain why you think the bus name should be explicit.
> > If the bus is ambiguous, it can be explicited by the user.
> >
> > I see some good benefits in being tolerant with the bus/device
> > representation. It provides a smooth transition to the bus model.
> >
> 
> We build libraries. The applications we build with the help of those
> libraries get notified by the OS about device events. Those devices
> are chields of their parent bus. At the time the event is fired the OS
> already knows about:
> 
> - the bus name (parent)
> - the device name (child)
> - additional event parameters (environment)
> 
> Blame me that I probably spent too much time with Kay Sievers and
> GregKH to understand that device naming is easy to get wrong. Just
> look at the hyperv device names and how they switched to the UUID
> scheme. I don't think that hyperv is the only bus that uses UUID as
> device identification. We should not codify a policy of how to deduce
> a bus name from a given device name if that is knowledge that is
> already present externally. Otherwise I fear this part of the EAL will
> be subject to constant churn.

I agree in the context of device events.
But this development concerns all device identification scheme, not only
in the context of hotplug where we can retrieve events giving proper
info. It is parsing user input as well (command line or configuration).

In this user-centric device specification, the issue is that the current
model expect the user to provide the bus implicitly by using the right
parameter (--vdev, -w). This identification *cannot* stay, as those are
parsed within the EAL and specifics are getting out.

What is left is thus a choice: either

* We let the current scheme work for a time, while the EAL is being cleaned
  out, during which we need some crutch to emulate the current behavior

* We force all users to migrate to the new format straight away with a
  full identification scheme.

I planned for both eventuality in my deprecation notice, by warning that
device parameters and definition might change this version. However,
while I thought that it was possible it would happen, I think it is best
to provide as much stability as possible while we work out the EAL
internals.

This API is only there because I choose to keep backward compatibility.
A compromise might be to make it private to the EAL (I proposed it
before but no one really responded so I haven't acted on it). This would
help the future transition to the fully-qualified-device-identifier that
we will have to require from our users.

I'd like to hear from other DPDK devs as I think that surprising users
is not something that should be done lightly. I understand your concern
and am not opposed to address it.
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gaëtan Rivet
> Sent: Thursday, June 29, 2017 11:24 AM
> To: Jan Blunck <jblunck@infradead.org>
> Cc: Thomas Monjalon <thomas@monjalon.net>; dev <dev@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH v5 5/7] bus: add helper to find a bus from
> a device name
> 
> On Thu, Jun 29, 2017 at 09:56:30AM +0200, Jan Blunck wrote:
> > On Wed, Jun 28, 2017 at 7:03 PM, Thomas Monjalon <thomas@monjalon.net>
> wrote:
> > > 27/06/2017 20:55, Jan Blunck:
> > >> On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet
> <gaetan.rivet@6wind.com> wrote:
> > >> >  /**
> > >> > + * Find a bus capable of identifying a device.
> > >> > + *
> > >> > + * @param str
> > >> > + *   A device identifier (PCI address, virtual PMD name, ...).
> > >> > + *
> > >> > + * @return
> > >> > + *   A valid bus handle if found.
> > >> > + *   NULL if no bus is able to parse this device.
> > >> > + */
> > >> > +struct rte_bus *rte_bus_from_dev(const char *str);
> > >>
> > >> I still don't agree with this. The bus name should be passed
> > >> explicitly by the user of the API.
> > >>
> > >> NAK.
> > >
> > > Please explain why you think the bus name should be explicit.
> > > If the bus is ambiguous, it can be explicited by the user.
> > >
> > > I see some good benefits in being tolerant with the bus/device
> > > representation. It provides a smooth transition to the bus model.
> > >
> >
> > We build libraries. The applications we build with the help of those
> > libraries get notified by the OS about device events. Those devices
> > are chields of their parent bus. At the time the event is fired the OS
> > already knows about:
> >
> > - the bus name (parent)
> > - the device name (child)
> > - additional event parameters (environment)
> >
> > Blame me that I probably spent too much time with Kay Sievers and
> > GregKH to understand that device naming is easy to get wrong. Just
> > look at the hyperv device names and how they switched to the UUID
> > scheme. I don't think that hyperv is the only bus that uses UUID as
> > device identification. We should not codify a policy of how to deduce
> > a bus name from a given device name if that is knowledge that is
> > already present externally. Otherwise I fear this part of the EAL will
> > be subject to constant churn.
> 
> I agree in the context of device events.
> But this development concerns all device identification scheme, not only
> in the context of hotplug where we can retrieve events giving proper info.
> It is parsing user input as well (command line or configuration).
> 
> In this user-centric device specification, the issue is that the current
> model expect the user to provide the bus implicitly by using the right
> parameter (--vdev, -w). This identification *cannot* stay, as those are
> parsed within the EAL and specifics are getting out.
> 
> What is left is thus a choice: either
> 
> * We let the current scheme work for a time, while the EAL is being
> cleaned
>   out, during which we need some crutch to emulate the current behavior
> 
> * We force all users to migrate to the new format straight away with a
>   full identification scheme.
> 
> I planned for both eventuality in my deprecation notice, by warning that
> device parameters and definition might change this version. However, while
> I thought that it was possible it would happen, I think it is best to
> provide as much stability as possible while we work out the EAL internals.
> 
> This API is only there because I choose to keep backward compatibility.
> A compromise might be to make it private to the EAL (I proposed it before
> but no one really responded so I haven't acted on it). This would help the
> future transition to the fully-qualified-device-identifier that we will
> have to require from our users.
> 
> I'd like to hear from other DPDK devs as I think that surprising users is
> not something that should be done lightly. I understand your concern and
> am not opposed to address it.
> 

I'm all in favour of stability and backward compatibility, so I don't like the option of forcing users to change just now - especially as the whole picture is not yet complete. Once we are sure we have a fully settled new scheme, then we can being to deprecate the old.

/Bruce