From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <antonio.fischetti@intel.com>
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" <antonio.fischetti@intel.com>
To: "Bie, Tiwei" <tiwei.bie@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "yliu@fridaylinux.org" <yliu@fridaylinux.org>,
 "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
 "jfreimann@redhat.com" <jfreimann@redhat.com>, "stable@dpdk.org"
 <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: <BCFD2BB875535045A5368D9ADBF2969935059706@IRSMSX101.ger.corp.intel.com>
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-stable] [dpdk-dev] [PATCH v2] net/virtio: fix an
	incorrect behavior of	device stop/start
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=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 <tiwei.bie@intel.com>
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>
> ---
> 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