From: Shreyansh Jain <shreyansh.jain@nxp.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: <david.marchand@6wind.com>, <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v5 01/12] eal/bus: introduce bus abstraction
Date: Fri, 6 Jan 2017 16:01:24 +0530 [thread overview]
Message-ID: <d2513eef-f231-b96e-c763-55d632d8598a@nxp.com> (raw)
In-Reply-To: <12579803.ZHBMo4GdSA@xps13>
On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote:
> 2016-12-26 18:53, Shreyansh Jain:
>> +DPDK_17.02 {
>> + global:
>> +
>> + rte_bus_list;
>> + rte_eal_bus_add_device;
>> + rte_eal_bus_add_driver;
>> + rte_eal_bus_get;
>> + rte_eal_bus_dump;
>> + rte_eal_bus_register;
>> + rte_eal_bus_insert_device;
>> + rte_eal_bus_remove_device;
>> + rte_eal_bus_remove_driver;
>> + rte_eal_bus_unregister;
>
> I think the prefix can be just rte_bus_ instead of rte_eal_bus_.
Ok. I will change these.
I thought as these symbols are part of librte_eal/*, naming should
reflect that.
>
>> +/** Double linked list of buses */
>> +TAILQ_HEAD(rte_bus_list, rte_bus);
>> +
>> +/* Global Bus list */
>> +extern struct rte_bus_list rte_bus_list;
>
> Why the bus list is public?
I will revisit this. When I made it public, intention was that some
element external to EAL (e.g. drivers/bus) might require to iterate
over this list. But, as of now I don't think any such user/case exists
(except test*).
>
>> +/**
>> + * A structure describing a generic bus.
>> + */
>> +struct rte_bus {
>> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
>> + struct rte_driver_list driver_list;
>> + /**< List of all drivers on bus */
>> + struct rte_device_list device_list;
>> + /**< List of all devices on bus */
>> + const char *name; /**< Name of the bus */
>> +};
>
> I am not convinced we should link a generic bus to drivers and devices.
> What do you think of having rte_pci_bus being a rte_bus and linking
> with rte_pci_device and rte_pci_driver lists?
This is different from what I had in mind.
You are saying:
Class: rte_bus
`-> No object instantiated for this class
Class: rte_pci_bus inheriting rte_bus
`-> object instantiated for this class.
Here, rte_bus is being treated as an abstract class which is only
inherited and rte_pci_bus is the base class which is instantiated.
And I was thinking:
Class: rte_bus
`-> Object: pci_bus (defined in */eal/eal_pci.c)
Here, rte_bus is that base class which is instantiated.
I agree that what you are suggesting is inline with current model:
rte_device -> abstract class (no one instantiates it)
`-> rte_pci_device -> Base class which inherits rte_device and
is instantiated.
When I choose not to create rte_pci_bus, it was because I didn't want
another indirection in form of rte_bus->rte_pci_bus->object.
There were no 'non-generic' bus functions which were only applicable for
rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance
of rte_bus.
>
> I'm thinking to something like that:
>
> struct rte_bus {
> TAILQ_ENTRY(rte_bus) next;
> const char *name;
> rte_bus_scan_t scan;
> rte_bus_match_t match;
> };
> struct rte_pci_bus {
> struct rte_bus bus;
> struct rte_pci_driver_list pci_drivers;
> struct rte_pci_device_list pci_devices;
> };
if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is
fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI)
because I don't see any 'non-generic' information in rte_pci_bus which
can't be put in rte_bus.
>
>> +/** Helper for Bus registration. The constructor has higher priority than
>> + * PMD constructors
>> + */
>> +#define RTE_REGISTER_BUS(nm, bus) \
>> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
>> +{\
>> + (bus).name = RTE_STR(nm);\
>> + rte_eal_bus_register(&bus); \
>> +}
>
> By removing the lists from rte_bus as suggested above, do you still need
> a priority for this constructor?
I think yes.
Even if we have rte_pci_bus as a class, object of rte_bus should be part
of Bus list *before* registration of a driver (because, driver
registration searches for bus by name).
(This is assuming that no global PCI/VDEV/XXX bus object is created
which is directly used within all PCI specific bus operations).
There was another suggestion on list which was to check for existence of
bus _within_ each driver registration and create/instantiate an object
in case no bus is registered. I didn't like the approach so I didn't use
it. From David [1], and me [2]
[1] http://dpdk.org/ml/archives/dev/2016-December/051689.html
[2] http://dpdk.org/ml/archives/dev/2016-December/051698.html
>
>> struct rte_device {
>> TAILQ_ENTRY(rte_device) next; /**< Next device */
>> + struct rte_bus *bus; /**< Device connected to this bus */
>> const struct rte_driver *driver;/**< Associated driver */
>> int numa_node; /**< NUMA node connection */
>> struct rte_devargs *devargs; /**< Device user arguments */
>> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>> */
>> struct rte_driver {
>> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
>> + struct rte_bus *bus; /**< Bus serviced by this driver */
>> const char *name; /**< Driver name. */
>> const char *alias; /**< Driver alias. */
>> };
>
> Do we need to know the bus associated to a driver in rte_driver?
> Bus and driver are already associated in rte_device.
>
>
Two reasons:
1/ A driver should be associated with a bus so that if required, all bus
can be directly extracted - even when probing has not been done.
2/ device->driver would only be updated after probe. device->driver->bus
would not be valid in such cases, if required.
Overall, I don't have objections for rte_bus->rte_pci_bus=>object as
compared to rte_bus=>PCI-object. But, I would still like to get a final
confirmation of a more preferred way.
Meanwhile, I will make changes to accommodate this change to save time
in case rte_pci_bus class is final/preferred method.
-
Shreyansh
next prev parent reply other threads:[~2017-01-06 10:27 UTC|newest]
Thread overview: 132+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-04 10:11 [dpdk-dev] [PATCH 00/13] Introducing EAL Bus-Device-Driver Model Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 01/13] eal: define container_of macro Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 02/13] eal/bus: introduce bus abstraction Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 03/13] test: add basic bus infrastructure tests Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 04/13] eal/bus: add scan and match support Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 05/13] eal/bus: add support for inserting a device on a bus Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 06/13] eal: integrate bus scan and probe with EAL Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 07/13] pci: replace probe and remove handlers with rte_driver Shreyansh Jain
2016-12-08 17:50 ` Ferruh Yigit
2016-12-09 4:59 ` Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 08/13] eal: enable probe and remove from bus infrastructure Shreyansh Jain
2016-12-06 10:45 ` Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 09/13] pci: split match and probe function Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 10/13] eal/pci: generalize args of PCI scan/match towards RTE device/driver Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 11/13] pci: Pass rte_pci_addr to functions instead of separate args Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 12/13] eal: enable PCI bus Shreyansh Jain
2016-12-04 10:11 ` [dpdk-dev] [PATCH 13/13] eal/pci: remove PCI probe and init calls Shreyansh Jain
2016-12-06 20:52 ` [dpdk-dev] [PATCH 00/13] Introducing EAL Bus-Device-Driver Model David Marchand
2016-12-07 9:55 ` Shreyansh Jain
2016-12-07 12:17 ` David Marchand
2016-12-07 13:10 ` Shreyansh Jain
2016-12-07 13:24 ` Thomas Monjalon
2016-12-08 5:04 ` Shreyansh Jain
2016-12-08 7:21 ` Thomas Monjalon
2016-12-08 7:53 ` Shreyansh Jain
2016-12-12 14:35 ` Jianbo Liu
2016-12-13 6:56 ` Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 00/12] " Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 01/12] eal: define container_of macro Shreyansh Jain
2016-12-13 22:24 ` Jan Blunck
2016-12-14 5:12 ` Shreyansh Jain
2016-12-16 8:14 ` Jan Blunck
2016-12-16 9:23 ` Adrien Mazarguil
2016-12-16 10:47 ` Jan Blunck
2016-12-16 11:21 ` Adrien Mazarguil
2016-12-16 11:54 ` Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 02/12] eal/bus: introduce bus abstraction Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 03/12] test: add basic bus infrastructure tests Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 04/12] eal/bus: add scan, match and insert support Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 05/12] eal: integrate bus scan and probe with EAL Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 06/12] eal: add probe and remove support for rte_driver Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 07/12] eal: enable probe from bus infrastructure Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 08/12] pci: split match and probe function Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 09/12] eal/pci: generalize args of PCI scan/match towards RTE device/driver Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 10/12] pci: Pass rte_pci_addr to functions instead of separate args Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 11/12] eal: enable PCI bus Shreyansh Jain
2016-12-13 13:37 ` [dpdk-dev] [PATCH v2 12/12] drivers: update PMDs to use rte_driver probe and remove Shreyansh Jain
2016-12-13 13:52 ` Andrew Rybchenko
2016-12-13 15:07 ` Ferruh Yigit
2016-12-14 5:14 ` Shreyansh Jain
2016-12-14 5:11 ` Shreyansh Jain
2016-12-14 9:49 ` Shreyansh Jain
2016-12-15 21:36 ` Jan Blunck
2016-12-26 9:14 ` Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 00/12] Introducing EAL Bus-Device-Driver Model Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 01/12] eal: define container_of macro Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 02/12] eal/bus: introduce bus abstraction Shreyansh Jain
2016-12-20 12:37 ` Hemant Agrawal
2016-12-20 13:17 ` Jan Blunck
2016-12-20 13:51 ` Shreyansh Jain
2016-12-20 17:11 ` Stephen Hemminger
2016-12-21 7:11 ` Shreyansh Jain
2016-12-21 15:38 ` Jan Blunck
2016-12-21 23:33 ` Stephen Hemminger
2016-12-22 5:12 ` Shreyansh Jain
2016-12-22 5:52 ` Shreyansh Jain
2016-12-25 17:39 ` Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 03/12] test: add basic bus infrastructure tests Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 04/12] eal/bus: add scan, match and insert support Shreyansh Jain
2016-12-16 13:25 ` Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 05/12] eal: integrate bus scan and probe with EAL Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 06/12] eal: add probe and remove support for rte_driver Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 07/12] eal: enable probe from bus infrastructure Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 08/12] pci: split match and probe function Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 09/12] eal/pci: generalize args of PCI scan/match towards RTE device/driver Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 10/12] pci: Pass rte_pci_addr to functions instead of separate args Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 11/12] eal: enable PCI bus and PCI test framework Shreyansh Jain
2016-12-16 13:20 ` Shreyansh Jain
2016-12-16 13:10 ` [dpdk-dev] [PATCH v3 12/12] drivers: update PMDs to use rte_driver probe and remove Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 00/12] Introducing EAL Bus-Device-Driver Model Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 01/12] eal/bus: introduce bus abstraction Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 02/12] test: add basic bus infrastructure tests Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 03/12] eal/bus: add scan, match and insert support Shreyansh Jain
2016-12-26 13:27 ` Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 04/12] eal: integrate bus scan and probe with EAL Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 05/12] eal: add probe and remove support for rte_driver Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 06/12] eal: enable probe from bus infrastructure Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 07/12] pci: split match and probe function Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 08/12] eal/pci: generalize args of PCI scan/match towards RTE device/driver Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 09/12] pci: Pass rte_pci_addr to functions instead of separate args Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 10/12] eal: enable PCI bus and PCI test framework Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 11/12] drivers: update PMDs to use rte_driver probe and remove Shreyansh Jain
2016-12-26 12:50 ` [dpdk-dev] [PATCH v4 12/12] eal/bus: add bus iteration macros Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 00/12] Introducing EAL Bus-Device-Driver Model Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 01/12] eal/bus: introduce bus abstraction Shreyansh Jain
2017-01-03 21:52 ` Thomas Monjalon
2017-01-06 10:31 ` Shreyansh Jain [this message]
2017-01-06 14:55 ` Thomas Monjalon
2017-01-09 6:24 ` Shreyansh Jain
2017-01-09 15:22 ` Ferruh Yigit
2017-01-10 4:07 ` Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 02/12] test: add basic bus infrastructure tests Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 03/12] eal/bus: add scan, match and insert support Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 04/12] eal: integrate bus scan and probe with EAL Shreyansh Jain
2017-01-03 21:46 ` Thomas Monjalon
2017-01-06 10:38 ` Shreyansh Jain
2017-01-06 12:00 ` Shreyansh Jain
2017-01-06 13:46 ` Thomas Monjalon
2017-01-09 6:35 ` Shreyansh Jain
2017-01-08 12:21 ` Rosen, Rami
2017-01-09 6:34 ` Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 05/12] eal: add probe and remove support for rte_driver Shreyansh Jain
2017-01-03 22:05 ` Thomas Monjalon
2017-01-06 11:44 ` Shreyansh Jain
2017-01-06 15:26 ` Thomas Monjalon
2017-01-09 6:28 ` Shreyansh Jain
2016-12-26 13:23 ` [dpdk-dev] [PATCH v5 06/12] eal: enable probe from bus infrastructure Shreyansh Jain
2016-12-26 13:24 ` [dpdk-dev] [PATCH v5 07/12] pci: split match and probe function Shreyansh Jain
2017-01-03 22:08 ` Thomas Monjalon
2016-12-26 13:24 ` [dpdk-dev] [PATCH v5 08/12] eal/pci: generalize args of PCI scan/match towards RTE device/driver Shreyansh Jain
2017-01-03 22:13 ` Thomas Monjalon
2017-01-06 12:03 ` Shreyansh Jain
2016-12-26 13:24 ` [dpdk-dev] [PATCH v5 09/12] pci: Pass rte_pci_addr to functions instead of separate args Shreyansh Jain
2016-12-26 13:24 ` [dpdk-dev] [PATCH v5 10/12] eal: enable PCI bus and PCI test framework Shreyansh Jain
2016-12-26 13:24 ` [dpdk-dev] [PATCH v5 11/12] drivers: update PMDs to use rte_driver probe and remove Shreyansh Jain
2017-01-09 15:19 ` Ferruh Yigit
2017-01-09 16:18 ` Ferruh Yigit
2017-01-10 4:09 ` Shreyansh Jain
2016-12-26 13:24 ` [dpdk-dev] [PATCH v5 12/12] eal/bus: add bus iteration macros Shreyansh Jain
2017-01-03 22:15 ` Thomas Monjalon
2017-01-03 22:22 ` [dpdk-dev] [PATCH v5 00/12] Introducing EAL Bus-Device-Driver Model Thomas Monjalon
2017-01-06 6:27 ` Shreyansh Jain
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d2513eef-f231-b96e-c763-55d632d8598a@nxp.com \
--to=shreyansh.jain@nxp.com \
--cc=david.marchand@6wind.com \
--cc=dev@dpdk.org \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).