[PATCH] staging: hv: Fix GARP not sent after Quick Migration

[PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Haiyang Zhang]

After Quick Migration, the network is not immediately operational in the
current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, I put
the netif_notify_peers() into a scheduled work, otherwise GARP packet will
not be sent after quick migration, and cause network disconnection.

Thanks to Mike Surcouf <mike@surcouf.co.uk> for reporting the bug and
testing the patch.

This patch should also be backported to stable kernel 2.6.32 and later.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Mike Surcouf <mike@surcouf.co.uk>
Cc: stable <stable@kernel.org>
---
 drivers/staging/hv/netvsc_drv.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 2d40f5f..417e5f7 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -46,6 +46,7 @@ struct net_device_context {
 	/* point back to our device context */
 	struct hv_device *device_ctx;
 	unsigned long avail;
+	struct work_struct work;
 };
 
 
@@ -219,6 +220,7 @@ static void netvsc_linkstatus_callback(struct hv_device *device_obj,
 				       unsigned int status)
 {
 	struct net_device *net = dev_get_drvdata(&device_obj->device);
+	struct net_device_context *ndev_ctx;
 
 	if (!net) {
 		DPRINT_ERR(NETVSC_DRV, "got link status but net device "
@@ -229,7 +231,8 @@ static void netvsc_linkstatus_callback(struct hv_device *device_obj,
 	if (status == 1) {
 		netif_carrier_on(net);
 		netif_wake_queue(net);
-		netif_notify_peers(net);
+		ndev_ctx = netdev_priv(net);
+		schedule_work(&ndev_ctx->work);
 	} else {
 		netif_carrier_off(net);
 		netif_stop_queue(net);
@@ -328,6 +331,17 @@ static const struct net_device_ops device_ops = {
 	.ndo_set_mac_address =		eth_mac_addr,
 };
 
+static void netvsc_send_garp(struct work_struct *w)
+{
+	struct net_device_context *ndev_ctx;
+	struct net_device *net;
+
+	ndev_ctx = container_of(w, struct net_device_context, work);
+	net = dev_get_drvdata(&ndev_ctx->device_ctx->device);
+	netif_notify_peers(net);
+}
+
+
 static int netvsc_probe(struct device *device)
 {
 	struct hv_driver *drv =
@@ -353,6 +367,7 @@ static int netvsc_probe(struct device *device)
 	net_device_ctx->device_ctx = device_obj;
 	net_device_ctx->avail = ring_size;
 	dev_set_drvdata(device, net);
+	INIT_WORK(&net_device_ctx->work, netvsc_send_garp);
 
 	/* Notify the netvsc driver of the new device */
 	ret = net_drv_obj->base.dev_add(device_obj, &device_info);
-- 
1.6.3.2

Re: [PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Greg KH]

On Tue, Apr 05, 2011 at 08:27:48AM -0700, Haiyang Zhang wrote:
> After Quick Migration, the network is not immediately operational in the
> current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, I put
> the netif_notify_peers() into a scheduled work, otherwise GARP packet will
> not be sent after quick migration, and cause network disconnection.

But that's a hypervisor bug, right?  It shouldn't be sending a "connect"
event on the ethernet device if the network really isn't present.  How
about getting it fixed there, which will resolve the issues for all
guest operating systems, not just Linux.

Or, if this is something that the hyperv developers are not going to
fix, we need to document it in the code itself so that people don't try
to remove it in a few years as an "optimization".

thanks,

greg k-h

RE: [PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Haiyang Zhang]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, April 05, 2011 11:42 AM
> But that's a hypervisor bug, right?  It shouldn't be sending a "connect"
> event on the ethernet device if the network really isn't present.  How
> about getting it fixed there, which will resolve the issues for all
> guest operating systems, not just Linux.
>
> Or, if this is something that the hyperv developers are not going to
> fix, we need to document it in the code itself so that people don't try
> to remove it in a few years as an "optimization".

Fix on the Hyper-V side is not available for now at least. I will add the
comments into code.

Thanks,
- Haiyang

Re: [PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Greg KH]

On Tue, Apr 05, 2011 at 03:54:54PM +0000, Haiyang Zhang wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Tuesday, April 05, 2011 11:42 AM
> > But that's a hypervisor bug, right?  It shouldn't be sending a "connect"
> > event on the ethernet device if the network really isn't present.  How
> > about getting it fixed there, which will resolve the issues for all
> > guest operating systems, not just Linux.
> >
> > Or, if this is something that the hyperv developers are not going to
> > fix, we need to document it in the code itself so that people don't try
> > to remove it in a few years as an "optimization".
> 
> Fix on the Hyper-V side is not available for now at least. I will add the
> comments into code.

What happens when the hyperv code is fixed?  Will this be removed?  Or
will all guests have this delay in them for no reason and the older
"unfixed" guests suddenly work faster?  That seems odd...

thanks,

greg k-h

[PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Haiyang Zhang]

After Quick Migration, the network is not immediately operational in the
current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, I put
the netif_notify_peers() into a scheduled work, otherwise GARP packet will
not be sent after quick migration, and cause network disconnection.

Thanks to Mike Surcouf <mike@surcouf.co.uk> for reporting the bug and
testing the patch.

This patch should also be backported to stable kernel 2.6.32 and later.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Mike Surcouf <mike@surcouf.co.uk>
Cc: stable <stable@kernel.org>
---
 drivers/staging/hv/netvsc_drv.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 2d40f5f..3566456 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -46,6 +46,7 @@ struct net_device_context {
 	/* point back to our device context */
 	struct hv_device *device_ctx;
 	unsigned long avail;
+	struct work_struct work;
 };
 
 
@@ -219,6 +220,7 @@ static void netvsc_linkstatus_callback(struct hv_device *device_obj,
 				       unsigned int status)
 {
 	struct net_device *net = dev_get_drvdata(&device_obj->device);
+	struct net_device_context *ndev_ctx;
 
 	if (!net) {
 		DPRINT_ERR(NETVSC_DRV, "got link status but net device "
@@ -229,7 +231,8 @@ static void netvsc_linkstatus_callback(struct hv_device *device_obj,
 	if (status == 1) {
 		netif_carrier_on(net);
 		netif_wake_queue(net);
-		netif_notify_peers(net);
+		ndev_ctx = netdev_priv(net);
+		schedule_work(&ndev_ctx->work);
 	} else {
 		netif_carrier_off(net);
 		netif_stop_queue(net);
@@ -328,6 +331,24 @@ static const struct net_device_ops device_ops = {
 	.ndo_set_mac_address =		eth_mac_addr,
 };
 
+/*
+ * Send GARP packet to network peers after migrations.
+ * After Quick Migration, the network is not immediately operational in the
+ * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, I put
+ * the netif_notify_peers() into a scheduled work, otherwise GARP packet will
+ * not be sent after quick migration, and cause network disconnection.
+ */
+static void netvsc_send_garp(struct work_struct *w)
+{
+	struct net_device_context *ndev_ctx;
+	struct net_device *net;
+
+	ndev_ctx = container_of(w, struct net_device_context, work);
+	net = dev_get_drvdata(&ndev_ctx->device_ctx->device);
+	netif_notify_peers(net);
+}
+
+
 static int netvsc_probe(struct device *device)
 {
 	struct hv_driver *drv =
@@ -353,6 +374,7 @@ static int netvsc_probe(struct device *device)
 	net_device_ctx->device_ctx = device_obj;
 	net_device_ctx->avail = ring_size;
 	dev_set_drvdata(device, net);
+	INIT_WORK(&net_device_ctx->work, netvsc_send_garp);
 
 	/* Notify the netvsc driver of the new device */
 	ret = net_drv_obj->base.dev_add(device_obj, &device_info);
-- 
1.7.4.1

Re: [PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Greg KH]

On Tue, Apr 05, 2011 at 09:24:15AM -0700, Haiyang Zhang wrote:
> After Quick Migration, the network is not immediately operational in the
> current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, I put
> the netif_notify_peers() into a scheduled work, otherwise GARP packet will
> not be sent after quick migration, and cause network disconnection.

Wait, what are you relying on here for the delay?  What happens when
processors get faster and that work queue gets run even sooner than
today?  You are relying on the scheduler to introduce a "delay" here and
you can't rely on that at all.

Instead, use a proper timer as you do know how long you need to wait,
right?  Don't abuse workqueues for something they were not designed for.

thanks,

greg k-h

RE: [PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Haiyang Zhang]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, April 05, 2011 12:23 PM
> Instead, use a proper timer as you do know how long you need to wait,
> right?  Don't abuse workqueues for something they were not designed for.

Thanks for the suggestion, I will add an msleep. And, yes, this delay will
be removed if the Hyper-V side is fixed.

Thanks,
- Haiyang

[PATCH] staging: hv: Fix GARP not sent after Quick Migration

[Haiyang Zhang]

After Quick Migration, the network is not immediately operational in the
current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, I added
another netif_notify_peers() into a scheduled work, otherwise GARP packet will
not be sent after quick migration, and cause network disconnection.

Thanks to Mike Surcouf <mike@surcouf.co.uk> for reporting the bug and
testing the patch.

This patch should also be backported to stable kernel 2.6.32 and later.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Mike Surcouf <mike@surcouf.co.uk>
Cc: stable <stable@kernel.org>
---
 drivers/staging/hv/netvsc_drv.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 54a203b..258233d 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -48,6 +48,7 @@ struct net_device_context {
 	/* point back to our device context */
 	struct hv_device *device_ctx;
 	unsigned long avail;
+	struct work_struct work;
 };
 
 
@@ -214,6 +215,7 @@ static void netvsc_linkstatus_callback(struct hv_device *device_obj,
 				       unsigned int status)
 {
 	struct net_device *net = dev_get_drvdata(&device_obj->device);
+	struct net_device_context *ndev_ctx;
 
 	if (!net) {
 		netdev_err(net, "got link status but net device "
@@ -225,6 +227,8 @@ static void netvsc_linkstatus_callback(struct hv_device *device_obj,
 		netif_carrier_on(net);
 		netif_wake_queue(net);
 		netif_notify_peers(net);
+		ndev_ctx = netdev_priv(net);
+		schedule_work(&ndev_ctx->work);
 	} else {
 		netif_carrier_off(net);
 		netif_stop_queue(net);
@@ -320,6 +324,25 @@ static const struct net_device_ops device_ops = {
 	.ndo_set_mac_address =		eth_mac_addr,
 };
 
+/*
+ * Send GARP packet to network peers after migrations.
+ * After Quick Migration, the network is not immediately operational in the
+ * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, add
+ * another netif_notify_peers() into a scheduled work, otherwise GARP packet
+ * will not be sent after quick migration, and cause network disconnection.
+ */
+static void netvsc_send_garp(struct work_struct *w)
+{
+	struct net_device_context *ndev_ctx;
+	struct net_device *net;
+
+	msleep(20);
+	ndev_ctx = container_of(w, struct net_device_context, work);
+	net = dev_get_drvdata(&ndev_ctx->device_ctx->device);
+	netif_notify_peers(net);
+}
+
+
 static int netvsc_probe(struct device *device)
 {
 	struct hv_driver *drv =
@@ -345,6 +368,7 @@ static int netvsc_probe(struct device *device)
 	net_device_ctx->device_ctx = device_obj;
 	net_device_ctx->avail = ring_size;
 	dev_set_drvdata(device, net);
+	INIT_WORK(&net_device_ctx->work, netvsc_send_garp);
 
 	/* Notify the netvsc driver of the new device */
 	ret = net_drv_obj->base.dev_add(device_obj, &device_info);
-- 
1.7.4.1