From: Jan Blunck <jblunck@infradead.org>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev <dev@dpdk.org>, Gaetan Rivet <gaetan.rivet@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v6 05/11] bus: introduce hotplug functionality
Date: Wed, 28 Jun 2017 17:11:46 +0200 [thread overview]
Message-ID: <CALe+Z00bCgi6NQmbswxDnFwnb=fPYCONKzzqcbybr5X9bO2N3w@mail.gmail.com> (raw)
In-Reply-To: <1765263.t0XI3ojoND@xps>
On Wed, Jun 28, 2017 at 3:30 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 28/06/2017 15:09, Jan Blunck:
>> On Wed, Jun 28, 2017 at 2:11 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>> > 28/06/2017 13:58, Jan Blunck:
>> >> On Wed, Jun 28, 2017 at 1:44 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>> >> > 27/06/2017 21:03, Jan Blunck:
>> >> >> On Tue, Jun 27, 2017 at 6:11 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
>> >> >> > --- a/lib/librte_eal/common/include/rte_bus.h
>> >> >> > +++ b/lib/librte_eal/common/include/rte_bus.h
>> >> >> > /**
>> >> >> > + * Implementation specific probe function which is responsible for linking
>> >> >> > + * devices on that bus with applicable drivers.
>> >> >> > + * The plugged device might already have been used previously by the bus,
>> >> >> > + * in which case some buses might prefer to detect and re-use the relevant
>> >> >> > + * information pertaining to this device.
>> >> >> > + *
>> >> >> > + * @param da
>> >> >> > + * Device declaration.
>> >> >> > + *
>> >> >> > + * @return
>> >> >> > + * The pointer to a valid rte_device usable by the bus on success.
>> >> >> > + * NULL on error. rte_errno is then set.
>> >> >> > + */
>> >> >> > +typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
>> >> >>
>> >> >> Shouldn't this be orthogonal to unplug() and take a rte_device. You
>> >> >> should only be able to plug devices that have been found by scan
>> >> >> before.
>> >> >
>> >> > Plugging a device that has been scanned before is a special case.
>> >> > In a true hotplug scenario, we could use this plug function passing
>> >> > a devargs.
>> >> > I don't see any issue passing rte_devargs to plug and rte_device to unplug.
>> >>
>> >> What do you mean by "true hotplug"?
>> >
>> > I mean a kernel notification of a new device.
>>
>> Does a "false hotplug" exist, too? :)
>
> The false hotplug was the original attach function which was just
> adding a new ethdev interface.
>
>> >> The problem with this is that passing just rte_devargs to plug()
>> >> requires the bus to parse and enrich the rte_devargs with bus
>> >> specifics. From there it gets folded into the to-be-created bus
>> >> specific rte_XXX_device. This makes it unnecessarily complicated and
>> >> even worse it adds a second code path how devices come alive.
>> >
>> > Just after the notification, the rte_device does not exist yet.
>> > So the plug function could share the same code as the scan function
>> > to get the metadata and create the device instance.
>>
>> Exactly this is what I want to avoid.
>
> Why do you want to avoid that?
> I think you mean it is not what you had in mind.
>
>> The plug() function would become a "scan-one and probe".
>
> Yes
>
>> From my point of view plug() and unplug() should be orthogonal.
>> The plug() and unplug() should only be responsible for adding drivers
>> with optional arguments. The EAL should allow the drivers to get
>> unplugged/re-plugged at run-time. I want to be able to change arguments
>> ... or even drivers :)
>
> It is a totally different thing.
> We are talking about hotplug of a device,
> and you are talking about changing drivers dynamically.
>
> So I still don't understand what is the issue with the plug/unplug
> functions proposed here.
>
I don't agree with the notion that plug() means "scan-one and probe".
>> >> When we get notified about a hotplug event we already know which bus
>> >> this event belongs to:
>> >>
>> >> 1. scan the bus for incoming devices
>> >
>> > No need to scan every devices here.
>>
>> This is a readdir followed by open+read+close for any new device. This
>> code belongs here anyway. Its lightweight if nothing changed. The scan
>> itself should be idempotent anyway.
>>
>> >> 2. plug single device with devargs and probe for drivers
>> >>
>> >> Makes sense?
>> >
>> > I want to make sure there is no misunderstanding first :)
>>
>> Which makes sense. That is probably my fault due to being too
>> distracted with other things and not communicating well enough while
>> Gaetan consumed my code.
>
> Your ideas are probably interesting, and I want to understand them.
> In the meantime, we need to progress on 17.08-rc1 which must be done
> in following days. Please let's separate the ideas which are not
> yet implemented from what we are already able to deliver.
Sorry, if you got the impression that this is taken from thin air but
this is how my original patch series looked like and I'm just
providing the rational behind it.
next prev parent reply other threads:[~2017-06-28 15:11 UTC|newest]
Thread overview: 117+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-26 0:21 [dpdk-dev] [PATCH v5 00/12] bus: attach / detach API Gaetan Rivet
2017-06-26 0:21 ` [dpdk-dev] [PATCH v5 01/12] bus: add bus iterator to find a bus Gaetan Rivet
2017-06-26 15:30 ` Bruce Richardson
2017-06-26 20:53 ` Gaëtan Rivet
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 02/12] bus: add device iterator method Gaetan Rivet
2017-06-26 16:20 ` Bruce Richardson
2017-06-26 21:13 ` Gaëtan Rivet
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 03/12] bus: add helper to find which bus holds a device Gaetan Rivet
2017-06-26 16:31 ` Bruce Richardson
2017-06-26 22:16 ` Gaëtan Rivet
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 04/12] bus: add bus iterator to find " Gaetan Rivet
2017-06-27 10:14 ` Bruce Richardson
2017-06-27 13:54 ` Bruce Richardson
2017-06-27 15:05 ` Gaëtan Rivet
2017-06-27 15:08 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 05/12] bus: introduce hotplug functionality Gaetan Rivet
2017-06-27 12:23 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 06/12] vdev: implement find_device bus operation Gaetan Rivet
2017-06-27 12:25 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 07/12] vdev: implement hotplug functionality Gaetan Rivet
2017-06-27 12:41 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 08/12] vdev: expose bus name Gaetan Rivet
2017-06-27 12:45 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 09/12] vdev: use standard bus registration function Gaetan Rivet
2017-06-27 12:59 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 10/12] pci: implement find_device bus operation Gaetan Rivet
2017-06-27 13:36 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 11/12] pci: implement hotplug " Gaetan Rivet
2017-06-27 13:49 ` Bruce Richardson
2017-06-26 0:22 ` [dpdk-dev] [PATCH v5 12/12] eal: make virtual driver probe and remove take rte_vdev_device Gaetan Rivet
2017-06-27 13:58 ` Bruce Richardson
2017-06-27 14:47 ` Gaëtan Rivet
2017-06-27 15:06 ` Bruce Richardson
2017-06-27 14:00 ` Bruce Richardson
2017-06-27 14:08 ` [dpdk-dev] [PATCH v5 00/12] bus: attach / detach API Bruce Richardson
2017-06-27 14:48 ` Gaëtan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 00/11] " Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 01/11] bus: add bus iterator to find a bus Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 02/11] bus: add device iterator method Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 03/11] bus: add helper to find which bus holds a device Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 04/11] bus: add bus iterator to find " Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 05/11] bus: introduce hotplug functionality Gaetan Rivet
2017-06-27 19:03 ` Jan Blunck
2017-06-28 11:44 ` Thomas Monjalon
2017-06-28 11:58 ` Jan Blunck
2017-06-28 12:11 ` Thomas Monjalon
2017-06-28 13:09 ` Jan Blunck
2017-06-28 13:30 ` Thomas Monjalon
2017-06-28 15:11 ` Jan Blunck [this message]
2017-06-28 15:33 ` Bruce Richardson
2017-06-29 12:59 ` Gaëtan Rivet
2017-06-29 19:20 ` Jan Blunck
2017-06-30 11:32 ` Gaëtan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 06/11] vdev: implement find_device bus operation Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 07/11] vdev: implement hotplug " Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 08/11] vdev: use standard bus registration function Gaetan Rivet
2017-07-03 22:15 ` Thomas Monjalon
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 09/11] pci: implement find_device bus operation Gaetan Rivet
2017-06-27 23:35 ` Stephen Hemminger
2017-06-28 9:17 ` Gaëtan Rivet
2017-06-28 9:52 ` Richardson, Bruce
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 10/11] pci: implement hotplug " Gaetan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 11/11] dev: use new hotplug API in attach / detach Gaetan Rivet
2017-06-29 18:21 ` [dpdk-dev] [PATCH 00/15] bus attach/detach API and hotplug rework Jan Blunck
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 01/15] bus: add bus iterator to find a bus Jan Blunck
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 02/15] bus: add find_device bus operation Jan Blunck
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 03/15] vdev: implement " Jan Blunck
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 04/15] pci: " Jan Blunck
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 05/15] bus/fslmc: " Jan Blunck
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 06/15] bus: add helper to find which bus holds a device Jan Blunck
2017-06-30 9:16 ` Thomas Monjalon
2017-06-30 16:46 ` Jan Blunck
2017-06-30 18:29 ` Thomas Monjalon
2017-06-30 21:24 ` Bruce Richardson
2017-06-30 21:25 ` Bruce Richardson
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 07/15] bus: add bus iterator to find " Jan Blunck
2017-06-30 9:17 ` Thomas Monjalon
2017-06-29 18:21 ` [dpdk-dev] [PATCH v7 08/15] bus: require buses to implement find_device operation Jan Blunck
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 09/15] bus: add rte_bus_find_by_name Jan Blunck
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 10/15] bus: introduce device plug/unplug functionality Jan Blunck
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 11/15] vdev: implement unplug bus operation Jan Blunck
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 12/15] pci: implement hotplug " Jan Blunck
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 13/15] eal: add hotplug add/remove functions Jan Blunck
2017-06-30 9:06 ` Thomas Monjalon
2017-06-30 9:11 ` Gaëtan Rivet
2017-06-30 9:20 ` Bruce Richardson
2017-06-30 15:44 ` Jan Blunck
2017-06-30 16:03 ` Thomas Monjalon
2017-06-30 16:13 ` Gaëtan Rivet
2017-06-30 16:25 ` Bruce Richardson
2017-06-30 16:29 ` Thomas Monjalon
2017-06-30 12:54 ` Thomas Monjalon
2017-06-30 15:12 ` Jan Blunck
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 14/15] dev: use new hotplug API in attach / detach Jan Blunck
2017-06-29 21:05 ` Thomas Monjalon
2017-06-29 18:22 ` [dpdk-dev] [PATCH v7 15/15] ethdev: Use embedded rte_device to detach driver Jan Blunck
2017-06-29 20:58 ` Thomas Monjalon
2017-06-30 9:59 ` [dpdk-dev] [PATCH 00/15] bus attach/detach API and hotplug rework Thomas Monjalon
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 00/14] " Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 01/14] bus: add bus iterator to find a bus Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 02/14] bus: add find_device bus operation Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 03/14] vdev: implement " Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 04/14] pci: " Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 05/14] bus/fslmc: " Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 06/14] bus: add helper to find which bus holds a device Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 07/14] bus: require buses to implement find_device operation Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 08/14] bus: add rte_bus_find_by_name Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 09/14] bus: introduce device plug/unplug functionality Jan Blunck
2017-06-30 18:38 ` Gaëtan Rivet
2017-06-30 18:52 ` Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 10/14] vdev: implement unplug bus operation Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 11/14] pci: implement hotplug " Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 12/14] eal: add hotplug add/remove functions Jan Blunck
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 13/14] ethdev: Use embedded rte_device to detach driver Jan Blunck
2017-07-03 23:17 ` Thomas Monjalon
2017-06-30 18:19 ` [dpdk-dev] [PATCH v8 14/14] dev: use new hotplug API in attach Jan Blunck
2017-07-03 22:59 ` [dpdk-dev] [PATCH v8 00/14] bus attach/detach API and hotplug rework Thomas Monjalon
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='CALe+Z00bCgi6NQmbswxDnFwnb=fPYCONKzzqcbybr5X9bO2N3w@mail.gmail.com' \
--to=jblunck@infradead.org \
--cc=dev@dpdk.org \
--cc=gaetan.rivet@6wind.com \
--cc=thomas@monjalon.net \
/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).