On 20/03/2020 16:23, Maxime Coquelin wrote: > > > On 3/19/20 6:15 PM, Kevin Traynor wrote: >> Hi Itsuro, >> >> On 05/03/2020 02:54, Itsuro Oda wrote: >>> If a vhost device is closed before eth_dev_configure is done >>> to the device, internal resources allocated to the device >>> would not be freed. This patch fixes it. >>> >>> Fixes: 3d01b759d267 ("net/vhost: delay driver setup") >>> Cc: sta...@dpdk.org >>> >>> Signed-off-by: Itsuro Oda <o...@valinux.co.jp> >>> Reviewed-by: Xiaolong Ye <xiaolong...@intel.com> >> >> This fixes an issue with the patch you backported for 18.11. Is the >> issue also present in the backported version? >> >> If so, this patch is not in upstream dpdk or gone through validation. So >> the choices are, >> >> 1. revert your patches from 18.11 >> 2. go ahead on stable without this patch >> 3. delay until this patch is in master (but not until validated) and >> then backport to stable >> >> Itsuro/Maxime, what do you think? > > I think you should drop Itsuro patches for now, as long as this patch is > not in master. Secondary process was broken for several revisions in > Vhost, so it is better to keep it broken for now than risking > regressions on primary process support. >
I agree, vhost primary is too well used to take risks on regression while there are still some outstanding issues being resolved with these patches for secondary process. Patches reverted. > If this patch is in master before the next 18.11 is done, then we can > pick all the patches. > Yes, we can do that. thanks, Kevin. > Thanks, > Maxime >> thanks, >> Kevin. >> >>> --- >>> v2: >>> - fix commit message >>> >>> v3: >>> - fix spell error of Reviewed-by >>> >>> drivers/net/vhost/rte_eth_vhost.c | 16 +++++++--------- >>> 1 file changed, 7 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/net/vhost/rte_eth_vhost.c >>> b/drivers/net/vhost/rte_eth_vhost.c >>> index 458ed58f5..1ed977e9b 100644 >>> --- a/drivers/net/vhost/rte_eth_vhost.c >>> +++ b/drivers/net/vhost/rte_eth_vhost.c >>> @@ -1065,16 +1065,14 @@ eth_dev_close(struct rte_eth_dev *dev) >>> >>> eth_dev_stop(dev); >>> >>> - rte_vhost_driver_unregister(internal->iface_name); >>> - >>> list = find_internal_resource(internal->iface_name); >>> - if (!list) >>> - return; >>> - >>> - pthread_mutex_lock(&internal_list_lock); >>> - TAILQ_REMOVE(&internal_list, list, next); >>> - pthread_mutex_unlock(&internal_list_lock); >>> - rte_free(list); >>> + if (list) { >>> + rte_vhost_driver_unregister(internal->iface_name); >>> + pthread_mutex_lock(&internal_list_lock); >>> + TAILQ_REMOVE(&internal_list, list, next); >>> + pthread_mutex_unlock(&internal_list_lock); >>> + rte_free(list); >>> + } >>> >>> if (dev->data->rx_queues) >>> for (i = 0; i < dev->data->nb_rx_queues; i++) >>> >>