RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> -Original Message- > From: Dan Williams > > > e.g., on a bare metal host with Ubuntu 14.04, when I plug the RJ45 cable > > > out of the network card and then plug the cable back into the network card > > > quickly -- in ~3 seconds, networkd doesn't trigger DHCP renew request: in > > > /var/log/syslog, we see > > > Aug 12 11:07:07 decui-lin NetworkManager[828]: (eth0): carrier > now OFF (device state 100, deferring action for 4 seconds) > > > Aug 12 11:07:07 decui-lin kernel: [ 246.975453] e1000e: eth0 NIC Link is > Down > > > Aug 12 11:07:10 decui-lin NetworkManager[828]: (eth0): carrier > now ON (device state 100) > > > Aug 12 11:07:10 decui-lin kernel: [ 250.028533] e1000e: eth0 NIC Link is > Up 100 Mbps Full Duplex, Flow Control: Rx/Tx > > > > > > It looks there is a delay of 4s. > > > I'm going to find out if there is a configurable parameter for this. > > > > Just to avoid any confusion: you are referring to "networkd" (and so > > did my comments), but the above logs are from "NetworkManager". > > And yes, NM does have a 4-second delay before processing a carrier down > event, and NM currently does not renew DHCP on link changes, but that's > certainly something we can/should change. > > Dan Hi Tom, Dan, Thanks a lot for the clarification! So the 4s delay comes from NM rather than networkd. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Wed, 2014-08-13 at 15:15 +0200, Tom Gundersen wrote: > On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui wrote: > >> From: Tom Gundersen > >> > Unluckily this logic doesn't work because the user-space daemons > >> > like ifplugd, usually don't renew the DHCP immediately as long as they > >> > receive a link-down message: they usually wait for some seconds and if > >> > they find the link becomes up soon, they won't trigger renew operations. > >> > (I guess this behavior can be somewhat reasonable: maybe the daemons > >> > try to not trigger DHCP renew on temporary link instability) > >> > > >> networkd does not suffer from this problem, and in ifplugd it can be > > I didn't have time to check the code of networkd, but I think it does have > > the > > same behavior. > > e.g., on a bare metal host with Ubuntu 14.04, when I plug the RJ45 cable out > > of the network card and then plug the cable back into the network card > > quickly -- in ~3 seconds, networkd doesn't trigger DHCP renew request: in > > /var/log/syslog, we see > > Aug 12 11:07:07 decui-lin NetworkManager[828]: (eth0): carrier now > > OFF (device state 100, deferring action for 4 seconds) > > Aug 12 11:07:07 decui-lin kernel: [ 246.975453] e1000e: eth0 NIC Link is > > Down > > Aug 12 11:07:10 decui-lin NetworkManager[828]: (eth0): carrier now > > ON (device state 100) > > Aug 12 11:07:10 decui-lin kernel: [ 250.028533] e1000e: eth0 NIC Link is > > Up 100 Mbps Full Duplex, Flow Control: Rx/Tx > > > > It looks there is a delay of 4s. > > I'm going to find out if there is a configurable parameter for this. > > Just to avoid any confusion: you are referring to "networkd" (and so > did my comments), but the above logs are from "NetworkManager". And yes, NM does have a 4-second delay before processing a carrier down event, and NM currently does not renew DHCP on link changes, but that's certainly something we can/should change. Dan > >> disabled. Most other network drivers will send > >> IFF_LOWER_DOWN/IFF_LOWER_UP upon suspend/resume so if you were to > >> do the same you will not work any worse than the others. What would be > > suspend/resume(ACPI S3?) is different as this is usually > 4 seconds? :-) > > Sure, but that's clearly not something we should rely on. > > >> nice, as mentioned by Dan and Lennart, would be to send an additional > >> explicit event such as "resumed from suspend" or "L3 may be wrong". > > Sorry, I neglected to reply this. > > IMHO even if we add the new event, we still need lots of efforts to > > make the various userland daemons(ifplugd, networkd, etc) to use the > > new event. > > And looks we're the first user of this new event. I'm not sure if this issue > > here can convince the network subsystem maintainers such a new event > > is a must. > > > >> That should be a generic thing though, to fix all such issues in one > >> go. > > I agree, though this requires we update all the userland daemons... > > > >> > I'm not sure our attempt to "fix" the daemons can be easily accepted. > >> > BTW, by CPUID, an application has a reliable way to determine if it's > >> > running on hyper-v on not. Maybe we can "fix" the behavior of the > >> > daemons when they run on hyper-v? > >> > BTW2, according to my limited experience, I doubt other VMMs can > >> > handle this auto-DHCP-renew-in-guest issue properly. > >> > >> To the extent this is a problem, it is a generic one, so we should not > >> need any hyper-v specific logic in userspace. > > OK, I understood. > > > >> > That was why Yue's patch wanted to add a SLEEP(10s) between the > >> > link-down and link-up events and hoped this could be an acceptable > >> > fix(while it turned out not, obviously), though we admit it's not so good > >> > to add such a magic number "10s" in a kernel driver. > >> > > >> > Please point it out if I missed or misunderstand something. > >> > > >> > Now I understand it's not good to pass the event to the udev daemon, > >> > and it's not good to use a SLEEP(10s) in the kernel space(even if it's > >> > in a > >> > "work" task here). > >> > >> Please just expose to userspace what is happening (link lost/gained, > >> resumed from suspend/...), and let us sort out how to react to that. > >> If you put assumptions about what kind of timeouts (long-dead) > >> userspace uses into your drivers you'll just create a mess. > > OK, I got it now. > > So I think I'm supposed to send out a netif_carrier_off()/on() patch, > > and I'd better do this after I verify the daemons can work with > > proper parameters specified. > > > >> > Please let me know if it's the correct direction to fix the user-space > >> > daemons (ifplugd, systemd-networkd, etc). > >> > If you think this is viable and we should do this, I'll submit a > >> > netif_carrier_off/on patch first and will start to work with the > >> > projects of ifplugd, systemd-networkd and many OSVs to make the > >> > while thing work eventually. > >> > >> Have you actually checked that carrier_off/on does
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui wrote: >> From: Tom Gundersen >> > Unluckily this logic doesn't work because the user-space daemons >> > like ifplugd, usually don't renew the DHCP immediately as long as they >> > receive a link-down message: they usually wait for some seconds and if >> > they find the link becomes up soon, they won't trigger renew operations. >> > (I guess this behavior can be somewhat reasonable: maybe the daemons >> > try to not trigger DHCP renew on temporary link instability) >> > >> networkd does not suffer from this problem, and in ifplugd it can be > I didn't have time to check the code of networkd, but I think it does have the > same behavior. > e.g., on a bare metal host with Ubuntu 14.04, when I plug the RJ45 cable out > of the network card and then plug the cable back into the network card > quickly -- in ~3 seconds, networkd doesn't trigger DHCP renew request: in > /var/log/syslog, we see > Aug 12 11:07:07 decui-lin NetworkManager[828]: (eth0): carrier now OFF > (device state 100, deferring action for 4 seconds) > Aug 12 11:07:07 decui-lin kernel: [ 246.975453] e1000e: eth0 NIC Link is Down > Aug 12 11:07:10 decui-lin NetworkManager[828]: (eth0): carrier now ON > (device state 100) > Aug 12 11:07:10 decui-lin kernel: [ 250.028533] e1000e: eth0 NIC Link is Up > 100 Mbps Full Duplex, Flow Control: Rx/Tx > > It looks there is a delay of 4s. > I'm going to find out if there is a configurable parameter for this. Just to avoid any confusion: you are referring to "networkd" (and so did my comments), but the above logs are from "NetworkManager". >> disabled. Most other network drivers will send >> IFF_LOWER_DOWN/IFF_LOWER_UP upon suspend/resume so if you were to >> do the same you will not work any worse than the others. What would be > suspend/resume(ACPI S3?) is different as this is usually > 4 seconds? :-) Sure, but that's clearly not something we should rely on. >> nice, as mentioned by Dan and Lennart, would be to send an additional >> explicit event such as "resumed from suspend" or "L3 may be wrong". > Sorry, I neglected to reply this. > IMHO even if we add the new event, we still need lots of efforts to > make the various userland daemons(ifplugd, networkd, etc) to use the > new event. > And looks we're the first user of this new event. I'm not sure if this issue > here can convince the network subsystem maintainers such a new event > is a must. > >> That should be a generic thing though, to fix all such issues in one >> go. > I agree, though this requires we update all the userland daemons... > >> > I'm not sure our attempt to "fix" the daemons can be easily accepted. >> > BTW, by CPUID, an application has a reliable way to determine if it's >> > running on hyper-v on not. Maybe we can "fix" the behavior of the >> > daemons when they run on hyper-v? >> > BTW2, according to my limited experience, I doubt other VMMs can >> > handle this auto-DHCP-renew-in-guest issue properly. >> >> To the extent this is a problem, it is a generic one, so we should not >> need any hyper-v specific logic in userspace. > OK, I understood. > >> > That was why Yue's patch wanted to add a SLEEP(10s) between the >> > link-down and link-up events and hoped this could be an acceptable >> > fix(while it turned out not, obviously), though we admit it's not so good >> > to add such a magic number "10s" in a kernel driver. >> > >> > Please point it out if I missed or misunderstand something. >> > >> > Now I understand it's not good to pass the event to the udev daemon, >> > and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a >> > "work" task here). >> >> Please just expose to userspace what is happening (link lost/gained, >> resumed from suspend/...), and let us sort out how to react to that. >> If you put assumptions about what kind of timeouts (long-dead) >> userspace uses into your drivers you'll just create a mess. > OK, I got it now. > So I think I'm supposed to send out a netif_carrier_off()/on() patch, > and I'd better do this after I verify the daemons can work with > proper parameters specified. > >> > Please let me know if it's the correct direction to fix the user-space >> > daemons (ifplugd, systemd-networkd, etc). >> > If you think this is viable and we should do this, I'll submit a >> > netif_carrier_off/on patch first and will start to work with the >> > projects of ifplugd, systemd-networkd and many OSVs to make the >> > while thing work eventually. >> >> Have you actually checked that carrier_off/on does not work on >> anything but ifplugd? It would surprise me... > I can confirm carrier_off/on with 0 delay between the off and on > doesn't work for ifplugd and networkd. > > Thanks, > -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Tom Gundersen > > Unluckily this logic doesn't work because the user-space daemons > > like ifplugd, usually don't renew the DHCP immediately as long as they > > receive a link-down message: they usually wait for some seconds and if > > they find the link becomes up soon, they won't trigger renew operations. > > (I guess this behavior can be somewhat reasonable: maybe the daemons > > try to not trigger DHCP renew on temporary link instability) > > > networkd does not suffer from this problem, and in ifplugd it can be I didn't have time to check the code of networkd, but I think it does have the same behavior. e.g., on a bare metal host with Ubuntu 14.04, when I plug the RJ45 cable out of the network card and then plug the cable back into the network card quickly -- in ~3 seconds, networkd doesn't trigger DHCP renew request: in /var/log/syslog, we see Aug 12 11:07:07 decui-lin NetworkManager[828]: (eth0): carrier now OFF (device state 100, deferring action for 4 seconds) Aug 12 11:07:07 decui-lin kernel: [ 246.975453] e1000e: eth0 NIC Link is Down Aug 12 11:07:10 decui-lin NetworkManager[828]: (eth0): carrier now ON (device state 100) Aug 12 11:07:10 decui-lin kernel: [ 250.028533] e1000e: eth0 NIC Link is Up 100 Mbps Full Duplex, Flow Control: Rx/Tx It looks there is a delay of 4s. I'm going to find out if there is a configurable parameter for this. > disabled. Most other network drivers will send > IFF_LOWER_DOWN/IFF_LOWER_UP upon suspend/resume so if you were to > do the same you will not work any worse than the others. What would be suspend/resume(ACPI S3?) is different as this is usually > 4 seconds? :-) > nice, as mentioned by Dan and Lennart, would be to send an additional > explicit event such as "resumed from suspend" or "L3 may be wrong". Sorry, I neglected to reply this. IMHO even if we add the new event, we still need lots of efforts to make the various userland daemons(ifplugd, networkd, etc) to use the new event. And looks we're the first user of this new event. I'm not sure if this issue here can convince the network subsystem maintainers such a new event is a must. > That should be a generic thing though, to fix all such issues in one > go. I agree, though this requires we update all the userland daemons... > > I'm not sure our attempt to "fix" the daemons can be easily accepted. > > BTW, by CPUID, an application has a reliable way to determine if it's > > running on hyper-v on not. Maybe we can "fix" the behavior of the > > daemons when they run on hyper-v? > > BTW2, according to my limited experience, I doubt other VMMs can > > handle this auto-DHCP-renew-in-guest issue properly. > > To the extent this is a problem, it is a generic one, so we should not > need any hyper-v specific logic in userspace. OK, I understood. > > That was why Yue's patch wanted to add a SLEEP(10s) between the > > link-down and link-up events and hoped this could be an acceptable > > fix(while it turned out not, obviously), though we admit it's not so good > > to add such a magic number "10s" in a kernel driver. > > > > Please point it out if I missed or misunderstand something. > > > > Now I understand it's not good to pass the event to the udev daemon, > > and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a > > "work" task here). > > Please just expose to userspace what is happening (link lost/gained, > resumed from suspend/...), and let us sort out how to react to that. > If you put assumptions about what kind of timeouts (long-dead) > userspace uses into your drivers you'll just create a mess. OK, I got it now. So I think I'm supposed to send out a netif_carrier_off()/on() patch, and I'd better do this after I verify the daemons can work with proper parameters specified. > > Please let me know if it's the correct direction to fix the user-space > > daemons (ifplugd, systemd-networkd, etc). > > If you think this is viable and we should do this, I'll submit a > > netif_carrier_off/on patch first and will start to work with the > > projects of ifplugd, systemd-networkd and many OSVs to make the > > while thing work eventually. > > Have you actually checked that carrier_off/on does not work on > anything but ifplugd? It would surprise me... I can confirm carrier_off/on with 0 delay between the off and on doesn't work for ifplugd and networkd. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> -Original Message- > From: Bill Fink > In the case of ifplugd, it has parameters -u | --delay-up= which > defaults to 0 seconds, and -d | --delay-down= which defaults to > 5 seconds. Maybe for hyperv you could specify --delay-down=0. > I don't know if other daemons such as systemd have similar options. > It might still be good to have some modest delay between the > netif_carrier_off(net) and netif_carrier_on(net). > > -Bill Thanks, Bill! I'll try these parameters and see if a modest delay is necessary here. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Monday, August 11, 2014 11:52 AM > > I suppose you meant the below logic: > > if (refresh) { > > rtnl_lock(); > > netif_carrier_off(net); > > netif_carrier_on(net); > > rtnl_unlock(); > > } > > > > We have discussed this in the previous mails of this thread itself: > > e.g., http://marc.info/?l=linux-driver-devel&m=140593811715975&w=2 > > > > Unluckily this logic doesn't work because the user-space daemons > > like ifplugd, usually don't renew the DHCP immediately as long as they > > receive a link-down message: they usually wait for some seconds and if > > they find the link becomes up soon, they won't trigger renew operations. > > (I guess this behavior can be somewhat reasonable: maybe the daemons > > try to not trigger DHCP renew on temporary link instability) > > Is that such a big deal? If you know you spend much of your time in > ifplugd, why not use something different that triggers a DHCP renewal > faster, or fix ifplugd? Hi Florian, Thanks for the comment! Yes, we can fix ifplugd and ALL other equivalent daemons, but that would need much more efforts, so I was trying to add a workaround in the kernel driver -- now I've realized this is not a good solution. :-) > > If we use this logic in the kernel space, we'll have to "fix" the user-space > > daemons, like ifplugd, systemd-networkd...,etc. > > You mean the opposite here don't you? If you put that logic in kernel > space you don't have to fix the userland. I think we'll have to fix the userland or customize the userland by passing proper parameters(if there is any) to the userland daemons(when they run in a hyper-v guest) if we use the below logic in the driver: if (refresh) { rtnl_lock(); netif_carrier_off(net); netif_carrier_on(net); rtnl_unlock(); } e.g., as Bill told in another mail, ifplugd has --delay-down. > > I'm not sure our attempt to "fix" the daemons can be easily accepted. > > BTW, by CPUID, an application has a reliable way to determine if it's > > running on hyper-v on not. Maybe we can "fix" the behavior of the > > daemons when they run on hyper-v? > > That is not acceptable as well, why would an user-space application > would have to care that much whether it runs on hyper-v or a physical > host? Not to mention that anytime someone develops a similar but new > application they would have to become aware of such platform and its > "specicities". Ok, I understood. > > BTW2, according to my limited experience, I doubt other VMMs can > > handle this auto-DHCP-renew-in-guest issue properly. > > > > That was why Yue's patch wanted to add a SLEEP(10s) between the > > link-down and link-up events and hoped this could be an acceptable > > fix(while it turned out not, obviously), though we admit it's not so good > > to add such a magic number "10s" in a kernel driver. > > > > Please point it out if I missed or misunderstand something. > > I think this is just an integration issue that you are having, and I > would not be focusing on any particular user-space implementation, but > rather put something in the driver that is sensible, just like what was > suggested before: toggling the carrier state. OK, let me send a netif_carrier_off()/on() patch after I verify the daemons can work with proper parameters specified. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Mon, Aug 11, 2014 at 5:23 AM, Dexuan Cui wrote: >> -Original Message- >> From: Greg KH [mailto:gre...@linuxfoundation.org] >> > > > >> > > > IMO the most feasible and need-the-least-change solution may be: >> > > > the hyperv network VSC driver passes the event >> > > > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? >> > > > >> > > No, don't do that, again, act like any other network device, drop the >> > > link and bring it up when it comes back. >> > > >> > Hi Greg, >> > Do you mean tearing down the net device and re-creating it (by >> > register_netdev() and unregister_netdev)? >> >> No, don't you have link-detect for your network device? Toggle that, I >> thought patches to do this were posted a while ago... >> >> But if you really want to tear the whole network device down and then >> back up again, sure, that would also work. > Hi Greg, Stephen, > > Thanks for the comments! > > I suppose you meant the below logic: > if (refresh) { > rtnl_lock(); > netif_carrier_off(net); > netif_carrier_on(net); > rtnl_unlock(); > } > > We have discussed this in the previous mails of this thread itself: > e.g., http://marc.info/?l=linux-driver-devel&m=140593811715975&w=2 > > Unluckily this logic doesn't work because the user-space daemons > like ifplugd, usually don't renew the DHCP immediately as long as they > receive a link-down message: they usually wait for some seconds and if > they find the link becomes up soon, they won't trigger renew operations. > (I guess this behavior can be somewhat reasonable: maybe the daemons > try to not trigger DHCP renew on temporary link instability) > > If we use this logic in the kernel space, we'll have to "fix" the user-space > daemons, like ifplugd, systemd-networkd...,etc. networkd does not suffer from this problem, and in ifplugd it can be disabled. Most other network drivers will send IFF_LOWER_DOWN/IFF_LOWER_UP upon suspend/resume so if you were to do the same you will not work any worse than the others. What would be nice, as mentioned by Dan and Lennart, would be to send an additional explicit event such as "resumed from suspend" or "L3 may be wrong". That should be a generic thing though, to fix all such issues in one go. > I'm not sure our attempt to "fix" the daemons can be easily accepted. > BTW, by CPUID, an application has a reliable way to determine if it's > running on hyper-v on not. Maybe we can "fix" the behavior of the > daemons when they run on hyper-v? > BTW2, according to my limited experience, I doubt other VMMs can > handle this auto-DHCP-renew-in-guest issue properly. To the extent this is a problem, it is a generic one, so we should not need any hyper-v specific logic in userspace. > That was why Yue's patch wanted to add a SLEEP(10s) between the > link-down and link-up events and hoped this could be an acceptable > fix(while it turned out not, obviously), though we admit it's not so good > to add such a magic number "10s" in a kernel driver. > > Please point it out if I missed or misunderstand something. > > Now I understand it's not good to pass the event to the udev daemon, > and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a > "work" task here). Please just expose to userspace what is happening (link lost/gained, resumed from suspend/...), and let us sort out how to react to that. If you put assumptions about what kind of timeouts (long-dead) userspace uses into your drivers you'll just create a mess. > Please let me know if it's the correct direction to fix the user-space > daemons (ifplugd, systemd-networkd, etc). > If you think this is viable and we should do this, I'll submit a > netif_carrier_off/on patch first and will start to work with the > projects of ifplugd, systemd-networkd and many OSVs to make the > while thing work eventually. Have you actually checked that carrier_off/on does not work on anything but ifplugd? It would surprise me... Cheers, Tom ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Sun, 10 Aug 2014, Florian Fainelli wrote: > Le 10/08/2014 20:23, Dexuan Cui a écrit : > >> -Original Message- > >> From: Greg KH [mailto:gre...@linuxfoundation.org] > > > > IMO the most feasible and need-the-least-change solution may be: > > the hyperv network VSC driver passes the event > > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? > > > No, don't do that, again, act like any other network device, drop the > link and bring it up when it comes back. > > >>> Hi Greg, > >>> Do you mean tearing down the net device and re-creating it (by > >>> register_netdev() and unregister_netdev)? > >> > >> No, don't you have link-detect for your network device? Toggle that, I > >> thought patches to do this were posted a while ago... > >> > >> But if you really want to tear the whole network device down and then > >> back up again, sure, that would also work. > > Hi Greg, Stephen, > > > > Thanks for the comments! > > > > I suppose you meant the below logic: > > if (refresh) { > > rtnl_lock(); > > netif_carrier_off(net); > > netif_carrier_on(net); > > rtnl_unlock(); > > } > > > > We have discussed this in the previous mails of this thread itself: > > e.g., http://marc.info/?l=linux-driver-devel&m=140593811715975&w=2 > > > > Unluckily this logic doesn't work because the user-space daemons > > like ifplugd, usually don't renew the DHCP immediately as long as they > > receive a link-down message: they usually wait for some seconds and if > > they find the link becomes up soon, they won't trigger renew operations. > > (I guess this behavior can be somewhat reasonable: maybe the daemons > > try to not trigger DHCP renew on temporary link instability) > > Is that such a big deal? If you know you spend much of your time in > ifplugd, why not use something different that triggers a DHCP renewal > faster, or fix ifplugd? In the case of ifplugd, it has parameters -u | --delay-up= which defaults to 0 seconds, and -d | --delay-down= which defaults to 5 seconds. Maybe for hyperv you could specify --delay-down=0. I don't know if other daemons such as systemd have similar options. It might still be good to have some modest delay between the netif_carrier_off(net) and netif_carrier_on(net). -Bill ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
Le 10/08/2014 20:23, Dexuan Cui a écrit : -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] IMO the most feasible and need-the-least-change solution may be: the hyperv network VSC driver passes the event RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? No, don't do that, again, act like any other network device, drop the link and bring it up when it comes back. Hi Greg, Do you mean tearing down the net device and re-creating it (by register_netdev() and unregister_netdev)? No, don't you have link-detect for your network device? Toggle that, I thought patches to do this were posted a while ago... But if you really want to tear the whole network device down and then back up again, sure, that would also work. Hi Greg, Stephen, Thanks for the comments! I suppose you meant the below logic: if (refresh) { rtnl_lock(); netif_carrier_off(net); netif_carrier_on(net); rtnl_unlock(); } We have discussed this in the previous mails of this thread itself: e.g., http://marc.info/?l=linux-driver-devel&m=140593811715975&w=2 Unluckily this logic doesn't work because the user-space daemons like ifplugd, usually don't renew the DHCP immediately as long as they receive a link-down message: they usually wait for some seconds and if they find the link becomes up soon, they won't trigger renew operations. (I guess this behavior can be somewhat reasonable: maybe the daemons try to not trigger DHCP renew on temporary link instability) Is that such a big deal? If you know you spend much of your time in ifplugd, why not use something different that triggers a DHCP renewal faster, or fix ifplugd? If we use this logic in the kernel space, we'll have to "fix" the user-space daemons, like ifplugd, systemd-networkd...,etc. You mean the opposite here don't you? If you put that logic in kernel space you don't have to fix the userland. I'm not sure our attempt to "fix" the daemons can be easily accepted. BTW, by CPUID, an application has a reliable way to determine if it's running on hyper-v on not. Maybe we can "fix" the behavior of the daemons when they run on hyper-v? That is not acceptable as well, why would an user-space application would have to care that much whether it runs on hyper-v or a physical host? Not to mention that anytime someone develops a similar but new application they would have to become aware of such platform and its "specicities". BTW2, according to my limited experience, I doubt other VMMs can handle this auto-DHCP-renew-in-guest issue properly. That was why Yue's patch wanted to add a SLEEP(10s) between the link-down and link-up events and hoped this could be an acceptable fix(while it turned out not, obviously), though we admit it's not so good to add such a magic number "10s" in a kernel driver. Please point it out if I missed or misunderstand something. I think this is just an integration issue that you are having, and I would not be focusing on any particular user-space implementation, but rather put something in the driver that is sensible, just like what was suggested before: toggling the carrier state. Now I understand it's not good to pass the event to the udev daemon, and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a "work" task here). Please let me know if it's the correct direction to fix the user-space daemons (ifplugd, systemd-networkd, etc). If you think this is viable and we should do this, I'll submit a netif_carrier_off/on patch first and will start to work with the projects of ifplugd, systemd-networkd and many OSVs to make the while thing work eventually. Thanks, -- Dexuan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Florian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> -Original Message- > From: Dexuan Cui > Sent: Monday, August 11, 2014 11:24 AM > Now I understand it's not good to pass the event to the udev daemon, > and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a > "work" task here). > > Please let me know if it's the correct direction to fix the user-space > daemons (ifplugd, systemd-networkd, etc). > If you think this is viable and we should do this, I'll submit a > netif_carrier_off/on patch first and will start to work with the > projects of ifplugd, systemd-networkd and many OSVs to make the > while thing work eventually. I forgot to mention: due to the huge efforts to fix the user-space daemons, I wish the "tearing down the net device and re-creating it" method could work with the daemons -- I'm going to try it. Anybody objects to this method? Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > > > > > > > > IMO the most feasible and need-the-least-change solution may be: > > > > the hyperv network VSC driver passes the event > > > > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? > > > > > > > No, don't do that, again, act like any other network device, drop the > > > link and bring it up when it comes back. > > > > > Hi Greg, > > Do you mean tearing down the net device and re-creating it (by > > register_netdev() and unregister_netdev)? > > No, don't you have link-detect for your network device? Toggle that, I > thought patches to do this were posted a while ago... > > But if you really want to tear the whole network device down and then > back up again, sure, that would also work. Hi Greg, Stephen, Thanks for the comments! I suppose you meant the below logic: if (refresh) { rtnl_lock(); netif_carrier_off(net); netif_carrier_on(net); rtnl_unlock(); } We have discussed this in the previous mails of this thread itself: e.g., http://marc.info/?l=linux-driver-devel&m=140593811715975&w=2 Unluckily this logic doesn't work because the user-space daemons like ifplugd, usually don't renew the DHCP immediately as long as they receive a link-down message: they usually wait for some seconds and if they find the link becomes up soon, they won't trigger renew operations. (I guess this behavior can be somewhat reasonable: maybe the daemons try to not trigger DHCP renew on temporary link instability) If we use this logic in the kernel space, we'll have to "fix" the user-space daemons, like ifplugd, systemd-networkd...,etc. I'm not sure our attempt to "fix" the daemons can be easily accepted. BTW, by CPUID, an application has a reliable way to determine if it's running on hyper-v on not. Maybe we can "fix" the behavior of the daemons when they run on hyper-v? BTW2, according to my limited experience, I doubt other VMMs can handle this auto-DHCP-renew-in-guest issue properly. That was why Yue's patch wanted to add a SLEEP(10s) between the link-down and link-up events and hoped this could be an acceptable fix(while it turned out not, obviously), though we admit it's not so good to add such a magic number "10s" in a kernel driver. Please point it out if I missed or misunderstand something. Now I understand it's not good to pass the event to the udev daemon, and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a "work" task here). Please let me know if it's the correct direction to fix the user-space daemons (ifplugd, systemd-networkd, etc). If you think this is viable and we should do this, I'll submit a netif_carrier_off/on patch first and will start to work with the projects of ifplugd, systemd-networkd and many OSVs to make the while thing work eventually. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Fri, 8 Aug 2014 06:45:49 -0700 Greg KH wrote: > On Fri, Aug 08, 2014 at 08:11:20AM +, Dexuan Cui wrote: > > > From: Greg KH [mailto:gre...@linuxfoundation.org] > > > Sent: Friday, August 8, 2014 11:32 AM > > > > Hi Richard and all, > > > > > > > > IMO the most feasible and need-the-least-change solution may be: > > > > the hyperv network VSC driver passes the event > > > > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? > > > > > > > > In this way, every distro only needs to add a udev rule, which should > > > > be simple. > > > > > > No, don't do that, again, act like any other network device, drop the > > > link and bring it up when it comes back. > > > > > > greg k-h > > > > Hi Greg, > > Thanks for the comment! > > > > Do you mean tearing down the net device and re-creating it (by > > register_netdev() and unregister_netdev)? > > No, don't you have link-detect for your network device? Toggle that, I > thought patches to do this were posted a while ago... > > But if you really want to tear the whole network device down and then > back up again, sure, that would also work. > > good luck, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Call netif_carrier_off then netif_carrier_on to toggle the link detect state of netdevice. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Fri, Aug 08, 2014 at 08:11:20AM +, Dexuan Cui wrote: > > From: Greg KH [mailto:gre...@linuxfoundation.org] > > Sent: Friday, August 8, 2014 11:32 AM > > > Hi Richard and all, > > > > > > IMO the most feasible and need-the-least-change solution may be: > > > the hyperv network VSC driver passes the event > > > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? > > > > > > In this way, every distro only needs to add a udev rule, which should > > > be simple. > > > > No, don't do that, again, act like any other network device, drop the > > link and bring it up when it comes back. > > > > greg k-h > > Hi Greg, > Thanks for the comment! > > Do you mean tearing down the net device and re-creating it (by > register_netdev() and unregister_netdev)? No, don't you have link-detect for your network device? Toggle that, I thought patches to do this were posted a while ago... But if you really want to tear the whole network device down and then back up again, sure, that would also work. good luck, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Friday, August 8, 2014 11:32 AM > > Hi Richard and all, > > > > IMO the most feasible and need-the-least-change solution may be: > > the hyperv network VSC driver passes the event > > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? > > > > In this way, every distro only needs to add a udev rule, which should > > be simple. > > No, don't do that, again, act like any other network device, drop the > link and bring it up when it comes back. > > greg k-h Hi Greg, Thanks for the comment! Do you mean tearing down the net device and re-creating it (by register_netdev() and unregister_netdev)? Sorry, I'm new to network drivers. I'll have to try this to see if this works or not, though I suppose it would work. -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Fri, Aug 08, 2014 at 03:13:58AM +, Dexuan Cui wrote: > > -Original Message- > > From: Richard Weinberger [mailto:richard.weinber...@gmail.com] > > Sent: Friday, August 8, 2014 6:37 AM > > To: David Miller; Yue Zhang (OSTC DEV) > > Cc: o...@aepfle.de; net...@vger.kernel.org; driverdev- > > de...@linuxdriverproject.org; LKML; Greg KH; jasow...@redhat.com; > > Haiyang Zhang; KY Srinivasan; Thomas Shao; Dexuan Cui > > Subject: Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation > > > > On Mon, Jul 21, 2014 at 11:32 PM, David Miller > > wrote: > > > From: Olaf Hering > > > Date: Mon, 21 Jul 2014 11:18:51 +0200 > > > > > >> On Mon, Jul 21, Richard Weinberger wrote: > > >> > > >>> My concern is that 10 seconds is maybe not a the right choice. > > >>> (As we cannot know all implementations) > > >> > > >> Until someone reports an issue with it, 10 is fine. Just like 20 or 666. > > > > > > Wrong, this is policy and belongs in userspace. > > > > The "/etc/init.d/network restart" nonsense now hit Linus' tree. > > Yue, what is your proposal to fix that? > > > > //richard > > Hi Richard and all, > Sorry for the late response -- actually we have been trying to > figure out a solution that's acceptable to all. > > IMO the most feasible and need-the-least-change solution may be: > the hyperv network VSC driver passes the event > RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? > > In this way, every distro only needs to add a udev rule, which should > be simple. No, don't do that, again, act like any other network device, drop the link and bring it up when it comes back. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> -Original Message- > From: Richard Weinberger [mailto:richard.weinber...@gmail.com] > Sent: Friday, August 8, 2014 6:37 AM > To: David Miller; Yue Zhang (OSTC DEV) > Cc: o...@aepfle.de; net...@vger.kernel.org; driverdev- > de...@linuxdriverproject.org; LKML; Greg KH; jasow...@redhat.com; > Haiyang Zhang; KY Srinivasan; Thomas Shao; Dexuan Cui > Subject: Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation > > On Mon, Jul 21, 2014 at 11:32 PM, David Miller > wrote: > > From: Olaf Hering > > Date: Mon, 21 Jul 2014 11:18:51 +0200 > > > >> On Mon, Jul 21, Richard Weinberger wrote: > >> > >>> My concern is that 10 seconds is maybe not a the right choice. > >>> (As we cannot know all implementations) > >> > >> Until someone reports an issue with it, 10 is fine. Just like 20 or 666. > > > > Wrong, this is policy and belongs in userspace. > > The "/etc/init.d/network restart" nonsense now hit Linus' tree. > Yue, what is your proposal to fix that? > > //richard Hi Richard and all, Sorry for the late response -- actually we have been trying to figure out a solution that's acceptable to all. IMO the most feasible and need-the-least-change solution may be: the hyperv network VSC driver passes the event RNDIS_STATUS_NETWORK_CHANGE to the udev daemon? In this way, every distro only needs to add a udev rule, which should be simple. Any comment? -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Mon, Jul 21, 2014 at 11:32 PM, David Miller wrote: > From: Olaf Hering > Date: Mon, 21 Jul 2014 11:18:51 +0200 > >> On Mon, Jul 21, Richard Weinberger wrote: >> >>> My concern is that 10 seconds is maybe not a the right choice. >>> (As we cannot know all implementations) >> >> Until someone reports an issue with it, 10 is fine. Just like 20 or 666. > > Wrong, this is policy and belongs in userspace. The "/etc/init.d/network restart" nonsense now hit Linus' tree. Yue, what is your proposal to fix that? -- Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
From: Olaf Hering Date: Mon, 21 Jul 2014 11:18:51 +0200 > On Mon, Jul 21, Richard Weinberger wrote: > >> My concern is that 10 seconds is maybe not a the right choice. >> (As we cannot know all implementations) > > Until someone reports an issue with it, 10 is fine. Just like 20 or 666. Wrong, this is policy and belongs in userspace. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Mon, 2014-07-21 at 15:11 +0200, Lennart Poettering wrote: > On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yue...@microsoft.com) wrote: > > > Some network monitoring daemon, like ifplugd has a deferring mechanism. > > When it detects carriers is offline, it doesn't trigger DHCP renew > > immediately. > > Instead it will wait for another 5 seconds to check whether carrier is back > > to > > online status. In that case, it will avoid renew DHCP lease. > > ifplugd doesn't renew DHCP leases anyway, one of the scripts it invokes > does. > > ifplugd is obsolete software. I wrote it more than 10 years ago, and > haven't really updated it since. it's sounds seriously wrong to add > multi-second waits to the kernel just to make this crappy, obsolete > software work. > > Please fix this properly, and work with the PM guys, so that we get a > sane userspace how the kernel can notify userspace about > suspends/hibernations triggered from the outside, so that userspace > daemons can subscribe to that and then refresh the DHCP leases on their > own. Yeah, like I've said before, there have been other cases where a "hey, my L3 address might be wrong now, so please confirm it" message would be useful. Carrier on/off doesn't necessarily mean that, but even if it did, the off interval is a really bad mechanism for that too. So I'd really like some kind of event for this that's distinct from carrier that userspace could use. Dan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Mon, Jul 21, 2014 at 12:21 PM, Yue Zhang (OSTC DEV) wrote: >> From: Tom Gundersen [mailto:t...@jklm.no] >> Sent: Monday, July 21, 2014 5:42 PM >> >> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang wrote: >> > From: Yue Zhang >> > >> > This patch addresses the comment from Olaf Hering and Greg KH >> > for a previous commit 3a494e710367 ("hyperv: Add handler for >> > RNDIS_STATUS_NETWORK_CHANGE event") >> > >> > In previous solution, the driver calls "network restart" to >> > force a DHCP renew when the host is back from hibernation. >> > >> > In this fix, the driver will keep network carrier offline for >> > 10 seconds and then bring it back. So that ifplugd daemon will >> > notice this change and refresh DHCP lease. >> > >> > Cc: Haiyang Zhang >> > Cc: K. Y. Srinivasan >> > >> > Signed-off-by: Yue Zhang >> > --- >> > drivers/net/hyperv/netvsc_drv.c | 21 + >> > 1 file changed, 17 insertions(+), 4 deletions(-) >> > >> > diff --git a/drivers/net/hyperv/netvsc_drv.c >> b/drivers/net/hyperv/netvsc_drv.c >> > index a9c5eaa..559c97d 100644 >> > --- a/drivers/net/hyperv/netvsc_drv.c >> > +++ b/drivers/net/hyperv/netvsc_drv.c >> > @@ -33,6 +33,7 @@ >> > #include >> > #include >> > #include >> > +#include >> > #include >> > #include >> > #include >> > @@ -792,8 +793,7 @@ static void netvsc_link_change(struct work_struct >> *w) >> > struct netvsc_device *net_device; >> > struct rndis_device *rdev; >> > bool notify, refresh = false; >> > - char *argv[] = { "/etc/init.d/network", "restart", NULL }; >> > - char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", >> NULL }; >> > + int delay; >> > >> > rtnl_lock(); >> > >> > @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct >> *w) >> > >> > rtnl_unlock(); >> > >> > - if (refresh) >> > - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); >> > + if (refresh) { >> > + /* >> > +* Keep the carrier offline for 10 seconds >> > +* to notify ifplugd daemon network change >> > +*/ >> > + for (delay = 0; delay < 10; delay++) { >> > + rtnl_lock(); >> > + netif_carrier_off(net); >> > + rtnl_unlock(); >> > + ssleep(1); >> > + } >> > + rtnl_lock(); >> > + netif_carrier_on(net); >> > + rtnl_unlock(); >> > + } >> >> Why is it necessary to wait for ten seconds? Why not just: >> >> if (refresh) { >> rtnl_lock(); >> netif_carrier_off(net); >> netif_carrier_on(net); >> rtnl_unlock(); >> } >> >> At least systemd-networkd will renew the dhcp lease as long as it gets >> NEWLINK messages indicating that the carrier was lost and regained, >> regardless of the time between events. Is there any reason not to do >> this? >> >> Cheers, >> >> Tom >> > > Hi, Tom > > Some network monitoring daemon, like ifplugd has a deferring mechanism. > When it detects carriers is offline, it doesn't trigger DHCP renew > immediately. > Instead it will wait for another 5 seconds to check whether carrier is back to > online status. In that case, it will avoid renew DHCP lease. > > And also there is some optimization in Linux's network stack. If link state > flipped so quickly, like the code you proposed. It is very likely the event > won't > be delivered to user space. Ah, ok, I don't know the kernel side of this too well, you may need some sort of flush or sync between the calls I suggested. At any rate, I would say that the solution should be to send a "lower down" followed immediately by "lower up" and never have any sort of timeout. All the drivers I have tried send out such events immediately when the machine is resumed, so I guess most network software should know how to deal with that. I really think the policy of what to do in response to the various events should be left to userspace to figure out. Cheers, Tom ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yue...@microsoft.com) wrote: > Some network monitoring daemon, like ifplugd has a deferring mechanism. > When it detects carriers is offline, it doesn't trigger DHCP renew > immediately. > Instead it will wait for another 5 seconds to check whether carrier is back > to > online status. In that case, it will avoid renew DHCP lease. ifplugd doesn't renew DHCP leases anyway, one of the scripts it invokes does. ifplugd is obsolete software. I wrote it more than 10 years ago, and haven't really updated it since. it's sounds seriously wrong to add multi-second waits to the kernel just to make this crappy, obsolete software work. Please fix this properly, and work with the PM guys, so that we get a sane userspace how the kernel can notify userspace about suspends/hibernations triggered from the outside, so that userspace daemons can subscribe to that and then refresh the DHCP leases on their own. Lennart -- Lennart Poettering, Red Hat ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Tom Gundersen [mailto:t...@jklm.no] > Sent: Monday, July 21, 2014 5:42 PM > > On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang wrote: > > From: Yue Zhang > > > > This patch addresses the comment from Olaf Hering and Greg KH > > for a previous commit 3a494e710367 ("hyperv: Add handler for > > RNDIS_STATUS_NETWORK_CHANGE event") > > > > In previous solution, the driver calls "network restart" to > > force a DHCP renew when the host is back from hibernation. > > > > In this fix, the driver will keep network carrier offline for > > 10 seconds and then bring it back. So that ifplugd daemon will > > notice this change and refresh DHCP lease. > > > > Cc: Haiyang Zhang > > Cc: K. Y. Srinivasan > > > > Signed-off-by: Yue Zhang > > --- > > drivers/net/hyperv/netvsc_drv.c | 21 + > > 1 file changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > > index a9c5eaa..559c97d 100644 > > --- a/drivers/net/hyperv/netvsc_drv.c > > +++ b/drivers/net/hyperv/netvsc_drv.c > > @@ -33,6 +33,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -792,8 +793,7 @@ static void netvsc_link_change(struct work_struct > *w) > > struct netvsc_device *net_device; > > struct rndis_device *rdev; > > bool notify, refresh = false; > > - char *argv[] = { "/etc/init.d/network", "restart", NULL }; > > - char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", > NULL }; > > + int delay; > > > > rtnl_lock(); > > > > @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct > *w) > > > > rtnl_unlock(); > > > > - if (refresh) > > - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); > > + if (refresh) { > > + /* > > +* Keep the carrier offline for 10 seconds > > +* to notify ifplugd daemon network change > > +*/ > > + for (delay = 0; delay < 10; delay++) { > > + rtnl_lock(); > > + netif_carrier_off(net); > > + rtnl_unlock(); > > + ssleep(1); > > + } > > + rtnl_lock(); > > + netif_carrier_on(net); > > + rtnl_unlock(); > > + } > > Why is it necessary to wait for ten seconds? Why not just: > > if (refresh) { > rtnl_lock(); > netif_carrier_off(net); > netif_carrier_on(net); > rtnl_unlock(); > } > > At least systemd-networkd will renew the dhcp lease as long as it gets > NEWLINK messages indicating that the carrier was lost and regained, > regardless of the time between events. Is there any reason not to do > this? > > Cheers, > > Tom > Hi, Tom Some network monitoring daemon, like ifplugd has a deferring mechanism. When it detects carriers is offline, it doesn't trigger DHCP renew immediately. Instead it will wait for another 5 seconds to check whether carrier is back to online status. In that case, it will avoid renew DHCP lease. And also there is some optimization in Linux's network stack. If link state flipped so quickly, like the code you proposed. It is very likely the event won't be delivered to user space. Thanks --- Yue ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang wrote: > From: Yue Zhang > > This patch addresses the comment from Olaf Hering and Greg KH > for a previous commit 3a494e710367 ("hyperv: Add handler for > RNDIS_STATUS_NETWORK_CHANGE event") > > In previous solution, the driver calls "network restart" to > force a DHCP renew when the host is back from hibernation. > > In this fix, the driver will keep network carrier offline for > 10 seconds and then bring it back. So that ifplugd daemon will > notice this change and refresh DHCP lease. > > Cc: Haiyang Zhang > Cc: K. Y. Srinivasan > > Signed-off-by: Yue Zhang > --- > drivers/net/hyperv/netvsc_drv.c | 21 + > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index a9c5eaa..559c97d 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -33,6 +33,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -792,8 +793,7 @@ static void netvsc_link_change(struct work_struct *w) > struct netvsc_device *net_device; > struct rndis_device *rdev; > bool notify, refresh = false; > - char *argv[] = { "/etc/init.d/network", "restart", NULL }; > - char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL > }; > + int delay; > > rtnl_lock(); > > @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct *w) > > rtnl_unlock(); > > - if (refresh) > - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); > + if (refresh) { > + /* > +* Keep the carrier offline for 10 seconds > +* to notify ifplugd daemon network change > +*/ > + for (delay = 0; delay < 10; delay++) { > + rtnl_lock(); > + netif_carrier_off(net); > + rtnl_unlock(); > + ssleep(1); > + } > + rtnl_lock(); > + netif_carrier_on(net); > + rtnl_unlock(); > + } Why is it necessary to wait for ten seconds? Why not just: if (refresh) { rtnl_lock(); netif_carrier_off(net); netif_carrier_on(net); rtnl_unlock(); } At least systemd-networkd will renew the dhcp lease as long as it gets NEWLINK messages indicating that the carrier was lost and regained, regardless of the time between events. Is there any reason not to do this? Cheers, Tom > if (notify) > netdev_notify_peers(net); > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Mon, Jul 21, Richard Weinberger wrote: > My concern is that 10 seconds is maybe not a the right choice. > (As we cannot know all implementations) Until someone reports an issue with it, 10 is fine. Just like 20 or 666. Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
Yue, Am 21.07.2014 10:44, schrieb Yue Zhang (OSTC DEV): > Hi, Richard > > IMHO, all networking implementations should handle the cable offline event. > Consider > this situation. I unplugged the network cable and connect it to a new network > switch > after 10 seconds. If the DHCP renew is not triggered, the network will break. > I think in > normal cases, it should already been handled properly. Unless there is a > strong > justification for not doing this. In that case, we shouldn't renew DHCP > anyway. I agree that they should handle the cable offline event. My concern is that 10 seconds is maybe not a the right choice. (As we cannot know all implementations) Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Richard Weinberger [mailto:rich...@nod.at] > Sent: Monday, July 21, 2014 4:18 PM > > Yue, > > Am 21.07.2014 10:05, schrieb Yue Zhang (OSTC DEV): > > The problem of systemd implementation is that in different distros, the > > ways > > to restart service are different. Propagating the event to userspace also > > doesn't > > help for this issue. > > This way each distro can provide their own restart script. > Same as every distro has custom start scripts, etc... > > > The advantage of current solution is that it simulates a cable plugging > > in/out event. IMHO, in all the distros, this simulated event has already > > been well > > handled. It is a dup effect to implement new hooks. > > Iff the current solution works for _all_ networking implementations. > > Thanks, > //richard Hi, Richard IMHO, all networking implementations should handle the cable offline event. Consider this situation. I unplugged the network cable and connect it to a new network switch after 10 seconds. If the DHCP renew is not triggered, the network will break. I think in normal cases, it should already been handled properly. Unless there is a strong justification for not doing this. In that case, we shouldn't renew DHCP anyway. Thanks Yue ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
Yue, Am 21.07.2014 10:05, schrieb Yue Zhang (OSTC DEV): > The problem of systemd implementation is that in different distros, the ways > to > restart service are different. Propagating the event to userspace also > doesn't help > for this issue. This way each distro can provide their own restart script. Same as every distro has custom start scripts, etc... > The advantage of current solution is that it simulates a cable plugging > in/out event. > IMHO, in all the distros, this simulated event has already been well handled. > It is a > dup effect to implement new hooks. Iff the current solution works for _all_ networking implementations. Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Richard Weinberger [mailto:rich...@nod.at] > Sent: Monday, July 21, 2014 2:55 PM > > Yue, > > Am 21.07.2014 04:44, schrieb Yue Zhang (OSTC DEV): > >> From: Richard Weinberger [mailto:richard.weinber...@gmail.com] > >> Why 10? Is this a random number which works by accident for ifplugd? > >> What about other networking implementations, is 10 also ok for them? > >> -- > >> Thanks, > >> //richard > > > > Hi, Richard > > > > I checked ifplugd's code. The deferring time is 5 seconds. That's how > comes > > the "10s". I agree with you this is a magic number and should be avoid. > However, > > this is the only feasible solution right now. If there is a better > > solution, I will > be > > glad to switch to it. > > > > I tested the fix in Redhat, Ubuntu and SUSE and it works in all of them. > > The problem I see is that there is no good way to trigger a DHCP renew from > a network device drivers. You're on the wrong layer. > 10 seconds may work but this is IMHO a hack which can easily break. > There are also more networking implementations than ifplugd. > Specially the systemd implementation looks promising. > > Can't you propagate the RNDIS_STATUS_NETWORK_CHANGE event to > userspace? > IIRC on HyperV guests already have a guest daemon. Let the daemon handle > the event such that distros can install their own hooks... > > Thanks, > //richard Hi, Richard The problem of systemd implementation is that in different distros, the ways to restart service are different. Propagating the event to userspace also doesn't help for this issue. The advantage of current solution is that it simulates a cable plugging in/out event. IMHO, in all the distros, this simulated event has already been well handled. It is a dup effect to implement new hooks. Thanks Yue ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
Yie, Am 21.07.2014 04:44, schrieb Yue Zhang (OSTC DEV): >> From: Richard Weinberger [mailto:richard.weinber...@gmail.com] >> Why 10? Is this a random number which works by accident for ifplugd? >> What about other networking implementations, is 10 also ok for them? >> -- >> Thanks, >> //richard > > Hi, Richard > > I checked ifplugd's code. The deferring time is 5 seconds. That's how comes > the "10s". I agree with you this is a magic number and should be avoid. > However, > this is the only feasible solution right now. If there is a better solution, > I will be > glad to switch to it. > > I tested the fix in Redhat, Ubuntu and SUSE and it works in all of them. The problem I see is that there is no good way to trigger a DHCP renew from a network device drivers. You're on the wrong layer. 10 seconds may work but this is IMHO a hack which can easily break. There are also more networking implementations than ifplugd. Specially the systemd implementation looks promising. Can't you propagate the RNDIS_STATUS_NETWORK_CHANGE event to userspace? IIRC on HyperV guests already have a guest daemon. Let the daemon handle the event such that distros can install their own hooks... Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Varka Bhadram [mailto:varkabhad...@gmail.com] > Sent: Friday, July 18, 2014 6:13 PM > > On 07/18/2014 04:25 PM, Yue Zhang wrote: > > @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct > *w) > > > > rtnl_unlock(); > > > > - if (refresh) > > - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); > > + if (refresh) { > > + /* > > +* Keep the carrier offline for 10 seconds > > +* to notify ifplugd daemon network change > > +*/ > > This should be networking comment style.. > > /* bla bla.. >* bla >*/ > > -- > Regards, > Varka Bhadram. I will fix this. Thanks Yue ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
> From: Richard Weinberger [mailto:richard.weinber...@gmail.com] > Sent: Friday, July 18, 2014 9:41 PM > > On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang wrote: > > From: Yue Zhang > > > > This patch addresses the comment from Olaf Hering and Greg KH > > for a previous commit 3a494e710367 ("hyperv: Add handler for > > RNDIS_STATUS_NETWORK_CHANGE event") > > > > In previous solution, the driver calls "network restart" to > > force a DHCP renew when the host is back from hibernation. > > > > In this fix, the driver will keep network carrier offline for > > 10 seconds and then bring it back. So that ifplugd daemon will > > notice this change and refresh DHCP lease. > > > > Cc: Haiyang Zhang > > Cc: K. Y. Srinivasan > > > > Signed-off-by: Yue Zhang > > --- > > drivers/net/hyperv/netvsc_drv.c | 21 + > > 1 file changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > > index a9c5eaa..559c97d 100644 > > --- a/drivers/net/hyperv/netvsc_drv.c > > +++ b/drivers/net/hyperv/netvsc_drv.c > > @@ -33,6 +33,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -792,8 +793,7 @@ static void netvsc_link_change(struct work_struct > *w) > > struct netvsc_device *net_device; > > struct rndis_device *rdev; > > bool notify, refresh = false; > > - char *argv[] = { "/etc/init.d/network", "restart", NULL }; > > - char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", > NULL }; > > + int delay; > > > > rtnl_lock(); > > > > @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct > *w) > > > > rtnl_unlock(); > > > > - if (refresh) > > - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); > > + if (refresh) { > > + /* > > +* Keep the carrier offline for 10 seconds > > +* to notify ifplugd daemon network change > > +*/ > > Why 10? Is this a random number which works by accident for ifplugd? > What about other networking implementations, is 10 also ok for them? > -- > Thanks, > //richard Hi, Richard I checked ifplugd's code. The deferring time is 5 seconds. That's how comes the "10s". I agree with you this is a magic number and should be avoid. However, this is the only feasible solution right now. If there is a better solution, I will be glad to switch to it. I tested the fix in Redhat, Ubuntu and SUSE and it works in all of them. Thanks Yie ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang wrote: > From: Yue Zhang > > This patch addresses the comment from Olaf Hering and Greg KH > for a previous commit 3a494e710367 ("hyperv: Add handler for > RNDIS_STATUS_NETWORK_CHANGE event") > > In previous solution, the driver calls "network restart" to > force a DHCP renew when the host is back from hibernation. > > In this fix, the driver will keep network carrier offline for > 10 seconds and then bring it back. So that ifplugd daemon will > notice this change and refresh DHCP lease. > > Cc: Haiyang Zhang > Cc: K. Y. Srinivasan > > Signed-off-by: Yue Zhang > --- > drivers/net/hyperv/netvsc_drv.c | 21 + > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index a9c5eaa..559c97d 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -33,6 +33,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -792,8 +793,7 @@ static void netvsc_link_change(struct work_struct *w) > struct netvsc_device *net_device; > struct rndis_device *rdev; > bool notify, refresh = false; > - char *argv[] = { "/etc/init.d/network", "restart", NULL }; > - char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL > }; > + int delay; > > rtnl_lock(); > > @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct *w) > > rtnl_unlock(); > > - if (refresh) > - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); > + if (refresh) { > + /* > +* Keep the carrier offline for 10 seconds > +* to notify ifplugd daemon network change > +*/ Why 10? Is this a random number which works by accident for ifplugd? What about other networking implementations, is 10 also ok for them? > + for (delay = 0; delay < 10; delay++) { > + rtnl_lock(); > + netif_carrier_off(net); > + rtnl_unlock(); > + ssleep(1); > + } > + rtnl_lock(); > + netif_carrier_on(net); > + rtnl_unlock(); > + } > > if (notify) > netdev_notify_peers(net); > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
On 07/18/2014 04:25 PM, Yue Zhang wrote: @@ -816,8 +816,21 @@ static void netvsc_link_change(struct work_struct *w) rtnl_unlock(); - if (refresh) - call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); + if (refresh) { + /* +* Keep the carrier offline for 10 seconds +* to notify ifplugd daemon network change +*/ This should be networking comment style.. /* bla bla.. * bla */ -- Regards, Varka Bhadram. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel