Re: [PATCH 0/9] fix hotplug API

From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Jan Blunck <jblunck@infradead.org>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	Shreyansh Jain <shreyansh.jain@nxp.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Bruce Richardson <bruce.richardson@intel.com>,
	dev@dpdk.org
Subject: Re: [PATCH 0/9] fix hotplug API
Date: Tue, 11 Jul 2017 21:21:02 +0200	[thread overview]
Message-ID: <20170711192102.GY11154@bidouze.vm.6wind.com> (raw)
In-Reply-To: <CALe+Z00Rc_6cn_ckWrK1cD2RsdWENM3s+ytOpxNymTmaqh8e0g@mail.gmail.com>

On Tue, Jul 11, 2017 at 03:07:02PM -0400, Jan Blunck wrote:
> On Tue, Jul 11, 2017 at 12:02 PM, Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> > On Sun, Jul 09, 2017 at 04:19:05PM +0200, Thomas Monjalon wrote:
> >> 09/07/2017 12:38, Jan Blunck:
> >> > On Sat, Jul 8, 2017 at 9:45 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> >> > > Sending those fixes as separate patches as they stand on their own.
> >> > > This series improves usability of the hotplug API and fixes a few issues
> >> > > with existing implementations.
> >> > >
> >> >
> >> > Interesting that you send this series as fixes. From what I can tell
> >> > this is a collection of changes that you have proposed before and I
> >> > have commented on and requested changes for. But I don't see that they
> >> > have been addressed at all:
> >> >
> >> > - you still concatenate the bus and device name just to pass it down
> >> > to the buses and parse it into its components again
> >> > - you still delegate the devargs parsing to the buses
> >> >
> >> > Please fix this issues. If you really want to return a device handle
> >> > from the hotplug API please present the use-case and in which cases
> >> > this solves a real-world problem.
> >>
> >> Please Jan, could you check in the 9 patches of this series
> >> which ones are OK for you?
> >> Thank you
> >>
> >
> > Hi Jan,
> >
> > I sent a new version, consisting mostly in fixes, apart from removing
> > the devarg argument from plug / unplug (which is closely tied to the
> > fixes I proposed). I removed the changes to rte_eal_hotplug_add return
> > value.
> >
> 
> Thanks for the notice. I'll have a look.
> 
> > What do you think of this version? The hotplug must be fixed before
> > the release, this way or another.
> >
> 
> Do you mean the hotplug of real devices or of virtual devices?

Well, both ideally. Virtual devices are not so different from real
devices in the end.

The vdev scan allocates rte_vdev_devices, which are linked to their
rte_devargs. after this scan, it is possible to use vdev->find_device() to get
back the handle to use during the (hypothetical) vdev->plug().

So it is not far from a regular bus. This proximity is a good thing IMO,
as it allows genericity in its use. No need to define an exception for
the vdev bus, it can be used with the others.

Which is useful, if we consider decoupling the EAL from the vdev bus.
Given that this change will not happen during this release it
is not strictly necessary, but it seems clean enough and should simplify
future work anyway.

-- 
Gaëtan Rivet
6WIND