[dpdk-dev] [PATCH 7/9] bus: add helper to find a bus from a device name

[Gaëtan Rivet]

On Sat, Jun 10, 2017 at 10:50:12AM +0200, Jan Blunck wrote:
> On Thu, Jun 8, 2017 at 2:51 PM, Gaëtan Rivet <gaetan.rivet at 6wind.com> wrote:
> > On Thu, Jun 08, 2017 at 01:40:46PM +0200, Jan Blunck wrote:
> >> On Thu, Jun 8, 2017 at 1:36 PM, Gaëtan Rivet <gaetan.rivet at 6wind.com> wrote:
> >> > On Thu, Jun 08, 2017 at 12:45:17PM +0200, Jan Blunck wrote:
> >> >> On Wed, Jun 7, 2017 at 10:03 PM, Gaëtan Rivet <gaetan.rivet at 6wind.com> wrote:
> >> >> > On Wed, Jun 07, 2017 at 07:28:07PM +0200, Jan Blunck wrote:
> >> >> >> On Wed, May 24, 2017 at 5:12 PM, Gaetan Rivet <gaetan.rivet at 6wind.com> wrote:
> >> >> >> > Find which bus should be able to parse this device name into an internal
> >> >> >> > device representation.
> >> >> >> >
> >> >> >>
> >> >> >> No, please don't add this. One should know to what bus a device
> >> >> >> belongs to before plugging it. Artificially encoding the parent bus
> >> >> >> into the device name is not the right thing to do. Please keep those
> >> >> >> things separate.
> >> >> >>
> >> >> >
> >> >>
> >> >> When plugging a device the users know about:
> >> >> - bus name
> >> >> - device name
> >> >>
> >> >> Its not the case that the users invent the device names out of thin
> >> >> air. The EAL shouldn't codify what the users of the EAL already know
> >> >> about.
> >> >>
> >> >>
> >> >
> >> > Yes, but in that case the user is forced to explicitly name the bus used
> >> > for a device.
> >> >
> >> > I think it might be sufficient to have this as a private function to the
> >> > EAL, as it is currently only used within the rte_devargs parsing.
> >> > Applications could use this helper to recognize a bus from a device
> >> > name, but this is contrived.
> >> >
> >>
> >> Just remove it. Putting the knowledge of what bus a device name could
> >> be for into code has failed before (e.g. biosdevname etc.). If the
> >> application doesn't know what bus the device is living on we have a
> >> different problem.
> >>
> >
> > This means that devices will be declared as follows:
> >
> >   -w PCI:00:02.0 -w virtual:net_ring0
> >
> > Without a way to keep the legacy behavior.
> > Do you agree with it?
> >
> 
> This adds ':' as another character not usable in identifiers for buses.
> 

I understand your issue with this.
However, we are not forced to define a separating character that would thus
be forbidden within bus names.

We can strncmp() the name of the device, limited to strlen(bus->name).
If no bus name could be read this way, then we are sure to have a device
name (if we want to keep backward compability) or to throw an error if
we want to force defining the bus explicitly.

My current approach has been to define what is allowed within a bus name
and to limit the name comparison to the valid characters.
This allowed to use any illegal character as separator, but I think it
was a mistake. This is not a good, stable API for the future and does
not give clear rules to bus writers as to what is, or will be allowed
within a bus name.

But the strncmp approach allows the use of any separating character,
legal or illegal within a bus name. Additionally, it's then possible not
to add a magic devargs that must be parsed prior to device probing.

This last point is important I think. I am strongly opposed to having
parsing done upon the devargs, and to have any special logic added to
the rte_devargs constructor. As you saw with the driver= special
parameter, this led you to either add rte_kvargs as a dependency or roll
out your own parser. This was an undocumented hack for the bonding PMD test,
and personally I do not support this kind of addition to the public
rte_bus API.

> I wouldn't touch the meaning of -w at all and instead use it as an alias for
> 
> --dev 00:02.0,bus=pci  --bus pci,whitelist
> 

I agree that we should add a --bus parameter or similar to offer bus
configuration.

I think that in the end, -w, -b and --vdev should disappear to only
leave a common --dev parameter, capable of handling all devices.

It may be possible to have a special rule for -w, -b for some time, that
would either prepend "PCI:" or append ",bus=PCI" to any device
definition, but this is hacky as it relies on PCI_BUS_NAME without
being able to include its definition.

But considering the possibility to declare a device without explicitly
naming its bus: if we go in this direction, then we already have a
public API that can be leveraged for this. The implementation is
simple and can be made private to the EAL if exposing it is a problem.
Additionally it does not inject implicit bus dependencies within the
EAL.

> This would clearly separate the device arguments from the bus arguments.
> 
> What do you think? Does this make sense?

-- 
Gaëtan Rivet
6WIND