[dpdk-dev,1/2] net/virtio: make control queue thread-safe

Message ID 1511521440-57724-2-git-send-email-xiao.w.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Xiao Wang Nov. 24, 2017, 11:03 a.m. UTC
  The virtio_send_command function may be called from app's configuration
routine, but also from an interrupt handler called when live migration is
done on the backup side. So this patch makes control queue thread-safe
first.

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 ++++++-
 drivers/net/virtio/virtio_rxtx.c   | 1 +
 drivers/net/virtio/virtio_rxtx.h   | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)
  

Comments

Tiwei Bie Nov. 24, 2017, 5:38 a.m. UTC | #1
On Fri, Nov 24, 2017 at 03:03:59AM -0800, Xiao Wang wrote:
[...]
> diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
> index 54f1e84..24e3026 100644
> --- a/drivers/net/virtio/virtio_rxtx.h
> +++ b/drivers/net/virtio/virtio_rxtx.h
> @@ -84,6 +84,7 @@ struct virtnet_ctl {
>  	rte_iova_t virtio_net_hdr_mem;  /**< hdr for each xmit packet */
>  	uint16_t port_id;               /**< Device port identifier. */
>  	const struct rte_memzone *mz;   /**< mem zone to populate CTL ring. */
> +	rte_spinlock_t sl;				/**< spinlock for control queue. */

Please use spaces instead of (4 spaces) tabs between the code and comments.

Best regards,
Tiwei Bie
  
Xiao Wang Nov. 30, 2017, 2:10 a.m. UTC | #2
> -----Original Message-----

> From: Bie, Tiwei

> Sent: Friday, November 24, 2017 1:39 PM

> To: Wang, Xiao W <xiao.w.wang@intel.com>

> Cc: dev@dpdk.org; yliu@fridaylinux.org

> Subject: Re: [dpdk-dev] [PATCH 1/2] net/virtio: make control queue thread-

> safe

> 

> On Fri, Nov 24, 2017 at 03:03:59AM -0800, Xiao Wang wrote:

> [...]

> > diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h

> > index 54f1e84..24e3026 100644

> > --- a/drivers/net/virtio/virtio_rxtx.h

> > +++ b/drivers/net/virtio/virtio_rxtx.h

> > @@ -84,6 +84,7 @@ struct virtnet_ctl {

> >  	rte_iova_t virtio_net_hdr_mem;  /**< hdr for each xmit packet */

> >  	uint16_t port_id;               /**< Device port identifier. */

> >  	const struct rte_memzone *mz;   /**< mem zone to populate CTL ring.

> */

> > +	rte_spinlock_t sl;				/**< spinlock for

> control queue. */

> 

> Please use spaces instead of (4 spaces) tabs between the code and comments.


Will change it in v2, thanks!

BRs,
Xiao
> 

> Best regards,

> Tiwei Bie
  
Stephen Hemminger Nov. 30, 2017, 2:59 a.m. UTC | #3
On Fri, 24 Nov 2017 03:03:59 -0800
Xiao Wang <xiao.w.wang@intel.com> wrote:

> @@ -184,8 +186,10 @@ struct rte_virtio_xstats_name_off {
>  		"vq->hw->cvq = %p vq = %p",
>  		vq->vq_desc_head_idx, status, vq->hw->cvq, vq);
>  
> -	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1))
> +	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1)) {

You ndon't need so many paranthesis.  
	if (vq->vq_free_cnt < pkt_num + 2 || pkt_num < 1)
  
Xiao Wang Dec. 1, 2017, 1:38 a.m. UTC | #4
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, November 30, 2017 10:59 AM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org; yliu@fridaylinux.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] net/virtio: make control queue thread-
> safe
> 
> On Fri, 24 Nov 2017 03:03:59 -0800
> Xiao Wang <xiao.w.wang@intel.com> wrote:
> 
> > @@ -184,8 +186,10 @@ struct rte_virtio_xstats_name_off {
> >  		"vq->hw->cvq = %p vq = %p",
> >  		vq->vq_desc_head_idx, status, vq->hw->cvq, vq);
> >
> > -	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1))
> > +	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1)) {
> 
> You ndon't need so many paranthesis.
> 	if (vq->vq_free_cnt < pkt_num + 2 || pkt_num < 1)

Yes, it looks better. Will change it in v2.

Thanks,
Xiao
  
Xiao Wang Dec. 4, 2017, 2:02 p.m. UTC | #5
When live migration is finished, the backup VM needs to proactively announce
its new location. DPDK vhost has implemented VHOST_USER_PROTOCOL_F_RARP to
generate a RARP packet to switch in dequeue path. Another method is to let
the guest proactively send out RARP packet using VIRTIO_NET_F_GUEST_ANNOUNCE
feature.

This patch set enables this feature in virtio pmd, to support VM running virtio
pmd be migrated without vhost supporting RARP generation.

v2:
- Use spaces instead of tabs between the code and comments.
- Remove unnecessary parentheses.
- Use rte_pktmbuf_mtod directly to get eth_hdr addr.
- Fix virtio_dev_pause return value check.

Xiao Wang (2):
  net/virtio: make control queue thread-safe
  net/virtio: support GUEST ANNOUNCE

 drivers/net/virtio/virtio_ethdev.c | 138 ++++++++++++++++++++++++++++++++++++-
 drivers/net/virtio/virtio_ethdev.h |   4 ++
 drivers/net/virtio/virtio_pci.h    |   1 +
 drivers/net/virtio/virtio_rxtx.c   |  82 ++++++++++++++++++++++
 drivers/net/virtio/virtio_rxtx.h   |   1 +
 drivers/net/virtio/virtqueue.h     |  11 +++
 6 files changed, 234 insertions(+), 3 deletions(-)
  

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e0328f6..1959b11 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -177,6 +177,8 @@  struct rte_virtio_xstats_name_off {
 		PMD_INIT_LOG(ERR, "Control queue is not supported.");
 		return -1;
 	}
+
+	rte_spinlock_lock(&cvq->sl);
 	vq = cvq->vq;
 	head = vq->vq_desc_head_idx;
 
@@ -184,8 +186,10 @@  struct rte_virtio_xstats_name_off {
 		"vq->hw->cvq = %p vq = %p",
 		vq->vq_desc_head_idx, status, vq->hw->cvq, vq);
 
-	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1))
+	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1)) {
+		rte_spinlock_unlock(&cvq->sl);
 		return -1;
+	}
 
 	memcpy(cvq->virtio_net_hdr_mz->addr, ctrl,
 		sizeof(struct virtio_pmd_ctrl));
@@ -261,6 +265,7 @@  struct rte_virtio_xstats_name_off {
 
 	result = cvq->virtio_net_hdr_mz->addr;
 
+	rte_spinlock_unlock(&cvq->sl);
 	return result->status;
 }
 
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 390c137..6a24fde 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -407,6 +407,7 @@ 
 	struct virtio_hw *hw = dev->data->dev_private;
 
 	if (hw->cvq && hw->cvq->vq) {
+		rte_spinlock_init(&hw->cvq->sl);
 		VIRTQUEUE_DUMP((struct virtqueue *)hw->cvq->vq);
 	}
 }
diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
index 54f1e84..24e3026 100644
--- a/drivers/net/virtio/virtio_rxtx.h
+++ b/drivers/net/virtio/virtio_rxtx.h
@@ -84,6 +84,7 @@  struct virtnet_ctl {
 	rte_iova_t virtio_net_hdr_mem;  /**< hdr for each xmit packet */
 	uint16_t port_id;               /**< Device port identifier. */
 	const struct rte_memzone *mz;   /**< mem zone to populate CTL ring. */
+	rte_spinlock_t sl;				/**< spinlock for control queue. */
 };
 
 int virtio_rxq_vec_setup(struct virtnet_rx *rxvq);