Re: [PATCH v3] pdump: fix vdev cleanup

[Thomas Monjalon <thomas@monjalon.net>]

15/01/2019 11:25, Pattan, Reshma:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 15/01/2019 11:09, Pattan, Reshma:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > 11/01/2019 18:51, Reshma Pattan:
> > > > > Virtual devices added in pdump application using
> > > > > rte_eal_hotplug_add should be removed explicitly while existing
> > > > > the pdump application, otherwise the
> > > >
> > > > existing -> exiting ?
> > > >
> > > > > subsequent run of the pdump application will fail with the reason
> > > > > that virtual devices with the same name already exists in primary.
> > > > >
> > > > > Fixes: 6362f362a2 ("app/pdump: use EAL hotplug instead of ethdev
> > > > > attach")
> > > > > CC: stable@dpdk.org
> > > > > CC: ferruh.yigit@intel.com
> > > >
> > > > Why is it marked as a fix of this commit?
> > > > Is it a miss in the conversion from ethdev attach calls?
> > > > Or a changed behaviour with hotplug functions?
> > >
> > > It is changed behaviour with hotplug functions.
> > 
> > Please, could you explain?
> > 
> 
> Attach calls were replaced with rte_eal_hotplug_remove() in pdump in the commit id given in Fixes line.
> 
> And after this change, the subsequent runs(other than 1st) of pdump application are failing with below error
> 
> 1)Pdump application error:
> 
> EAL: Failed to hotplug add device
> EAL: Error - exiting with code: 1
>   Cause: vdev creation failed:create_mp_ring_vdev:706
> 
> 2)And error on primary application side is below:
> 	testpmd> Ethernet device with name net_pcap_rx_0 already allocated
>                 EAL: Failed to hotplug add device on primary
> 
> So, the dev added in secondary should be removed during its exit by calling rte_eal_hotplug_remove ().
> 
> So,  that is the reason I chose the Fixes: commit as the one where rte_eal_hotplug_add() replaced the attach.
> 
> Earlier with attach calls though not detaching the devices upon application exit, these issues are not seen.

OK
I won't dig to understand why this error was hidden with previous API.
I'll assume there is no need to backport this fix in 17.11.