Re: [PATCH 3/4] net/virtio-user: support server mode

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

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Wednesday, February 28, 2018 2:02 AM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org;
> yliu@fridaylinux.org; Tan, Jianfeng <jianfeng.tan@intel.com>; Bie, Tiwei
> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
> Cc: Wang, Dong1 <dong1.wang@intel.com>
> Subject: Re: [PATCH 3/4] net/virtio-user: support server mode
> 
> 
> 
> On 02/14/2018 03:53 PM, Zhiyong Yang wrote:
> > virtio user adds to support for server mode.
> >
> > Virtio user with server mode creates socket file and then starts to
> > wait for first connection from vhost user with client mode in blocking mode.
> >
> > Server mode virtio user supports many times' vhost reconnections with
> > same configurations.
> >
> > Support only one connection at the same time in server mode.
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > ---
> >   drivers/net/virtio/virtio_ethdev.c               |  9 ++-
> >   drivers/net/virtio/virtio_user/vhost_user.c      | 77
> ++++++++++++++++++++--
> >   drivers/net/virtio/virtio_user/virtio_user_dev.c | 44 +++++++++----
> >   drivers/net/virtio/virtio_user_ethdev.c          | 81
> ++++++++++++++++++++++--
> >   4 files changed, 186 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> > b/drivers/net/virtio/virtio_ethdev.c
> > index 884f74ad0..44d037d6b 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -1273,9 +1273,13 @@ static void
> >   virtio_notify_peers(struct rte_eth_dev *dev)
> >   {
> >   	struct virtio_hw *hw = dev->data->dev_private;
> > -	struct virtnet_rx *rxvq = dev->data->rx_queues[0];
> > +	struct virtnet_rx *rxvq = NULL;
> I don't think it is needed to assign to NULL here.

Ok.
> 
> >   	struct rte_mbuf *rarp_mbuf;
> >
> > +	if (!dev->data->rx_queues)
> > +		return;
> > +
> > +	rxvq = dev->data->rx_queues[0];
> The above change is valid, but I think it should be in a dedicated patch, as it
> might be backported to -stable.
> 
Ok, it will cause crash in some cases. For example, the code goes here before memory allocation of  rxvq is done.

> >   	rarp_mbuf = rte_net_make_rarp_packet(rxvq->mpool,
> >   			(struct ether_addr *)hw->mac_addr);
> >   	if (rarp_mbuf == NULL) {
> > @@ -1333,7 +1337,8 @@ virtio_interrupt_handler(void *param)
> >
> >   	if (isr & VIRTIO_NET_S_ANNOUNCE) {
> >   		virtio_notify_peers(dev);
> > -		virtio_ack_link_announce(dev);
> > +		if (hw->cvq) > +
> 	virtio_ack_link_announce(dev);
> Is this change also related to server mode?
> It may deserve a dedicated patch too.

Above changes are not related to server mode. :). 
Just looks more reasonable.

If format extra two patches,  Should I put them in this series or in another series?

Thanks
Zhiyong

> >   	}
> >   }
> 
> Thanks,
> Maxime