| Message ID | 1403228076-7596-1-git-send-email-haiyangz@microsoft.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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
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?
> -----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
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.
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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