Re: [dpdk-dev] [PATCH v3 4/4] net/vhost: add NULL pointer checking

["Yang, Zhiyong" <zhiyong.yang@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yang, Zhiyong
> Sent: Friday, March 30, 2018 10:01 AM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org
> Cc: Tan, Jianfeng <jianfeng.tan@intel.com>; Wang, Zhihong
> <zhihong.wang@intel.com>; thomas@monjalon.net; Wang, Dong1
> <dong1.wang@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 4/4] net/vhost: add NULL pointer
> checking
> 
> Hi Maxime,
> 
> > -----Original Message-----
> > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> > Sent: Thursday, March 29, 2018 9:20 PM
> > To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
> > Cc: Tan, Jianfeng <jianfeng.tan@intel.com>; Wang, Zhihong
> > <zhihong.wang@intel.com>; thomas@monjalon.net; Wang, Dong1
> > <dong1.wang@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>
> > Subject: Re: [PATCH v3 4/4] net/vhost: add NULL pointer checking
> >
> > Hi,
> >
> > On 03/21/2018 04:03 AM, zhiyong.yang@intel.com wrote:
> > > When vhost user PMD works in client mode to connect/reconnect
> > > virtio-user with server mode, new thread sometimes may run to
> > > new_device before queue_setup has been done, So have to wait until
> > > memory allocation is done.
> > >
> > > Release note is updated in the patch.
> > >
> > > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > > ---
> > >   doc/guides/rel_notes/release_18_05.rst | 7 +++++++
> > >   drivers/net/vhost/rte_eth_vhost.c      | 9 +++++++++
> > >   2 files changed, 16 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/release_18_05.rst
> > > b/doc/guides/rel_notes/release_18_05.rst
> > > index 3923dc253..7b301f021 100644
> > > --- a/doc/guides/rel_notes/release_18_05.rst
> > > +++ b/doc/guides/rel_notes/release_18_05.rst
> > > @@ -41,6 +41,13 @@ New Features
> > >        Also, make sure to start the actual text at the margin.
> > >
> > =========================================================
> > >
> > > +* **Added support for virtio-user server mode.**
> > > +
> > > +  In a container environment if the vhost-user backend restarts,
> > > + there's no way  for it to reconnect to virtio-user. To address
> > > + this, support for server mode  is added. In this mode the socket
> > > + file is created by virtio-user, which the  backend then connects
> > > + to. This means that if the backend restarts, it can  reconnect to
> > > + virtio-user and
> > continue communications.
> >
> > I think this shouldn't be part of this patch.
> >
> Ok, I can merge it with the previous patch 3/4.
> 
> > >
> > >   API Changes
> > >   -----------
> > > diff --git a/drivers/net/vhost/rte_eth_vhost.c
> > > b/drivers/net/vhost/rte_eth_vhost.c
> > > index 3aae01c39..2490bad0b 100644
> > > --- a/drivers/net/vhost/rte_eth_vhost.c
> > > +++ b/drivers/net/vhost/rte_eth_vhost.c
> > > @@ -580,6 +580,15 @@ new_device(int vid)
> > >   		eth_dev->data->numa_node = newnode;
> > >   #endif
> > >
> > > +	/* The thread may run here before eth_dev->data->rx_queues or
> > > +	 * eth_dev->data->tx_queues have gotten valid memory, so have to
> > > +	 * wait until memory allocation is done.
> > > +	 */
> > > +	while (!eth_dev->data->rx_queues ||
> > > +	       !eth_dev->data->tx_queues) {
> > > +		usleep(1);
> > > +	}
> > > +
> > >   	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> > >   		vq = eth_dev->data->rx_queues[i];
> > >   		if (vq == NULL)
> > >
> >
> > I don't like the idea of polling here.
> > It looks like Junjie is addressing the problem in a different way [0],
> > do you confirm it would work in your case?
> >
> 
> Great to hear that.  I have to fix it when the issue is found.
> It's better to have another solution. I will test it later.
> 

Junjie's patch can fix the existing issue and then drop this patch.

Here is the link.
http://www.dpdk.org/dev/patchwork/patch/36766/

thanks
Zhiyong