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 27F541B22C; Tue, 14 Nov 2017 18:38:36 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Nov 2017 09:38:35 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,395,1505804400"; d="scan'208";a="1244007989" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by fmsmga002.fm.intel.com with ESMTP; 14 Nov 2017 09:38:33 -0800 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.22]) by IRSMSX106.ger.corp.intel.com ([169.254.8.36]) with mapi id 14.03.0319.002; Tue, 14 Nov 2017 17:38:32 +0000 From: "Fischetti, Antonio" To: "Bie, Tiwei" , "dev@dpdk.org" CC: "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" , "jfreimann@redhat.com" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of device stop/start Thread-Index: AQHTSUi182nEWudyNkKC9McnSRi8UKMUQMjg Date: Tue, 14 Nov 2017 17:38:32 +0000 Message-ID: References: <20170829082601.30349-1-tiwei.bie@intel.com> <1508465368-36746-1-git-send-email-tiwei.bie@intel.com> In-Reply-To: <1508465368-36746-1-git-send-email-tiwei.bie@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjY5MWVmMGEtNDU4Yy00MDQ1LWEzNTYtOGJjMDdhNDgzODQ1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiI4N3RObFJaM3dzZ1B1NlZxNDI2TjhRK1FtYUpBK0ZGZTlCSUVZd0ZESUg0WjljVDNIK0Z6d3FIYkdRZlVMUVRWIn0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Tue, 14 Nov 2017 17:38:37 -0000 Hi Tiwei, I'm doing some regression tests with v17.11-rc4. I ran=20 into a hitch with testpmd running into a guest VM. It happens=20 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=20 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 =3D 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=20 sending packets before I launch testpmd in the guest. In my testbench I have Open-vSwitch (OvS-DPDK) which launches=20 a VM with 2 vhostuserclient ports (vhu0 and vhu1), each with=20 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=20 vhostuserclient ports. While the traffic generator is already sending packets, in the=20 guest VM I launch=20 ./testpmd -c 0x3 -n 4 --socket-mem 512 -- --burst=3D64 -i --txqflags=3D0x= f00 --disable-hw-vlan The issue is that I see no packet received on the traffic generator=20 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=20 more info on my testbench. 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 >=20 > 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. >=20 > Fixes: a85786dc816f ("virtio: fix states handling during > initialization") > Cc: stable@dpdk.org >=20 > Signed-off-by: Tiwei Bie > Reviewed-by: Jens Freimann > --- > v2: > - Use the existing `for` loop > - Improve the commit log >=20 > 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(-) >=20 > 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) >=20 > for (i =3D 0; i < dev->data->nb_rx_queues; i++) { > rxvq =3D dev->data->rx_queues[i]; > + /* Flush the old packets */ > + virtqueue_flush(rxvq->vq); > virtqueue_notify(rxvq->vq); > } >=20 > 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) >=3D offset; > } >=20 > -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 =3D VIRTQUEUE_NUSED(vq); > + > + for (i =3D 0; i < nb_used; i++) { > + used_idx =3D vq->vq_used_cons_idx & (vq->vq_nentries - 1); > + uep =3D &vq->vq_ring.used->ring[used_idx]; > + desc_idx =3D (uint16_t)uep->id; > + dxp =3D &vq->vq_descx[desc_idx]; > + if (dxp->cookie !=3D NULL) { > + rte_pktmbuf_free(dxp->cookie); > + dxp->cookie =3D 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); >=20 > +/* 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) >=20 > #define VIRTQUEUE_NUSED(vq) ((uint16_t)((vq)->vq_ring.used->idx - (vq)- > >vq_used_cons_idx)) >=20 > +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