[dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Zhiyong Yang]

Current dpdk code virtio_dev_stop only disables interrupt and marks link down,
When it is invoked, tx/rx traffic flows still work. This is a strange behavior.
The patchset supports the switch of flow by calling virtio_dev_start/stop.

The implementation refers to vhost pmd.

Zhiyong Yang (2):
  net/virtio: add data elements to turn on/off traffic flow
  net/virtio: support to turn on/off the traffic flow

 drivers/net/virtio/virtio_ethdev.c           | 37 +++++++++++++++++++++++++++-
 drivers/net/virtio/virtio_pci.h              |  1 +
 drivers/net/virtio/virtio_rxtx.c             | 12 +++++++++
 drivers/net/virtio/virtio_rxtx.h             |  6 +++++
 drivers/net/virtio/virtio_rxtx_simple.c      |  4 +++
 drivers/net/virtio/virtio_rxtx_simple_neon.c |  5 +++-
 drivers/net/virtio/virtio_rxtx_simple_sse.c  |  5 +++-
 drivers/net/virtio/virtio_user_ethdev.c      |  1 +
 8 files changed, 68 insertions(+), 3 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/2] net/virtio: add data elements to turn on/off traffic flow

[Zhiyong Yang]

Add "rte_atomic32_t started" to turn on/off the RX/TX flow from
per port perspective.

Add rte_atomic32_t allow_queuing to control traffic flow from
per Queue perspective.

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---
 drivers/net/virtio/virtio_pci.h  | 1 +
 drivers/net/virtio/virtio_rxtx.h | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 0362acd..952ad62 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -253,6 +253,7 @@ struct virtio_hw {
 	uint64_t    req_guest_features;
 	uint64_t    guest_features;
 	uint32_t    max_queue_pairs;
+	rte_atomic32_t started;
 	uint16_t	max_mtu;
 	uint16_t    vtnet_hdr_size;
 	uint8_t	    vlan_strip;
diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
index 28f82d6..03357ae 100644
--- a/drivers/net/virtio/virtio_rxtx.h
+++ b/drivers/net/virtio/virtio_rxtx.h
@@ -60,6 +60,9 @@ struct virtnet_rx {
 	struct virtnet_stats stats;
 
 	const struct rte_memzone *mz; /**< mem zone to populate RX ring. */
+
+	/* a flag indicates whether allow to receive pkts. */
+	rte_atomic32_t allow_queuing;
 };
 
 struct virtnet_tx {
@@ -75,6 +78,9 @@ struct virtnet_tx {
 	struct virtnet_stats stats;
 
 	const struct rte_memzone *mz;    /**< mem zone to populate TX ring. */
+
+	/* a flag indicates whether allow to transmit pkts. */
+	rte_atomic32_t allow_queuing;
 };
 
 struct virtnet_ctl {
-- 
2.7.4

[dpdk-dev] [PATCH 2/2] net/virtio: support to turn on/off the traffic flow

[Zhiyong Yang]

virtio_dev_start/stop support the switch of traffic flow.

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c           | 37 +++++++++++++++++++++++++++-
 drivers/net/virtio/virtio_rxtx.c             | 12 +++++++++
 drivers/net/virtio/virtio_rxtx_simple.c      |  4 +++
 drivers/net/virtio/virtio_rxtx_simple_neon.c |  5 +++-
 drivers/net/virtio/virtio_rxtx_simple_sse.c  |  5 +++-
 drivers/net/virtio/virtio_user_ethdev.c      |  1 +
 6 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d9986ab..5955443 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1707,6 +1707,33 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static void
+update_queuing_status(struct rte_eth_dev *dev)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	unsigned int i;
+	struct virtnet_rx *vnet_rx;
+	struct virtnet_tx *vnet_tx;
+	int allow_queuing = 1;
+
+	if (rte_atomic32_read(&hw->started) == 0)
+		allow_queuing = 0;
+
+	/* Wait until rx/tx_pkt_burst stops accessing virtio device */
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		vnet_rx = dev->data->rx_queues[i];
+		if (!vnet_rx)
+			continue;
+		rte_atomic32_set(&vnet_rx->allow_queuing, allow_queuing);
+	}
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		vnet_tx = dev->data->tx_queues[i];
+		if (!vnet_tx)
+			continue;
+		rte_atomic32_set(&vnet_tx->allow_queuing, allow_queuing);
+	}
+}
 
 static int
 virtio_dev_start(struct rte_eth_dev *dev)
@@ -1758,6 +1785,9 @@ virtio_dev_start(struct rte_eth_dev *dev)
 		virtqueue_notify(rxvq->vq);
 	}
 
+	rte_atomic32_set(&hw->started, 1);
+	update_queuing_status(dev);
+
 	PMD_INIT_LOG(DEBUG, "Notified backend at initialization");
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
@@ -1819,13 +1849,15 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
 }
 
 /*
- * Stop device: disable interrupt and mark link down
+ * Stop device: disable interrupt, mark link down
+ *              and stop the traffic flow.
  */
 static void
 virtio_dev_stop(struct rte_eth_dev *dev)
 {
 	struct rte_eth_link link;
 	struct rte_intr_conf *intr_conf = &dev->data->dev_conf.intr_conf;
+	struct virtio_hw *hw = dev->data->dev_private;
 
 	PMD_INIT_LOG(DEBUG, "stop");
 
@@ -1834,6 +1866,9 @@ virtio_dev_stop(struct rte_eth_dev *dev)
 
 	memset(&link, 0, sizeof(link));
 	virtio_dev_atomic_write_link_status(dev, &link);
+
+	rte_atomic32_set(&hw->started, 0);
+	update_queuing_status(dev);
 }
 
 static int
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index fcd9e93..3ef9678 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -736,6 +736,10 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	int offload;
 	struct virtio_net_hdr *hdr;
 
+	nb_rx = 0;
+	if (unlikely(rte_atomic32_read(&rxvq->allow_queuing) == 0))
+		return nb_rx;
+
 	nb_used = VIRTQUEUE_NUSED(vq);
 
 	virtio_rmb();
@@ -850,6 +854,10 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 	uint32_t hdr_size;
 	int offload;
 
+	nb_rx = 0;
+	if (unlikely(rte_atomic32_read(&rxvq->allow_queuing) == 0))
+		return nb_rx;
+
 	nb_used = VIRTQUEUE_NUSED(vq);
 
 	virtio_rmb();
@@ -1012,6 +1020,10 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	uint16_t nb_used, nb_tx;
 	int error;
 
+	nb_tx = 0;
+	if (unlikely(rte_atomic32_read(&txvq->allow_queuing) == 0))
+		return nb_tx;
+
 	if (unlikely(nb_pkts < 1))
 		return nb_pkts;
 
diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index b651e53..d061e72 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c
@@ -95,6 +95,10 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 	uint16_t nb_tail, nb_commit;
 	int i;
 	uint16_t desc_idx_max = (vq->vq_nentries >> 1) - 1;
+	uint16_t nb_tx = 0;
+
+	if (unlikely(rte_atomic32_read(&txvq->allow_queuing) == 0))
+		return nb_tx;
 
 	nb_used = VIRTQUEUE_NUSED(vq);
 	rte_compiler_barrier();
diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c b/drivers/net/virtio/virtio_rxtx_simple_neon.c
index 793eefb..5aa7e2f 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
@@ -77,7 +77,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	struct vring_used_elem *rused;
 	struct rte_mbuf **sw_ring;
 	struct rte_mbuf **sw_ring_end;
-	uint16_t nb_pkts_received;
+	uint16_t nb_pkts_received = 0;
 
 	uint8x16_t shuf_msk1 = {
 		0xFF, 0xFF, 0xFF, 0xFF, /* packet type */
@@ -106,6 +106,9 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		0, 0
 	};
 
+	if (unlikely(rte_atomic32_read(&rxvq->allow_queuing) == 0))
+		return nb_pkts_received;
+
 	if (unlikely(nb_pkts < RTE_VIRTIO_DESC_PER_LOOP))
 		return 0;
 
diff --git a/drivers/net/virtio/virtio_rxtx_simple_sse.c b/drivers/net/virtio/virtio_rxtx_simple_sse.c
index 87bb5c6..39fe557 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_sse.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_sse.c
@@ -79,7 +79,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	struct vring_used_elem *rused;
 	struct rte_mbuf **sw_ring;
 	struct rte_mbuf **sw_ring_end;
-	uint16_t nb_pkts_received;
+	uint16_t nb_pkts_received = 0;
 	__m128i shuf_msk1, shuf_msk2, len_adjust;
 
 	shuf_msk1 = _mm_set_epi8(
@@ -109,6 +109,9 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		0, (uint16_t)-vq->hw->vtnet_hdr_size,
 		0, 0);
 
+	if (unlikely(rte_atomic32_read(&rxvq->allow_queuing) == 0))
+		return nb_pkts_received;
+
 	if (unlikely(nb_pkts < RTE_VIRTIO_DESC_PER_LOOP))
 		return 0;
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index d3a5cac..00b35e9 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -318,6 +318,7 @@ virtio_user_eth_dev_alloc(const char *name)
 	hw->modern   = 0;
 	hw->use_simple_rxtx = 0;
 	hw->virtio_user_dev = dev;
+	rte_atomic32_set(&hw->started, 0);
 	data->dev_private = hw;
 	data->drv_name = virtio_user_driver.driver.name;
 	data->numa_node = SOCKET_ID_ANY;
-- 
2.7.4

Re: [dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Yuanhan Liu]

On Fri, Mar 31, 2017 at 07:40:17PM +0800, Zhiyong Yang wrote:
> Current dpdk code virtio_dev_stop only disables interrupt and marks link down,
> When it is invoked, tx/rx traffic flows still work. This is a strange behavior.
> The patchset supports the switch of flow by calling virtio_dev_start/stop.
> 
> The implementation refers to vhost pmd.

That's a difference story. Vhost pmd uses 2 vars to track the
status, whereas you are using only one here. So why not
setting/clearing "started" at dev_start/stop, respectively?
Then we can check "started" at Rx/Tx functions.

BTW, why does it have to be atomic?

	--yliu
> 
> Zhiyong Yang (2):
>   net/virtio: add data elements to turn on/off traffic flow
>   net/virtio: support to turn on/off the traffic flow
> 
>  drivers/net/virtio/virtio_ethdev.c           | 37 +++++++++++++++++++++++++++-
>  drivers/net/virtio/virtio_pci.h              |  1 +
>  drivers/net/virtio/virtio_rxtx.c             | 12 +++++++++
>  drivers/net/virtio/virtio_rxtx.h             |  6 +++++
>  drivers/net/virtio/virtio_rxtx_simple.c      |  4 +++
>  drivers/net/virtio/virtio_rxtx_simple_neon.c |  5 +++-
>  drivers/net/virtio/virtio_rxtx_simple_sse.c  |  5 +++-
>  drivers/net/virtio/virtio_user_ethdev.c      |  1 +
>  8 files changed, 68 insertions(+), 3 deletions(-)
> 
> -- 
> 2.7.4

Re: [dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Yang, Zhiyong]

Hi, yuanhan:
	Sorry for the delay reply due to my annual leave.

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Thursday, April 6, 2017 12:00 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com
> Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the traffic flow
> 
> On Fri, Mar 31, 2017 at 07:40:17PM +0800, Zhiyong Yang wrote:
> > Current dpdk code virtio_dev_stop only disables interrupt and marks
> > link down, When it is invoked, tx/rx traffic flows still work. This is a strange
> behavior.
> > The patchset supports the switch of flow by calling virtio_dev_start/stop.
> >
> > The implementation refers to vhost pmd.
> 
> That's a difference story. Vhost pmd uses 2 vars to track the status, whereas you
> are using only one here. So why not setting/clearing "started" at dev_start/stop,
> respectively?
> Then we can check "started" at Rx/Tx functions.

Yes, I use only one var since I think vhost pmd using two is too complex and it is unnecessary.
I'm setting/clearing started at virtio_dev_start/stop,  update_queuing_status is added to avoid
duplicate code. I don't understand your question.

> 
> BTW, why does it have to be atomic?
> 

Consider again. It is not necessary to use atomic here. But It seems that it doesn't  have an negative effect.

Thanks
Zhiyong

> 	--yliu
> >
> > Zhiyong Yang (2):
> >   net/virtio: add data elements to turn on/off traffic flow
> >   net/virtio: support to turn on/off the traffic flow
> >
> >  drivers/net/virtio/virtio_ethdev.c           | 37 +++++++++++++++++++++++++++-
> >  drivers/net/virtio/virtio_pci.h              |  1 +
> >  drivers/net/virtio/virtio_rxtx.c             | 12 +++++++++
> >  drivers/net/virtio/virtio_rxtx.h             |  6 +++++
> >  drivers/net/virtio/virtio_rxtx_simple.c      |  4 +++
> >  drivers/net/virtio/virtio_rxtx_simple_neon.c |  5 +++-
> > drivers/net/virtio/virtio_rxtx_simple_sse.c  |  5 +++-
> >  drivers/net/virtio/virtio_user_ethdev.c      |  1 +
> >  8 files changed, 68 insertions(+), 3 deletions(-)
> >
> > --
> > 2.7.4

Re: [dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Yuanhan Liu]

On Mon, Apr 17, 2017 at 08:50:52AM +0000, Yang, Zhiyong wrote:
> Hi, yuanhan:
> 	Sorry for the delay reply due to my annual leave.
> 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Thursday, April 6, 2017 12:00 PM
> > To: Yang, Zhiyong <zhiyong.yang@intel.com>
> > Cc: dev@dpdk.org; maxime.coquelin@redhat.com
> > Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the traffic flow
> > 
> > On Fri, Mar 31, 2017 at 07:40:17PM +0800, Zhiyong Yang wrote:
> > > Current dpdk code virtio_dev_stop only disables interrupt and marks
> > > link down, When it is invoked, tx/rx traffic flows still work. This is a strange
> > behavior.
> > > The patchset supports the switch of flow by calling virtio_dev_start/stop.
> > >
> > > The implementation refers to vhost pmd.
> > 
> > That's a difference story. Vhost pmd uses 2 vars to track the status, whereas you
> > are using only one here. So why not setting/clearing "started" at dev_start/stop,
> > respectively?
> > Then we can check "started" at Rx/Tx functions.
> 
> Yes, I use only one var since I think vhost pmd using two is too complex and it is unnecessary.

No, it's needed. For vhost-user pmd, we can only do Rx when both below
items are met:

- port is started
- new_device() is invoked, aka, the device is connected

For that reason, two vars is used to track it.

> I'm setting/clearing started at virtio_dev_start/stop,  update_queuing_status is added to avoid
> duplicate code.

It's not about duplicate code. While we could make the var per-device,
you make it per-queue. That's complex and unnecessary.

> I don't understand your question.
> 
> > 
> > BTW, why does it have to be atomic?
> > 
> 
> Consider again. It is not necessary to use atomic here. But It seems that it doesn't  have an negative effect.

Hmm... that's a good reason to keep it, just because it has no negative
effect? Talking about the negative effect, badly, it really has. The
atomic is more expensive.

	--yliu

Re: [dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Yang, Zhiyong]

Hi，yuanhan:
	
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, April 19, 2017 10:03 AM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com
> Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the traffic flow
> 
> On Mon, Apr 17, 2017 at 08:50:52AM +0000, Yang, Zhiyong wrote:
> > Hi, yuanhan:
> > 	Sorry for the delay reply due to my annual leave.
> >
> > > -----Original Message-----
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Thursday, April 6, 2017 12:00 PM
> > > To: Yang, Zhiyong <zhiyong.yang@intel.com>
> > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com
> > > Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the
> > > traffic flow
> > >
> > > On Fri, Mar 31, 2017 at 07:40:17PM +0800, Zhiyong Yang wrote:
> > > > Current dpdk code virtio_dev_stop only disables interrupt and
> > > > marks link down, When it is invoked, tx/rx traffic flows still
> > > > work. This is a strange
> > > behavior.
> > > > The patchset supports the switch of flow by calling virtio_dev_start/stop.
> > > >
> > > > The implementation refers to vhost pmd.
> > >
> > > That's a difference story. Vhost pmd uses 2 vars to track the
> > > status, whereas you are using only one here. So why not
> > > setting/clearing "started" at dev_start/stop, respectively?
> > > Then we can check "started" at Rx/Tx functions.
> >
> > Yes, I use only one var since I think vhost pmd using two is too complex and it
> is unnecessary.
> 
> No, it's needed. For vhost-user pmd, we can only do Rx when both below items
> are met:
> 
> - port is started
> - new_device() is invoked, aka, the device is connected
> 
> For that reason, two vars is used to track it.
> 

Got it. Thanks for your explanation.

> > I'm setting/clearing started at virtio_dev_start/stop,
> > update_queuing_status is added to avoid duplicate code.
> 
> It's not about duplicate code. While we could make the var per-device, you
> make it per-queue. That's complex and unnecessary.
> 

Agree with you.  The var per-device is good.  This can make code simplified.

> > I don't understand your question.
> >
> > >
> > > BTW, why does it have to be atomic?
> > >
> >
> > Consider again. It is not necessary to use atomic here. But It seems that it
> doesn't  have an negative effect.
> 
> Hmm... that's a good reason to keep it, just because it has no negative effect?
> Talking about the negative effect, badly, it really has. The atomic is more
> expensive.

Haha, you are right. 
I will rework it and send V2.

Thanks
Zhiyong
> 
> 	--yliu

Re: [dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Yuanhan Liu]

On Wed, Apr 19, 2017 at 02:31:58AM +0000, Yang, Zhiyong wrote:
> Haha, you are right. 
> I will rework it and send V2.

Note that the "started" field is added in one of my recent patch, to
addres the link status always being UP.

So you should pull the latest virtio code and make a patch there: you
don't need to introduce it any more.

	--yliu

Re: [dpdk-dev] [PATCH 0/2] net/virtio: support to turn on/off the traffic flow

[Yang, Zhiyong]

Ok, yuanhan. I see.

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, April 19, 2017 10:58 AM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com
> Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the traffic flow
> 
> On Wed, Apr 19, 2017 at 02:31:58AM +0000, Yang, Zhiyong wrote:
> > Haha, you are right.
> > I will rework it and send V2.
> 
> Note that the "started" field is added in one of my recent patch, to addres the
> link status always being UP.
> 
> So you should pull the latest virtio code and make a patch there: you don't need
> to introduce it any more.
> 
> 	--yliu

[dpdk-dev] [PATCH v2] net/virtio: support to turn on/off traffic flow

[Zhiyong Yang]

Current virtio_dev_stop only disables interrupt and marks link down,
When it is invoked, tx/rx traffic flows still work. This is a strange
behavior. The patch supports the switch of flow.

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---

Changes in V2:
Use the flag "started" introduced by yuanhan's applied patch, rx/tx
check the per-device defined var "started". 

 drivers/net/virtio/virtio_rxtx.c             | 21 ++++++++++++++-------
 drivers/net/virtio/virtio_rxtx_simple.c      |  5 +++++
 drivers/net/virtio/virtio_rxtx_simple_neon.c |  6 +++++-
 drivers/net/virtio/virtio_rxtx_simple_sse.c  |  6 +++++-
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index ea0bd9d..fbc96df 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -725,7 +725,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct virtnet_rx *rxvq = rx_queue;
 	struct virtqueue *vq = rxvq->vq;
-	struct virtio_hw *hw;
+	struct virtio_hw *hw = vq->hw;
 	struct rte_mbuf *rxm, *new_mbuf;
 	uint16_t nb_used, num, nb_rx;
 	uint32_t len[VIRTIO_MBUF_BURST_SZ];
@@ -736,6 +736,10 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	int offload;
 	struct virtio_net_hdr *hdr;
 
+	nb_rx = 0;
+	if (unlikely(hw->started == 0))
+		return nb_rx;
+
 	nb_used = VIRTQUEUE_NUSED(vq);
 
 	virtio_rmb();
@@ -748,8 +752,6 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	num = virtqueue_dequeue_burst_rx(vq, rcv_pkts, len, num);
 	PMD_RX_LOG(DEBUG, "used:%d dequeue:%d", nb_used, num);
 
-	hw = vq->hw;
-	nb_rx = 0;
 	nb_enqueued = 0;
 	hdr_size = hw->vtnet_hdr_size;
 	offload = rx_offload_enabled(hw);
@@ -834,7 +836,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 {
 	struct virtnet_rx *rxvq = rx_queue;
 	struct virtqueue *vq = rxvq->vq;
-	struct virtio_hw *hw;
+	struct virtio_hw *hw = vq->hw;
 	struct rte_mbuf *rxm, *new_mbuf;
 	uint16_t nb_used, num, nb_rx;
 	uint32_t len[VIRTIO_MBUF_BURST_SZ];
@@ -848,14 +850,16 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 	uint32_t hdr_size;
 	int offload;
 
+	nb_rx = 0;
+	if (unlikely(hw->started == 0))
+		return nb_rx;
+
 	nb_used = VIRTQUEUE_NUSED(vq);
 
 	virtio_rmb();
 
 	PMD_RX_LOG(DEBUG, "used:%d", nb_used);
 
-	hw = vq->hw;
-	nb_rx = 0;
 	i = 0;
 	nb_enqueued = 0;
 	seg_num = 0;
@@ -1005,9 +1009,12 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	struct virtqueue *vq = txvq->vq;
 	struct virtio_hw *hw = vq->hw;
 	uint16_t hdr_size = hw->vtnet_hdr_size;
-	uint16_t nb_used, nb_tx;
+	uint16_t nb_used, nb_tx = 0;
 	int error;
 
+	if (unlikely(hw->started == 0))
+		return nb_tx;
+
 	if (unlikely(nb_pkts < 1))
 		return nb_pkts;
 
diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index b651e53..542cf80 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c
@@ -89,12 +89,17 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 {
 	struct virtnet_tx *txvq = tx_queue;
 	struct virtqueue *vq = txvq->vq;
+	struct virtio_hw *hw = vq->hw;
 	uint16_t nb_used;
 	uint16_t desc_idx;
 	struct vring_desc *start_dp;
 	uint16_t nb_tail, nb_commit;
 	int i;
 	uint16_t desc_idx_max = (vq->vq_nentries >> 1) - 1;
+	uint16_t nb_tx = 0;
+
+	if (unlikely(hw->started == 0))
+		return nb_tx;
 
 	nb_used = VIRTQUEUE_NUSED(vq);
 	rte_compiler_barrier();
diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c b/drivers/net/virtio/virtio_rxtx_simple_neon.c
index 793eefb..ecc62ad 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
@@ -72,12 +72,13 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 {
 	struct virtnet_rx *rxvq = rx_queue;
 	struct virtqueue *vq = rxvq->vq;
+	struct virtio_hw *hw = vq->hw;
 	uint16_t nb_used;
 	uint16_t desc_idx;
 	struct vring_used_elem *rused;
 	struct rte_mbuf **sw_ring;
 	struct rte_mbuf **sw_ring_end;
-	uint16_t nb_pkts_received;
+	uint16_t nb_pkts_received = 0;
 
 	uint8x16_t shuf_msk1 = {
 		0xFF, 0xFF, 0xFF, 0xFF, /* packet type */
@@ -106,6 +107,9 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		0, 0
 	};
 
+	if (unlikely(hw->started == 0))
+		return nb_pkts_received;
+
 	if (unlikely(nb_pkts < RTE_VIRTIO_DESC_PER_LOOP))
 		return 0;
 
diff --git a/drivers/net/virtio/virtio_rxtx_simple_sse.c b/drivers/net/virtio/virtio_rxtx_simple_sse.c
index 87bb5c6..7cf0f8b 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_sse.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_sse.c
@@ -74,12 +74,13 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 {
 	struct virtnet_rx *rxvq = rx_queue;
 	struct virtqueue *vq = rxvq->vq;
+	struct virtio_hw *hw = vq->hw;
 	uint16_t nb_used;
 	uint16_t desc_idx;
 	struct vring_used_elem *rused;
 	struct rte_mbuf **sw_ring;
 	struct rte_mbuf **sw_ring_end;
-	uint16_t nb_pkts_received;
+	uint16_t nb_pkts_received = 0;
 	__m128i shuf_msk1, shuf_msk2, len_adjust;
 
 	shuf_msk1 = _mm_set_epi8(
@@ -109,6 +110,9 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		0, (uint16_t)-vq->hw->vtnet_hdr_size,
 		0, 0);
 
+	if (unlikely(hw->started == 0))
+		return nb_pkts_received;
+
 	if (unlikely(nb_pkts < RTE_VIRTIO_DESC_PER_LOOP))
 		return 0;
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] net/virtio: support to turn on/off traffic flow

[Maxime Coquelin]

On 04/19/2017 08:29 AM, Zhiyong Yang wrote:
> Current virtio_dev_stop only disables interrupt and marks link down,
> When it is invoked, tx/rx traffic flows still work. This is a strange
> behavior. The patch supports the switch of flow.
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> ---
> 
> Changes in V2:
> Use the flag "started" introduced by yuanhan's applied patch, rx/tx
> check the per-device defined var "started".
> 
>   drivers/net/virtio/virtio_rxtx.c             | 21 ++++++++++++++-------
>   drivers/net/virtio/virtio_rxtx_simple.c      |  5 +++++
>   drivers/net/virtio/virtio_rxtx_simple_neon.c |  6 +++++-
>   drivers/net/virtio/virtio_rxtx_simple_sse.c  |  6 +++++-
>   4 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index ea0bd9d..fbc96df 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -725,7 +725,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>   {
>   	struct virtnet_rx *rxvq = rx_queue;
>   	struct virtqueue *vq = rxvq->vq;
> -	struct virtio_hw *hw;
> +	struct virtio_hw *hw = vq->hw;
>   	struct rte_mbuf *rxm, *new_mbuf;
>   	uint16_t nb_used, num, nb_rx;
>   	uint32_t len[VIRTIO_MBUF_BURST_SZ];
> @@ -736,6 +736,10 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>   	int offload;
>   	struct virtio_net_hdr *hdr;
>   
> +	nb_rx = 0;

Only cosmetics, but I would do like in Tx path, i.e. set nb_rx to 0
directly at declaration time.

> +	if (unlikely(hw->started == 0))
> +		return nb_rx;
> +
>   	nb_used = VIRTQUEUE_NUSED(vq);
>   
>   	virtio_rmb();
> @@ -748,8 +752,6 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>   	num = virtqueue_dequeue_burst_rx(vq, rcv_pkts, len, num);
>   	PMD_RX_LOG(DEBUG, "used:%d dequeue:%d", nb_used, num);
>   
> -	hw = vq->hw;
> -	nb_rx = 0;
>   	nb_enqueued = 0;
>   	hdr_size = hw->vtnet_hdr_size;
>   	offload = rx_offload_enabled(hw);
> @@ -834,7 +836,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
>   {
>   	struct virtnet_rx *rxvq = rx_queue;
>   	struct virtqueue *vq = rxvq->vq;
> -	struct virtio_hw *hw;
> +	struct virtio_hw *hw = vq->hw;
>   	struct rte_mbuf *rxm, *new_mbuf;
>   	uint16_t nb_used, num, nb_rx;
>   	uint32_t len[VIRTIO_MBUF_BURST_SZ];
> @@ -848,14 +850,16 @@ virtio_recv_mergeable_pkts(void *rx_queue,
>   	uint32_t hdr_size;
>   	int offload;
>   
> +	nb_rx = 0;
Ditto.

> +	if (unlikely(hw->started == 0))
> +		return nb_rx;
> +
>   	nb_used = VIRTQUEUE_NUSED(vq);
>   
>   	virtio_rmb();
>   
>   	PMD_RX_LOG(DEBUG, "used:%d", nb_used);
>   
> -	hw = vq->hw;
> -	nb_rx = 0;
>   	i = 0;
>   	nb_enqueued = 0;
>   	seg_num = 0;
> @@ -1005,9 +1009,12 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>   	struct virtqueue *vq = txvq->vq;
>   	struct virtio_hw *hw = vq->hw;
>   	uint16_t hdr_size = hw->vtnet_hdr_size;
> -	uint16_t nb_used, nb_tx;
> +	uint16_t nb_used, nb_tx = 0;
>   	int error;
>   
> +	if (unlikely(hw->started == 0))
> +		return nb_tx;
> +
>   	if (unlikely(nb_pkts < 1))
>   		return nb_pkts;
>   
> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
> index b651e53..542cf80 100644
> --- a/drivers/net/virtio/virtio_rxtx_simple.c
> +++ b/drivers/net/virtio/virtio_rxtx_simple.c
> @@ -89,12 +89,17 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
>   {
>   	struct virtnet_tx *txvq = tx_queue;
>   	struct virtqueue *vq = txvq->vq;
> +	struct virtio_hw *hw = vq->hw;
>   	uint16_t nb_used;
>   	uint16_t desc_idx;
>   	struct vring_desc *start_dp;
>   	uint16_t nb_tail, nb_commit;
>   	int i;
>   	uint16_t desc_idx_max = (vq->vq_nentries >> 1) - 1;
> +	uint16_t nb_tx = 0;
> +
> +	if (unlikely(hw->started == 0))
> +		return nb_tx;
Maybe just return 0 directly, nb_tx really needed here?

>   
>   	nb_used = VIRTQUEUE_NUSED(vq);
>   	rte_compiler_barrier();
> diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> index 793eefb..ecc62ad 100644
> --- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
> +++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> @@ -72,12 +72,13 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>   {
>   	struct virtnet_rx *rxvq = rx_queue;
>   	struct virtqueue *vq = rxvq->vq;
> +	struct virtio_hw *hw = vq->hw;
>   	uint16_t nb_used;
>   	uint16_t desc_idx;
>   	struct vring_used_elem *rused;
>   	struct rte_mbuf **sw_ring;
>   	struct rte_mbuf **sw_ring_end;
> -	uint16_t nb_pkts_received;
> +	uint16_t nb_pkts_received = 0;
>   
>   	uint8x16_t shuf_msk1 = {
>   		0xFF, 0xFF, 0xFF, 0xFF, /* packet type */
> @@ -106,6 +107,9 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>   		0, 0
>   	};
>   
> +	if (unlikely(hw->started == 0))
> +		return nb_pkts_received;
Ditto.

> +
>   	if (unlikely(nb_pkts < RTE_VIRTIO_DESC_PER_LOOP))
>   		return 0;
>   
> diff --git a/drivers/net/virtio/virtio_rxtx_simple_sse.c b/drivers/net/virtio/virtio_rxtx_simple_sse.c
> index 87bb5c6..7cf0f8b 100644
> --- a/drivers/net/virtio/virtio_rxtx_simple_sse.c
> +++ b/drivers/net/virtio/virtio_rxtx_simple_sse.c
> @@ -74,12 +74,13 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>   {
>   	struct virtnet_rx *rxvq = rx_queue;
>   	struct virtqueue *vq = rxvq->vq;
> +	struct virtio_hw *hw = vq->hw;
>   	uint16_t nb_used;
>   	uint16_t desc_idx;
>   	struct vring_used_elem *rused;
>   	struct rte_mbuf **sw_ring;
>   	struct rte_mbuf **sw_ring_end;
> -	uint16_t nb_pkts_received;
> +	uint16_t nb_pkts_received = 0;
>   	__m128i shuf_msk1, shuf_msk2, len_adjust;
>   
>   	shuf_msk1 = _mm_set_epi8(
> @@ -109,6 +110,9 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>   		0, (uint16_t)-vq->hw->vtnet_hdr_size,
>   		0, 0);
>   
> +	if (unlikely(hw->started == 0))
> +		return nb_pkts_received;
Ditto.

> +
>   	if (unlikely(nb_pkts < RTE_VIRTIO_DESC_PER_LOOP))
>   		return 0;


As said, only cosmetic comments, it looks otherwise valid to me.
If you fix these, feel free to add my:
Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v2] net/virtio: support to turn on/off traffic flow

[Yuanhan Liu]

On Wed, Apr 26, 2017 at 09:45:42AM +0200, Maxime Coquelin wrote:
> As said, only cosmetic comments, it looks otherwise valid to me.
> If you fix these, feel free to add my:
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Oops, I forgot to reply this thread, that I have already applied it :/

	--yliu

Re: [dpdk-dev] [PATCH v2] net/virtio: support to turn on/off traffic flow

[Maxime Coquelin]

On 04/26/2017 09:56 AM, Yuanhan Liu wrote:
> On Wed, Apr 26, 2017 at 09:45:42AM +0200, Maxime Coquelin wrote:
>> As said, only cosmetic comments, it looks otherwise valid to me.
>> If you fix these, feel free to add my:
>> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Oops, I forgot to reply this thread, that I have already applied it :/

No worries, not a big deal :)
The fix is valid, which is the most important.

Cheers,
Maxime