From: David Marchand <david.marchand@6wind.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC PATCH 0/6] Restructure EAL device model for bus support
Date: Sun, 20 Nov 2016 16:30:06 +0100 [thread overview]
Message-ID: <CALwxeUuDNdS67YJm3=Ga6vikh=0T8eyou+_jBNO3_HxtanTJxg@mail.gmail.com> (raw)
In-Reply-To: <1479360605-20558-1-git-send-email-shreyansh.jain@nxp.com>
On Thu, Nov 17, 2016 at 6:29 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> DPDK has been inherently a PCI inclined framework. Because of this, the
> design of device tree (or list) within DPDK is also PCI inclined. A non-PCI
> device doesn't have a way of being expressed without using hooks started from
> EAL to PMD.
>
> With this cover letter, some patches are presented which try to break this
> strict linkage of EAL with PCI devices. Aim is to generalize the device
> hierarchy on the lines of how Linux handles it:
>
> device A1
> |
> +==.===='==============.============+ Bus A.
> | `--> driver A11 \
> device A2 `-> driver A12 \______
> |CPU |
> /`````
> device A1 /
> | /
> +==.===='==============.============+ Bus A`
> | `--> driver B11
> device A2 `-> driver B12
>
> Simply put:
> - a bus is connect to CPU (or core)
> - devices are conneted to Bus
> - drivers are running instances which manage one or more devices
> - bus is responsible for identifying devices (and interrupt propogation)
> - driver is responsible for initializing the device
>
> (*Reusing text from email [1])
> In context of DPDK EAL:
> - a generic bus (not a driver, not a device). I don't know how to categorize
> a bus. It is certainly not a device, and then handler for a bus (physical)
> can be considered a 'bus driver'. So, just 'rte_bus'.
> - there is a bus for each physical implementation (or virtual). So, a rte_bus
> Object for PCI, VDEV, ABC, DEF and so on.
> - Buses are registered just like a PMD - RTE_PMD_BUS_REGISTER()
> - Each registered bus is part of a doubly list.
> -- Each device inherits rte_bus
> -- Each driver inherits rte_bus
> -- Device and Drivers lists are part of rte_bus
> - eth_driver is no more required - it was just a holder for PMDs to register
> themselves. It can be replaced with rte_xxx_driver and corresponding init/
> uninit moved to rte_driver
> - rte_eth_dev modified to disassociate itself from rte_pci_device and connect
> to generic rte_device
>
> Once again, improvising from [1]:
>
> __ rte_bus_list
> /
> +----------'---+
> |rte_bus |
> | driver_list------> List of rte_bus specific
> | device_list---- devices
> | scan | `-> List of rte_bus associated
> | match | drivers
> | dump |
> | ..some refcnt| (#)
> +--|------|----+
> _________/ \_________
> +--------/----+ +-\---------------+
> |rte_device | |rte_driver |
> | rte_bus | | rte_bus |
> | rte_driver |(#) | init |
> | | | uninit |
> | devargs | | dev_private_size|
> +---||--------+ | drv_flags |(#)
> || | intr_handle(2*) |(#)
> | \ +----------\\\----+
> | \_____________ \\\
> | \ |||
> +------|---------+ +----|----------+ |||
> |rte_pci_device | |rte_xxx_device | (4*) |||
> | PCI specific | | xxx device | |||
> | info (mem,) | | specific fns | / | \
> +----------------+ +---------------+ / | \
> _____________________/ / \
> / ___/ \
> +-------------'--+ +------------'---+ +--'------------+
> |rte_pci_driver | |rte_vdev_driver | |rte_xxx_driver |
> | PCI id table, | | <probably, | | .... |
> | other driver | | nothing> | +---------------+
> | data | +----------------+
> +----------------+
>
> (1*) Problem is that probe functions have different arguments. So,
> generalizing them might be some rework in the respective device
> layers
> (2*) Interrupt handling for each driver type might be different. I am not
> sure how to generalize that either. This is grey area for me.
> (3*) Probably exposing a bitmask for device capabilities. Nothing similar
> exists now to relate it. Don't know if that is useful. Allowing
> applications to question a device about what it supports and what it
> doesn't - making it more flexible at application layer (but more code
> as well.)
> (4*) Even vdev would be an instantiated as a device. It is not being done
> currently.
> (#) Items which are still pending
>
> With this cover letter, some patches have been posted. These are _only_ for
> discussion purpose. They are not complete and neither compilable.
> All the patches, except 0001, have sufficient context about what the changes
> are and rationale for same. Obviously, code is best answer.
As said by Jan B., I too think that splitting the patches into smaller
patchsets is important.
Looking at your datamodel, some quick comments :
- Why init/uninit in rte_driver ? Why not probe/remove ?
- I don't think dev_private_size makes sense in rte_driver. The bus is
responsible for creating rte_device objects (embedded or not in
rte_$bus_device objects). If a driver needs to allocate some priv (for
whatever needs), the driver should do the allocation itself (or maybe
a helper for current eth_driver), then reference the original
rte_$bus_device object it got from the probe method.
For a first patchset, I would see:
- introduce the rte_bus object. In rte_eal_init, for each bus, we call
the scan method. Then, for each bus, we find the appropriate
rte_driver using the bus match method then call the probe method. If
the probe succeeds, the rte_device points to the associated
rte_driver,
- migrate the pci scan code to a pci bus (scan looks at sysfs for
linux / ioctl for bsd + devargs for blacklist / whitelist ?), match is
the same at what is done in rte_eal_pci_probe_one_driver() at the
moment,
- migrate the vdev init code to a vdev bus (scan looks at devargs):
this is new, we must create rte_device objects for vdev drivers to use
later
Then we can talk about the next steps once the bus is in place.
--
David Marchand
next prev parent reply other threads:[~2016-11-20 15:30 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-17 5:29 Shreyansh Jain
2016-11-17 5:30 ` [dpdk-dev] [RFC PATCH 1/6] eal: define container macro Shreyansh Jain
2016-11-17 12:06 ` Jan Blunck
2016-11-17 13:01 ` Shreyansh Jain
2016-11-17 5:30 ` [dpdk-dev] [RFC PATCH 2/6] eal: introduce bus-device-driver structure Shreyansh Jain
2016-11-17 11:19 ` Jan Blunck
2016-11-17 13:00 ` Shreyansh Jain
2016-11-17 16:13 ` Jan Blunck
2016-11-17 5:30 ` [dpdk-dev] [RFC PATCH 3/6] bus: add bus driver layer Shreyansh Jain
2016-11-17 5:30 ` [dpdk-dev] [RFC PATCH 4/6] eal/common: handle bus abstraction for device/driver objects Shreyansh Jain
2016-11-17 5:30 ` [dpdk-dev] [RFC PATCH 5/6] eal: supporting bus model in init process Shreyansh Jain
2016-11-17 5:30 ` [dpdk-dev] [RFC PATCH 6/6] eal: removing eth_driver Shreyansh Jain
2016-11-17 12:53 ` Jan Blunck
2016-11-18 13:05 ` Shreyansh Jain
2016-11-17 11:55 ` [dpdk-dev] [RFC PATCH 0/6] Restructure EAL device model for bus support Jan Blunck
2016-11-17 13:08 ` Shreyansh Jain
2016-11-17 16:54 ` Jan Blunck
2016-11-20 15:30 ` David Marchand [this message]
2016-11-21 9:08 ` Thomas Monjalon
2016-11-21 10:47 ` Jan Blunck
2016-11-23 9:45 ` 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='CALwxeUuDNdS67YJm3=Ga6vikh=0T8eyou+_jBNO3_HxtanTJxg@mail.gmail.com' \
--to=david.marchand@6wind.com \
--cc=dev@dpdk.org \
--cc=shreyansh.jain@nxp.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).