[dpdk-dev,v11,5/5] net/virtio: support GUEST ANNOUNCE

Message ID 20180116214103.67803-6-xiao.w.wang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply patch file failure

Commit Message

Xiao Wang Jan. 16, 2018, 9:41 p.m. UTC
  When live migration is done, for the backup VM, either the virtio
frontend or the vhost backend needs to send out gratuitous RARP packet
to announce its new network location.

This patch enables VIRTIO_NET_F_GUEST_ANNOUNCE feature to support live
migration scenario where the vhost backend doesn't have the ability to
generate RARP packet.

Brief introduction of the work flow:
1. QEMU finishes live migration, pokes the backup VM with an interrupt.
2. Virtio interrupt handler reads out the interrupt status value, and
   realizes it needs to send out RARP packet to announce its location.
3. Pause device to stop worker thread touching the queues.
4. Inject a RARP packet into a Tx Queue.
5. Ack the interrupt via control queue.
6. Resume device to continue packet processing.

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 50 ++++++++++++++++++++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h |  1 +
 drivers/net/virtio/virtqueue.h     | 11 +++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)
  

Comments

Ferruh Yigit Jan. 19, 2018, 5:33 p.m. UTC | #1
On 1/16/2018 9:41 PM, Xiao Wang wrote:
> When live migration is done, for the backup VM, either the virtio
> frontend or the vhost backend needs to send out gratuitous RARP packet
> to announce its new network location.
> 
> This patch enables VIRTIO_NET_F_GUEST_ANNOUNCE feature to support live
> migration scenario where the vhost backend doesn't have the ability to
> generate RARP packet.
> 
> Brief introduction of the work flow:
> 1. QEMU finishes live migration, pokes the backup VM with an interrupt.
> 2. Virtio interrupt handler reads out the interrupt status value, and
>    realizes it needs to send out RARP packet to announce its location.
> 3. Pause device to stop worker thread touching the queues.
> 4. Inject a RARP packet into a Tx Queue.
> 5. Ack the interrupt via control queue.
> 6. Resume device to continue packet processing.
> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>


Hi Yuanhan,

This commit breaks the build!

As far as I understand you send a fix but merged into other patch, which leaves
this commit still broken.

What do you think sending a fix that can be mergable to this one, so I can
squash it on next-net?

Thanks,
ferruh
  
Ferruh Yigit Jan. 20, 2018, 2:31 p.m. UTC | #2
On 1/19/2018 5:33 PM, Ferruh Yigit wrote:
> On 1/16/2018 9:41 PM, Xiao Wang wrote:
>> When live migration is done, for the backup VM, either the virtio
>> frontend or the vhost backend needs to send out gratuitous RARP packet
>> to announce its new network location.
>>
>> This patch enables VIRTIO_NET_F_GUEST_ANNOUNCE feature to support live
>> migration scenario where the vhost backend doesn't have the ability to
>> generate RARP packet.
>>
>> Brief introduction of the work flow:
>> 1. QEMU finishes live migration, pokes the backup VM with an interrupt.
>> 2. Virtio interrupt handler reads out the interrupt status value, and
>>    realizes it needs to send out RARP packet to announce its location.
>> 3. Pause device to stop worker thread touching the queues.
>> 4. Inject a RARP packet into a Tx Queue.
>> 5. Ack the interrupt via control queue.
>> 6. Resume device to continue packet processing.
>>
>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> 
> Hi Yuanhan,
> 
> This commit breaks the build!

I switched two patches and problem gone, like:
first: net: fixup RARP generation
second: net/virtio: support GUEST ANNOUNCE

From my point of view nothing more needs to be done, but can you please double
check the patches.

Thanks,
ferruh

> 
> As far as I understand you send a fix but merged into other patch, which leaves
> this commit still broken.
> 
> What do you think sending a fix that can be mergable to this one, so I can
> squash it on next-net?
> 
> Thanks,
> ferruh
>
  
Xiao Wang Jan. 21, 2018, 1:31 a.m. UTC | #3
> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Saturday, January 20, 2018 10:31 PM

> To: Wang, Xiao W <xiao.w.wang@intel.com>; yliu@fridaylinux.org;

> olivier.matz@6wind.com; maxime.coquelin@redhat.com; Thomas Monjalon

> <thomas@monjalon.net>

> Cc: dev@dpdk.org; Bie, Tiwei <tiwei.bie@intel.com>;

> stephen@networkplumber.org

> Subject: Re: [dpdk-dev] [PATCH v11 5/5] net/virtio: support GUEST ANNOUNCE

> 

> On 1/19/2018 5:33 PM, Ferruh Yigit wrote:

> > On 1/16/2018 9:41 PM, Xiao Wang wrote:

> >> When live migration is done, for the backup VM, either the virtio

> >> frontend or the vhost backend needs to send out gratuitous RARP packet

> >> to announce its new network location.

> >>

> >> This patch enables VIRTIO_NET_F_GUEST_ANNOUNCE feature to support

> live

> >> migration scenario where the vhost backend doesn't have the ability to

> >> generate RARP packet.

> >>

> >> Brief introduction of the work flow:

> >> 1. QEMU finishes live migration, pokes the backup VM with an interrupt.

> >> 2. Virtio interrupt handler reads out the interrupt status value, and

> >>    realizes it needs to send out RARP packet to announce its location.

> >> 3. Pause device to stop worker thread touching the queues.

> >> 4. Inject a RARP packet into a Tx Queue.

> >> 5. Ack the interrupt via control queue.

> >> 6. Resume device to continue packet processing.

> >>

> >> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>

> >> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

> >

> >

> > Hi Yuanhan,

> >

> > This commit breaks the build!

> 

> I switched two patches and problem gone, like:

> first: net: fixup RARP generation

> second: net/virtio: support GUEST ANNOUNCE

> 

> From my point of view nothing more needs to be done, but can you please

> double

> check the patches.


The 2 patches are OK.
Thanks!

BRs,
Xiao
> 

> Thanks,

> ferruh

> 

> >

> > As far as I understand you send a fix but merged into other patch, which

> leaves

> > this commit still broken.

> >

> > What do you think sending a fix that can be mergable to this one, so I can

> > squash it on next-net?

> >

> > Thanks,

> > ferruh

> >
  

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e8ff1e449..4f60d1367 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -19,6 +19,8 @@ 
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
 #include <rte_ether.h>
+#include <rte_ip.h>
+#include <rte_arp.h>
 #include <rte_common.h>
 #include <rte_errno.h>
 #include <rte_cpuflags.h>
@@ -78,6 +80,9 @@  static int virtio_dev_queue_stats_mapping_set(
 	uint8_t stat_idx,
 	uint8_t is_rx);
 
+static void virtio_notify_peers(struct rte_eth_dev *dev);
+static void virtio_ack_link_announce(struct rte_eth_dev *dev);
+
 /*
  * The set of PCI devices this driver supports
  */
@@ -1272,9 +1277,46 @@  virtio_inject_pkts(struct rte_eth_dev *dev, struct rte_mbuf **tx_pkts,
 	return ret;
 }
 
+static void
+virtio_notify_peers(struct rte_eth_dev *dev)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	struct virtnet_rx *rxvq = dev->data->rx_queues[0];
+	struct rte_mbuf *rarp_mbuf;
+
+	rarp_mbuf = rte_net_make_rarp_packet(rxvq->mpool,
+			(struct ether_addr *)hw->mac_addr);
+	if (rarp_mbuf == NULL) {
+		PMD_DRV_LOG(ERR, "failed to make RARP packet.");
+		return;
+	}
+
+	/* If virtio port just stopped, no need to send RARP */
+	if (virtio_dev_pause(dev) < 0) {
+		rte_pktmbuf_free(rarp_mbuf);
+		return;
+	}
+
+	virtio_inject_pkts(dev, &rarp_mbuf, 1);
+	virtio_dev_resume(dev);
+}
+
+static void
+virtio_ack_link_announce(struct rte_eth_dev *dev)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	struct virtio_pmd_ctrl ctrl;
+
+	ctrl.hdr.class = VIRTIO_NET_CTRL_ANNOUNCE;
+	ctrl.hdr.cmd = VIRTIO_NET_CTRL_ANNOUNCE_ACK;
+
+	virtio_send_command(hw->cvq, &ctrl, NULL, 0);
+}
+
 /*
- * Process Virtio Config changed interrupt and call the callback
- * if link state changed.
+ * Process virtio config changed interrupt. Call the callback
+ * if link state changed, generate gratuitous RARP packet if
+ * the status indicates an ANNOUNCE.
  */
 void
 virtio_interrupt_handler(void *param)
@@ -1297,6 +1339,10 @@  virtio_interrupt_handler(void *param)
 						      NULL, NULL);
 	}
 
+	if (isr & VIRTIO_NET_S_ANNOUNCE) {
+		virtio_notify_peers(dev);
+		virtio_ack_link_announce(dev);
+	}
 }
 
 /* set rx and tx handlers according to what is supported */
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 69b30b7e1..09ebc5fb5 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -38,6 +38,7 @@ 
 	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
 	 1u << VIRTIO_NET_F_MTU	| \
+	 1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE |	\
 	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
 	 1ULL << VIRTIO_F_VERSION_1       |	\
 	 1ULL << VIRTIO_F_IOMMU_PLATFORM)
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 1482a951d..60df359b3 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -129,6 +129,17 @@  struct virtio_net_ctrl_mac {
 #define VIRTIO_NET_CTRL_VLAN_ADD 0
 #define VIRTIO_NET_CTRL_VLAN_DEL 1
 
+/*
+ * Control link announce acknowledgement
+ *
+ * The command VIRTIO_NET_CTRL_ANNOUNCE_ACK is used to indicate that
+ * driver has recevied the notification; device would clear the
+ * VIRTIO_NET_S_ANNOUNCE bit in the status field after it receives
+ * this command.
+ */
+#define VIRTIO_NET_CTRL_ANNOUNCE     3
+#define VIRTIO_NET_CTRL_ANNOUNCE_ACK 0
+
 struct virtio_net_ctrl_hdr {
 	uint8_t class;
 	uint8_t cmd;