lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAG-2HqXNi1_yNOTLcvDys3hHJ4wHU+aJLOCgdu4uiYLBTwyFdw@mail.gmail.com> Date: Mon, 21 Jul 2014 16:06:48 +0200 From: Tom Gundersen <teg@...m.no> To: "Yue Zhang (OSTC DEV)" <yuezha@...rosoft.com> Cc: netdev <netdev@...r.kernel.org>, "driverdev-devel@...uxdriverproject.org" <driverdev-devel@...uxdriverproject.org>, LKML <linux-kernel@...r.kernel.org>, Greg KH <gregkh@...uxfoundation.org>, "olaf@...fle.de" <olaf@...fle.de>, "jasowang@...hat.com" <jasowang@...hat.com>, David Miller <davem@...emloft.net>, Haiyang Zhang <haiyangz@...rosoft.com>, KY Srinivasan <kys@...rosoft.com>, Thomas Shao <huishao@...rosoft.com>, Dexuan Cui <decui@...rosoft.com>, Lennart Poettering <lennart@...ttering.net> Subject: Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation On Mon, Jul 21, 2014 at 12:21 PM, Yue Zhang (OSTC DEV) <yuezha@...rosoft.com> wrote: >> From: Tom Gundersen [mailto:teg@...m.no] >> Sent: Monday, July 21, 2014 5:42 PM >> >> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@...rosoft.com> wrote: >> > From: Yue Zhang <yuezha@...rosoft.com> >> > >> > 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 <haiyangz@...rosoft.com> >> > Cc: K. Y. Srinivasan <kys@...rosoft.com> >> > >> > Signed-off-by: Yue Zhang <yuezha@...rosoft.com> >> > --- >> > 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 <linux/if_vlan.h> >> > #include <linux/in.h> >> > #include <linux/slab.h> >> > +#include <linux/delay.h> >> > #include <net/arp.h> >> > #include <net/route.h> >> > #include <net/sock.h> >> > @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists