Re: [PATCH v2] net/virtio: fix an incorrect behavior of device stop/start
From: Fischetti, Antonio <hidden>
Date: 2017-11-14 17:38:32
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. Thanks, Antonio
quoted hunk ↗ jump to hunk
-----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 <redacted> Reviewed-by: Jens Freimann <redacted> --- 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.cb/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.cb/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.cb/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.hb/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)-quoted
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