From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 5BE141B345; Thu, 18 Jan 2018 16:48:36 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jan 2018 07:48:35 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,378,1511856000"; d="scan'208";a="196651969" Received: from debian-xvivbkq.sh.intel.com ([10.67.104.226]) by fmsmga005.fm.intel.com with ESMTP; 18 Jan 2018 07:48:34 -0800 Date: Thu, 18 Jan 2018 23:48:09 +0800 From: Tiwei Bie To: Olivier Matz Cc: dev@dpdk.org, Yuanhan Liu , Maxime Coquelin , stable@dpdk.org Message-ID: <20180118154809.lkc7cjbagg3pg6k2@debian-xvivbkq.sh.intel.com> References: <20180118090733.12728-1-olivier.matz@6wind.com> <20180118090733.12728-3-olivier.matz@6wind.com> <20180118140549.tnqzaytw36grtaff@debian-xvivbkq.sh.intel.com> <20180118145553.4sttsn7l3dcdmvfw@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180118145553.4sttsn7l3dcdmvfw@platinum> User-Agent: NeoMutt/20170609 (1.8.3) Subject: Re: [dpdk-dev] [PATCH 2/3] net/virtio: rationalize queue flushing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jan 2018 15:48:36 -0000 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@dpdk.org > > > > > > Signed-off-by: Olivier Matz > > > --- > > > 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