[dpdk-dev] [PATCH 2/3] net/virtio: rationalize queue flushing

[Tiwei Bie]

On Thu, Jan 18, 2018 at 03:55:53PM +0100, Olivier Matz wrote:
> On Thu, Jan 18, 2018 at 10:05:49PM +0800, Tiwei Bie wrote:
> > On Thu, Jan 18, 2018 at 10:07:32AM +0100, Olivier Matz wrote:
> > > Rationalize the function virtio_dev_free_mbufs():
> > > 
> > > - skip NULL vqs instead of crashing: this is required for the
> > >   next commit
> > > - use the same kind of loop than in virtio_free_queues()
> > > - also flush mbufs from the control queue (this is useless yet)
> > > - factorize common code between rxq, txq, cq
> > > 
> > > Cc: stable at dpdk.org
> > > 
> > > Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
> > > ---
> > >  drivers/net/virtio/virtio_ethdev.c | 55 ++++++++++++++++++--------------------
> > >  1 file changed, 26 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > > index c7426951c..d8b3b8ed7 100644
> > > --- a/drivers/net/virtio/virtio_ethdev.c
> > > +++ b/drivers/net/virtio/virtio_ethdev.c
> > > @@ -1868,47 +1868,44 @@ virtio_dev_start(struct rte_eth_dev *dev)
> > >  
> > >  static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
> > >  {
> > [...]
> > >  
> > > -		mbuf_num = 0;
> > > -		while ((buf = virtqueue_detach_unused(txvq->vq)) != NULL) {
> > > +		while ((buf = virtqueue_detach_unused(vq)) != NULL) {
> > 
> > Thanks for working on this! The virtqueue_detach_unused() can't
> > handle the vector Rx case correctly. Because vq->vq_descx[] is
> > initialized for vector Rx, but isn't updated by the vector Rx.
> > So together with the next commit, it may cause problems during
> > dev_stop/dev_configure/dev_start if vector Rx is used.
> 
> What would be the appropriate way to fix it?
> 
> Either we fix the vector functions to set vq->vq_descx, or we find
> another method to flush the mbufs in case of vector rx. Can we use
> vq->sw_ring[] to get the mbufs instead?

The first one may hurt the performance. If not, it would be the
best choice IMO. For the second one and your question, there is
some related code in virtqueue_rxvq_flush() you could refer to:

https://github.com/DPDK/dpdk/blob/e00093f381a1/drivers/net/virtio/virtqueue.c#L51

Thanks,
Tiwei