rte_bus parse API
cover rte_bus parse API
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
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(-)
Signature · 1 line
2.1.4
PATCH 1/9 bus: fix bus name registration
The default bus registration function should not result in buses registering with double quotes within their names.
--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_eal/common/include/rte_bus.h +1 -1
@@ -222,7 +222,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ }
Signature · 1 line
2.1.4
PATCH 2/9 bus: verify bus name on registration
In reply to Gaetan RivetVerify that a bus name is legal.
--- lib/librte_eal/common/eal_common_bus.c | 13 +++++++++++++ lib/librte_eal/common/eal_private.h | 16 ++++++++++++++++ 2 files changed, 29 insertions(+)
lib/librte_eal/common/eal_common_bus.c +13 -0
@@ -47,6 +47,8 @@ rte_bus_register(struct rte_bus *bus) {RTE_VERIFY (bus);RTE_VERIFY (bus->name &&strlen (bus->name)); +/* Its whole name should be comprised only of valid characters */ +RTE_VERIFY (rte_bus_name_valid (bus->name) ==strlen (bus->name));/* A bus should mandatorily have the scan implemented */ RTE_VERIFY (bus->scan);RTE_VERIFY (bus->probe);
@@ -210,3 +212,14 @@ rte_bus_find_device(const struct rte_device *start,return dev; } + +/* Validate a bus name. */ +size_t +rte_bus_name_valid (const char *name) +{ +size_t i =0 ; + +while (isalnum (name[i]) || name[i] =='_' ) + i++; +return i; +}
lib/librte_eal/common/eal_private.h +16 -0
@@ -338,4 +338,20 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/* +* Validate a bus name. +* +* Counts the number of valid characters in a given name, +* starting from the beginning. +* A legal bus name is defined by the pattern: +* [:alnum:_]+ +* +* @param name +* bus name +* +* @return +* number of valid characters in name. +*/ +size_t rte_bus_name_valid (const char *name); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
PATCH 3/9 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -103,6 +103,26 @@ typedef int (*rte_bus_attach_t)(struct rte_device *dev);typedef int (*rte_bus_detach_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -113,6 +133,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_attach_t attach;/**< Probe single device for drivers */ rte_bus_detach_t detach;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
PATCH 4/9 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 62 ++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +33 -29
@@ -102,6 +102,31 @@ static char *parse_driver_arg(const char *args)return str; } +/* +* typeof(addr): (struct rte_vdev_driver **) +*/ +static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (!strncmp (driver->driver.name, name, +strlen (driver->driver.name))) + goto ret; +if (driver->driver.alias && + !strncmp (driver->driver.alias, name, +strlen (driver->driver.alias))) + goto ret; + } + driver =NULL ; +ret: +if (addr !=NULL ) + *out = driver; +return !driver; +} +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {
@@ -116,36 +141,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } - } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -368,6 +371,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, /* .attach = NULL, see comment on vdev_detach */ .detach = vdev_detach, + .parse = vdev_parse, };RTE_INIT (rte_vdev_bus_register);
Signature · 1 line
2.1.4
PATCH 5/9 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
lib/librte_eal/common/eal_common_pci.c +19 -0
@@ -474,6 +474,24 @@ rte_pci_dump(FILE *f) } } +/* +* Parse a device entry +* typeof(addr): (struct rte_pci_addr *) +*/ +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +int parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr) + *out = pci_addr; +return parse; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -530,6 +548,7 @@ struct rte_pci_bus rte_pci_bus = { .scan = rte_pci_scan, .probe = rte_pci_probe, .find_device = pci_find_device, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH 6/9 bus: add helper to find bus from a name
In reply to Gaetan Rivet--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ lib/librte_eal/common/eal_common_bus.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_bus.h | 14 ++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ 4 files changed, 47 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +7 -0
@@ -197,3 +197,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +19 -0
@@ -223,3 +223,22 @@ rte_bus_name_valid(const char *name) i++;return i; } + +static int +bus_cmp_name (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; +size_t i =0 ; + + i =rte_bus_name_valid (name); +return (!strncmp (bus->name, name, i)); +} + +/* find a bus by its name */ +struct rte_bus * +rte_bus_from_name (const char *str) +{ +if (rte_bus_name_valid (str) ==0 ) +return NULL ; +return rte_bus_find (bus_cmp_name, str); +}
lib/librte_eal/common/include/rte_bus.h +14 -0
@@ -236,6 +236,20 @@ rte_bus_find_device(const struct rte_device *start,*/ struct rte_bus *rte_bus_find_by_device (const struct rte_device *dev); +/* +* Find a bus handle by its name. +* Compares the name of each bus up until any invalid character +* in the matched pattern. +* +* @param str +* A null terminated character string. +* +* @return +* A pointer to a bus if found. +* NULL if no bus matches. +*/ +struct rte_bus *rte_bus_from_name (const char *str); +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors.
lib/librte_eal/linuxapp/eal/rte_eal_version.map +7 -0
@@ -201,3 +201,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
Signature · 1 line
2.1.4
PATCH 7/9 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- 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(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +15 -0
@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}
lib/librte_eal/common/include/rte_bus.h +12 -0
@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
Signature · 1 line
2.1.4
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 3 lines · Gaetan Rivet
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.
Quoted 79 lines
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}lib/librte_eal/common/include/rte_bus.h +12
-0@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-1@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05; --2.1.4
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 10 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
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.
The EAL has no way to know this currently. As you noted, it has to know onto which bus a device belongs before plugging it.
Quoted 79 lines
Quoted 79 lines
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}lib/librte_eal/common/include/rte_bus.h +12
-0@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-1@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05; --2.1.4
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 6 lines · Gaëtan Rivet
Quoted 5 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Quoted 6 lines
Quoted 5 lines
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.
Quoted 86 lines
The EAL has no way to know this currently. As you noted, it has to know onto which bus a device belongs before plugging it.
Quoted 72 lines
Quoted 72 lines
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}lib/librte_eal/common/include/rte_bus.h +12
-0@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +0
-0@@ -206,5 +206,6 @@ DPDK_17.08 { global:Quoted 7 lines
Quoted 7 lines
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-1rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05; --2.1.4
--
Gaėtan Rivet
6WIND
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 14 lines · Jan Blunck
Quoted 6 lines · Gaëtan Rivet
Quoted 5 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Quoted 6 lines
Quoted 5 lines
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.
Quoted 9 lines
When plugging a device the users know about:
- bus name
- device nameIts 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.
Quoted 86 lines
Quoted 86 lines
The EAL has no way to know this currently. As you noted, it has to know onto which bus a device belongs before plugging it.
Quoted 72 lines
Quoted 72 lines
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}lib/librte_eal/common/include/rte_bus.h +12
-0@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +0
-0@@ -206,5 +206,6 @@ DPDK_17.08 { global:Quoted 7 lines
Quoted 7 lines
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-1rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05; --2.1.4
--
Gaėtan Rivet
6WIND
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 25 lines · Gaëtan Rivet
Quoted 14 lines · Jan Blunck
Quoted 6 lines · Gaëtan Rivet
Quoted 5 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Quoted 6 lines
Quoted 5 lines
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.
Quoted 9 lines
When plugging a device the users know about:
- bus name
- device nameIts 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.
Quoted 8 lines
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_devargsparsing. 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.
Quoted 90 lines
Quoted 78 lines
Quoted 78 lines
The EAL has no way to know this currently. As you noted, it has to know onto which bus a device belongs before plugging it.
Quoted 75 lines
Quoted 75 lines
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}lib/librte_eal/common/include/rte_bus.h +12
-0@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-0@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev;Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0
-1} DPDK_17.05; --2.1.4
--
Gaėtan Rivet
6WIND--
Gaëtan Rivet
6WIND
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 35 lines · Jan Blunck
Quoted 25 lines · Gaëtan Rivet
Quoted 14 lines · Jan Blunck
Quoted 6 lines · Gaëtan Rivet
Quoted 5 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Quoted 6 lines
Quoted 5 lines
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.
Quoted 9 lines
When plugging a device the users know about:
- bus name
- device nameIts 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.
Quoted 8 lines
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_devargsparsing. Applications could use this helper to recognize a bus from a device name, but this is contrived.
Quoted 5 lines
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?
Quoted 90 lines
Quoted 90 lines
Quoted 78 lines
Quoted 78 lines
The EAL has no way to know this currently. As you noted, it has to know onto which bus a device belongs before plugging it.
Quoted 75 lines
Quoted 75 lines
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}lib/librte_eal/common/include/rte_bus.h +12
-0@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-0@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev;Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0
-1} DPDK_17.05; --2.1.4
--
Gaėtan Rivet
6WIND--
Gaëtan Rivet
6WIND
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 42 lines · Gaëtan Rivet
Quoted 35 lines · Jan Blunck
Quoted 25 lines · Gaëtan Rivet
Quoted 14 lines · Jan Blunck
Quoted 6 lines · Gaëtan Rivet
Quoted 5 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Quoted 6 lines
Quoted 5 lines
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.
Quoted 9 lines
When plugging a device the users know about:
- bus name
- device nameIts 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.
Quoted 8 lines
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_devargsparsing. Applications could use this helper to recognize a bus from a device name, but this is contrived.Quoted 5 lines
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.
Quoted 7 lines
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 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
This would clearly separate the device arguments from the bus arguments.
What do you think? Does this make sense?
Re: [PATCH 7/9] bus: add helper to find a bus from a device name
Quoted 45 lines · Jan Blunck
Quoted 44 lines · Gaëtan Rivet
Quoted 35 lines · Jan Blunck
Quoted 25 lines · Gaëtan Rivet
Quoted 14 lines · Jan Blunck
Quoted 6 lines · Gaëtan Rivet
Quoted 5 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Quoted 6 lines
Quoted 5 lines
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.
Quoted 9 lines
When plugging a device the users know about:
- bus name
- device nameIts 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.
Quoted 8 lines
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_devargsparsing. Applications could use this helper to recognize a bus from a device name, but this is contrived.Quoted 5 lines
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:
Quoted 8 lines
Quoted 5 lines
-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.
Quoted 4 lines
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.
Quoted 3 lines
This would clearly separate the device arguments from the bus arguments.
What do you think? Does this make sense?
Signature · 2 lines
Gaëtan Rivet 6WIND
PATCH 8/9 vdev: expose bus name
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_vdev.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -384,6 +384,6 @@ static void rte_vdev_bus_register(void)return ; registered =1 ; - rte_vdev_bus.name =RTE_STR (virtual); + rte_vdev_bus.name =VIRTUAL_BUS_NAME ;rte_bus_register (&rte_vdev_bus); }
lib/librte_eal/common/include/rte_vdev.h +2 -0
@@ -41,6 +41,8 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> +#define VIRTUAL_BUS_NAME "virtual" +struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
PATCH 9/9 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 17 ++++++++++++++++- lib/librte_eal/common/eal_common_vdev.c | 6 ++++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +16 -1
@@ -83,7 +83,10 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) { struct rte_devargs *devargs = NULL; +const char *dev = devargs_str; +struct rte_bus *bus;char *buf =NULL ; +size_t i;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,7 +97,19 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) + bus =rte_bus_from_name (dev); +if (bus) { + i =rte_bus_name_valid (dev); + dev += i +1 ; + }else { + bus =rte_bus_from_dev (dev); +if (!bus) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus; +if (rte_eal_parse_devargs_str (dev, &buf, &devargs->args)) goto fail;switch (devargs->type) {
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -182,6 +182,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_from_name (VIRTUAL_BUS_NAME );if (args) devargs->args =strdup (args);
@@ -294,12 +295,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_from_name (VIRTUAL_BUS_NAME ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
v2 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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
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/
v1 -> v2:
* fix PCI parse implementation
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(-)
Signature · 1 line
2.1.4
PATCH v2 1/9 bus: fix bus name registration
The default bus registration function should not result in buses registering with double quotes within their names.
--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_eal/common/include/rte_bus.h +1 -1
@@ -222,7 +222,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ }
Signature · 1 line
2.1.4
PATCH v2 2/9 bus: verify bus name on registration
In reply to Gaetan RivetVerify that a bus name is legal.
--- lib/librte_eal/common/eal_common_bus.c | 13 +++++++++++++ lib/librte_eal/common/eal_private.h | 16 ++++++++++++++++ 2 files changed, 29 insertions(+)
lib/librte_eal/common/eal_common_bus.c +13 -0
@@ -47,6 +47,8 @@ rte_bus_register(struct rte_bus *bus) {RTE_VERIFY (bus);RTE_VERIFY (bus->name &&strlen (bus->name)); +/* Its whole name should be comprised only of valid characters */ +RTE_VERIFY (rte_bus_name_valid (bus->name) ==strlen (bus->name));/* A bus should mandatorily have the scan implemented */ RTE_VERIFY (bus->scan);RTE_VERIFY (bus->probe);
@@ -210,3 +212,14 @@ rte_bus_find_device(const struct rte_device *start,return dev; } + +/* Validate a bus name. */ +size_t +rte_bus_name_valid (const char *name) +{ +size_t i =0 ; + +while (isalnum (name[i]) || name[i] =='_' ) + i++; +return i; +}
lib/librte_eal/common/eal_private.h +16 -0
@@ -338,4 +338,20 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/* +* Validate a bus name. +* +* Counts the number of valid characters in a given name, +* starting from the beginning. +* A legal bus name is defined by the pattern: +* [:alnum:_]+ +* +* @param name +* bus name +* +* @return +* number of valid characters in name. +*/ +size_t rte_bus_name_valid (const char *name); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
PATCH v2 3/9 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -103,6 +103,26 @@ typedef int (*rte_bus_attach_t)(struct rte_device *dev);typedef int (*rte_bus_detach_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -113,6 +133,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_attach_t attach;/**< Probe single device for drivers */ rte_bus_detach_t detach;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
PATCH v2 4/9 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 62 ++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +33 -29
@@ -102,6 +102,31 @@ static char *parse_driver_arg(const char *args)return str; } +/* +* typeof(addr): (struct rte_vdev_driver **) +*/ +static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (!strncmp (driver->driver.name, name, +strlen (driver->driver.name))) + goto ret; +if (driver->driver.alias && + !strncmp (driver->driver.alias, name, +strlen (driver->driver.alias))) + goto ret; + } + driver =NULL ; +ret: +if (addr !=NULL ) + *out = driver; +return !driver; +} +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {
@@ -116,36 +141,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } - } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -368,6 +371,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, /* .attach = NULL, see comment on vdev_detach */ .detach = vdev_detach, + .parse = vdev_parse, };RTE_INIT (rte_vdev_bus_register);
Signature · 1 line
2.1.4
PATCH v2 5/9 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
lib/librte_eal/common/eal_common_pci.c +19 -0
@@ -474,6 +474,24 @@ rte_pci_dump(FILE *f) } } +/* +* Parse a device entry +* typeof(addr): (struct rte_pci_addr *) +*/ +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +int parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr) + *out = pci_addr; +return !parse; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -530,6 +548,7 @@ struct rte_pci_bus rte_pci_bus = { .scan = rte_pci_scan, .probe = rte_pci_probe, .find_device = pci_find_device, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH v2 6/9 bus: add helper to find bus from a name
In reply to Gaetan Rivet--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ lib/librte_eal/common/eal_common_bus.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_bus.h | 14 ++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ 4 files changed, 47 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +7 -0
@@ -197,3 +197,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +19 -0
@@ -223,3 +223,22 @@ rte_bus_name_valid(const char *name) i++;return i; } + +static int +bus_cmp_name (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; +size_t i =0 ; + + i =rte_bus_name_valid (name); +return (!strncmp (bus->name, name, i)); +} + +/* find a bus by its name */ +struct rte_bus * +rte_bus_from_name (const char *str) +{ +if (rte_bus_name_valid (str) ==0 ) +return NULL ; +return rte_bus_find (bus_cmp_name, str); +}
lib/librte_eal/common/include/rte_bus.h +14 -0
@@ -236,6 +236,20 @@ rte_bus_find_device(const struct rte_device *start,*/ struct rte_bus *rte_bus_find_by_device (const struct rte_device *dev); +/* +* Find a bus handle by its name. +* Compares the name of each bus up until any invalid character +* in the matched pattern. +* +* @param str +* A null terminated character string. +* +* @return +* A pointer to a bus if found. +* NULL if no bus matches. +*/ +struct rte_bus *rte_bus_from_name (const char *str); +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors.
lib/librte_eal/linuxapp/eal/rte_eal_version.map +7 -0
@@ -201,3 +201,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
Signature · 1 line
2.1.4
PATCH v2 7/9 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- 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(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +15 -0
@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}
lib/librte_eal/common/include/rte_bus.h +12 -0
@@ -251,6 +251,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
Signature · 1 line
2.1.4
PATCH v2 8/9 vdev: expose bus name
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_vdev.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -384,6 +384,6 @@ static void rte_vdev_bus_register(void)return ; registered =1 ; - rte_vdev_bus.name =RTE_STR (virtual); + rte_vdev_bus.name =VIRTUAL_BUS_NAME ;rte_bus_register (&rte_vdev_bus); }
lib/librte_eal/common/include/rte_vdev.h +2 -0
@@ -41,6 +41,8 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> +#define VIRTUAL_BUS_NAME "virtual" +struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
PATCH v2 9/9 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 17 ++++++++++++++++- lib/librte_eal/common/eal_common_vdev.c | 6 ++++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +16 -1
@@ -83,7 +83,10 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) { struct rte_devargs *devargs = NULL; +const char *dev = devargs_str; +struct rte_bus *bus;char *buf =NULL ; +size_t i;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,7 +97,19 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) + bus =rte_bus_from_name (dev); +if (bus) { + i =rte_bus_name_valid (dev); + dev += i +1 ; + }else { + bus =rte_bus_from_dev (dev); +if (!bus) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus; +if (rte_eal_parse_devargs_str (dev, &buf, &devargs->args)) goto fail;switch (devargs->type) {
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -182,6 +182,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_from_name (VIRTUAL_BUS_NAME );if (args) devargs->args =strdup (args);
@@ -294,12 +295,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_from_name (VIRTUAL_BUS_NAME ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
v3 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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
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/
v1 -> v2:
* fix PCI parse implementation
v2 --> v3
* Rebase the series on the new plug / unplug API
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(-)
Signature · 1 line
2.1.4
PATCH v3 1/9 bus: fix bus name registration
The default bus registration function should not result in buses registering with double quotes within their names.
--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_eal/common/include/rte_bus.h +1 -1
@@ -234,7 +234,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ }
Signature · 1 line
2.1.4
PATCH v3 2/9 bus: verify bus name on registration
In reply to Gaetan RivetVerify that a bus name is legal.
--- lib/librte_eal/common/eal_common_bus.c | 13 +++++++++++++ lib/librte_eal/common/eal_private.h | 16 ++++++++++++++++ 2 files changed, 29 insertions(+)
lib/librte_eal/common/eal_common_bus.c +13 -0
@@ -47,6 +47,8 @@ rte_bus_register(struct rte_bus *bus) {RTE_VERIFY (bus);RTE_VERIFY (bus->name &&strlen (bus->name)); +/* Its whole name should be comprised only of valid characters */ +RTE_VERIFY (rte_bus_name_valid (bus->name) ==strlen (bus->name));/* A bus should mandatorily have the scan implemented */ RTE_VERIFY (bus->scan);RTE_VERIFY (bus->probe);
@@ -210,3 +212,14 @@ rte_bus_find_device(const struct rte_device *start, }return dev; } + +/* Validate a bus name. */ +size_t +rte_bus_name_valid (const char *name) +{ +size_t i =0 ; + +while (isalnum (name[i]) || name[i] =='_' ) + i++; +return i; +}
lib/librte_eal/common/eal_private.h +16 -0
@@ -338,4 +338,20 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/* +* Validate a bus name. +* +* Counts the number of valid characters in a given name, +* starting from the beginning. +* A legal bus name is defined by the pattern: +* [:alnum:_]+ +* +* @param name +* bus name +* +* @return +* number of valid characters in name. +*/ +size_t rte_bus_name_valid (const char *name); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
PATCH v3 3/9 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -106,6 +106,26 @@ typedef int (*rte_bus_plug_t)(struct rte_devargs *da);typedef int (*rte_bus_unplug_t )(struct rte_devargs *da);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -116,6 +136,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
PATCH v3 4/9 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 62 ++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +33 -29
@@ -102,6 +102,31 @@ static char *parse_driver_arg(const char *args)return str; } +/* +* typeof(addr): (struct rte_vdev_driver **) +*/ +static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (!strncmp (driver->driver.name, name, +strlen (driver->driver.name))) + goto ret; +if (driver->driver.alias && + !strncmp (driver->driver.alias, name, +strlen (driver->driver.alias))) + goto ret; + } + driver =NULL ; +ret: +if (addr !=NULL ) + *out = driver; +return !driver; +} +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {
@@ -116,36 +141,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } - } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -368,6 +371,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, + .parse = vdev_parse, };RTE_INIT (rte_vdev_bus_register);
Signature · 1 line
2.1.4
PATCH v3 5/9 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
lib/librte_eal/common/eal_common_pci.c +19 -0
@@ -474,6 +474,24 @@ rte_pci_dump(FILE *f) } } +/* +* Parse a device entry +* typeof(addr): (struct rte_pci_addr *) +*/ +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +int parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr) + *out = pci_addr; +return !parse; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -529,6 +547,7 @@ struct rte_pci_bus rte_pci_bus = { .scan = rte_pci_scan, .probe = rte_pci_probe, .find_device = pci_find_device, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH v3 6/9 bus: add helper to find bus from a name
In reply to Gaetan Rivet--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ lib/librte_eal/common/eal_common_bus.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_bus.h | 14 ++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ 4 files changed, 47 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +7 -0
@@ -197,3 +197,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +19 -0
@@ -223,3 +223,22 @@ rte_bus_name_valid(const char *name) i++;return i; } + +static int +bus_cmp_name (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; +size_t i =0 ; + + i =rte_bus_name_valid (name); +return (!strncmp (bus->name, name, i)); +} + +/* find a bus by its name */ +struct rte_bus * +rte_bus_from_name (const char *str) +{ +if (rte_bus_name_valid (str) ==0 ) +return NULL ; +return rte_bus_find (bus_cmp_name, str); +}
lib/librte_eal/common/include/rte_bus.h +14 -0
@@ -248,6 +248,20 @@ rte_bus_find_device(const struct rte_device *start,*/ struct rte_bus *rte_bus_find_by_device (const struct rte_device *dev); +/* +* Find a bus handle by its name. +* Compares the name of each bus up until any invalid character +* in the matched pattern. +* +* @param str +* A null terminated character string. +* +* @return +* A pointer to a bus if found. +* NULL if no bus matches. +*/ +struct rte_bus *rte_bus_from_name (const char *str); +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors.
lib/librte_eal/linuxapp/eal/rte_eal_version.map +7 -0
@@ -201,3 +201,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
Signature · 1 line
2.1.4
PATCH v3 7/9 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- 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(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +15 -0
@@ -242,3 +242,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return (bus->parse && !bus->parse (name,NULL )); +} + +/* 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); +}
lib/librte_eal/common/include/rte_bus.h +12 -0
@@ -263,6 +263,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 structure if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_from_dev (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
Signature · 1 line
2.1.4
PATCH v3 8/9 vdev: expose bus name
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_vdev.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -384,6 +384,6 @@ static void rte_vdev_bus_register(void)return ; registered =1 ; - rte_vdev_bus.name =RTE_STR (virtual); + rte_vdev_bus.name =VIRTUAL_BUS_NAME ;rte_bus_register (&rte_vdev_bus); }
lib/librte_eal/common/include/rte_vdev.h +2 -0
@@ -41,6 +41,8 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> +#define VIRTUAL_BUS_NAME "virtual" +struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
PATCH v3 9/9 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 17 ++++++++++++++++- lib/librte_eal/common/eal_common_vdev.c | 6 ++++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +16 -1
@@ -83,7 +83,10 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) { struct rte_devargs *devargs = NULL; +const char *dev = devargs_str; +struct rte_bus *bus;char *buf =NULL ; +size_t i;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,7 +97,19 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) + bus =rte_bus_from_name (dev); +if (bus) { + i =rte_bus_name_valid (dev); + dev += i +1 ; + }else { + bus =rte_bus_from_dev (dev); +if (!bus) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus; +if (rte_eal_parse_devargs_str (dev, &buf, &devargs->args)) goto fail;switch (devargs->type) {
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -182,6 +182,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_from_name (VIRTUAL_BUS_NAME );if (args) devargs->args =strdup (args);
@@ -294,12 +295,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_from_name (VIRTUAL_BUS_NAME ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
v4 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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
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/
v1 -> v2:
* fix PCI parse implementation
v2 --> v3:
* Rebase the series on the new plug / unplug API
v3 --> v4:
* Several bug fixes, commit log rewrite.
* Follow the changes to the plug / unplug API.
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 a bus from a bus 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 | 68 +++++++++++++------------ 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, 203 insertions(+), 34 deletions(-)
Signature · 1 line
2.1.4
PATCH v4 1/9 bus: fix bus name registration
The default bus registration function should not result in buses registering with double quotes within their names.
--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_eal/common/include/rte_bus.h +1 -1
@@ -247,7 +247,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ }
Signature · 1 line
2.1.4
PATCH v4 2/9 bus: verify bus name on registration
In reply to Gaetan RivetVerify that a bus name is legal.
--- lib/librte_eal/common/eal_common_bus.c | 13 +++++++++++++ lib/librte_eal/common/eal_private.h | 16 ++++++++++++++++ 2 files changed, 29 insertions(+)
lib/librte_eal/common/eal_common_bus.c +13 -0
@@ -47,6 +47,8 @@ rte_bus_register(struct rte_bus *bus) {RTE_VERIFY (bus);RTE_VERIFY (bus->name &&strlen (bus->name)); +/* Its whole name should be comprised only of valid characters */ +RTE_VERIFY (rte_bus_name_valid (bus->name) ==strlen (bus->name));/* A bus should mandatorily have the scan implemented */ RTE_VERIFY (bus->scan);RTE_VERIFY (bus->probe);
@@ -207,3 +209,14 @@ rte_bus_find_device(const struct rte_device *start, }return dev; } + +/* Validate a bus name. */ +size_t +rte_bus_name_valid (const char *name) +{ +size_t i =0 ; + +while (isalnum (name[i]) || name[i] =='_' ) + i++; +return i; +}
lib/librte_eal/common/eal_private.h +16 -0
@@ -338,4 +338,20 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/* +* Validate a bus name. +* +* Counts the number of valid characters in a given name, +* starting from the beginning. +* A legal bus name is defined by the pattern: +* [:alnum:_]+ +* +* @param name +* bus name +* +* @return +* number of valid characters in name. +*/ +size_t rte_bus_name_valid (const char *name); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
PATCH v4 3/9 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -117,6 +117,26 @@ typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -127,6 +147,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
PATCH v4 4/9 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 60 +++++++++++++++++---------------- 1 file changed, 31 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +31 -29
@@ -104,6 +104,29 @@ static char *parse_driver_arg(const char *args)return str; } +/* +* typeof(addr): (struct rte_vdev_driver **) +*/ +static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver =NULL ; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (!strncmp (driver->driver.name, name, +strlen (driver->driver.name))) +break ; +if (driver->driver.alias && + !strncmp (driver->driver.alias, name, +strlen (driver->driver.alias))) +break ; + } +if (addr !=NULL ) + *out = driver; +return !driver; +} +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {
@@ -118,36 +141,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } - } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -390,6 +391,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, + .parse = vdev_parse, };RTE_INIT (rte_vdev_bus_register);
Signature · 1 line
2.1.4
PATCH v4 5/9 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
lib/librte_eal/common/eal_common_pci.c +19 -0
@@ -445,6 +445,24 @@ rte_pci_dump(FILE *f) } } +/* +* Parse a device entry +* typeof(addr): (struct rte_pci_addr *) +*/ +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +int parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr) + *out = pci_addr; +return !parse; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -500,6 +518,7 @@ struct rte_pci_bus rte_pci_bus = { .scan = rte_pci_scan, .probe = rte_pci_probe, .find_device = pci_find_device, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH v4 6/9 bus: add helper to find a bus from a bus name
In reply to Gaetan Rivet--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ lib/librte_eal/common/eal_common_bus.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_bus.h | 14 ++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ 4 files changed, 47 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +7 -0
@@ -197,3 +197,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +19 -0
@@ -220,3 +220,22 @@ rte_bus_name_valid(const char *name) i++;return i; } + +static int +bus_cmp_name (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; +size_t i =0 ; + + i =rte_bus_name_valid (name); +return strncmp (bus->name, name, i); +} + +/* find a bus by its name */ +struct rte_bus * +rte_bus_from_name (const char *str) +{ +if (rte_bus_name_valid (str) ==0 ) +return NULL ; +return rte_bus_find (bus_cmp_name, str); +}
lib/librte_eal/common/include/rte_bus.h +14 -0
@@ -261,6 +261,20 @@ rte_bus_find_device(const struct rte_device *start,*/ struct rte_bus *rte_bus_find_by_device (const struct rte_device *dev); +/* +* Find a bus handle by its name. +* Compares the name of each bus up until any invalid character +* in the matched pattern. +* +* @param str +* A null terminated character string. +* +* @return +* A pointer to a bus if found. +* NULL if no bus matches. +*/ +struct rte_bus *rte_bus_from_name (const char *str); +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors.
lib/librte_eal/linuxapp/eal/rte_eal_version.map +7 -0
@@ -201,3 +201,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
Signature · 1 line
2.1.4
PATCH v4 7/9 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- 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(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +15 -0
@@ -239,3 +239,18 @@ rte_bus_from_name(const char *str)return NULL ;return rte_bus_find (bus_cmp_name, str); } + +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); +}
lib/librte_eal/common/include/rte_bus.h +12 -0
@@ -276,6 +276,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); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
Signature · 1 line
2.1.4
PATCH v4 8/9 vdev: expose bus name
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_vdev.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -404,6 +404,6 @@ static void rte_vdev_bus_register(void)return ; registered =1 ; - rte_vdev_bus.name =RTE_STR (virtual); + rte_vdev_bus.name =VIRTUAL_BUS_NAME ;rte_bus_register (&rte_vdev_bus); }
lib/librte_eal/common/include/rte_vdev.h +2 -0
@@ -41,6 +41,8 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> +#define VIRTUAL_BUS_NAME "virtual" +struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
PATCH v4 9/9 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 17 ++++++++++++++++- lib/librte_eal/common/eal_common_vdev.c | 6 ++++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +16 -1
@@ -83,7 +83,10 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) { struct rte_devargs *devargs = NULL; +const char *dev = devargs_str; +struct rte_bus *bus;char *buf =NULL ; +size_t i;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,7 +97,19 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) + bus =rte_bus_from_name (dev); +if (bus) { + i =rte_bus_name_valid (dev); + dev += i +1 ; + }else { + bus =rte_bus_from_dev (dev); +if (!bus) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus; +if (rte_eal_parse_devargs_str (dev, &buf, &devargs->args)) goto fail;switch (devargs->type) {
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -182,6 +182,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_from_name (VIRTUAL_BUS_NAME );if (args) devargs->args =strdup (args);
@@ -294,12 +295,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_from_name (VIRTUAL_BUS_NAME ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
v5 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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
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/
v1 -> v2:
* fix PCI parse implementation
v2 --> v3:
* Rebase the series on the new plug / unplug API
v3 --> v4:
* Several bug fixes, commit log rewrite.
* Follow the changes to the plug / unplug API.
v5:
* Do not verify bus name on bus registration.
Actually, a legal bus name is not defined anymore.
The bus/device separator in a device declaration can be anything, as long as it is a single character.
The behavior is otherwise unchanged.
Gaetan Rivet (7):
bus: fix bus name registration
bus: introduce parsing functionality
vdev: implement parse bus operation
pci: implement parse bus operation
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 | 31 +++++++++++ lib/librte_eal/common/eal_common_devargs.c | 15 +++++- lib/librte_eal/common/eal_common_pci.c | 19 +++++++ lib/librte_eal/common/eal_common_vdev.c | 68 +++++++++++++------------ 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 +++ 9 files changed, 169 insertions(+), 34 deletions(-)
Signature · 1 line
2.1.4
PATCH v5 1/7 bus: fix bus name registration
The default bus registration function should not result in buses registering with double quotes within their names.
--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_eal/common/include/rte_bus.h +1 -1
@@ -254,7 +254,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ }
Signature · 1 line
2.1.4
Re: [PATCH v5 1/7] bus: fix bus name registration
Quoted 7 lines · Gaetan Rivet
The default bus registration function should not result in buses registering with double quotes within their names.
Fixes a97725791eec ("bus: introduce bus abstraction")Signed-off-by Gaetan Rivet <[email protected]>
Re: [PATCH v5 1/7] bus: fix bus name registration
In reply to Gaetan RivetQuoted 3 lines · Gaetan Rivet
The default bus registration function should not result in buses registering with double quotes within their names.
This is breaking expectations with users. All other registration macro pass the names through the stringification. The problem is that you pass in the name as a string already ("PCI" instead of PCI).
Quoted 24 lines
Fixes a97725791eec ("bus: introduce bus abstraction")Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)lib/librte_eal/common/include/rte_bus.h +1
-2@@ -254,7 +254,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ } --2.1.4
Re: [PATCH v5 1/7] bus: fix bus name registration
Quoted 10 lines · Jan Blunck
Quoted 3 lines · Gaetan Rivet
The default bus registration function should not result in buses registering with double quotes within their names.
This is breaking expectations with users. All other registration macro pass the names through the stringification. The problem is that you pass in the name as a string already ("PCI" instead of PCI).
I agree that it does not behave as expected. However, all buses are currently using it this way, and this cannot be fixed if maintainers keep using public define's for their bus name.
Quoted 24 lines
Quoted 24 lines
Fixes a97725791eec ("bus: introduce bus abstraction")Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)lib/librte_eal/common/include/rte_bus.h +1
-2@@ -254,7 +254,7 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ } --2.1.4
Signature · 2 lines
Gaëtan Rivet 6WIND
PATCH v5 2/7 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -117,6 +117,26 @@ typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -127,6 +147,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
Re: [PATCH v5 2/7] bus: introduce parsing functionality
Quoted 5 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Signed-off-by Gaetan Rivet <[email protected]>
Re: [PATCH v5 2/7] bus: introduce parsing functionality
In reply to Gaetan RivetQuoted 4 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Again, I don't think this makes sense to have. Also there is no user for this as far as I can see. The bus specific device representation should come from the scan function instead of this.
Quoted 47 lines
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)lib/librte_eal/common/include/rte_bus.h +21
-1@@ -117,6 +117,26 @@ typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {@@ -127,6 +147,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/** -- 2.1.4
Re: [PATCH v5 2/7] bus: introduce parsing functionality
Quoted 11 lines · Jan Blunck
Quoted 4 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Again, I don't think this makes sense to have. Also there is no user for this as far as I can see. The bus specific device representation should come from the scan function instead of this.
I think it makes sense that scanning would store this representation yes. The issue is that it requires another rte_bus_scan API evolution, which is beyond this patchset.
Keep in mind that I was bound when writing this to follow the existing API. I agree that some cleanup could come up in the EAL, but it cannot all happen at once. These first steps (generic devargs / vdev & PCI move to drivers/bus) serve to trim the EAL before these changes.
Quoted 47 lines
Quoted 47 lines
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)lib/librte_eal/common/include/rte_bus.h +21
-1@@ -117,6 +117,26 @@ typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {@@ -127,6 +147,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find device on bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/** -- 2.1.4
Signature · 2 lines
Gaëtan Rivet 6WIND
PATCH v5 3/7 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 60 +++++++++++++++++---------------- 1 file changed, 31 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +31 -29
@@ -104,6 +104,29 @@ static char *parse_driver_arg(const char *args)return str; } +/* +* typeof(addr): (struct rte_vdev_driver **) +*/ +static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver =NULL ; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (!strncmp (driver->driver.name, name, +strlen (driver->driver.name))) +break ; +if (driver->driver.alias && + !strncmp (driver->driver.alias, name, +strlen (driver->driver.alias))) +break ; + } +if (addr !=NULL ) + *out = driver; +return !driver; +} +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {
@@ -118,36 +141,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } - } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -390,6 +391,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, + .parse = vdev_parse, };RTE_INIT (rte_vdev_bus_register);
Signature · 1 line
2.1.4
Re: [PATCH v5 3/7] vdev: implement parse bus operation
Quoted 26 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_vdev.c | 60 +++++++++++++++++---------------- 1 file changed, 31 insertions(+), 29 deletions(-)lib/librte_eal/common/eal_common_vdev.c +13
-0@@ -104,6 +104,29 @@ static char *parse_driver_arg(const char *args)return str; } +/* +* typeof(addr): (struct rte_vdev_driver **) +*/ +static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver =NULL ; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (!strncmp (driver->driver.name, name, +strlen (driver->driver.name))) +break ;
lib/librte_eal/common/eal_common_vdev.c static char *parse_driver_arg(const char *args) · line 129
Quoted 10 lines
lib/librte_eal/common/eal_common_vdev.c +10
-0+if (driver->driver.alias && + !strncmp (driver->driver.alias, name, +strlen (driver->driver.alias))) +break ; + } +if (addr !=NULL ) + *out = driver; +return !driver; +} +
Prefer == 0 to use of "!" in return value from strncmp. Otherwise:
PATCH v5 4/7 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ lib/librte_eal/common/eal_common_bus.c | 16 ++++++++++++++++ lib/librte_eal/common/eal_common_pci.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_bus.h | 14 ++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ 5 files changed, 63 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +7 -0
@@ -197,3 +197,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +16 -0
@@ -215,3 +215,19 @@ rte_bus_find_device(const struct rte_device *start, }return dev; } + +static int +bus_cmp_name (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name)); +} + +/* find a bus by its name */ +struct rte_bus * +rte_bus_from_name (const char *str) +{ +return rte_bus_find (bus_cmp_name, str,NULL ); +}
lib/librte_eal/common/eal_common_pci.c +19 -0
@@ -450,6 +450,24 @@ rte_pci_dump(FILE *f) } } +/* +* Parse a device entry +* typeof(addr): (struct rte_pci_addr *) +*/ +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +int parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr) + *out = pci_addr; +return !parse; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -505,6 +523,7 @@ struct rte_pci_bus rte_pci_bus = { .scan = rte_pci_scan, .probe = rte_pci_probe, .find_device = pci_find_device, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
lib/librte_eal/common/include/rte_bus.h +14 -0
@@ -268,6 +268,20 @@ rte_bus_find_device(const struct rte_device *start,*/ struct rte_bus *rte_bus_find_by_device (const struct rte_device *dev); +/* +* Find a bus handle by its name. +* Compares the name of each bus up until any invalid character +* in the matched pattern. +* +* @param str +* A null terminated character string. +* +* @return +* A pointer to a bus if found. +* NULL if no bus matches. +*/ +struct rte_bus *rte_bus_from_name (const char *str); +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors.
lib/librte_eal/linuxapp/eal/rte_eal_version.map +7 -0
@@ -201,3 +201,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;
Signature · 1 line
2.1.4
Re: [PATCH v5 4/7] pci: implement parse bus operation
Quoted 38 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ lib/librte_eal/common/eal_common_bus.c | 16 ++++++++++++++++ lib/librte_eal/common/eal_common_pci.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_bus.h | 14 ++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ 5 files changed, 63 insertions(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +7
-0@@ -197,3 +197,10 @@ DPDK_17.05 { vfio_get_group_no; } DPDK_17.02; + +DPDK_17.08 { + global: + + rte_bus_from_name; + +} DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +6
-0@@ -215,3 +215,19 @@ rte_bus_find_device(const struct rte_device *start, }return dev; } + +static int +bus_cmp_name (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; +
lib/librte_eal/common/eal_common_bus.c rte_bus_find_device(const struct rte_device *start, · line 233
Quoted 10 lines
lib/librte_eal/common/eal_common_bus.c +10
-0+return strncmp (bus->name, name, +strlen (bus->name)); +} + +/* find a bus by its name */ +struct rte_bus * +rte_bus_from_name (const char *str) +{ +return rte_bus_find (bus_cmp_name, str,NULL ); +}
I think the rte_bus_from_name changes should go in a separate patch, as they are not part of the PCI changes.
Content-wise, it looks good though, so keep my ack on both patches if split.
PATCH v5 5/7 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- 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(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
lib/librte_eal/common/eal_common_bus.c +15 -0
@@ -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 ); +}
lib/librte_eal/common/include/rte_bus.h +12 -0
@@ -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); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;
Signature · 1 line
2.1.4
Re: [PATCH v5 5/7] bus: add helper to find a bus from a device name
Quoted 5 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Signed-off-by Gaetan Rivet <[email protected]>---
Jan Blunck
NAKRe: [PATCH v5 5/7] bus: add helper to find a bus from a device name
In reply to Gaetan RivetQuoted 53 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Signed-off-by Gaetan Rivet <[email protected]>--- 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(+)lib/librte_eal/bsdapp/eal/rte_eal_version.map +1
-0@@ -202,5 +202,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05;lib/librte_eal/common/eal_common_bus.c +15
-0@@ -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 ); +}lib/librte_eal/common/include/rte_bus.h +0
-0@@ -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);/**
lib/librte_eal/common/include/rte_bus.h struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev); · line 295
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h +10
-0+* 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.
Quoted 19 lines
lib/librte_eal/common/include/rte_bus.h +2
-0+ +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */ lib/librte_eal/linuxapp/eal/rte_eal_version.map +1
-1@@ -206,5 +206,6 @@ DPDK_17.08 { global: rte_bus_from_name; + rte_bus_from_dev; } DPDK_17.05; --2.1.4
Re: [PATCH v5 5/7] bus: add helper to find a bus from a device name
27/06/2017 20:55, Jan Blunck:
Quoted 13 lines · Gaetan Rivet
Quoted 11 lines
/**
+ * 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.
+ */
+structrte_bus*rte_bus_from_dev(const char *str);
Quoted 4 lines
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.
Re: [PATCH v5 5/7] bus: add helper to find a bus from a device name
Quoted 17 lines · Thomas Monjalon
27/06/2017 20:55, Jan Blunck:
Quoted 16 lines · Gaetan Rivet
Quoted 11 lines
/**
+ * 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.
+ */
+structrte_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.
Quoted 8 lines
Quoted 1 line
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.
Re: [PATCH v5 5/7] bus: add helper to find a bus from a device name
29/06/2017 09:56, Jan Blunck:
Quoted 36 lines · Thomas Monjalon
Quoted 17 lines
27/06/2017 20:55, Jan Blunck:
Quoted 16 lines · Gaetan Rivet
Quoted 11 lines
/**
+ * 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.
+ */
+structrte_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.
Quoted 8 lines
Quoted 1 line
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)
Quoted 8 lines
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?
Re: [PATCH v5 5/7] bus: add helper to find a bus from a device name
In reply to Jan BlunckQuoted 36 lines · Jan Blunck
Quoted 17 lines · Thomas Monjalon
27/06/2017 20:55, Jan Blunck:
Quoted 16 lines · Gaetan Rivet
Quoted 11 lines
/**
+ * 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.
+ */
+structrte_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.
Quoted 8 lines
Quoted 1 line
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)
Quoted 8 lines
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.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v5 5/7] bus: add helper to find a bus from a device name
Quoted 88 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Gaëtan Rivet
Sent: Thursday, June 29, 2017 11:24 AM
To: Jan Blunck <[email protected]>
Cc: Thomas Monjalon <[email protected]>; dev <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v5 5/7] bus: add helper to find a bus from a device nameQuoted 1 line · Jan Blunck
On Wed, Jun 28, 2017 at 7:03 PM, Thomas Monjalon <[email protected]>
Quoted 2 lines · wrote:
Quoted 2 lines
27/06/2017 20:55, Jan Blunck:
Quoted 1 line
On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet
Quoted 33 lines · <[email protected]> wrote:
Quoted 15 lines
Quoted 15 lines
Quoted 11 lines
/**
+ * 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.
+ */
+structrte_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.
Quoted 8 lines
Quoted 1 line
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)Quoted 8 lines
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.
Quoted 4 lines
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
PATCH v5 6/7 vdev: expose bus name
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_vdev.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -404,6 +404,6 @@ static void rte_vdev_bus_register(void)return ; registered =1 ; - rte_vdev_bus.name =RTE_STR (virtual); + rte_vdev_bus.name =VIRTUAL_BUS_NAME ;rte_bus_register (&rte_vdev_bus); }
lib/librte_eal/common/include/rte_vdev.h +2 -0
@@ -41,6 +41,8 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> +#define VIRTUAL_BUS_NAME "virtual" +struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
PATCH v5 7/7 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 15 ++++++++++++++- lib/librte_eal/common/eal_common_vdev.c | 6 ++++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 21 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +14 -1
@@ -83,6 +83,8 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) { struct rte_devargs *devargs = NULL; +const char *dev = devargs_str; +struct rte_bus *bus;char *buf =NULL ;int ret;
@@ -94,7 +96,18 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) + bus =rte_bus_from_name (dev); +if (bus) { + dev +=strlen (bus->name) +1 ; + }else { + bus =rte_bus_from_dev (dev); +if (!bus) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus; +if (rte_eal_parse_devargs_str (dev, &buf, &devargs->args)) goto fail;switch (devargs->type) {
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -182,6 +182,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_from_name (VIRTUAL_BUS_NAME );if (args) devargs->args =strdup (args);
@@ -294,12 +295,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_from_name (VIRTUAL_BUS_NAME ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
Quoted 31 lines · Gaetan Rivet
Following the evolutions announced in [1], here is the first part of the
rte_devargsrework planned for 17.08. The rationale has been partially explained in [2].This first part covers the introduction of the necessary facilities in
rte_busto allow for generic device parsing. This API is implemented for the virtual and PCI buses. Additionally, thisrte_busevolution is being used withinrte_devargsto 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_busto help bus recognition:-
rte_bus_from_name
-rte_bus_from_devThese 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 modifiedrte_devargsparsing 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.0This 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.
Quoted 9 lines
[1]: http://dpdk.org/ml/archives/dev/2017-May/065634.html
[2]: http://dpdk.org/ml/archives/dev/2017-May/065670.htmlThis patchset depends on:
bus: attach / detach API
http://dpdk.org/ml/archives/dev/2017-May/066330.html
http://dpdk.org/dev/patchwork/patch/24489/
Just to note there are some errors on apply with this set after applying the latest version of the dependent patches. Nothing that's hard to fix, mind - this set just needs a new version after the attach/detach API set is applied.
/Bruce
v6 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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.
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
This explicit bus designation is optional; no evolution is currently forced on users to migrate to this new format. The separating character is arbitrary.
[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 and hotplug rework
http://dpdk.org/ml/archives/dev/2017-June/069643.html
http://dpdk.org/dev/patchwork/patch/26135/
v1 -> v2:
* fix PCI parse implementation
v2 --> v3:
* Rebase the series on the new plug / unplug API
v3 --> v4:
* Several bug fixes, commit log rewrite.
* Follow the changes to the plug / unplug API.
v5:
* Do not verify bus name on bus registration.
Actually, a legal bus name is not defined anymore.
The bus/device separator in a device declaration can be anything, as long as it is a single character.
The behavior is otherwise unchanged.
v6:
* Rebase upon new hotplug patchset version.
* Rename rte_bus_from_dev as rte_bus_find_by_device_name. This function is now private to the EAL.
* Remove rte_bus_from_name, as it has been integrated as rte_bus_find_by_name by Jan Blunck.
* Reduce parsing ambiguity in rte_devargs.
Gaetan Rivet (6):
bus: fix bus name registration
bus: introduce parsing functionality
vdev: implement parse bus operation
pci: implement parse bus operation
bus: add helper to find a bus from a device name
devargs: parse bus info
lib/librte_eal/common/eal_common_bus.c | 15 +++++++ lib/librte_eal/common/eal_common_devargs.c | 42 +++++++++++++++---- lib/librte_eal/common/eal_common_pci.c | 15 +++++++ lib/librte_eal/common/eal_common_vdev.c | 64 +++++++++++++++-------------- lib/librte_eal/common/eal_private.h | 12 ++++++ lib/librte_eal/common/include/rte_bus.h | 23 ++++++++++- lib/librte_eal/common/include/rte_devargs.h | 3 ++ 7 files changed, 135 insertions(+), 39 deletions(-)
Signature · 1 line
2.1.4
PATCH v6 1/6 bus: fix bus name registration
The default bus registration function should not result in buses registering with double quotes within their names.
--- lib/librte_eal/common/include/rte_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_eal/common/include/rte_bus.h +1 -1
@@ -258,7 +258,7 @@ struct rte_bus *rte_bus_find_by_name(const char *busname);#define RTE_REGISTER_BUS (nm, bus) \ static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ {\ - (bus).name = RTE_STR(nm);\ + (bus).name = nm;\ rte_bus_register(&bus); \ }
Signature · 1 line
2.1.4
PATCH 1/1 bus: fix driver registration
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.
There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it was businitfn_PCI_BUS_NAME instead of businitfn_pci.
The bus registration macro is also changed to use
the new RTE_INIT_PRIO macro, similar to RTE_INIT used for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.
---
This patch is a proposal to replace the patch
"bus: fix bus name registration" in the series "rte_bus parse API".
--- drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 --- lib/librte_eal/common/eal_common_pci.c | 2 +- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_bus.h | 3 ++- lib/librte_eal/common/include/rte_eal.h | 3 +++ lib/librte_eal/common/include/rte_pci.h | 3 --- lib/librte_eal/common/include/rte_vdev.h | 2 -- 8 files changed, 8 insertions(+), 12 deletions(-)
drivers/bus/fslmc/fslmc_bus.c +1 -1
@@ -161,4 +161,4 @@ struct rte_fslmc_bus rte_fslmc_bus = { .driver_list = TAILQ_HEAD_INITIALIZER(rte_fslmc_bus.driver_list), }; -RTE_REGISTER_BUS (FSLMC_BUS_NAME , rte_fslmc_bus.bus); +RTE_REGISTER_BUS (fslmc, rte_fslmc_bus.bus);
drivers/bus/fslmc/rte_fslmc.h +0 -3
@@ -56,9 +56,6 @@ extern "C" {#include <rte_dev.h> #include <rte_bus.h> -/** Name of FSLMC Bus */ -#define FSLMC_BUS_NAME "FSLMC" -struct rte_dpaa2_driver ;/* DPAA2 Device and Driver lists for FSLMC bus */
lib/librte_eal/common/eal_common_pci.c +1 -1
@@ -549,4 +549,4 @@ struct rte_pci_bus rte_pci_bus = { .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), }; -RTE_REGISTER_BUS (PCI_BUS_NAME , rte_pci_bus.bus); +RTE_REGISTER_BUS (pci, rte_pci_bus.bus);
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -374,4 +374,4 @@ static struct rte_bus rte_vdev_bus = { .unplug = vdev_unplug, }; -RTE_REGISTER_BUS (VIRTUAL_BUS_NAME , rte_vdev_bus); +RTE_REGISTER_BUS (vdev, rte_vdev_bus);
lib/librte_eal/common/include/rte_bus.h +2 -1
@@ -256,7 +256,8 @@ struct rte_bus *rte_bus_find_by_name(const char *busname);* The constructor has higher priority than PMD constructors. */ #define RTE_REGISTER_BUS (nm, bus) \ -static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ +RTE_INIT_PRIO(businitfn_ ##nm, 101); \ +static void businitfn_ ##nm(void) \ {\ (bus).name = RTE_STR(nm);\ rte_bus_register(&bus); \
lib/librte_eal/common/include/rte_eal.h +3 -0
@@ -286,6 +286,9 @@ static inline int rte_gettid(void)#define RTE_INIT (func) \ static void __attribute__((constructor, used)) func(void) +#define RTE_INIT_PRIO (func, prio) \ +static void __attribute__((constructor(prio), used)) func(void) + #ifdef __cplusplus }#endif
lib/librte_eal/common/include/rte_pci.h +0 -3
@@ -77,9 +77,6 @@ const char *pci_get_sysfs_path(void);/** Maximum number of PCI resources. */ #define PCI_MAX_RESOURCE 6 -/** Name of PCI Bus */ -#define PCI_BUS_NAME "PCI" -/* Forward declarations */ struct rte_pci_device ;struct rte_pci_driver ;
lib/librte_eal/common/include/rte_vdev.h +0 -2
@@ -41,8 +41,6 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> -#define VIRTUAL_BUS_NAME "virtual" -struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.13.1
Re: [PATCH] bus: fix driver registration
Quoted 24 lines · Thomas Monjalon
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it wasbusinitfn_PCI_BUS_NAMEinstead ofbusinitfn_pci.The bus registration macro is also changed to use
the newRTE_INIT_PRIOmacro, similar toRTE_INITused for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.Fixes 0fd1a0eaae19 ("pci: add bus driver")Fixes fea892e35f21 ("bus/vdev: use standard bus registration")Fixes 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")Signed-off-by Thomas Monjalon <[email protected]>---
This patch is a proposal to replace the patch
"bus: fix bus name registration" in the series "rte_busparse API".
Quoted 11 lines
--- drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 --- lib/librte_eal/common/eal_common_pci.c | 2 +- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_bus.h | 3 ++- lib/librte_eal/common/include/rte_eal.h | 3 +++ lib/librte_eal/common/include/rte_pci.h | 3 --- lib/librte_eal/common/include/rte_vdev.h | 2 -- 8 files changed, 8 insertions(+), 12 deletions(-)
For DPAA2 as well as generic change:
Re: [PATCH] bus: fix driver registration
Quoted 36 lines · Shreyansh Jain
Quoted 24 lines · Thomas Monjalon
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it wasbusinitfn_PCI_BUS_NAMEinstead ofbusinitfn_pci.The bus registration macro is also changed to use
the newRTE_INIT_PRIOmacro, similar toRTE_INITused for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.Fixes 0fd1a0eaae19 ("pci: add bus driver")Fixes fea892e35f21 ("bus/vdev: use standard bus registration")Fixes 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")Signed-off-by Thomas Monjalon <[email protected]>---
This patch is a proposal to replace the patch
"bus: fix bus name registration" in the series "rte_busparse API".Quoted 11 lines
--- drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 --- lib/librte_eal/common/eal_common_pci.c | 2 +- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_bus.h | 3 ++- lib/librte_eal/common/include/rte_eal.h | 3 +++ lib/librte_eal/common/include/rte_pci.h | 3 --- lib/librte_eal/common/include/rte_vdev.h | 2 -- 8 files changed, 8 insertions(+), 12 deletions(-)
Quoted 4 lines
For DPAA2 as well as generic change:
Acked-by Shreyansh Jain <[email protected]>
I just noticed that Bruce has already Acked the patch which is super-seeded by this. I didn't mean to conflict here.
I don't have a reservation to either of these - but, I like this patch as it does away with priority as well as need for drivers to define an internal name strings. Thus, my ACK to this.
PATCH v6 2/6 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -148,6 +168,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find a device on the bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
PATCH v6 3/6 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 58 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +29 -29
@@ -102,6 +102,27 @@ static char *parse_driver_arg(const char *args) }static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver =NULL ; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (strncmp (driver->driver.name, name, +strlen (driver->driver.name)) ==0 ) +break ; +if (driver->driver.alias && +strncmp (driver->driver.alias, name, +strlen (driver->driver.alias)) ==0 ) +break ; + } +if (driver !=NULL && + addr !=NULL ) + *out = driver; +return driver ==NULL ; +} + +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {const char *name;
@@ -115,36 +136,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } - } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -372,6 +371,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, /* .plug = NULL, see comment on vdev_unplug */ .unplug = vdev_unplug, + .parse = vdev_parse, };RTE_REGISTER_BUS (VIRTUAL_BUS_NAME , rte_vdev_bus);
Signature · 1 line
2.1.4
PATCH v6 4/6 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
lib/librte_eal/common/eal_common_pci.c +15 -0
@@ -448,6 +448,20 @@ rte_pci_dump(FILE *f) } } +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +bool parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr !=NULL ) + *out = pci_addr; +return parse == false; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -544,6 +558,7 @@ struct rte_pci_bus rte_pci_bus = { .find_device = pci_find_device, .plug = pci_plug, .unplug = pci_unplug, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH v6 5/6 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- lib/librte_eal/common/eal_common_bus.c | 15 +++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 27 insertions(+)
lib/librte_eal/common/eal_common_bus.c +15 -0
@@ -204,3 +204,18 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +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_find_by_device_name (const char *str) +{ +return rte_bus_find (NULL , bus_can_parse, str); +}
lib/librte_eal/common/eal_private.h +12 -0
@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* 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_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
Re: [PATCH v6 5/6] bus: add helper to find a bus from a device name
I made this API private, however we can see the bonding PMD could use this kind of facility:
http://dpdk.org/ml/archives/dev/2017-July/070087.html
I know the failsafe could leverage this, as well as another bus idea I threw together a few weeks ago[1].
So maybe I should make it public? This can always be fixed past-integration though, and I'd prefer being able to go forward on the PCI move.
[1]: http://dpdk.org/ml/archives/dev/2017-June/067546.html
Quoted 56 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_bus.c | 15 +++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 27 insertions(+)lib/librte_eal/common/eal_common_bus.c +15
-0@@ -204,3 +204,18 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +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_find_by_device_name (const char *str) +{ +return rte_bus_find (NULL , bus_can_parse, str); +}lib/librte_eal/common/eal_private.h +12
-0@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* 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_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */ Signature · 1 line
2.1.4
Signature · 2 lines
Gaëtan Rivet 6WIND
PATCH v6 6/6 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++----- lib/librte_eal/common/eal_common_vdev.c | 6 +++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +35 -7
@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name)); +} +/* store a whitelist parameter for later parsing */ int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ; -char *buf =NULL ; +struct rte_bus *bus =NULL ; +char *dev =NULL ; +const char *devname;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) +if (rte_eal_parse_devargs_str (devargs_str, &dev, &devargs->args)) goto fail; + devname = dev; +do { + bus =rte_bus_find (bus, bus_name_cmp, dev); +if (bus ==NULL ) +break ; + devname = dev +strlen (bus->name) +1 ; +if (rte_bus_find_by_device_name (devname) == bus) +break ; + devname = dev; + }while (1 ); +if (bus ==NULL ) { + bus =rte_bus_find_by_device_name (devname); +if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus;switch (devargs->type) {case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;break ;case RTE_DEVTYPE_VIRTUAL :/* save driver name */ ret =snprintf (devargs->virt.drv_name, -sizeof (devargs->virt.drv_name),"%s" , buf); +sizeof (devargs->virt.drv_name),"%s" , devname);if (ret <0 || ret >= (int )sizeof (devargs->virt.drv_name)) goto fail;break ; } -free (buf); +free (dev);TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ; fail: -free (buf); +free (dev);if (devargs) {free (devargs->args);free (devargs);
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_find_by_name (VIRTUAL_BUS_NAME );if (args) devargs->args =strdup (args);
@@ -289,12 +290,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_find_by_name (VIRTUAL_BUS_NAME ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
v7 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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.
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
This explicit bus designation is optional; no evolution is currently forced on users to migrate to this new format. The separating character is arbitrary.
[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 and hotplug rework
http://dpdk.org/ml/archives/dev/2017-June/069643.html
http://dpdk.org/dev/patchwork/patch/26135/
v1 -> v2:
* fix PCI parse implementation
v2 --> v3:
* Rebase the series on the new plug / unplug API
v3 --> v4:
* Several bug fixes, commit log rewrite.
* Follow the changes to the plug / unplug API.
v5:
* Do not verify bus name on bus registration.
Actually, a legal bus name is not defined anymore.
The bus/device separator in a device declaration can be anything, as long as it is a single character.
The behavior is otherwise unchanged.
v6:
* Rebase upon new hotplug patchset version.
* Rename rte_bus_from_dev as rte_bus_find_by_device_name. This function is now private to the EAL.
* Remove rte_bus_from_name, as it has been integrated as rte_bus_find_by_name by Jan Blunck.
* Reduce parsing ambiguity in rte_devargs.
v7:
* Use Thomas' bus registration fix.
* Improve rte_bus_find_by_device_name, by making more resistant to shaky bus parse implementation.
Gaetan Rivet (5):
bus: introduce parsing functionality
vdev: implement parse bus operation
pci: implement parse bus operation
bus: add helper to find a bus from a device name
devargs: parse bus info
Thomas Monjalon (1):
bus: fix driver registration
drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 -- lib/librte_eal/common/eal_common_bus.c | 21 +++++++++ lib/librte_eal/common/eal_common_devargs.c | 42 +++++++++++++++--- lib/librte_eal/common/eal_common_pci.c | 17 +++++++- lib/librte_eal/common/eal_common_vdev.c | 66 +++++++++++++++-------------- lib/librte_eal/common/eal_private.h | 12 ++++++ lib/librte_eal/common/include/rte_bus.h | 24 ++++++++++- lib/librte_eal/common/include/rte_devargs.h | 3 ++ lib/librte_eal/common/include/rte_eal.h | 3 ++ lib/librte_eal/common/include/rte_pci.h | 3 -- lib/librte_eal/common/include/rte_vdev.h | 2 - 12 files changed, 148 insertions(+), 50 deletions(-)
Signature · 1 line
2.1.4
PATCH v7 1/6 bus: fix driver registration
From: Thomas Monjalon <[email protected]>
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.
There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it was businitfn_PCI_BUS_NAME instead of businitfn_pci.
The bus registration macro is also changed to use
the new RTE_INIT_PRIO macro, similar to RTE_INIT used for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.
--- drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 --- lib/librte_eal/common/eal_common_pci.c | 2 +- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_bus.h | 3 ++- lib/librte_eal/common/include/rte_eal.h | 3 +++ lib/librte_eal/common/include/rte_pci.h | 3 --- lib/librte_eal/common/include/rte_vdev.h | 2 -- 8 files changed, 8 insertions(+), 12 deletions(-)
drivers/bus/fslmc/fslmc_bus.c +1 -1
@@ -161,4 +161,4 @@ struct rte_fslmc_bus rte_fslmc_bus = { .driver_list = TAILQ_HEAD_INITIALIZER(rte_fslmc_bus.driver_list), }; -RTE_REGISTER_BUS (FSLMC_BUS_NAME , rte_fslmc_bus.bus); +RTE_REGISTER_BUS (fslmc, rte_fslmc_bus.bus);
drivers/bus/fslmc/rte_fslmc.h +0 -3
@@ -56,9 +56,6 @@ extern "C" {#include <rte_dev.h> #include <rte_bus.h> -/** Name of FSLMC Bus */ -#define FSLMC_BUS_NAME "FSLMC" -struct rte_dpaa2_driver ;/* DPAA2 Device and Driver lists for FSLMC bus */
lib/librte_eal/common/eal_common_pci.c +1 -1
@@ -549,4 +549,4 @@ struct rte_pci_bus rte_pci_bus = { .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), }; -RTE_REGISTER_BUS (PCI_BUS_NAME , rte_pci_bus.bus); +RTE_REGISTER_BUS (pci, rte_pci_bus.bus);
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -374,4 +374,4 @@ static struct rte_bus rte_vdev_bus = { .unplug = vdev_unplug, }; -RTE_REGISTER_BUS (VIRTUAL_BUS_NAME , rte_vdev_bus); +RTE_REGISTER_BUS (vdev, rte_vdev_bus);
lib/librte_eal/common/include/rte_bus.h +2 -1
@@ -256,7 +256,8 @@ struct rte_bus *rte_bus_find_by_name(const char *busname);* The constructor has higher priority than PMD constructors. */ #define RTE_REGISTER_BUS (nm, bus) \ -static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ +RTE_INIT_PRIO(businitfn_ ##nm, 101); \ +static void businitfn_ ##nm(void) \ {\ (bus).name = RTE_STR(nm);\ rte_bus_register(&bus); \
lib/librte_eal/common/include/rte_eal.h +3 -0
@@ -286,6 +286,9 @@ static inline int rte_gettid(void)#define RTE_INIT (func) \ static void __attribute__((constructor, used)) func(void) +#define RTE_INIT_PRIO (func, prio) \ +static void __attribute__((constructor(prio), used)) func(void) + #ifdef __cplusplus }#endif
lib/librte_eal/common/include/rte_pci.h +0 -3
@@ -77,9 +77,6 @@ const char *pci_get_sysfs_path(void);/** Maximum number of PCI resources. */ #define PCI_MAX_RESOURCE 6 -/** Name of PCI Bus */ -#define PCI_BUS_NAME "PCI" -/* Forward declarations */ struct rte_pci_device ;struct rte_pci_driver ;
lib/librte_eal/common/include/rte_vdev.h +0 -2
@@ -41,8 +41,6 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> -#define VIRTUAL_BUS_NAME "virtual" -struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
Re: [PATCH v7 1/6] bus: fix driver registration
Quoted 18 lines · Gaetan Rivet
From: Thomas Monjalon <[email protected]>
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it wasbusinitfn_PCI_BUS_NAMEinstead ofbusinitfn_pci.The bus registration macro is also changed to use
the newRTE_INIT_PRIOmacro, similar toRTE_INITused for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.
Quoted 6 lines
Fixes 0fd1a0eaae19 ("pci: add bus driver")Fixes fea892e35f21 ("bus/vdev: use standard bus registration")Fixes 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")Signed-off-by Thomas Monjalon <[email protected]>---
Though I had already acked it in v6,
(from DPAA2 (bus/fslmc) perspective, as well as generically):
-
Shreyansh
Re: [PATCH v7 1/6] bus: fix driver registration
In reply to Gaetan RivetQuoted 18 lines · Gaetan Rivet
From: Thomas Monjalon <[email protected]>
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it wasbusinitfn_PCI_BUS_NAMEinstead ofbusinitfn_pci.The bus registration macro is also changed to use
the newRTE_INIT_PRIOmacro, similar toRTE_INITused for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.
Quoted 6 lines
Fixes 0fd1a0eaae19 ("pci: add bus driver")Fixes fea892e35f21 ("bus/vdev: use standard bus registration")Fixes 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")Signed-off-by Thomas Monjalon <[email protected]>---
LGTM, Reviewed-by: Santosh Shukla <[email protected]>
PATCH v7 2/6 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -148,6 +168,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find a device on the bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
Re: [PATCH v7 2/6] bus: introduce parsing functionality
Quoted 6 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Signed-off-by Gaetan Rivet <[email protected]>---
Re: [PATCH v7 2/6] bus: introduce parsing functionality
In reply to Gaetan RivetHi Gaetan,
Quoted 20 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)lib/librte_eal/common/include/rte_bus.h +3
-0@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific
lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 153
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h +10
-0+* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +*
r / is not a error/ is valid?
lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 159
Quoted 6 lines
lib/librte_eal/common/include/rte_bus.h +6
-0+* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); +
_parse handler in _common_vdev or common_pci file return boolean value i.e..0 for success and 1 for error, right? if so then
!0 for any error would be like '1' for error case.. make sense?
Re: [PATCH v7 2/6] bus: introduce parsing functionality
Quoted 20 lines · santosh
Hi Gaetan,
Quoted 16 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)lib/librte_eal/common/include/rte_bus.h +0
-0@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);
Quoted 17 lines
Quoted 4 lines
lib/librte_eal/common/include/rte_bus.h +3
-0/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 153
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h +10
-0+* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* r / is not a error/ is valid?
I'm partial to "is not an error" here, but it doesn't matter that much and I can change it if you prefer.
Quoted 11 lines
lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 159
Quoted 6 lines
lib/librte_eal/common/include/rte_bus.h +6
-0+* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); +
_parsehandler in_common_vdevorcommon_pcifile return boolean value i.e..0 for success and 1 for error, right? if so then
!0 for any error would be like '1' for error case.. make sense?
I thought of that yes, and actually your suggestion was the first version I used.
Ultimately however, this function is not only saying "can parse": it is not merely a test of being able to process the input, but also the process itself. The test value is then a byproduct.
As such, I decided to settle on the standard "0 means nothing of note happened, carry on".
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v7 2/6] bus: introduce parsing functionality
Quoted 53 lines · Gaëtan Rivet
Quoted 20 lines · santosh
Hi Gaetan,
Quoted 16 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)lib/librte_eal/common/include/rte_bus.h +0
-0@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);Quoted 16 lines
Quoted 4 lines
lib/librte_eal/common/include/rte_bus.h +3
-0/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 153
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h +10
-0+* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* r / is not a error/ is valid?
I'm partial to "is not an error" here, but it doesn't matter that much and I can change it if you prefer.
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 159
Quoted 6 lines
lib/librte_eal/common/include/rte_bus.h +6
-0+* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); +
_parsehandler in_common_vdevorcommon_pcifile return boolean value i.e..0 for success and 1 for error, right? if so then
!0 for any error would be like '1' for error case.. make sense?I thought of that yes, and actually your suggestion was the first version I used.
Quoted 6 lines
Ultimately however, this function is not only saying "can parse": it is not merely a test of being able to process the input, but also the process itself. The test value is then a byproduct.
As such, I decided to settle on the standard "0 means nothing of note happened, carry on".
I'm not aware of past work history, catching up with stuff so no strong opinion but In my opinion: if we're sure about return values then better mention them explicitly.
Thanks.
Re: [PATCH v7 2/6] bus: introduce parsing functionality
Quoted 59 lines · santosh
Quoted 57 lines · Gaëtan Rivet
Quoted 20 lines · santosh
Hi Gaetan,
Quoted 16 lines · Gaetan Rivet
This operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)lib/librte_eal/common/include/rte_bus.h +0
-0@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);Quoted 16 lines
Quoted 4 lines
lib/librte_eal/common/include/rte_bus.h +3
-0/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 153
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h +10
-0+* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* r / is not a error/ is valid?
I'm partial to "is not an error" here, but it doesn't matter that much and I can change it if you prefer.
Quoted 10 lines
lib/librte_eal/common/include/rte_bus.h typedef int (*rte_bus_plug_t)(struct rte_device *dev, · line 159
Quoted 6 lines
lib/librte_eal/common/include/rte_bus.h +6
-0+* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); +
_parsehandler in_common_vdevorcommon_pcifile return boolean value i.e..0 for success and 1 for error, right? if so then
!0 for any error would be like '1' for error case.. make sense?I thought of that yes, and actually your suggestion was the first version I used.
Ultimately however, this function is not only saying "can parse": it is not merely a test of being able to process the input, but also the process itself. The test value is then a byproduct.
Quoted 7 lines
Quoted 2 lines
As such, I decided to settle on the standard "0 means nothing of note happened, carry on".
I'm not aware of past work history, catching up with stuff so no strong opinion but In my opinion: if we're sure about return values then better mention them explicitly.
That's the way it has been implemented in vdev and pci, but other buses might want to use internal helpers in their version, that might return something else than 1 on error.
As such, I think it's better to push the weakest specification sufficient to match our needs and not force unnecessary hoops on developers downstream.
Quoted 3 lines
Thanks.
Signature · 2 lines
Gaëtan Rivet 6WIND
PATCH v7 3/6 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 58 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +29 -29
@@ -102,6 +102,27 @@ static char *parse_driver_arg(const char *args) }static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver =NULL ; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (strncmp (driver->driver.name, name, +strlen (driver->driver.name)) ==0 ) +break ; +if (driver->driver.alias && +strncmp (driver->driver.alias, name, +strlen (driver->driver.alias)) ==0 ) +break ; + } +if (driver !=NULL && + addr !=NULL ) + *out = driver; +return driver ==NULL ; +} + +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {const char *name;
@@ -115,36 +136,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } - } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -372,6 +371,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, /* .plug = NULL, see comment on vdev_unplug */ .unplug = vdev_unplug, + .parse = vdev_parse, };RTE_REGISTER_BUS (vdev, rte_vdev_bus);
Signature · 1 line
2.1.4
Re: [PATCH v7 3/6] vdev: implement parse bus operation
Quoted 2 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>---
Re: [PATCH v7 3/6] vdev: implement parse bus operation
In reply to Gaetan RivetQuoted 2 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>---
LGTM, Reviewed-by: Santosh Shukla <[email protected]>
PATCH v7 4/6 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
lib/librte_eal/common/eal_common_pci.c +15 -0
@@ -448,6 +448,20 @@ rte_pci_dump(FILE *f) } } +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +bool parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr !=NULL ) + *out = pci_addr; +return parse == false; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -544,6 +558,7 @@ struct rte_pci_bus rte_pci_bus = { .find_device = pci_find_device, .plug = pci_plug, .unplug = pci_unplug, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH v7 5/6 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 33 insertions(+)
lib/librte_eal/common/eal_common_bus.c +21 -0
@@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +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 ); +} + +struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[32 ]; +char *c; + +snprintf (name,sizeof (name),"%s" , str); + c =strchr (name,',' ); +if (c !=NULL ) + c[0 ] ='\0' ; +return rte_bus_find (NULL , bus_can_parse, name); +}
lib/librte_eal/common/eal_private.h +12 -0
@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* 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_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
Re: [PATCH v7 5/6] bus: add helper to find a bus from a device name
Hello Gaetan,
Some comments inline:
Quoted 20 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 33 insertions(+)lib/librte_eal/common/eal_common_bus.c +3
-0@@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name)
lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 219
Quoted 10 lines
lib/librte_eal/common/eal_common_bus.c +10
-0+{ +const char *name = _name; + +return !(bus->parse && bus->parse (name,NULL ) ==0 ); +} + +struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[32 ];
It is possible to use a constant here? Basically, I am not sure why '32' has been chosen - or maybe, it might remind a reader in future the reason for this value.
Just to clarify: is there any documented limit on bus name? Until this point, the name (and length) of bus was responsibility of bus driver implementation. eal_common_bus.c doesn't codify any limit - so, this may have to be advertised, even if just within the code.
lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 223
Quoted 4 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+char *c; + +snprintf (name,sizeof (name),"%s" , str); + c =strchr (name,',' );
I saw a lot of discussion on the naming assumptions/presumptions. Though, I am not sure I have an understood conclusion from that. So, this might be incorrect/ill-informed:
Is it assumed that ',' is not present in device name?
Do you think it would be better if this is documented that ',' in a device name is reserved (as per devargs)? (or, is this already known?)
For example, in case of DPAA2 devices, I didn't consider this as a case while generating names while scanning the bus (though, I didn't have a ',' in the name). But, if a well known fact, bus drivers can normalize their device names before creating device names.
Quoted 16 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+if (c !=NULL ) + c[0 ] ='\0' ; +return rte_bus_find (NULL , bus_can_parse, name); +}lib/librte_eal/common/eal_private.h +3
-0@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* Find a bus capable of identifying a device. +*
lib/librte_eal/common/eal_private.h int rte_eal_hugepage_attach(void); · line 353
Quoted 10 lines
lib/librte_eal/common/eal_private.h +9
-0+* @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_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */
(Apologies for commenting so late in series - feel free to ignore if this disrupts your cycle or sounds out-of-context.)
-
Shreyansh
Re: [PATCH v7 5/6] bus: add helper to find a bus from a device name
Hi Shreyansh,
Quoted 40 lines · Shreyansh Jain
Hello Gaetan,
Some comments inline:
Quoted 20 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 33 insertions(+)lib/librte_eal/common/eal_common_bus.c +3
-0@@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name)lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 219
Quoted 10 lines
lib/librte_eal/common/eal_common_bus.c +10
-0+{ +const char *name = _name; + +return !(bus->parse && bus->parse (name,NULL ) ==0 ); +} + +struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[32 ];It is possible to use a constant here? Basically, I am not sure why '32' has been chosen - or maybe, it might remind a reader in future the reason for this value.
Quoted 5 lines
Just to clarify: is there any documented limit on bus name? Until this point, the name (and length) of bus was responsibility of bus driver implementation.
eal_common_bus.cdoesn't codify any limit - so, this may have to be advertised, even if just within the code.
I agree that this is not clear. First thing however: this is a limit on the device name length, not bus.
This problem is fixed in [1], as a single common device name location is defined. The problem is that it is in another patchset (even if both are closely linked and were only separated to ease integration).
This limit was currently taken from the arbitrary limit of device in name in the original rte_devargs for virtual devices. I think it could be exported as a define by rte_dev.h, used there and reused in [1] to define the new rte_devargs structure.
Would that be ok with you?
[1]: http://dpdk.org/ml/archives/dev/2017-July/070225.html
Quoted 11 lines
lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 223
Quoted 4 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+char *c; + +snprintf (name,sizeof (name),"%s" , str); + c =strchr (name,',' );I saw a lot of discussion on the naming assumptions/presumptions. Though, I am not sure I have an understood conclusion from that.
So, this might be incorrect/ill-informed:Is it assumed that ',' is not present in device name?
Quoted 8 lines
Do you think it would be better if this is documented that ',' in a device name is reserved (as per devargs)? (or, is this already known?)
For example, in case of DPAA2 devices, I didn't consider this as a case while generating names while scanning the bus (though, I didn't have a ',' in the name). But, if a well known fact, bus drivers can normalize their device names before creating device names.
For this release cycle the discussion was mostly about forbidding (or not forbidding) characters in bus names. This character is one limitation previously defined for device names, to separate the device name from its kvargs.
I am thus following the current rte_devargs API.
Not sure if that's documented. There are examples in doc, but maybe this device name limitation is not explicitly said.
Quoted 20 lines
Quoted 20 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+if (c !=NULL ) + c[0 ] ='\0' ; +return rte_bus_find (NULL , bus_can_parse, name); +}lib/librte_eal/common/eal_private.h +9
-0@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* 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.
Quoted 9 lines
lib/librte_eal/common/eal_private.h int rte_eal_hugepage_attach(void);
Quoted 5 lines
lib/librte_eal/common/eal_private.h +3
-0+*/ +struct rte_bus *rte_bus_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */ (Apologies for commenting so late in series - feel free to ignore if this disrupts your cycle or sounds out-of-context.)
No problem, thanks for reviewing so quickly this patchset.
Quoted 2 lines
-
Shreyansh
Signature · 2 lines
Gaëtan Rivet 6WIND
Shreyansh Jain
Acked-byRe: [PATCH v7 5/6] bus: add helper to find a bus from a device name
Quoted 43 lines · Gaëtan Rivet
Hi Shreyansh,
Quoted 40 lines · Shreyansh Jain
Hello Gaetan,
Some comments inline:
Quoted 20 lines · Gaetan Rivet
Find which bus should be able to parse this device name into an internal device representation.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 33 insertions(+)lib/librte_eal/common/eal_common_bus.c +3
-0@@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +static int +bus_can_parse (const struct rte_bus *bus,const void *_name)lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 219
Quoted 10 lines
lib/librte_eal/common/eal_common_bus.c +10
-0+{ +const char *name = _name; + +return !(bus->parse && bus->parse (name,NULL ) ==0 ); +} + +struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[32 ];It is possible to use a constant here? Basically, I am not sure why '32' has been chosen - or maybe, it might remind a reader in future the reason for this value.
Quoted 8 lines
Quoted 5 lines
Just to clarify: is there any documented limit on bus name? Until this point, the name (and length) of bus was responsibility of bus driver implementation.
eal_common_bus.cdoesn't codify any limit - so, this may have to be advertised, even if just within the code.I agree that this is not clear. First thing however: this is a limit on the device name length, not bus.
Ah, yes, I see that now. I mixed up.
Quoted 4 lines
This problem is fixed in [1], as a single common device name location is defined. The problem is that it is in another patchset (even if both are closely linked and were only separated to ease integration).
Ok. I hadn't yet gone through it yet - so, didn't notice.
I will look through that as well.
But, if this is already taken care of in that series, consider my comment resolved.
Quoted 7 lines
This limit was currently taken from the arbitrary limit of device in name in the original
rte_devargsfor virtual devices. I think it could be exported as a define byrte_dev.h, used there and reused in [1] to define the newrte_devargsstructure.Would that be ok with you?
For the purpose of this patch, this is OK.
I will read through that patch series and comment if there is something I have comments on. Thanks for highlighting.
Quoted 23 lines
[1]: http://dpdk.org/ml/archives/dev/2017-July/070225.html
Quoted 11 lines
lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 223
Quoted 4 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+char *c; + +snprintf (name,sizeof (name),"%s" , str); + c =strchr (name,',' );I saw a lot of discussion on the naming assumptions/presumptions. Though, I am not sure I have an understood conclusion from that.
So, this might be incorrect/ill-informed:Is it assumed that ',' is not present in device name?
Quoted 8 lines
Do you think it would be better if this is documented that ',' in a device name is reserved (as per devargs)? (or, is this already known?)
For example, in case of DPAA2 devices, I didn't consider this as a case while generating names while scanning the bus (though, I didn't have a ',' in the name). But, if a well known fact, bus drivers can normalize their device names before creating device names.
Quoted 8 lines
For this release cycle the discussion was mostly about forbidding (or not forbidding) characters in bus names. This character is one limitation previously defined for device names, to separate the device name from its kvargs.
I am thus following the current
rte_devargsAPI.
Not sure if that's documented. There are examples in doc, but maybe this device name limitation is not explicitly said.
I agree. There doesn't seem to be much devargs related documentation anyways. (is there?)
If someone uses a ',' name (ignorantly), they would get a failure from this function because of partial name match.
But its worth documenting - now that EAL (Bus scan) allows device an arbitrary naming (no more just PCI and VDEV). (I will try to write something and push as patch - that is, if I have enough understanding of devargs subsystem.)
Quoted 31 lines
Quoted 21 lines
Quoted 21 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+if (c !=NULL ) + c[0 ] ='\0' ; +return rte_bus_find (NULL , bus_can_parse, name); +}lib/librte_eal/common/eal_private.h +9
-0@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* 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. Quoted 9 lines
lib/librte_eal/common/eal_private.h int rte_eal_hugepage_attach(void);
Quoted 5 lines
lib/librte_eal/common/eal_private.h +3
-0+*/ +struct rte_bus *rte_bus_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */ (Apologies for commenting so late in series - feel free to ignore if this disrupts your cycle or sounds out-of-context.)
Quoted 5 lines
No problem, thanks for reviewing so quickly this patchset.
Quoted 2 lines
-
Shreyansh
Other than the 'device name length issue', which would be revisited in other patch series:
Re: [PATCH v7 5/6] bus: add helper to find a bus from a device name
In reply to Gaëtan Rivet05/07/2017 15:45, Gaëtan Rivet:
Quoted 18 lines · Shreyansh Jain
Quoted 9 lines
lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 219
Quoted 4 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[32 ];It is possible to use a constant here? Basically, I am not sure why '32' has been chosen - or maybe, it might remind a reader in future the reason for this value.
Quoted 4 lines
Just to clarify: is there any documented limit on bus name? Until this point, the name (and length) of bus was responsibility of bus driver implementation.
eal_common_bus.cdoesn't codify any limit - so, this may have to be advertised, even if just within the code.I agree that this is not clear. First thing however: this is a limit on the device name length, not bus.
Quoted 8 lines
This problem is fixed in [1], as a single common device name location is defined. The problem is that it is in another patchset (even if both are closely linked and were only separated to ease integration).
This limit was currently taken from the arbitrary limit of device in name in the original
rte_devargsfor virtual devices. I think it could be exported as a define byrte_dev.h, used there and reused in [1] to define the newrte_devargsstructure.
Please check this patch which was integrated yesterday:
http://dpdk.org/commit/48d8675c9cf
Now you can use RTE_DEV_NAME_MAX_LEN instead of hardcoded 32.
Re: [PATCH v7 5/6] bus: add helper to find a bus from a device name
Quoted 23 lines · Thomas Monjalon
05/07/2017 15:45, Gaëtan Rivet:
Quoted 22 lines · Shreyansh Jain
Quoted 9 lines
lib/librte_eal/common/eal_common_bus.c rte_bus_find_by_name(const char *busname) · line 219
Quoted 4 lines
lib/librte_eal/common/eal_common_bus.c +4
-0+struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[32 ];It is possible to use a constant here? Basically, I am not sure why '32' has been chosen - or maybe, it might remind a reader in future the reason for this value.
Quoted 4 lines
Just to clarify: is there any documented limit on bus name? Until this point, the name (and length) of bus was responsibility of bus driver implementation.
eal_common_bus.cdoesn't codify any limit - so, this may have to be advertised, even if just within the code.I agree that this is not clear. First thing however: this is a limit on the device name length, not bus.
This problem is fixed in [1], as a single common device name location is defined. The problem is that it is in another patchset (even if both are closely linked and were only separated to ease integration).
Quoted 9 lines
Quoted 4 lines
This limit was currently taken from the arbitrary limit of device in name in the original
rte_devargsfor virtual devices. I think it could be exported as a define byrte_dev.h, used there and reused in [1] to define the newrte_devargsstructure.Please check this patch which was integrated yesterday:
http://dpdk.org/commit/48d8675c9cfNow you can use
RTE_DEV_NAME_MAX_LENinstead of hardcoded 32.
Yes I just saw that while rebasing, it's exactly what I needed. It will be in the next version.
Signature · 2 lines
Gaëtan Rivet 6WIND
PATCH v7 6/6 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++----- lib/librte_eal/common/eal_common_vdev.c | 6 +++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +35 -7
@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name)); +} +/* store a whitelist parameter for later parsing */ int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ; -char *buf =NULL ; +struct rte_bus *bus =NULL ; +char *dev =NULL ; +const char *devname;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) +if (rte_eal_parse_devargs_str (devargs_str, &dev, &devargs->args)) goto fail; + devname = dev; +do { + bus =rte_bus_find (bus, bus_name_cmp, dev); +if (bus ==NULL ) +break ; + devname = dev +strlen (bus->name) +1 ; +if (rte_bus_find_by_device_name (devname) == bus) +break ; + devname = dev; + }while (1 ); +if (bus ==NULL ) { + bus =rte_bus_find_by_device_name (devname); +if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus;switch (devargs->type) {case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;break ;case RTE_DEVTYPE_VIRTUAL :/* save driver name */ ret =snprintf (devargs->virt.drv_name, -sizeof (devargs->virt.drv_name),"%s" , buf); +sizeof (devargs->virt.drv_name),"%s" , devname);if (ret <0 || ret >= (int )sizeof (devargs->virt.drv_name)) goto fail;break ; } -free (buf); +free (dev);TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ; fail: -free (buf); +free (dev);if (devargs) {free (devargs->args);free (devargs);
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_find_by_name ("vdev" );if (args) devargs->args =strdup (args);
@@ -289,12 +290,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_find_by_name ("vdev" ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
Re: [PATCH v7 6/6] devargs: parse bus info
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 134
Quoted 7 lines · On Wed, 5 Jul 2017 01:55:23 +0200 Gaetan Rivet <[email protected]> wrote:
lib/librte_eal/common/eal_common_devargs.c +1
-2case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;
Shouldn't these go under bus args for PCI?
It would be good to get all the bus specific args out of generic code.
Re: [PATCH v7 6/6] devargs: parse bus info
In reply to Gaetan RivetHello Gaetan,
Quoted 12 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++----- lib/librte_eal/common/eal_common_vdev.c | 6 +++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-)lib/librte_eal/common/eal_common_devargs.c +0
-0
lib/librte_eal/common/eal_common_devargs.c rte_eal_parse_devargs_str(const char *devargs_str, · line 87
Quoted 10 lines
lib/librte_eal/common/eal_common_devargs.c +7
-0@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name));
Trivial: Any specific reason why this is split across multiple lines even though it is less than 80 chars in total?
Quoted 15 lines
lib/librte_eal/common/eal_common_devargs.c +5
-1+} +/* store a whitelist parameter for later parsing */ int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ; -char *buf =NULL ; +struct rte_bus *bus =NULL ; +char *dev =NULL ; +const char *devname;int ret;/* use malloc instead of rte_malloc as it's called early at init */
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 108
Quoted 5 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) +if (rte_eal_parse_devargs_str (devargs_str, &dev, &devargs->args))
A very basic (and possibly incorrect) question:
here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" was passed to application, which would mean;
dev = "net_pcap0" (name of the device) devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 115
Quoted 7 lines
lib/librte_eal/common/eal_common_devargs.c +6
-0goto fail; + devname = dev; + do { + bus = rte_bus_find(bus, bus_name_cmp, dev); + if (bus == NULL) + break; + devname = dev + strlen(bus->name) + 1;
Assuming "vdev" bus:
"net_pcap0"
|devname points here --------' (4+1) chars from start of "net_pcap0". Is that the expectation?
Probably I am missing something here (maybe the input already has a bus name.)
Or, if this is not the case, then we will have to change the test application (test_devargs.c) which passes such strings to "rte_eal_devargs_add".
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 116
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c +1
-0+if (rte_bus_find_by_device_name (devname) == bus)
Obviously, if my assumption is correct, this fails. Or, maybe I am completely off the mark.
Quoted 21 lines
lib/librte_eal/common/eal_common_devargs.c +12
-2+break ; + devname = dev; + }while (1 ); +if (bus ==NULL ) { + bus =rte_bus_find_by_device_name (devname); +if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus;switch (devargs->type) {case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;break ;
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 145
Quoted 10 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1case RTE_DEVTYPE_VIRTUAL :/* save driver name */ ret =snprintf (devargs->virt.drv_name, -sizeof (devargs->virt.drv_name),"%s" , buf); +sizeof (devargs->virt.drv_name),"%s" , devname);if (ret <0 || ret >= (int )sizeof (devargs->virt.drv_name)) goto fail;break ; }
I think all this will change in the devargs series.
Quoted 54 lines
lib/librte_eal/common/eal_common_devargs.c +2
-2-free (buf); +free (dev);TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ; fail: -free (buf); +free (dev);if (devargs) {free (devargs->args);free (devargs);lib/librte_eal/common/eal_common_vdev.c +4
-2@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_find_by_name ("vdev" );if (args) devargs->args =strdup (args);@@ -289,12 +290,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_find_by_name ("vdev" ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);lib/librte_eal/common/include/rte_devargs.h +1
-0@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 97
Quoted 8 lines
lib/librte_eal/common/include/rte_devargs.h +2
-0@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
-
Shreyansh
Re: [PATCH v7 6/6] devargs: parse bus info
Quoted 19 lines · Shreyansh Jain
Hello Gaetan,
Quoted 16 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++----- lib/librte_eal/common/eal_common_vdev.c | 6 +++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-)lib/librte_eal/common/eal_common_devargs.c +2
-0@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +static int +bus_name_cmp (const struct rte_bus *bus,const void *_name)
Quoted 9 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_parse_devargs_str(const char *devargs_str, · line 87
Quoted 5 lines
lib/librte_eal/common/eal_common_devargs.c +5
-0+{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name));Trivial: Any specific reason why this is split across multiple lines even though it is less than 80 chars in total?
Not really, it's only a matter of taste.
If is mostly to hightlight that we are limiting the comparison to bus->name length, by putting it on its own line.
Quoted 36 lines
Quoted 14 lines
lib/librte_eal/common/eal_common_devargs.c +5
-1+} +/* store a whitelist parameter for later parsing */ int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ; -char *buf =NULL ; +struct rte_bus *bus =NULL ; +char *dev =NULL ; +const char *devname;int ret;/* use malloc instead of rte_malloc as it's called early at init */ lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 107
Quoted 4 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) +if (rte_eal_parse_devargs_str (devargs_str, &dev, &devargs->args))A very basic (and possibly incorrect) question:
here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" was passed to application, which would mean;
dev = "
net_pcap0" (name of the device) devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 115
Quoted 7 lines
lib/librte_eal/common/eal_common_devargs.c +6
-0goto fail; + devname = dev; + do { + bus = rte_bus_find(bus, bus_name_cmp, dev); + if (bus == NULL) + break; + devname = dev + strlen(bus->name) + 1;Assuming "vdev" bus:
Quoted 5 lines
"net_pcap0" |devname points here --------' (4+1) chars from start of "
net_pcap0". Is that the expectation?
Yes :)
Well, actually, almost. The lines
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 114
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c +3
-0+ bus =rte_bus_find (bus, bus_name_cmp, dev); +if (bus ==NULL ) +break ;
Mean that at this point, we would already have broken out of the loop. But to continue with your example, assuming that we have a bus that is named "net_p":
Quoted 12 lines
Probably I am missing something here (maybe the input already has a bus name.)
Or, if this is not the case, then we will have to change the test application (
test_devargs.c) which passes such strings to "rte_eal_devargs_add".lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 116
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c +1
-0+if (rte_bus_find_by_device_name (devname) == bus)Obviously, if my assumption is correct, this fails. Or, maybe I am completely off the mark.
It should not be necessary to update the tests (yet).
This bit of code tries to recognize bus names at the start of the declaration. However, some device names might be ambiguous and start like bus names for any reason. Device names have no specification beside not having commas within, bus names have no specification at all.
Thus, we first try here to recognize a bus name within dev, but we do not stop here. We also verify afterward that the resulting device would be correct, and that its bus handler is actually the same as the bus we first recognized.
In your example, the line
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 116
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c +1
-0+if (rte_bus_find_by_device_name (devname) == bus)
Fails, as the device is incorrect and rte_bus_find_by_device_name returns NULL as no bus is able to handle a device named
Quoted 1 line
"cap0,rx_pcap=input.pcap,tx_pcap=output.pcap"
Considering this, we should break from this loop with no recognized bus. As such, we enter afterward in the branch:
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 121
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c +2
-0+break ; + devname = dev;
Note here that devname is set back to the start of dev.
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 121
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c +3
-0+ }while (1 ); +if (bus ==NULL ) { + bus =rte_bus_find_by_device_name (devname);
Which will try to recognize a bus from the device name ("net_pcap0"), which should succeed in recognizing vdev.
It is a little contrived, but I wanted this parsing to both be flexible and perform as many checks as possible.
I am developing a new bus that have a high probability of name conflicts with other device names, so I am extra careful on this side.
Quoted 18 lines
Quoted 18 lines
lib/librte_eal/common/eal_common_devargs.c +7
-2+if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus;switch (devargs->type) {case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;break ;case RTE_DEVTYPE_VIRTUAL :/* save driver name */ ret = snprintf(devargs->virt.drv_name,
Quoted 9 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 145
Quoted 6 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1- sizeof(devargs->virt.drv_name), "%s", buf); + sizeof(devargs->virt.drv_name), "%s", devname);if (ret <0 || ret >= (int )sizeof (devargs->virt.drv_name)) goto fail;break ; }I think all this will change in the devargs series.
Indeed :)
Quoted 56 lines
Quoted 46 lines
lib/librte_eal/common/eal_common_devargs.c +2
-2-free (buf); +free (dev);TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ; fail: -free (buf); +free (dev);if (devargs) {free (devargs->args);free (devargs);lib/librte_eal/common/eal_common_vdev.c +4
-2@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_find_by_name ("vdev" );if (args) devargs->args =strdup (args);@@ -289,12 +290,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_find_by_name ("vdev" ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);lib/librte_eal/common/include/rte_devargs.h +1
-0@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 97
Quoted 8 lines
lib/librte_eal/common/include/rte_devargs.h +2
-0@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };-
Shreyansh
Signature · 2 lines
Gaëtan Rivet 6WIND
Shreyansh Jain
Acked-byRe: [PATCH v7 6/6] devargs: parse bus info
Hello Gaetan,
Quoted 30 lines · Gaëtan Rivet
Quoted 19 lines · Shreyansh Jain
Hello Gaetan,
Quoted 16 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++----- lib/librte_eal/common/eal_common_vdev.c | 6 +++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-)lib/librte_eal/common/eal_common_devargs.c +2
-0@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +static int +bus_name_cmp (const struct rte_bus *bus,const void *_name)Quoted 9 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_parse_devargs_str(const char *devargs_str, · line 87
Quoted 5 lines
lib/librte_eal/common/eal_common_devargs.c +5
-0+{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name));Trivial: Any specific reason why this is split across multiple lines even though it is less than 80 chars in total?
Quoted 3 lines
Not really, it's only a matter of taste.
If is mostly to hightlight that we are limiting the comparison tobus->namelength, by putting it on its own line.
Ok. I am ok with this.
Quoted 103 lines
Quoted 37 lines
Quoted 15 lines
lib/librte_eal/common/eal_common_devargs.c +5
-1+} +/* store a whitelist parameter for later parsing */ int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ; -char *buf =NULL ; +struct rte_bus *bus =NULL ; +char *dev =NULL ; +const char *devname;int ret;/* use malloc instead of rte_malloc as it's called early at init */ lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 107
Quoted 4 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) +if (rte_eal_parse_devargs_str (devargs_str, &dev, &devargs->args))A very basic (and possibly incorrect) question:
here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" was passed to application, which would mean;
dev = "
net_pcap0" (name of the device) devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 115
Quoted 7 lines
lib/librte_eal/common/eal_common_devargs.c +6
-0goto fail; + devname = dev; + do { + bus = rte_bus_find(bus, bus_name_cmp, dev); + if (bus == NULL) + break; + devname = dev + strlen(bus->name) + 1;Assuming "vdev" bus:
Quoted 5 lines
"net_pcap0" |devname points here --------' (4+1) chars from start of "
net_pcap0". Is that the expectation?Yes :)
Well, actually, almost. The linesQuoted 3 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 114
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c +3
-0+ bus =rte_bus_find (bus, bus_name_cmp, dev); +if (bus ==NULL ) +break ;Mean that at this point, we would already have broken out of the loop. But to continue with your example, assuming that we have a bus that is named "
net_p":Quoted 12 lines
Probably I am missing something here (maybe the input already has a bus name.)
Or, if this is not the case, then we will have to change the test application (
test_devargs.c) which passes such strings to "rte_eal_devargs_add".lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 116
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c +1
-0+if (rte_bus_find_by_device_name (devname) == bus)Obviously, if my assumption is correct, this fails. Or, maybe I am completely off the mark.
It should not be necessary to update the tests (yet).
This bit of code tries to recognize bus names at the start of the declaration. However, some device names might be ambiguous and start like bus names for any reason. Device names have no specification beside not having commas within, bus names have no specification at all.Thus, we first try here to recognize a bus name within dev, but we do not stop here. We also verify afterward that the resulting device would be correct, and that its bus handler is actually the same as the bus we first recognized.
In your example, the line
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 116
Quoted 1 line
lib/librte_eal/common/eal_common_devargs.c +1
-0+if (rte_bus_find_by_device_name (devname) == bus)Fails, as the device is incorrect and
rte_bus_find_by_device_namereturns NULL as no bus is able to handle a device namedQuoted 1 line
"cap0,rx_pcap=input.pcap,tx_pcap=output.pcap"
Considering this, we should break from this loop with no recognized bus. As such, we enter afterward in the branch:
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 121
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c +2
-0+break ; + devname = dev;Note here that devname is set back to the start of dev.
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 121
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c +3
-0+ }while (1 ); +if (bus ==NULL ) { + bus =rte_bus_find_by_device_name (devname);Which will try to recognize a bus from the device name ("
net_pcap0"), which should succeed in recognizing vdev.
Quoted 5 lines
It is a little contrived, but I wanted this parsing to both be flexible and perform as many checks as possible.
I am developing a new bus that have a high probability of name conflicts with other device names, so I am extra careful on this side.
Ok. I completely missed this logic.
Thanks.
Quoted 29 lines
Quoted 19 lines
Quoted 19 lines
lib/librte_eal/common/eal_common_devargs.c +7
-2+if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus;switch (devargs->type) {case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;break ;case RTE_DEVTYPE_VIRTUAL :/* save driver name */ ret = snprintf(devargs->virt.drv_name,Quoted 9 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) · line 145
Quoted 6 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1- sizeof(devargs->virt.drv_name), "%s", buf); + sizeof(devargs->virt.drv_name), "%s", devname);if (ret <0 || ret >= (int )sizeof (devargs->virt.drv_name)) goto fail;break ; }I think all this will change in the devargs series.
Quoted 59 lines
Indeed :)
Quoted 56 lines
Quoted 46 lines
lib/librte_eal/common/eal_common_devargs.c +2
-2-free (buf); +free (dev);TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ; fail: -free (buf); +free (dev);if (devargs) {free (devargs->args);free (devargs);lib/librte_eal/common/eal_common_vdev.c +4
-2@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_find_by_name ("vdev" );if (args) devargs->args =strdup (args);@@ -289,12 +290,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_find_by_name ("vdev" ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);lib/librte_eal/common/include/rte_devargs.h +1
-0@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 97
Quoted 8 lines
lib/librte_eal/common/include/rte_devargs.h +2
-0@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[32 ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };-
Shreyansh
And, for this patch as well:
v8 cover rte_bus parse API
In reply to Gaetan RivetFollowing 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.
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
This explicit bus designation is optional; no evolution is currently forced on users to migrate to this new format. The separating character is arbitrary.
[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 and hotplug rework
http://dpdk.org/ml/archives/dev/2017-June/069643.html
http://dpdk.org/dev/patchwork/patch/26135/
v1 -> v2:
* fix PCI parse implementation
v2 --> v3:
* Rebase the series on the new plug / unplug API
v3 --> v4:
* Several bug fixes, commit log rewrite.
* Follow the changes to the plug / unplug API.
v5:
* Do not verify bus name on bus registration.
Actually, a legal bus name is not defined anymore.
The bus/device separator in a device declaration can be anything, as long as it is a single character.
The behavior is otherwise unchanged.
v6:
* Rebase upon new hotplug patchset version.
* Rename rte_bus_from_dev as rte_bus_find_by_device_name. This function is now private to the EAL.
* Remove rte_bus_from_name, as it has been integrated as rte_bus_find_by_name by Jan Blunck.
* Reduce parsing ambiguity in rte_devargs.
v7:
* Use Thomas' bus registration fix.
* Improve rte_bus_find_by_device_name, by making more resistant to shaky bus parse implementation.
v8:
* Fix a few incorrect bus name references.
* Add a few acks.
* Clarify the standing of the bus parse API in its introductory commit log.
Gaetan Rivet (5):
bus: introduce parsing functionality
vdev: implement parse bus operation
pci: implement parse bus operation
bus: add helper to find a bus from a device name
devargs: parse bus info
Thomas Monjalon (1):
bus: fix driver registration
drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 -- lib/librte_eal/common/eal_common_bus.c | 23 +++++++++- lib/librte_eal/common/eal_common_dev.c | 2 +- lib/librte_eal/common/eal_common_devargs.c | 42 +++++++++++++++--- lib/librte_eal/common/eal_common_pci.c | 17 +++++++- lib/librte_eal/common/eal_common_vdev.c | 66 +++++++++++++++-------------- lib/librte_eal/common/eal_private.h | 12 ++++++ lib/librte_eal/common/include/rte_bus.h | 24 ++++++++++- lib/librte_eal/common/include/rte_devargs.h | 3 ++ lib/librte_eal/common/include/rte_eal.h | 3 ++ lib/librte_eal/common/include/rte_pci.h | 3 -- lib/librte_eal/common/include/rte_vdev.h | 2 - 13 files changed, 150 insertions(+), 52 deletions(-)
Signature · 1 line
2.1.4
PATCH v8 1/6 bus: fix driver registration
From: Thomas Monjalon <[email protected]>
The bus name was stored with embedded double quotes. Indeed the bus name is given with a string in a macro, which is not used elsewhere.
These macros are useless because the buses are drivers, so they must not have any API for the application writer. The registration can be done with a hardcoded value without quotes.
There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it was businitfn_PCI_BUS_NAME instead of businitfn_pci.
The bus registration macro is also changed to use
the new RTE_INIT_PRIO macro, similar to RTE_INIT used for other drivers. The priority is the highest (101) in order to be sure that the bus driver is registered before its device drivers.
--- drivers/bus/fslmc/fslmc_bus.c | 2 +- drivers/bus/fslmc/rte_fslmc.h | 3 --- lib/librte_eal/common/eal_common_bus.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 2 +- lib/librte_eal/common/eal_common_pci.c | 2 +- lib/librte_eal/common/eal_common_vdev.c | 2 +- lib/librte_eal/common/include/rte_bus.h | 3 ++- lib/librte_eal/common/include/rte_eal.h | 3 +++ lib/librte_eal/common/include/rte_pci.h | 3 --- lib/librte_eal/common/include/rte_vdev.h | 2 -- 10 files changed, 10 insertions(+), 14 deletions(-)
drivers/bus/fslmc/fslmc_bus.c +1 -1
@@ -161,4 +161,4 @@ struct rte_fslmc_bus rte_fslmc_bus = { .driver_list = TAILQ_HEAD_INITIALIZER(rte_fslmc_bus.driver_list), }; -RTE_REGISTER_BUS (FSLMC_BUS_NAME , rte_fslmc_bus.bus); +RTE_REGISTER_BUS (fslmc, rte_fslmc_bus.bus);
drivers/bus/fslmc/rte_fslmc.h +0 -3
@@ -56,9 +56,6 @@ extern "C" {#include <rte_dev.h> #include <rte_bus.h> -/** Name of FSLMC Bus */ -#define FSLMC_BUS_NAME "FSLMC" -struct rte_dpaa2_driver ;/* DPAA2 Device and Driver lists for FSLMC bus */
lib/librte_eal/common/eal_common_bus.c +1 -1
@@ -92,7 +92,7 @@ rte_bus_probe(void)struct rte_bus *bus, *vbus =NULL ; TAILQ_FOREACH(bus, &rte_bus_list, next) { -if (!strcmp (bus->name,"virtual" )) { +if (!strcmp (bus->name,"vdev" )) { vbus = bus;continue ; }
lib/librte_eal/common/eal_common_dev.c +1 -1
@@ -74,7 +74,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs)return -EINVAL ; } - ret =rte_eal_hotplug_add ("PCI" , name, devargs); + ret =rte_eal_hotplug_add ("pci" , name, devargs);if (ret && ret != -EINVAL )return ret;
lib/librte_eal/common/eal_common_pci.c +1 -1
@@ -549,4 +549,4 @@ struct rte_pci_bus rte_pci_bus = { .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), }; -RTE_REGISTER_BUS (PCI_BUS_NAME , rte_pci_bus.bus); +RTE_REGISTER_BUS (pci, rte_pci_bus.bus);
lib/librte_eal/common/eal_common_vdev.c +1 -1
@@ -374,4 +374,4 @@ static struct rte_bus rte_vdev_bus = { .unplug = vdev_unplug, }; -RTE_REGISTER_BUS (VIRTUAL_BUS_NAME , rte_vdev_bus); +RTE_REGISTER_BUS (vdev, rte_vdev_bus);
lib/librte_eal/common/include/rte_bus.h +2 -1
@@ -256,7 +256,8 @@ struct rte_bus *rte_bus_find_by_name(const char *busname);* The constructor has higher priority than PMD constructors. */ #define RTE_REGISTER_BUS (nm, bus) \ -static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ +RTE_INIT_PRIO(businitfn_ ##nm, 101); \ +static void businitfn_ ##nm(void) \ {\ (bus).name = RTE_STR(nm);\ rte_bus_register(&bus); \
lib/librte_eal/common/include/rte_eal.h +3 -0
@@ -286,6 +286,9 @@ static inline int rte_gettid(void)#define RTE_INIT (func) \ static void __attribute__((constructor, used)) func(void) +#define RTE_INIT_PRIO (func, prio) \ +static void __attribute__((constructor(prio), used)) func(void) + #ifdef __cplusplus }#endif
lib/librte_eal/common/include/rte_pci.h +0 -3
@@ -77,9 +77,6 @@ const char *pci_get_sysfs_path(void);/** Maximum number of PCI resources. */ #define PCI_MAX_RESOURCE 6 -/** Name of PCI Bus */ -#define PCI_BUS_NAME "PCI" -/* Forward declarations */ struct rte_pci_device ;struct rte_pci_driver ;
lib/librte_eal/common/include/rte_vdev.h +0 -2
@@ -41,8 +41,6 @@ extern "C" {#include <rte_dev.h> #include <rte_devargs.h> -#define VIRTUAL_BUS_NAME "virtual" -struct rte_vdev_device {TAILQ_ENTRY (rte_vdev_device) next;/**< Next attached vdev */ struct rte_device device;/**< Inherit core device */
Signature · 1 line
2.1.4
PATCH v8 2/6 bus: introduce parsing functionality
In reply to Gaetan RivetThis operation can be used either to validate that a device representation can be understood by a bus, as well as store the resulting specialized device representation in any format determined by the bus.
Implementing this function allows EAL initialization routines to infer which bus should handle a device. This is used as a way to respect backward compatibility.
This API will disappear once this compatibility is not enforced anymore.
--- lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
lib/librte_eal/common/include/rte_bus.h +21 -0
@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,typedef int (*rte_bus_unplug_t )(struct rte_device *dev);/** +* Bus specific parsing function. +* Validates the syntax used in the textual representation of a device, +* If the syntax is valid and ``addr`` is not NULL, writes the bus-specific +* device representation to ``addr``. +* +* @param[in] name +* device textual description +* +* @param[out] addr +* device information location address, into which parsed info +* should be written. If NULL, nothing should be written, which +* is not an error. +* +* @return +* 0 if parsing was successful. +* !0 for any error. +*/ +typedef int (*rte_bus_parse_t )(const char *name,void *addr); + +/** * A structure describing a generic bus. */ struct rte_bus {
@@ -148,6 +168,7 @@ struct rte_bus {rte_bus_find_device_t find_device;/**< Find a device on the bus */ rte_bus_plug_t plug;/**< Probe single device for drivers */ rte_bus_unplug_t unplug;/**< Remove single device from driver */ +rte_bus_parse_t parse;/**< Parse a device name */ };/**
Signature · 1 line
2.1.4
PATCH v8 3/6 vdev: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_vdev.c | 58 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +29 -29
@@ -102,6 +102,27 @@ static char *parse_driver_arg(const char *args) }static int +vdev_parse (const char *name,void *addr) +{ +struct rte_vdev_driver **out = addr; +struct rte_vdev_driver *driver =NULL ; + + TAILQ_FOREACH(driver, &vdev_driver_list, next) { +if (strncmp (driver->driver.name, name, +strlen (driver->driver.name)) ==0 ) +break ; +if (driver->driver.alias && +strncmp (driver->driver.alias, name, +strlen (driver->driver.alias)) ==0 ) +break ; + } +if (driver !=NULL && + addr !=NULL ) + *out = driver; +return driver ==NULL ; +} + +static int vdev_probe_all_drivers (struct rte_vdev_device *dev) {const char *name;
@@ -115,36 +136,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)RTE_LOG (DEBUG ,EAL ,"Search driver %s to probe device %s\n" , name,rte_vdev_device_name (dev)); - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -/* -* search a driver prefix in virtual device name. -* For example, if the driver is pcap PMD, driver->name -* will be "net_pcap", but "name" will be "net_pcapN". -* So use strncmp to compare. -*/ -if (!strncmp (driver->driver.name, name, -strlen (driver->driver.name))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; - goto out; - } - } - -/* Give new names precedence over aliases. */ - TAILQ_FOREACH(driver, &vdev_driver_list, next) { -if (driver->driver.alias && - !strncmp (driver->driver.alias, name, -strlen (driver->driver.alias))) { - dev->device.driver = &driver->driver; - ret = driver->probe (dev); -if (ret) - dev->device.driver =NULL ; -break ; - } +if (vdev_parse (name, &driver)) { + ret =-1 ; + goto out; } - + dev->device.driver = &driver->driver; + ret = driver->probe (dev); +if (ret) + dev->device.driver =NULL ; out:free (drv_name);return ret;
@@ -372,6 +371,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = vdev_find_device, /* .plug = NULL, see comment on vdev_unplug */ .unplug = vdev_unplug, + .parse = vdev_parse, };RTE_REGISTER_BUS (vdev, rte_vdev_bus);
Signature · 1 line
2.1.4
PATCH v8 4/6 pci: implement parse bus operation
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_pci.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
lib/librte_eal/common/eal_common_pci.c +15 -0
@@ -448,6 +448,20 @@ rte_pci_dump(FILE *f) } } +static int +pci_parse (const char *name,void *addr) +{ +struct rte_pci_addr *out = addr; +struct rte_pci_addr pci_addr; +bool parse; + + parse = (eal_parse_pci_BDF (name, &pci_addr) ==0 || +eal_parse_pci_DomBDF (name, &pci_addr) ==0 ); +if (parse && addr !=NULL ) + *out = pci_addr; +return parse == false; +} +/* register a driver */ void rte_pci_register (struct rte_pci_driver *driver)
@@ -544,6 +558,7 @@ struct rte_pci_bus rte_pci_bus = { .find_device = pci_find_device, .plug = pci_plug, .unplug = pci_unplug, + .parse = pci_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
Signature · 1 line
2.1.4
PATCH v8 5/6 bus: add helper to find a bus from a device name
In reply to Gaetan RivetFind which bus should be able to parse this device name into an internal device representation.
--- lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 12 ++++++++++++ 2 files changed, 33 insertions(+)
lib/librte_eal/common/eal_common_bus.c +21 -0
@@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname) {return rte_bus_find (NULL , cmp_bus_name, (const void *)busname); } + +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 ); +} + +struct rte_bus * +rte_bus_find_by_device_name (const char *str) +{ +char name[RTE_DEV_NAME_MAX_LEN ]; +char *c; + +snprintf (name,sizeof (name),"%s" , str); + c =strchr (name,',' ); +if (c !=NULL ) + c[0 ] ='\0' ; +return rte_bus_find (NULL , bus_can_parse, name); +}
lib/librte_eal/common/eal_private.h +12 -0
@@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);*/ bool rte_eal_using_phys_addrs (void ); +/** +* 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_find_by_device_name (const char *str); +#endif /* _EAL_PRIVATE_H_ */
Signature · 1 line
2.1.4
PATCH v8 6/6 devargs: parse bus info
In reply to Gaetan Rivet--- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++----- lib/librte_eal/common/eal_common_vdev.c | 6 +++-- lib/librte_eal/common/include/rte_devargs.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +35 -7
@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name)); +} +/* store a whitelist parameter for later parsing */ int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ; -char *buf =NULL ; +struct rte_bus *bus =NULL ; +char *dev =NULL ; +const char *devname;int ret;/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)memset (devargs,0 ,sizeof (*devargs)); devargs->type = devtype; -if (rte_eal_parse_devargs_str (devargs_str, &buf, &devargs->args)) +if (rte_eal_parse_devargs_str (devargs_str, &dev, &devargs->args)) goto fail; + devname = dev; +do { + bus =rte_bus_find (bus, bus_name_cmp, dev); +if (bus ==NULL ) +break ; + devname = dev +strlen (bus->name) +1 ; +if (rte_bus_find_by_device_name (devname) == bus) +break ; + devname = dev; + }while (1 ); +if (bus ==NULL ) { + bus =rte_bus_find_by_device_name (devname); +if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus info from device declaration\n" ); + goto fail; + } + } + devargs->bus = bus;switch (devargs->type) {case RTE_DEVTYPE_WHITELISTED_PCI :case RTE_DEVTYPE_BLACKLISTED_PCI :/* try to parse pci identifier */ -if (eal_parse_pci_BDF (buf, &devargs->pci.addr) !=0 && -eal_parse_pci_DomBDF (buf, &devargs->pci.addr) !=0 ) +if (bus->parse (devname, &devargs->pci.addr) !=0 ) goto fail;break ;case RTE_DEVTYPE_VIRTUAL :/* save driver name */ ret =snprintf (devargs->virt.drv_name, -sizeof (devargs->virt.drv_name),"%s" , buf); +sizeof (devargs->virt.drv_name),"%s" , devname);if (ret <0 || ret >= (int )sizeof (devargs->virt.drv_name)) goto fail;break ; } -free (buf); +free (dev);TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ; fail: -free (buf); +free (dev);if (devargs) {free (devargs->args);free (devargs);
lib/librte_eal/common/eal_common_vdev.c +4 -2
@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)return NULL ; devargs->type =RTE_DEVTYPE_VIRTUAL ; + devargs->bus =rte_bus_find_by_name ("vdev" );if (args) devargs->args =strdup (args);
@@ -289,12 +290,13 @@ vdev_scan(void) {struct rte_vdev_device *dev;struct rte_devargs *devargs; +struct rte_bus *vbus;/* for virtual devices we scan the devargs_list populated via cmdline */ - + vbus =rte_bus_find_by_name ("vdev" ); TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type !=RTE_DEVTYPE_VIRTUAL ) +if (devargs->bus != vbus)continue ; dev =find_vdev (devargs->virt.drv_name);
lib/librte_eal/common/include/rte_devargs.h +3 -0
@@ -51,6 +51,7 @@ extern "C" {#include <stdio.h> #include <sys/queue.h> #include <rte_pci.h> +#include <rte_bus.h> /** * Type of generic device
@@ -89,6 +90,8 @@ struct rte_devargs {char drv_name[RTE_DEV_NAME_MAX_LEN ]; } virt; }; +/** Bus handle for the device. */ +struct rte_bus *bus;/** Arguments string as given by user or "" for no argument. */ char *args; };
Signature · 1 line
2.1.4
Re: [PATCH v8 6/6] devargs: parse bus info
lib/librte_eal/common/eal_common_devargs.c rte_eal_parse_devargs_str(const char *devargs_str, · line 87
Quoted 7 lines · On Fri, 7 Jul 2017 02:03:12 +0200 Gaetan Rivet <[email protected]> wrote:
lib/librte_eal/common/eal_common_devargs.c +7
-0+static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name));
I don't think you need the _name variable hack. C will cast the argument to strncmp
Re: [PATCH v8 6/6] devargs: parse bus info
Quoted 12 lines · Stephen Hemminger
lib/librte_eal/common/eal_common_devargs.c rte_eal_parse_devargs_str(const char *devargs_str, · line 87
Quoted 7 lines · On Fri, 7 Jul 2017 02:03:12 +0200 Gaetan Rivet <[email protected]> wrote:
lib/librte_eal/common/eal_common_devargs.c +7
-0+static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name));I don't think you need the
_namevariable hack. C will cast the argument to strncmp
You're right, and it was just habit. Usually I prefer giving a type to a generic parameter, but in this case it may be too trivial.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v8 6/6] devargs: parse bus info
07/07/2017 10:31, Gaëtan Rivet:
Quoted 14 lines · Stephen Hemminger
Quoted 12 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_parse_devargs_str(const char *devargs_str, · line 87
Quoted 7 lines · On Fri, 7 Jul 2017 02:03:12 +0200 Gaetan Rivet <[email protected]> wrote:
lib/librte_eal/common/eal_common_devargs.c +7
-0+static int +bus_name_cmp (const struct rte_bus *bus,const void *_name) +{ +const char *name = _name; + +return strncmp (bus->name, name, +strlen (bus->name));I don't think you need the
_namevariable hack. C will cast the argument to strncmp
Quoted 2 lines
You're right, and it was just habit. Usually I prefer giving a type to a generic parameter, but in this case it may be too trivial.
Simplified when applying.
Quoted 9 lines
Gaetan Rivet (5):
bus: introduce parsing functionality
vdev: implement parse bus operation
pci: implement parse bus operation
bus: add helper to find a bus from a device name
devargs: parse bus infoThomas Monjalon (1):
bus: fix driver registration
Series applied with the small change suggested by Stephen, thanks
Quoted 16 lines · Gaetan Rivet
Following the evolutions announced in [1], here is the first part of the
rte_devargsrework planned for 17.08. The rationale has been partially explained in [2].This first part covers the introduction of the necessary facilities in
rte_busto allow for generic device parsing. This API is implemented for the virtual and PCI buses. Additionally, thisrte_busevolution is being used withinrte_devargsto 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_busto help bus recognition:-
rte_bus_from_name
-rte_bus_from_dev
Quoted 9 lines
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 modifiedrte_devargsparsing 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?
Quoted 41 lines
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.htmlThis 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 infolib/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
Quoted 27 lines · Jan Blunck
Quoted 16 lines · Gaetan Rivet
Following the evolutions announced in [1], here is the first part of the
rte_devargsrework planned for 17.08. The rationale has been partially explained in [2].This first part covers the introduction of the necessary facilities in
rte_busto allow for generic device parsing. This API is implemented for the virtual and PCI buses. Additionally, thisrte_busevolution is being used withinrte_devargsto 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_busto help bus recognition:-
rte_bus_from_name
-rte_bus_from_devQuoted 9 lines
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 modifiedrte_devargsparsing 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
Quoted 9 lines
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 43 lines
Quoted 43 lines
[2]: http://dpdk.org/ml/archives/dev/2017-May/065670.html
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.htmlThis 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 infolib/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
Signature · 2 lines
Gaëtan Rivet 6WIND
Quoted 38 lines · Gaëtan Rivet
Quoted 27 lines · Jan Blunck
Quoted 16 lines · Gaetan Rivet
Following the evolutions announced in [1], here is the first part of the
rte_devargsrework planned for 17.08. The rationale has been partially explained in [2].This first part covers the introduction of the necessary facilities in
rte_busto allow for generic device parsing. This API is implemented for the virtual and PCI buses. Additionally, thisrte_busevolution is being used withinrte_devargsto 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_busto help bus recognition:-
rte_bus_from_name
-rte_bus_from_devQuoted 9 lines
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 modifiedrte_devargsparsing 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.0Quoted 9 lines
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?
Quoted 9 lines
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_devargsrework has been to make the same functionality generic to allrte_bus. As seen quickly in [2],rte_devargshas two functions:* Validating a device declaration
* Keeping the relevant device info until it has been processed.
I don't agree with the validation step. This is highly device/driver/bus specific and I don't believe that just because you have created a rte_devargs it is a guarantee that the device is valid. Besides that it makes statically embedding rte_devargs into other structures impossible. As I see it rte_devargs is a key-value list with some keys that are generic. This would make application development much easier.
Quoted 74 lines
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_devargsallocation. 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 40 lines
Quoted 40 lines
[2]: http://dpdk.org/ml/archives/dev/2017-May/065670.html
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.htmlThis 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 infolib/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(-)Quoted 3 lines
Quoted 3 lines
--
2.1.4--
Gaėtan Rivet
6WIND
Quoted 45 lines · Jan Blunck
Quoted 44 lines · Gaëtan Rivet
Quoted 27 lines · Jan Blunck
Quoted 16 lines · Gaetan Rivet
Following the evolutions announced in [1], here is the first part of the
rte_devargsrework planned for 17.08. The rationale has been partially explained in [2].This first part covers the introduction of the necessary facilities in
rte_busto allow for generic device parsing. This API is implemented for the virtual and PCI buses. Additionally, thisrte_busevolution is being used withinrte_devargsto 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_busto help bus recognition:-
rte_bus_from_name
-rte_bus_from_devQuoted 9 lines
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 modifiedrte_devargsparsing 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.0Quoted 9 lines
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_devargsrework has been to make the same functionality generic to allrte_bus. As seen quickly in [2],rte_devargshas two functions:
Quoted 7 lines
Quoted 3 lines
* Validating a device declaration
* Keeping the relevant device info until it has been processed.I don't agree with the validation step. This is highly device/driver/bus specific and I don't believe that just because you have created a
rte_devargsit is a guarantee that the device is valid.
It is the current API. If an rte_devargs is added to the global list, then it means that the embedded information has been validated. I kept the same behavior but made it generic in the new version.
Quoted 2 lines
Besides that it makes statically embedding
rte_devargsinto other structures impossible.
Why? In the failsafe PMD, I statically embed an rte_devargs to avoid having to insert one in the global device list to remove it afterward. What limitation to this do you see? I think I misunderstand what you mean.
Quoted 5 lines
As I see it
rte_devargsis a key-value list
with some keys that are generic. This would make application development much easier.
The previously encoded information was:
- Scan policy / bus (in the type field)
- Device designation (PCI location / virtual driver)
- kvargs
Beside the kvargs, none were generic, so I made them so. I divided the type field into two separate elements as it was conflating two concepts.
I do not understand how much simpler we can make this. The fields have not changed, nor has the information itself. Everything has however been genericized, which is necessary.
Quoted 74 lines
Quoted 74 lines
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_devargsallocation. 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 40 lines
Quoted 40 lines
[2]: http://dpdk.org/ml/archives/dev/2017-May/065670.html
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.htmlThis 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 infolib/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(-)Quoted 3 lines
Quoted 3 lines
--
2.1.4--
Gaėtan Rivet
6WIND
Signature · 2 lines
Gaëtan Rivet 6WIND