[PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

The RNDIS_STATUS_NETWORK_CHANGE event is received after the Hyper-V host
sleep or hibernation. We refresh network at this time.
MS-TFS: 135162

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/net/hyperv/hyperv_net.h   |    3 ++-
 drivers/net/hyperv/netvsc_drv.c   |   30 ++++++++++++++++++++++++++----
 drivers/net/hyperv/rndis_filter.c |   21 +--------------------
 include/linux/rndis.h             |    1 +
 4 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6cc37c1..24441ae 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -170,6 +170,7 @@ struct rndis_device {
 
 	enum rndis_device_state state;
 	bool link_state;
+	bool link_change;
 	atomic_t new_req_id;
 
 	spinlock_t request_lock;
@@ -185,7 +186,7 @@ int netvsc_device_remove(struct hv_device *device);
 int netvsc_send(struct hv_device *device,
 		struct hv_netvsc_packet *packet);
 void netvsc_linkstatus_callback(struct hv_device *device_obj,
-				unsigned int status);
+				struct rndis_message *resp);
 int netvsc_recv_callback(struct hv_device *device_obj,
 			struct hv_netvsc_packet *packet,
 			struct ndis_tcp_ip_checksum_info *csum_info);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 4fd71b7..9b27ca8 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -579,8 +579,9 @@ drop:
  * netvsc_linkstatus_callback - Link up/down notification
  */
 void netvsc_linkstatus_callback(struct hv_device *device_obj,
-				       unsigned int status)
+				struct rndis_message *resp)
 {
+	struct rndis_indicate_status *indicate = &resp->msg.indicate_status;
 	struct net_device *net;
 	struct net_device_context *ndev_ctx;
 	struct netvsc_device *net_device;
@@ -589,7 +590,19 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
 	net_device = hv_get_drvdata(device_obj);
 	rdev = net_device->extension;
 
-	rdev->link_state = status != 1;
+	switch (indicate->status) {
+	case RNDIS_STATUS_MEDIA_CONNECT:
+		rdev->link_state = false;
+		break;
+	case RNDIS_STATUS_MEDIA_DISCONNECT:
+		rdev->link_state = true;
+		break;
+	case RNDIS_STATUS_NETWORK_CHANGE:
+		rdev->link_change = true;
+		break;
+	default:
+		return;
+	}
 
 	net = net_device->ndev;
 
@@ -597,7 +610,7 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
 		return;
 
 	ndev_ctx = netdev_priv(net);
-	if (status == 1) {
+	if (!rdev->link_state) {
 		schedule_delayed_work(&ndev_ctx->dwork, 0);
 		schedule_delayed_work(&ndev_ctx->dwork, msecs_to_jiffies(20));
 	} else {
@@ -767,7 +780,9 @@ static void netvsc_link_change(struct work_struct *w)
 	struct net_device *net;
 	struct netvsc_device *net_device;
 	struct rndis_device *rdev;
-	bool notify;
+	bool notify, refresh = false;
+	char *argv[] = { "/etc/init.d/network", "restart", NULL };
+	char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
 
 	rtnl_lock();
 
@@ -782,10 +797,17 @@ static void netvsc_link_change(struct work_struct *w)
 	} else {
 		netif_carrier_on(net);
 		notify = true;
+		if (rdev->link_change) {
+			rdev->link_change = false;
+			refresh = true;
+		}
 	}
 
 	rtnl_unlock();
 
+	if (refresh)
+		call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
+
 	if (notify)
 		netdev_notify_peers(net);
 }
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 99c527a..2b86f0b 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -320,25 +320,6 @@ static void rndis_filter_receive_response(struct rndis_device *dev,
 	}
 }
 
-static void rndis_filter_receive_indicate_status(struct rndis_device *dev,
-					     struct rndis_message *resp)
-{
-	struct rndis_indicate_status *indicate =
-			&resp->msg.indicate_status;
-
-	if (indicate->status == RNDIS_STATUS_MEDIA_CONNECT) {
-		netvsc_linkstatus_callback(
-			dev->net_dev->dev, 1);
-	} else if (indicate->status == RNDIS_STATUS_MEDIA_DISCONNECT) {
-		netvsc_linkstatus_callback(
-			dev->net_dev->dev, 0);
-	} else {
-		/*
-		 * TODO:
-		 */
-	}
-}
-
 /*
  * Get the Per-Packet-Info with the specified type
  * return NULL if not found.
@@ -464,7 +445,7 @@ int rndis_filter_receive(struct hv_device *dev,
 
 	case RNDIS_MSG_INDICATE:
 		/* notification msgs */
-		rndis_filter_receive_indicate_status(rndis_dev, rndis_msg);
+		netvsc_linkstatus_callback(dev, rndis_msg);
 		break;
 	default:
 		netdev_err(ndev,
diff --git a/include/linux/rndis.h b/include/linux/rndis.h
index 0c8dc71..93c0a64 100644
--- a/include/linux/rndis.h
+++ b/include/linux/rndis.h
@@ -65,6 +65,7 @@
 #define	RNDIS_STATUS_MEDIA_SPECIFIC_INDICATION	0x40010012
 #define RNDIS_STATUS_WW_INDICATION		RDIA_SPECIFIC_INDICATION
 #define RNDIS_STATUS_LINK_SPEED_CHANGE		0x40010013L
+#define RNDIS_STATUS_NETWORK_CHANGE		0x40010018
 
 #define RNDIS_STATUS_NOT_RESETTABLE		0x80010001
 #define RNDIS_STATUS_SOFT_ERRORS		0x80010003
-- 
1.7.4.1

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[David Miller]

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Thu, 19 Jun 2014 18:34:36 -0700

> The RNDIS_STATUS_NETWORK_CHANGE event is received after the Hyper-V host
> sleep or hibernation. We refresh network at this time.
> MS-TFS: 135162
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

Applied, thanks.

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Olaf Hering]

On Thu, Jun 19, Haiyang Zhang wrote:

> The RNDIS_STATUS_NETWORK_CHANGE event is received after the Hyper-V host
> sleep or hibernation. We refresh network at this time.

> +	char *argv[] = { "/etc/init.d/network", "restart", NULL };

What happens if that file does not exist? Dead network in the guest?
I tend to think if a VM with PV drivers goes to sleep it has to go
through the whole suspend/resume cycle, very much like the "LID closed"
event. So I think this and the other fbdev change that is floating
around is wrong.

Olaf

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Greg KH]

On Fri, Jun 20, 2014 at 06:57:04AM +0200, Olaf Hering wrote:
> On Thu, Jun 19, Haiyang Zhang wrote:
> 
> > The RNDIS_STATUS_NETWORK_CHANGE event is received after the Hyper-V host
> > sleep or hibernation. We refresh network at this time.
> 
> > +	char *argv[] = { "/etc/init.d/network", "restart", NULL };
> 
> What happens if that file does not exist? Dead network in the guest?
> I tend to think if a VM with PV drivers goes to sleep it has to go
> through the whole suspend/resume cycle, very much like the "LID closed"
> event. So I think this and the other fbdev change that is floating
> around is wrong.

Ah, and what about systems with no /etc/init.d/ at all (like
systemd-based ones)?  You can't have a kernel driver ask userspace to
restart all networking connections, that seems really wrong.

greg k-h

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Friday, June 20, 2014 1:12 AM
> To: Olaf Hering
> Cc: Haiyang Zhang; netdev@vger.kernel.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Fri, Jun 20, 2014 at 06:57:04AM +0200, Olaf Hering wrote:
> > On Thu, Jun 19, Haiyang Zhang wrote:
> >
> > > The RNDIS_STATUS_NETWORK_CHANGE event is received after the Hyper-V
> host
> > > sleep or hibernation. We refresh network at this time.
> >
> > > +	char *argv[] = { "/etc/init.d/network", "restart", NULL };
> >
> > What happens if that file does not exist? Dead network in the guest?
> > I tend to think if a VM with PV drivers goes to sleep it has to go
> > through the whole suspend/resume cycle, very much like the "LID
> closed"
> > event. So I think this and the other fbdev change that is floating
> > around is wrong.
> 
> Ah, and what about systems with no /etc/init.d/ at all (like
> systemd-based ones)?  You can't have a kernel driver ask userspace to
> restart all networking connections, that seems really wrong.

Olaf and Greg,

Thanks for your reviews. 
On Server Hyper-V, the host sleep/hibernation is not supported. So this 
event won't happen on most deployment of Hyper-V.

On Client Hyper-V (e.g. Windows 8+ on a laptop), the host sleep/hibernation
is supported. If the laptop is moved to another network during sleep, we 
previously need to manually refresh the network (network restart) to renew 
DHCP. With this patch, the network is refreshed on the event.

This command ("/etc/init.d/network restart") exists on our supported distros
currently. We will also look into some better ways to refresh the network for
the distros without this command. I have tried setting IF_OPER_DORMANT then 
IF_OPER_UP, but not working. I will look into the suspend/resume cycle as
well.

Thanks,
- Haiyang

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Olaf Hering]

On Fri, Jun 20, Haiyang Zhang wrote:

> This command ("/etc/init.d/network restart") exists on our supported distros
> currently. We will also look into some better ways to refresh the network for
> the distros without this command. I have tried setting IF_OPER_DORMANT then 
> IF_OPER_UP, but not working. I will look into the suspend/resume cycle as
> well.

I think its reasonable to expect guest config changes on this new kind
of host. Would a link-down/link-up event work? I'm sure it will, there
is enough code floating around in the guests which handles cable unplug. 

Olaf

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1320 bytes --]



> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, June 23, 2014 4:03 AM
> To: Haiyang Zhang
> Cc: Greg KH; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Fri, Jun 20, Haiyang Zhang wrote:
> 
> > This command ("/etc/init.d/network restart") exists on our supported
> distros
> > currently. We will also look into some better ways to refresh the
> network for
> > the distros without this command. I have tried setting IF_OPER_DORMANT
> then
> > IF_OPER_UP, but not working. I will look into the suspend/resume cycle
> as
> > well.
> 
> I think its reasonable to expect guest config changes on this new kind
> of host. Would a link-down/link-up event work? I'm sure it will, there
> is enough code floating around in the guests which handles cable unplug.

Do you mean netif_carrier_off() / netif_carrier_on()? They are already 
called in the code before this patch, but DHCP renew is not triggered by
them.

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Olaf Hering]

On Mon, Jun 23, Haiyang Zhang wrote:

> > I think its reasonable to expect guest config changes on this new kind
> > of host. Would a link-down/link-up event work? I'm sure it will, there
> > is enough code floating around in the guests which handles cable unplug.
> 
> Do you mean netif_carrier_off() / netif_carrier_on()? They are already 
> called in the code before this patch, but DHCP renew is not triggered by
> them.

I do not know how to simulate a cable unplug. The point is that calling
/etc/init.d/network will fail, at least in SLES12.
Maybe some sort of "DHCP refresh required" event is required?
Maybe the DHCP clients need to renew on cable unplug?
No idea what the solution to the issue really is.

Olaf

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1804 bytes --]



> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, June 23, 2014 9:17 AM
> To: Haiyang Zhang
> Cc: Greg KH; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Mon, Jun 23, Haiyang Zhang wrote:
> 
> > > I think its reasonable to expect guest config changes on this new
> > > kind of host. Would a link-down/link-up event work? I'm sure it
> > > will, there is enough code floating around in the guests which handles cable
> unplug.
> >
> > Do you mean netif_carrier_off() / netif_carrier_on()? They are already
> > called in the code before this patch, but DHCP renew is not triggered
> > by them.
> 
> I do not know how to simulate a cable unplug. The point is that calling
> /etc/init.d/network will fail, at least in SLES12.
> Maybe some sort of "DHCP refresh required" event is required?
> Maybe the DHCP clients need to renew on cable unplug?
> No idea what the solution to the issue really is.

Yes, it will be great if there is such a "DHCP refresh required" event, or DHCP clients
are triggered when netif_carrier_off() then netif_carrier_on().

I have tried some possibilities, like IF_OPER_DORMANT then IF_OPER_UP with 
netdev_state_change() etc. but not able to trigger DHCP review. I will look at this
further...

So, what's the equivalent or similar command to "network restart" on SLES12? Could
you update the command line for the usermodehelper when porting this patch to SLES
12?

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Olaf Hering]

On Mon, Jun 23, Haiyang Zhang wrote:

> I have tried some possibilities, like IF_OPER_DORMANT then IF_OPER_UP with 
> netdev_state_change() etc. but not able to trigger DHCP review. I will look at this
> further...

Is there a link down/up event anyway?
If the interface is configured to "on hotplug" or "on cable plugged in"
or whatever the setting name is, the network scripts should do
something. Even if its not a DHCP renew.

> So, what's the equivalent or similar command to "network restart" on SLES12? Could
> you update the command line for the usermodehelper when porting this patch to SLES
> 12?

I see a /sbin/rcnetwork in SLES, which links either to
'/etc/init.d/network' or 'service'. This could be used as a quick
workaround.

Olaf

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1595 bytes --]



> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, June 23, 2014 12:27 PM
> To: Haiyang Zhang
> Cc: Greg KH; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Mon, Jun 23, Haiyang Zhang wrote:
> 
> > I have tried some possibilities, like IF_OPER_DORMANT then IF_OPER_UP
> > with
> > netdev_state_change() etc. but not able to trigger DHCP review. I will
> > look at this further...
> 
> Is there a link down/up event anyway?
> If the interface is configured to "on hotplug" or "on cable plugged in"
> or whatever the setting name is, the network scripts should do something. Even
> if its not a DHCP renew.

Yes, there is a link down/up event from the host, we currently call netif_carrier_off() 
/ netif_carrier_on() with these events. Will hotplug scripts be triggered by 
netif_carrier_off/on? Where are the scripts located at (SLES)?

> 
> > So, what's the equivalent or similar command to "network restart" on
> > SLES12? Could you update the command line for the usermodehelper when
> > porting this patch to SLES 12?
> 
> I see a /sbin/rcnetwork in SLES, which links either to '/etc/init.d/network' or
> 'service'. This could be used as a quick workaround.

Thanks for the info!

Thanks,
- Haiyang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Olaf Hering]

On Mon, Jun 23, Haiyang Zhang wrote:

> Yes, there is a link down/up event from the host, we currently call netif_carrier_off() 
> / netif_carrier_on() with these events. Will hotplug scripts be triggered by 
> netif_carrier_off/on? Where are the scripts located at (SLES)?

In /etc/sysconfig/network/ifcfg-eth0 STARTMODE= should be set to
"ifplugd", this will most likely fix this sort of setup.

Olaf

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1015 bytes --]



> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Thursday, June 26, 2014 4:46 AM
> To: Haiyang Zhang
> Cc: Greg KH; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Mon, Jun 23, Haiyang Zhang wrote:
> 
> > Yes, there is a link down/up event from the host, we currently call
> > netif_carrier_off() / netif_carrier_on() with these events. Will
> > hotplug scripts be triggered by netif_carrier_off/on? Where are the scripts
> located at (SLES)?
> 
> In /etc/sysconfig/network/ifcfg-eth0 STARTMODE= should be set to "ifplugd",
> this will most likely fix this sort of setup.

Thanks for the info! I'll let our team know.

- Haiyang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Greg KH]

On Mon, Jun 23, 2014 at 04:09:59PM +0000, Haiyang Zhang wrote:
> 
> 
> > -----Original Message-----
> > From: Olaf Hering [mailto:olaf@aepfle.de]
> > Sent: Monday, June 23, 2014 9:17 AM
> > To: Haiyang Zhang
> > Cc: Greg KH; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> > devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > davem@davemloft.net
> > Subject: Re: [PATCH net-next] hyperv: Add handler for
> > RNDIS_STATUS_NETWORK_CHANGE event
> > 
> > On Mon, Jun 23, Haiyang Zhang wrote:
> > 
> > > > I think its reasonable to expect guest config changes on this new
> > > > kind of host. Would a link-down/link-up event work? I'm sure it
> > > > will, there is enough code floating around in the guests which handles cable
> > unplug.
> > >
> > > Do you mean netif_carrier_off() / netif_carrier_on()? They are already
> > > called in the code before this patch, but DHCP renew is not triggered
> > > by them.
> > 
> > I do not know how to simulate a cable unplug. The point is that calling
> > /etc/init.d/network will fail, at least in SLES12.
> > Maybe some sort of "DHCP refresh required" event is required?
> > Maybe the DHCP clients need to renew on cable unplug?
> > No idea what the solution to the issue really is.
> 
> Yes, it will be great if there is such a "DHCP refresh required" event, or DHCP clients
> are triggered when netif_carrier_off() then netif_carrier_on().
> 
> I have tried some possibilities, like IF_OPER_DORMANT then IF_OPER_UP with 
> netdev_state_change() etc. but not able to trigger DHCP review. I will look at this
> further...
> 
> So, what's the equivalent or similar command to "network restart" on SLES12? Could
> you update the command line for the usermodehelper when porting this patch to SLES
> 12?

Given that this change will fail on all future distro releases, and
almost all of the community distros today, I don't see how this is
acceptable at all.  Nor would it be any better if you switch to a
systemd command line script as well.  You should just work like any
other network device works in this situation when it comes to
enabling/disabling the device.  Worse case, just tear down the whole
network device at suspend time, and recreate it at resume.

greg k-h

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, June 23, 2014 12:29 PM
> To: Haiyang Zhang
> Cc: Olaf Hering; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Mon, Jun 23, 2014 at 04:09:59PM +0000, Haiyang Zhang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Olaf Hering [mailto:olaf@aepfle.de]
> > > Sent: Monday, June 23, 2014 9:17 AM
> > > To: Haiyang Zhang
> > > Cc: Greg KH; netdev@vger.kernel.org; jasowang@redhat.com; driverdev-
> > > devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > davem@davemloft.net
> > > Subject: Re: [PATCH net-next] hyperv: Add handler for
> > > RNDIS_STATUS_NETWORK_CHANGE event
> > >
> > > On Mon, Jun 23, Haiyang Zhang wrote:
> > >
> > > > > I think its reasonable to expect guest config changes on this
> > > > > new kind of host. Would a link-down/link-up event work? I'm sure
> > > > > it will, there is enough code floating around in the guests
> > > > > which handles cable
> > > unplug.
> > > >
> > > > Do you mean netif_carrier_off() / netif_carrier_on()? They are
> > > > already called in the code before this patch, but DHCP renew is
> > > > not triggered by them.
> > >
> > > I do not know how to simulate a cable unplug. The point is that
> > > calling /etc/init.d/network will fail, at least in SLES12.
> > > Maybe some sort of "DHCP refresh required" event is required?
> > > Maybe the DHCP clients need to renew on cable unplug?
> > > No idea what the solution to the issue really is.
> >
> > Yes, it will be great if there is such a "DHCP refresh required"
> > event, or DHCP clients are triggered when netif_carrier_off() then
> netif_carrier_on().
> >
> > I have tried some possibilities, like IF_OPER_DORMANT then IF_OPER_UP
> > with
> > netdev_state_change() etc. but not able to trigger DHCP review. I will
> > look at this further...
> >
> > So, what's the equivalent or similar command to "network restart" on
> > SLES12? Could you update the command line for the usermodehelper when
> > porting this patch to SLES 12?
> 
> Given that this change will fail on all future distro releases, and almost all of
> the community distros today, I don't see how this is acceptable at all.  Nor
> would it be any better if you switch to a systemd command line script as well.
> You should just work like any other network device works in this situation when
> it comes to enabling/disabling the device.  Worse case, just tear down the
> whole network device at suspend time, and recreate it at resume.

I will look into these options.

Thanks,
- Haiyang

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[David Miller]

From: Greg KH <greg@kroah.com>
Date: Mon, 23 Jun 2014 12:29:11 -0400

> Given that this change will fail on all future distro releases, and
> almost all of the community distros today, I don't see how this is
> acceptable at all.  Nor would it be any better if you switch to a
> systemd command line script as well.  You should just work like any
> other network device works in this situation when it comes to
> enabling/disabling the device.  Worse case, just tear down the whole
> network device at suspend time, and recreate it at resume.

Agreed, and I've strongly asked them to remove this code immediately.

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[David Miller]

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Mon, 23 Jun 2014 16:09:59 +0000

> So, what's the equivalent or similar command to "network restart" on SLES12? Could
> you update the command line for the usermodehelper when porting this patch to SLES
> 12?

No, you are not going to keep the usermodehelper invocation in your driver
please remove it.  It is absolutely inappropriate, and I strictly do not want
to keep it in there because other people will copy it and then we'll have a
real mess on our hands.

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, June 23, 2014 4:10 PM
> To: Haiyang Zhang
> Cc: olaf@aepfle.de; greg@kroah.com; netdev@vger.kernel.org;
> jasowang@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Date: Mon, 23 Jun 2014 16:09:59 +0000
> 
> > So, what's the equivalent or similar command to "network restart" on
> > SLES12? Could you update the command line for the usermodehelper when
> > porting this patch to SLES 12?
> 
> No, you are not going to keep the usermodehelper invocation in your driver
> please remove it.  It is absolutely inappropriate, and I strictly do not want to
> keep it in there because other people will copy it and then we'll have a real
> mess on our hands.

I'm working on it, and will have a patch to replace the usermodehelper soon.

Thanks,
- Haiyang

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Richard Weinberger]

On Mon, Jun 23, 2014 at 10:10 PM, David Miller <davem@davemloft.net> wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Date: Mon, 23 Jun 2014 16:09:59 +0000
>
>> So, what's the equivalent or similar command to "network restart" on SLES12? Could
>> you update the command line for the usermodehelper when porting this patch to SLES
>> 12?
>
> No, you are not going to keep the usermodehelper invocation in your driver
> please remove it.  It is absolutely inappropriate, and I strictly do not want
> to keep it in there because other people will copy it and then we'll have a
> real mess on our hands.

Sorry for digging up this old thread.
While talking with some guys about usermodehelper abuses I came across this gem.
Mainline still contains that "/etc/init.d/network restart" code.
Haiyang, care to cleanup?

-- 
Thanks,
//richard

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1582 bytes --]



> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Tuesday, October 27, 2015 6:36 PM
> To: David Miller <davem@davemloft.net>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; olaf@aepfle.de; Greg Kroah-
> Hartman <greg@kroah.com>; netdev@vger.kernel.org;
> jasowang@redhat.com; driverdev-devel@linuxdriverproject.org; LKML
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Mon, Jun 23, 2014 at 10:10 PM, David Miller <davem@davemloft.net>
> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> > Date: Mon, 23 Jun 2014 16:09:59 +0000
> >
> >> So, what's the equivalent or similar command to "network restart" on
> >> SLES12? Could you update the command line for the usermodehelper
> when
> >> porting this patch to SLES 12?
> >
> > No, you are not going to keep the usermodehelper invocation in your
> > driver please remove it.  It is absolutely inappropriate, and I
> > strictly do not want to keep it in there because other people will
> > copy it and then we'll have a real mess on our hands.
> 
> Sorry for digging up this old thread.
> While talking with some guys about usermodehelper abuses I came across
> this gem.
> Mainline still contains that "/etc/init.d/network restart" code.
> Haiyang, care to cleanup?

I will clean up the usermode helper soon.

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1748 bytes --]



> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Tuesday, October 27, 2015 6:36 PM
> To: David Miller <davem@davemloft.net>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; olaf@aepfle.de; Greg Kroah-
> Hartman <greg@kroah.com>; netdev@vger.kernel.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; LKML <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Mon, Jun 23, 2014 at 10:10 PM, David Miller <davem@davemloft.net>
> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> > Date: Mon, 23 Jun 2014 16:09:59 +0000
> >
> >> So, what's the equivalent or similar command to "network restart" on
> SLES12? Could
> >> you update the command line for the usermodehelper when porting this
> patch to SLES
> >> 12?
> >
> > No, you are not going to keep the usermodehelper invocation in your
> driver
> > please remove it.  It is absolutely inappropriate, and I strictly do
> not want
> > to keep it in there because other people will copy it and then we'll
> have a
> > real mess on our hands.
> 
> Sorry for digging up this old thread.
> While talking with some guys about usermodehelper abuses I came across
> this gem.
> Mainline still contains that "/etc/init.d/network restart" code.
> Haiyang, care to cleanup?

Hi Richard and others,

Thanks for the reminder. I will clean up the usermode helper.

Do you have suggestions of trigger DHCP refresh from kernel mode? Any 
sample code in the existing kernel code?

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Vitaly Kuznetsov]

Haiyang Zhang <haiyangz@microsoft.com> writes:

>> -----Original Message-----
>> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
>> Sent: Tuesday, October 27, 2015 6:36 PM
>> To: David Miller <davem@davemloft.net>
>> Cc: Haiyang Zhang <haiyangz@microsoft.com>; olaf@aepfle.de; Greg Kroah-
>> Hartman <greg@kroah.com>; netdev@vger.kernel.org; jasowang@redhat.com;
>> driverdev-devel@linuxdriverproject.org; LKML <linux-
>> kernel@vger.kernel.org>
>> Subject: Re: [PATCH net-next] hyperv: Add handler for
>> RNDIS_STATUS_NETWORK_CHANGE event
>> 
>> On Mon, Jun 23, 2014 at 10:10 PM, David Miller <davem@davemloft.net>
>> wrote:
>> > From: Haiyang Zhang <haiyangz@microsoft.com>
>> > Date: Mon, 23 Jun 2014 16:09:59 +0000
>> >
>> >> So, what's the equivalent or similar command to "network restart" on
>> SLES12? Could
>> >> you update the command line for the usermodehelper when porting this
>> patch to SLES
>> >> 12?
>> >
>> > No, you are not going to keep the usermodehelper invocation in your
>> driver
>> > please remove it.  It is absolutely inappropriate, and I strictly do
>> not want
>> > to keep it in there because other people will copy it and then we'll
>> have a
>> > real mess on our hands.
>> 
>> Sorry for digging up this old thread.
>> While talking with some guys about usermodehelper abuses I came across
>> this gem.
>> Mainline still contains that "/etc/init.d/network restart" code.
>> Haiyang, care to cleanup?
>
> Hi Richard and others,
>
> Thanks for the reminder. I will clean up the usermode helper.
>
> Do you have suggestions of trigger DHCP refresh from kernel mode? Any 
> sample code in the existing kernel code?
>

I think it's wrong to call dhcp refresh from kernel. What happens when
we reconnect normal hardware adapter to another network? Link goes down
and then up and userspace is supposed to react accordingly. I think we
should emulate something similar for RNDIS_STATUS_NETWORK_CHANGE.

-- 
  Vitaly

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Friday, October 30, 2015 6:56 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Richard Weinberger <richard.weinberger@gmail.com>; David Miller
> <davem@davemloft.net>; olaf@aepfle.de; jasowang@redhat.com; driverdev-
> devel@linuxdriverproject.org; LKML <linux-kernel@vger.kernel.org>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> Haiyang Zhang <haiyangz@microsoft.com> writes:
> 
> >> -----Original Message-----
> >> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> >> Sent: Tuesday, October 27, 2015 6:36 PM
> >> To: David Miller <davem@davemloft.net>
> >> Cc: Haiyang Zhang <haiyangz@microsoft.com>; olaf@aepfle.de; Greg
> Kroah-
> >> Hartman <greg@kroah.com>; netdev@vger.kernel.org; jasowang@redhat.com;
> >> driverdev-devel@linuxdriverproject.org; LKML <linux-
> >> kernel@vger.kernel.org>
> >> Subject: Re: [PATCH net-next] hyperv: Add handler for
> >> RNDIS_STATUS_NETWORK_CHANGE event
> >>
> >> On Mon, Jun 23, 2014 at 10:10 PM, David Miller <davem@davemloft.net>
> >> wrote:
> >> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >> > Date: Mon, 23 Jun 2014 16:09:59 +0000
> >> >
> >> >> So, what's the equivalent or similar command to "network restart"
> on
> >> SLES12? Could
> >> >> you update the command line for the usermodehelper when porting
> this
> >> patch to SLES
> >> >> 12?
> >> >
> >> > No, you are not going to keep the usermodehelper invocation in your
> >> driver
> >> > please remove it.  It is absolutely inappropriate, and I strictly
> do
> >> not want
> >> > to keep it in there because other people will copy it and then
> we'll
> >> have a
> >> > real mess on our hands.
> >>
> >> Sorry for digging up this old thread.
> >> While talking with some guys about usermodehelper abuses I came
> across
> >> this gem.
> >> Mainline still contains that "/etc/init.d/network restart" code.
> >> Haiyang, care to cleanup?
> >
> > Hi Richard and others,
> >
> > Thanks for the reminder. I will clean up the usermode helper.
> >
> > Do you have suggestions of trigger DHCP refresh from kernel mode? Any
> > sample code in the existing kernel code?
> >
> 
> I think it's wrong to call dhcp refresh from kernel. What happens when
> we reconnect normal hardware adapter to another network? Link goes down
> and then up and userspace is supposed to react accordingly. I think we
> should emulate something similar for RNDIS_STATUS_NETWORK_CHANGE.

When link is down physically for a few seconds, the DHCP will automatically 
refresh. I will add code to emulate this. There were some discussions around 
this and other possibilities previously... I agree emulating what happens 
with physically plug/unplug a cable is a reasonable way to trigger the DHCP 
refresh.

Thanks,
- Haiyang

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Richard Weinberger]

Am 30.10.2015 um 23:03 schrieb Haiyang Zhang:
> 
> 
>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Friday, October 30, 2015 6:56 AM
>> To: Haiyang Zhang <haiyangz@microsoft.com>
>> Cc: Richard Weinberger <richard.weinberger@gmail.com>; David Miller
>> <davem@davemloft.net>; olaf@aepfle.de; jasowang@redhat.com; driverdev-
>> devel@linuxdriverproject.org; LKML <linux-kernel@vger.kernel.org>;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH net-next] hyperv: Add handler for
>> RNDIS_STATUS_NETWORK_CHANGE event
>>
>> Haiyang Zhang <haiyangz@microsoft.com> writes:
>>
>>>> -----Original Message-----
>>>> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
>>>> Sent: Tuesday, October 27, 2015 6:36 PM
>>>> To: David Miller <davem@davemloft.net>
>>>> Cc: Haiyang Zhang <haiyangz@microsoft.com>; olaf@aepfle.de; Greg
>> Kroah-
>>>> Hartman <greg@kroah.com>; netdev@vger.kernel.org; jasowang@redhat.com;
>>>> driverdev-devel@linuxdriverproject.org; LKML <linux-
>>>> kernel@vger.kernel.org>
>>>> Subject: Re: [PATCH net-next] hyperv: Add handler for
>>>> RNDIS_STATUS_NETWORK_CHANGE event
>>>>
>>>> On Mon, Jun 23, 2014 at 10:10 PM, David Miller <davem@davemloft.net>
>>>> wrote:
>>>>> From: Haiyang Zhang <haiyangz@microsoft.com>
>>>>> Date: Mon, 23 Jun 2014 16:09:59 +0000
>>>>>
>>>>>> So, what's the equivalent or similar command to "network restart"
>> on
>>>> SLES12? Could
>>>>>> you update the command line for the usermodehelper when porting
>> this
>>>> patch to SLES
>>>>>> 12?
>>>>>
>>>>> No, you are not going to keep the usermodehelper invocation in your
>>>> driver
>>>>> please remove it.  It is absolutely inappropriate, and I strictly
>> do
>>>> not want
>>>>> to keep it in there because other people will copy it and then
>> we'll
>>>> have a
>>>>> real mess on our hands.
>>>>
>>>> Sorry for digging up this old thread.
>>>> While talking with some guys about usermodehelper abuses I came
>> across
>>>> this gem.
>>>> Mainline still contains that "/etc/init.d/network restart" code.
>>>> Haiyang, care to cleanup?
>>>
>>> Hi Richard and others,
>>>
>>> Thanks for the reminder. I will clean up the usermode helper.
>>>
>>> Do you have suggestions of trigger DHCP refresh from kernel mode? Any
>>> sample code in the existing kernel code?
>>>
>>
>> I think it's wrong to call dhcp refresh from kernel. What happens when
>> we reconnect normal hardware adapter to another network? Link goes down
>> and then up and userspace is supposed to react accordingly. I think we
>> should emulate something similar for RNDIS_STATUS_NETWORK_CHANGE.
> 
> When link is down physically for a few seconds, the DHCP will automatically 
> refresh. I will add code to emulate this. There were some discussions around 
> this and other possibilities previously... I agree emulating what happens 
> with physically plug/unplug a cable is a reasonable way to trigger the DHCP 
> refresh.

Can't you propagate the event to userspace and let it take an appropriate
action?

Thanks,
//richard

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[David Miller]

From: Olaf Hering <olaf@aepfle.de>
Date: Mon, 23 Jun 2014 15:17:23 +0200

> On Mon, Jun 23, Haiyang Zhang wrote:
> 
>> > I think its reasonable to expect guest config changes on this new kind
>> > of host. Would a link-down/link-up event work? I'm sure it will, there
>> > is enough code floating around in the guests which handles cable unplug.
>> 
>> Do you mean netif_carrier_off() / netif_carrier_on()? They are already 
>> called in the code before this patch, but DHCP renew is not triggered by
>> them.
> 
> I do not know how to simulate a cable unplug. The point is that calling
> /etc/init.d/network will fail, at least in SLES12.
> Maybe some sort of "DHCP refresh required" event is required?
> Maybe the DHCP clients need to renew on cable unplug?
> No idea what the solution to the issue really is.

What's in the driver right now is definitely not it, and I want this
network restart code removed immediately.

Re: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Dan Carpenter]

On Thu, Jun 19, 2014 at 06:34:36PM -0700, Haiyang Zhang wrote:
> @@ -589,7 +590,19 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
>  	net_device = hv_get_drvdata(device_obj);
>  	rdev = net_device->extension;
>  
> -	rdev->link_state = status != 1;
> +	switch (indicate->status) {
> +	case RNDIS_STATUS_MEDIA_CONNECT:
> +		rdev->link_state = false;

link_state false means that we want to connect?

> +		break;
> +	case RNDIS_STATUS_MEDIA_DISCONNECT:
> +		rdev->link_state = true;

link_state true means that we are disconnecting.

> +		break;
> +	case RNDIS_STATUS_NETWORK_CHANGE:
> +		rdev->link_change = true;
> +		break;
> +	default:
> +		return;
> +	}
>  
>  	net = net_device->ndev;
>  
> @@ -597,7 +610,7 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,

> @@ -782,10 +797,17 @@ static void netvsc_link_change(struct work_struct *w)
>  	} else {
>  		netif_carrier_on(net);
>  		notify = true;
> +		if (rdev->link_change) {
> +			rdev->link_change = false;
> +			refresh = true;
> +		}

How do we know that we received a RNDIS_STATUS_MEDIA_CONNECT before we
received the RNDIS_STATUS_NETWORK_CHANGE?  In other words, why does
RNDIS_STATUS_NETWORK_CHANGE imply that the link_state is false?

>  	}
>  
>  	rtnl_unlock();
>  
> +	if (refresh)
> +		call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);

You may as well use UMH_NO_WAIT since there is no error handling if
/etc/init.d/network is not found.

regards,
dan carpenter

RE: [PATCH net-next] hyperv: Add handler for RNDIS_STATUS_NETWORK_CHANGE event

[Haiyang Zhang]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Friday, June 20, 2014 4:42 AM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; olaf@aepfle.de;
> jasowang@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next] hyperv: Add handler for
> RNDIS_STATUS_NETWORK_CHANGE event
> 
> On Thu, Jun 19, 2014 at 06:34:36PM -0700, Haiyang Zhang wrote:
> > @@ -589,7 +590,19 @@ void netvsc_linkstatus_callback(struct hv_device
> *device_obj,
> >  	net_device = hv_get_drvdata(device_obj);
> >  	rdev = net_device->extension;
> >
> > -	rdev->link_state = status != 1;
> > +	switch (indicate->status) {
> > +	case RNDIS_STATUS_MEDIA_CONNECT:
> > +		rdev->link_state = false;
> 
> link_state false means that we want to connect?
Yes

> 
> > +		break;
> > +	case RNDIS_STATUS_MEDIA_DISCONNECT:
> > +		rdev->link_state = true;
> 
> link_state true means that we are disconnecting.
Yes.

> > @@ -782,10 +797,17 @@ static void netvsc_link_change(struct
> work_struct *w)
> >  	} else {
> >  		netif_carrier_on(net);
> >  		notify = true;
> > +		if (rdev->link_change) {
> > +			rdev->link_change = false;
> > +			refresh = true;
> > +		}
> 
> How do we know that we received a RNDIS_STATUS_MEDIA_CONNECT before we
> received the RNDIS_STATUS_NETWORK_CHANGE?  In other words, why does
> RNDIS_STATUS_NETWORK_CHANGE imply that the link_state is false?

After host sleep, both RNDIS_STATUS_MEDIA_CONNECT and 
RNDIS_STATUS_NETWORK_CHANGE events are received, but not necessarily in
this order. If RNDIS_STATUS_MEDIA_CONNECT arrives later, the flag saved
in rdev->link_change previously will trigger the refresh, not in the
RNDIS_STATUS_NETWORK_CHANGE event, but in the latter RNDIS_STATUS_MEDIA_CONNECT
event.

> 
> >  	}
> >
> >  	rtnl_unlock();
> >
> > +	if (refresh)
> > +		call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
> 
> You may as well use UMH_NO_WAIT since there is no error handling if
> /etc/init.d/network is not found.

I previously tried UMH_NO_WAIT, but not working. We need to wait for the 
exec (not process completion) in this case. Since it's in the work queue,
a bit of waiting is OK.

Thanks,
- Haiyang