Re: [dpdk-dev] [PATCH v7 00/22] Device querying

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

On Mon, Apr 23, 2018 at 12:54:54AM +0200, Thomas Monjalon wrote:
> 15/04/2018 17:07, Gaetan Rivet:
> > This patchset introduces a new EAL API for querying devices,
> > filtered by arbitrary properties.
> > 
> > The following elements are introduced to this end:
> > 
> >  * A new object, "rte_class", is used to describe
> >    the device class abstraction layer (eth, crypto, ...).
> > 
> >  * Both rte_bus and rte_class now offer a way to
> >    list their devices and filter the result
> >    using locally defined properties.
> > 
> >  * The rte_dev API now has an rte_dev_iterator, which
> >    is the way for the user to define the device filter
> >    and iterate upon the resulting set.
> > 
> > As an example, the "eth" device class is implemented.
> > 
> > Additionally, the device filters for
> > 
> >   + rte_bus_pci
> >   + rte_bus_vdev
> >   + rte_class_eth
> > 
> > are implemented and can be used with some
> > properties each, to show how to extend those.
> > 
> > Some example of filters:
> > 
> >   "bus=pci/class=eth"
> >   "bus=pci"
> >   "class=eth"
> >   "class=eth,name=net_ring0"
> >   "bus=pci,id=00:00.0"
> >   "bus=vdev,driver=net_ring"
> 
> Thanks for the very big work Gaetan.
> 
> This series has some cleanups, fixes, and more importantly,
> introduces the framework for a generic device syntax.
> 
> I am not clear what is missing in the properties parsing?

For properties, parsing is *mostly* done. The general framework is there
for other devs to populate as they see fit.

A thing that will arise when doing so, is that librte_kvargs is somewhat
cumbersome to use as it is, when the "/" character is not considered
"end-of-string". This adds cruft in a lot of places otherwise. So I
think we need to add some way to configure this (or even have this
behavior by default). Otherwise most things are available.

> In various buses and classes implementations?
> 
> Are we close of the next step, revisiting -w/-b and --vdev options?
> 

I started this part to see the scope. It's quite large.
I have issues with the current device declaration syntax, that needs to
be discussed.

1.

One issue is that buses are working on a best-effort basis to probe
drivers. When the "driver=" layer is given, a driver is then explicitly
choosen by the user. This contradicts the current bus logic. The
"driver" layer is necessary to give any device parameters, and choosing
the proper driver beforehand seems user-hostile.

I think the "driver=" part should actually be dropped altogether, which
would simplify the declaration. We'll see if another solution can be
attained.

2.

A second issue is regarding the standing of the "class" layer. The
parsing of devices goes like this:

  0. bus, class and driver is parsed and put in its relevant part
     in the devargs.
  1. Bus parses the devargs and uses its part within it (necessary to
     find the proper device identifier for example).
  2. Bus probes all drivers to find the one able to handle the device.
     At this point, the devargs is passed down to the driver, with all
     three layers (bus, class, driver) parts.
     Driver uses the driver part for its initialization.
  3. Driver calls the device creation function of a class (passing the
     devargs), for example to spawn an rte_eth_dev.
     The class thus parses the class part of the devargs and uses it for
     initialization.

In matter of layer hierarchy, this is not neat and clean:
  bus -> driver -> class.

Drivers are the one having to choose whether to spawn an eth or crypto
of compress device (or any other, in any numbers). Having the class
initialize after the driver however seems prone to conflicts in
configuration (between the hardware init and the higher device class
properties for example).

Another issue that I sidestepped in the device ident, is how to
recognize and handle ambiguous device declarations. Something like:

  --dev bus=pci

would thus match all PCI devices on the system. All would be probed
(akin to blacklist mode), with default parameters. Not aberrant.

However something like

  --dev class=eth

does not mean anything. This cannot be recognized at devargs parsing
time. Such devargs would not actually identify a specific device on a
bus. PCI might be able to handle such case, vdev not. Other buses might
not as well.

So I think it is necessary that at least the bus identifier is given.
The bus identifier is a bus property that is bus-specific: for PCI it is
the "id" property. For vdev it is "driver" (for driver name). This is
not user-friendly. Requiring the bus identifier is not actually, but
this is what was implicitly done by the current devargs "name" field.

But let's think about possible secondary properties: for example, in PCI
bus the vendor id could be a property. Should a devargs
"bus=pci,vendor=0x8086" be accepted, as a set of PCI device could actually
be matched? If multiple devices match the devargs, should the devargs be
cloned for each? I'm tempted to say no, for the sake of simplicity, but
this might be worth thinking about it.

So, as you see, this work is far from done and there are standing issues
that will I think require some discussion from the community.

-- 
Gaëtan Rivet
6WIND