Re: [PATCH 0/9] rte_bus parse API

On Wed, Jun 07, 2017 at 07:22:05PM +0200, Jan Blunck wrote:

On Wed, May 24, 2017 at 5:12 PM, Gaetan Rivet [off-list ref] wrote:

quoted

Following the evolutions announced in [1], here is the first part of
the rte_devargs rework planned for 17.08. The rationale has been partially
explained in [2].

This first part covers the introduction of the necessary facilities in
rte_bus to allow for generic device parsing. This API is implemented for
the virtual and PCI buses. Additionally, this rte_bus evolution is being
used within rte_devargs to characterize a device type by its bus.
This work is the first of two parts to reduce the dependency of the EAL
upon specific bus implementations.

Two public functions are added to rte_bus to help bus recognition:

- rte_bus_from_name
- rte_bus_from_dev

These functions are made public because the bus handle within devargs
becomes the generic device type. Recognizing device types is useful for
buses and PMDs alike.
The modified rte_devargs parsing allows declaring on the EAL command line
explicit buses to handle a device. The format is as follow:

  --vdev="virtual:net_ring0" --vdev="net_tap0,iface=tap0"
  -w PCI:00:02.0 -w 00:03.0

I don't see the point of doing this. The --vdev parameter implicitly
defines the bus by its name (--vdev aka virtual device).

Why don't you add a commandline "--dev" parameter that supports a
"bus=" devarg? You would need to clarify what that means for other
busses than the virtual one. Is the bus switched into whitelist mode
by that?

We cannot keep the current -w, -b and --vdev parameter. Those are
processed by the EAL, and use specifics from the virtual and PCI buses.

The rte_devargs rework has been to make the same functionality generic
to all rte_bus. As seen quickly in [2], rte_devargs has two functions:

* Validating a device declaration
* Keeping the relevant device info until it has been processed.

Both functionalities have been genericized. This results in all parameters
being able to be used with all types of devices. This is inherent to the
EAL becoming bus-agnostic.

Now, it is absolutely possible to rename for example -w as --dev, as it
is the expected behavior from users. This however should be discussed by
the community, last time I talked about the possibility of switching the
default of the PCI bus to whitelist mode the community wasn't all that
enthused by the prospect.

Finally, I do not like the idea of a special devarg just for declaring
explicitly buses for devices. The bus is not a device modifier, nor is
it a driver parameter. The bus is a way to define the location of the
device on the system. Adding a special "bus=" devargs means having some
preprocessing done on devargs upon rte_devargs allocation. This was
already abused by the bonding PMD with the driver= parameter. I do not
support this and did not want to repeat it. Passing down the device args
is a simple process and we should keep it as simple as possible.

I know you do not like having the bus as part of the device name.
As a compromise, I made the current system flexible and allowed the legacy
device definition to be kept.

However with a purely generic process, it is necessary to at least offer
the possibility to the user to explicitly use a bus, as nothing prevents
conflicting device names from existing.

quoted

[2]: http://dpdk.org/ml/archives/dev/2017-May/065670.html

quoted

This explicit bus designation is optional; no evolution is currently
forced on users to migrate to this new format. The separating character is
arbitrary and can be any character illegal within a bus name.
Subsequently, what is allowed within a bus name has been formally
defined and is now enforced.

[1]: http://dpdk.org/ml/archives/dev/2017-May/065634.html
[2]: http://dpdk.org/ml/archives/dev/2017-May/065670.html

This patchset depends on:

bus: attach / detach API
http://dpdk.org/ml/archives/dev/2017-May/066330.html
http://dpdk.org/dev/patchwork/patch/24489/

Gaetan Rivet (9):
  bus: fix bus name registration
  bus: verify bus name on registration
  bus: introduce parsing functionality
  vdev: implement parse bus operation
  pci: implement parse bus operation
  bus: add helper to find bus from a name
  bus: add helper to find a bus from a device name
  vdev: expose bus name
  devargs: parse bus info

 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  8 +++
 lib/librte_eal/common/eal_common_bus.c          | 47 +++++++++++++++++
 lib/librte_eal/common/eal_common_devargs.c      | 17 +++++-
 lib/librte_eal/common/eal_common_pci.c          | 19 +++++++
 lib/librte_eal/common/eal_common_vdev.c         | 70 ++++++++++++++-----------
 lib/librte_eal/common/eal_private.h             | 16 ++++++
 lib/librte_eal/common/include/rte_bus.h         | 49 ++++++++++++++++-
 lib/librte_eal/common/include/rte_devargs.h     |  3 ++
 lib/librte_eal/common/include/rte_vdev.h        |  2 +
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  8 +++
 10 files changed, 205 insertions(+), 34 deletions(-)

--
2.1.4

-- 
Gaëtan Rivet
6WIND