From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 83F53324A; Sat, 2 Dec 2017 02:25:09 +0100 (CET) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Dec 2017 17:25:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,348,1508828400"; d="scan'208";a="182642702" Received: from deepin-15.sh.intel.com (HELO debian-xvivbkq) ([10.67.104.165]) by fmsmga005.fm.intel.com with ESMTP; 01 Dec 2017 17:25:06 -0800 Date: Sat, 2 Dec 2017 09:24:36 +0800 From: Tiwei Bie To: "Fischetti, Antonio" Cc: "dev@dpdk.org" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" , "jfreimann@redhat.com" , "stable@dpdk.org" Message-ID: <20171202012436.7q73lizpzo4dgr7d@debian-xvivbkq> References: <20170829082601.30349-1-tiwei.bie@intel.com> <1508465368-36746-1-git-send-email-tiwei.bie@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of device stop/start 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: Sat, 02 Dec 2017 01:25:10 -0000 On Wed, Nov 15, 2017 at 01:38:32AM +0800, Fischetti, Antonio wrote: > Hi Tiwei, > > I'm doing some regression tests with v17.11-rc4. I ran > into a hitch with testpmd running into a guest VM. It happens > that no packet gets forwarded by testpmd. > The issue seems to appear after this patch was upstreamed. > > I saw there's a way to make it work, ie by avoiding to > increment the last consumed descriptor: > > --- a/drivers/net/virtio/virtqueue.c > +++ b/drivers/net/virtio/virtqueue.c > @@ -80,7 +80,7 @@ virtqueue_flush(struct virtqueue *vq) > rte_pktmbuf_free(dxp->cookie); > dxp->cookie = NULL; > } > - vq->vq_used_cons_idx++; > + //vq->vq_used_cons_idx++; > vq_ring_free_chain(vq, desc_idx); > > Not quite sure if this change make any sense to you? > > Some details below. > > The issue appears only if the traffic generator is already > sending packets before I launch testpmd in the guest. > > In my testbench I have Open-vSwitch (OvS-DPDK) which launches > a VM with 2 vhostuserclient ports (vhu0 and vhu1), each with > a single queue. > My OvS has 2 physical ports: dpdk0 and dpdk1. > dpdk0 forwards packets back and forth from/to the generator > to/from vhu0. > Similarly, dpdk1 forwards packets back and forth from/to the generator > to/from vhu1. > > In OvS there are 2 different PMD threads serving the 2 > vhostuserclient ports. > > While the traffic generator is already sending packets, in the > guest VM I launch > ./testpmd -c 0x3 -n 4 --socket-mem 512 -- --burst=64 -i --txqflags=0xf00 --disable-hw-vlan > > The issue is that I see no packet received on the traffic generator > and in fact testpmd shows > > ---------------------- Forward statistics for port 0 ---------------------- > RX-packets: 0 RX-dropped: 0 RX-total: 0 > TX-packets: 0 TX-dropped: 0 TX-total: 0 > ---------------------------------------------------------------------------- > > ---------------------- Forward statistics for port 1 ---------------------- > RX-packets: 0 RX-dropped: 0 RX-total: 0 > TX-packets: 0 TX-dropped: 0 TX-total: 0 > ---------------------------------------------------------------------------- > > +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++ > RX-packets: 0 RX-dropped: 0 RX-total: 0 > TX-packets: 0 TX-dropped: 0 TX-total: 0 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > Please let me know if I missed something or if you need > more info on my testbench. > Hi Antonio, I'm very sorry, I missed this mail before.. Thank you so much for reporting this issue and the detailed info. I'll look into this! Best regards, Tiwei Bie > > Thanks, > Antonio > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tiwei Bie > > Sent: Friday, October 20, 2017 3:09 AM > > To: dev@dpdk.org > > Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com; > > jfreimann@redhat.com; stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of > > device stop/start > > > > After starting a device, the driver shouldn't deliver the > > packets that already existed before the device is started > > to applications. Otherwise it will lead to incorrect packet > > collection for port state. This patch fixes this issue by > > flushing the Rx queues when starting the device. > > > > Fixes: a85786dc816f ("virtio: fix states handling during > > initialization") > > Cc: stable@dpdk.org > > > > Signed-off-by: Tiwei Bie > > Reviewed-by: Jens Freimann > > --- > > v2: > > - Use the existing `for` loop > > - Improve the commit log > > > > drivers/net/virtio/virtio_ethdev.c | 2 ++ > > drivers/net/virtio/virtio_rxtx.c | 2 +- > > drivers/net/virtio/virtqueue.c | 25 +++++++++++++++++++++++++ > > drivers/net/virtio/virtqueue.h | 5 +++++ > > 4 files changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/virtio/virtio_ethdev.c > > b/drivers/net/virtio/virtio_ethdev.c > > index 42c2836..9ccb0f4 100644 > > --- a/drivers/net/virtio/virtio_ethdev.c > > +++ b/drivers/net/virtio/virtio_ethdev.c > > @@ -1819,6 +1819,8 @@ virtio_dev_start(struct rte_eth_dev *dev) > > > > for (i = 0; i < dev->data->nb_rx_queues; i++) { > > rxvq = dev->data->rx_queues[i]; > > + /* Flush the old packets */ > > + virtqueue_flush(rxvq->vq); > > virtqueue_notify(rxvq->vq); > > } > > > > diff --git a/drivers/net/virtio/virtio_rxtx.c > > b/drivers/net/virtio/virtio_rxtx.c > > index 609b413..f5b6f94 100644 > > --- a/drivers/net/virtio/virtio_rxtx.c > > +++ b/drivers/net/virtio/virtio_rxtx.c > > @@ -80,7 +80,7 @@ virtio_dev_rx_queue_done(void *rxq, uint16_t offset) > > return VIRTQUEUE_NUSED(vq) >= offset; > > } > > > > -static void > > +void > > vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx) > > { > > struct vring_desc *dp, *dp_tail; > > diff --git a/drivers/net/virtio/virtqueue.c > > b/drivers/net/virtio/virtqueue.c > > index 9ad77b8..c3a536f 100644 > > --- a/drivers/net/virtio/virtqueue.c > > +++ b/drivers/net/virtio/virtqueue.c > > @@ -59,3 +59,28 @@ virtqueue_detatch_unused(struct virtqueue *vq) > > } > > return NULL; > > } > > + > > +/* Flush the elements in the used ring. */ > > +void > > +virtqueue_flush(struct virtqueue *vq) > > +{ > > + struct vring_used_elem *uep; > > + struct vq_desc_extra *dxp; > > + uint16_t used_idx, desc_idx; > > + uint16_t nb_used, i; > > + > > + nb_used = VIRTQUEUE_NUSED(vq); > > + > > + for (i = 0; i < nb_used; i++) { > > + used_idx = vq->vq_used_cons_idx & (vq->vq_nentries - 1); > > + uep = &vq->vq_ring.used->ring[used_idx]; > > + desc_idx = (uint16_t)uep->id; > > + dxp = &vq->vq_descx[desc_idx]; > > + if (dxp->cookie != NULL) { > > + rte_pktmbuf_free(dxp->cookie); > > + dxp->cookie = NULL; > > + } > > + vq->vq_used_cons_idx++; > > + vq_ring_free_chain(vq, desc_idx); > > + } > > +} > > diff --git a/drivers/net/virtio/virtqueue.h > > b/drivers/net/virtio/virtqueue.h > > index 9c4f96d..11059fb 100644 > > --- a/drivers/net/virtio/virtqueue.h > > +++ b/drivers/net/virtio/virtqueue.h > > @@ -304,6 +304,9 @@ void virtqueue_dump(struct virtqueue *vq); > > */ > > struct rte_mbuf *virtqueue_detatch_unused(struct virtqueue *vq); > > > > +/* Flush the elements in the used ring. */ > > +void virtqueue_flush(struct virtqueue *vq); > > + > > static inline int > > virtqueue_full(const struct virtqueue *vq) > > { > > @@ -312,6 +315,8 @@ virtqueue_full(const struct virtqueue *vq) > > > > #define VIRTQUEUE_NUSED(vq) ((uint16_t)((vq)->vq_ring.used->idx - (vq)- > > >vq_used_cons_idx)) > > > > +void vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx); > > + > > static inline void > > vq_update_avail_idx(struct virtqueue *vq) > > { > > -- > > 2.7.4 >