Re: [dpdk-dev] [PATCH] bus/vdev: fix probe same device twice

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Tuesday, November 6, 2018 4:34 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] bus/vdev: fix probe same device twice
> 
> On Tue, Nov 06, 2018 at 09:36:22PM +0100, Thomas Monjalon wrote:
> > 06/11/2018 16:46, Zhang, Qi Z:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > >
> > > > Hi,
> > > >
> > > > 06/11/2018 01:31, Qi Zhang:
> > > > > When probe the same device at second time
> > > >
> > > > Sorry I stop on this first sentence.
> > > > How and why do you probe a vdev twice?
> > >
> > > if we do rte_dev_hotplug_add or rte_dev_proble on a probed device.
> > > (yes, this is not usually what an application want, but it can
> > > happen by miss-operation, and this is covered by our test case, it
> > > make sense to me that hotplug API should be robust enough to handle
> > > that situation.)
> >
> > Yes I agree we must handle this situation.
> >
> > > we will failed at the second time as expected, but will not able to
> > > detach the device any more, since during the second scan, original
> vdev->device.devargs is corrupted.
> >
> > The root cause is we remove a devargs which was referenced.
> > Could we overwrite the first devargs instead of removing it?
> >
> >
> 
> It's also possible to add a back-reference to an rte_device in [1], but that can
> only work if only one rte_device references a devargs.
> It seems to be the case now, but it might be good to enforce explicitly that
> when a bus scans its devices, it should do a 1-to-1 map to devargs.
> 
> If mapping rte_device to rte_devargs needs to respect rules, it could help bus
> developpers to have a function that will do the job: verify that the devargs is
> not currently used, add the back-reference to the rte_device.
> 
> With the proper back-reference, it is possible to clean-up the device when
> removing the devargs 

This may still not work for vdev, since the old reference is used in vdev_find to find a exist device by name during scan.
(For PCI device, we have pci_addr, but vdev we use devargs->name to identify device, anyway this can be fixed in vdev, but that required a clone on the device name also break the coupling somehow.)
I just don't understand "why we must tight the tighten the device -> devargs coupling, not loosen it"

(and also to add the rte_devargs_extract() function
> that would allow keeping the original devargs and insert it back if the hotplug
> fails, then the mapping must be restored).

> 
> [1]: https://mails.dpdk.org/archives/dev/2018-November/118274.html
> 
> --
> Gaëtan Rivet
> 6WIND