Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Varka Bhadram]

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.

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Varka Bhadram]

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.

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Varka Bhadram]

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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang]

From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>

Signed-off-by: Yue Zhang <yuezha@microsoft.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();
+	}
 
 	if (notify)
 		netdev_notify_peers(net);
-- 
1.9.1

[PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang]

From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>

Signed-off-by: Yue Zhang <yuezha@microsoft.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();
+	}
 
 	if (notify)
 		netdev_notify_peers(net);
-- 
1.9.1

[PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang]

From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>

Signed-off-by: Yue Zhang <yuezha@microsoft.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();
+	}
 
 	if (notify)
 		netdev_notify_peers(net);
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> Cc: K. Y. Srinivasan <kys@microsoft.com>
>
> Signed-off-by: Yue Zhang <yuezha@microsoft.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
> +                */

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 majordomo@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> Cc: K. Y. Srinivasan <kys@microsoft.com>
>
> Signed-off-by: Yue Zhang <yuezha@microsoft.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
> +                */

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 majordomo@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> Cc: K. Y. Srinivasan <kys@microsoft.com>
>
> Signed-off-by: Yue Zhang <yuezha@microsoft.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
> +                */

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 majordomo@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

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

> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Friday, July 18, 2014 9:41 PM
> 
> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Signed-off-by: Yue Zhang <yuezha@microsoft.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
> > +                */
> 
> 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
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Friday, July 18, 2014 9:41 PM
> 
> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Signed-off-by: Yue Zhang <yuezha@microsoft.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
> > +                */
> 
> 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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Friday, July 18, 2014 9:41 PM
> 
> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Signed-off-by: Yue Zhang <yuezha@microsoft.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
> > +                */
> 
> 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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Varka Bhadram [mailto:varkabhadram@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Varka Bhadram [mailto:varkabhadram@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Varka Bhadram [mailto:varkabhadram@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

Yie,

Am 21.07.2014 04:44, schrieb Yue Zhang (OSTC DEV):
>> From: Richard Weinberger [mailto:richard.weinberger@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

Yie,

Am 21.07.2014 04:44, schrieb Yue Zhang (OSTC DEV):
>> From: Richard Weinberger [mailto:richard.weinberger@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

Yie,

Am 21.07.2014 04:44, schrieb Yue Zhang (OSTC DEV):
>> From: Richard Weinberger [mailto:richard.weinberger@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

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

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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Richard Weinberger [mailto:richard@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.weinberger@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Richard Weinberger [mailto:richard@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.weinberger@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

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

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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Richard Weinberger [mailto:richard@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Richard Weinberger [mailto:richard@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Olaf Hering]

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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Olaf Hering]

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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Olaf Hering]

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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> Cc: K. Y. Srinivasan <kys@microsoft.com>
>
> Signed-off-by: Yue Zhang <yuezha@microsoft.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

>         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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

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

> From: Tom Gundersen [mailto:teg@jklm.no]
> Sent: Monday, July 21, 2014 5:42 PM
> 
> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Signed-off-by: Yue Zhang <yuezha@microsoft.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.

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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Tom Gundersen [mailto:teg@jklm.no]
> Sent: Monday, July 21, 2014 5:42 PM
> 
> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Signed-off-by: Yue Zhang <yuezha@microsoft.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.

Thanks
---
Yue

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Yue Zhang (OSTC DEV)]

> From: Tom Gundersen [mailto:teg@jklm.no]
> Sent: Monday, July 21, 2014 5:42 PM
> 
> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Signed-off-by: Yue Zhang <yuezha@microsoft.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.

Thanks
---
Yue
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Lennart Poettering]

On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yuezha@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Lennart Poettering]

On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yuezha@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Lennart Poettering]

On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yuezha@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Mon, Jul 21, 2014 at 12:21 PM, Yue Zhang (OSTC DEV)
<yuezha@microsoft.com> wrote:
>> From: Tom Gundersen [mailto:teg@jklm.no]
>> Sent: Monday, July 21, 2014 5:42 PM
>>
>> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
>> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
>> > Cc: K. Y. Srinivasan <kys@microsoft.com>
>> >
>> > Signed-off-by: Yue Zhang <yuezha@microsoft.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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Mon, Jul 21, 2014 at 12:21 PM, Yue Zhang (OSTC DEV)
<yuezha@microsoft.com> wrote:
>> From: Tom Gundersen [mailto:teg@jklm.no]
>> Sent: Monday, July 21, 2014 5:42 PM
>>
>> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
>> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
>> > Cc: K. Y. Srinivasan <kys@microsoft.com>
>> >
>> > Signed-off-by: Yue Zhang <yuezha@microsoft.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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Mon, Jul 21, 2014 at 12:21 PM, Yue Zhang (OSTC DEV)
<yuezha@microsoft.com> wrote:
>> From: Tom Gundersen [mailto:teg@jklm.no]
>> Sent: Monday, July 21, 2014 5:42 PM
>>
>> On Fri, Jul 18, 2014 at 12:55 PM, Yue Zhang <yuezha@microsoft.com> wrote:
>> > From: Yue Zhang <yuezha@microsoft.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@microsoft.com>
>> > Cc: K. Y. Srinivasan <kys@microsoft.com>
>> >
>> > Signed-off-by: Yue Zhang <yuezha@microsoft.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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dan Williams]

On Mon, 2014-07-21 at 15:11 +0200, Lennart Poettering wrote:
> On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yuezha@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dan Williams]

On Mon, 2014-07-21 at 15:11 +0200, Lennart Poettering wrote:
> On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yuezha@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dan Williams]

On Mon, 2014-07-21 at 15:11 +0200, Lennart Poettering wrote:
> On Mon, 21.07.14 10:21, Yue Zhang (OSTC DEV) (yuezha@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[David Miller]

From: Olaf Hering <olaf@aepfle.de>
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.

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[David Miller]

From: Olaf Hering <olaf@aepfle.de>
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.

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[David Miller]

From: Olaf Hering <olaf@aepfle.de>
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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

On Mon, Jul 21, 2014 at 11:32 PM, David Miller <davem@davemloft.net> wrote:
> From: Olaf Hering <olaf@aepfle.de>
> 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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Richard Weinberger]

On Mon, Jul 21, 2014 at 11:32 PM, David Miller <davem@davemloft.net> wrote:
> From: Olaf Hering <olaf@aepfle.de>
> 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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

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

> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Friday, August 8, 2014 6:37 AM
> To: David Miller; Yue Zhang (OSTC DEV)
> Cc: olaf@aepfle.de; netdev@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; LKML; Greg KH; jasowang@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 <davem@davemloft.net>
> wrote:
> > From: Olaf Hering <olaf@aepfle.de>
> > 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
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Friday, August 8, 2014 6:37 AM
> To: David Miller; Yue Zhang (OSTC DEV)
> Cc: olaf@aepfle.de; netdev@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; LKML; Greg KH; jasowang@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 <davem@davemloft.net>
> wrote:
> > From: Olaf Hering <olaf@aepfle.de>
> > 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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Friday, August 8, 2014 6:37 AM
> To: David Miller; Yue Zhang (OSTC DEV)
> Cc: olaf@aepfle.de; netdev@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; LKML; Greg KH; jasowang@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 <davem@davemloft.net>
> wrote:
> > From: Olaf Hering <olaf@aepfle.de>
> > 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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Greg KH]

On Fri, Aug 08, 2014 at 03:13:58AM +0000, Dexuan Cui wrote:
> > -----Original Message-----
> > From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> > Sent: Friday, August 8, 2014 6:37 AM
> > To: David Miller; Yue Zhang (OSTC DEV)
> > Cc: olaf@aepfle.de; netdev@vger.kernel.org; driverdev-
> > devel@linuxdriverproject.org; LKML; Greg KH; jasowang@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 <davem@davemloft.net>
> > wrote:
> > > From: Olaf Hering <olaf@aepfle.de>
> > > 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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Greg KH]

On Fri, Aug 08, 2014 at 03:13:58AM +0000, Dexuan Cui wrote:
> > -----Original Message-----
> > From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> > Sent: Friday, August 8, 2014 6:37 AM
> > To: David Miller; Yue Zhang (OSTC DEV)
> > Cc: olaf@aepfle.de; netdev@vger.kernel.org; driverdev-
> > devel@linuxdriverproject.org; LKML; Greg KH; jasowang@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 <davem@davemloft.net>
> > wrote:
> > > From: Olaf Hering <olaf@aepfle.de>
> > > 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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Greg KH]

On Fri, Aug 08, 2014 at 03:13:58AM +0000, Dexuan Cui wrote:
> > -----Original Message-----
> > From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> > Sent: Friday, August 8, 2014 6:37 AM
> > To: David Miller; Yue Zhang (OSTC DEV)
> > Cc: olaf@aepfle.de; netdev@vger.kernel.org; driverdev-
> > devel@linuxdriverproject.org; LKML; Greg KH; jasowang@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 <davem@davemloft.net>
> > wrote:
> > > From: Olaf Hering <olaf@aepfle.de>
> > > 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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> From: Greg KH [mailto:gregkh@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> From: Greg KH [mailto:gregkh@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> From: Greg KH [mailto:gregkh@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Greg KH]

On Fri, Aug 08, 2014 at 08:11:20AM +0000, Dexuan Cui wrote:
> > From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Greg KH]

On Fri, Aug 08, 2014 at 08:11:20AM +0000, Dexuan Cui wrote:
> > From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Greg KH]

On Fri, Aug 08, 2014 at 08:11:20AM +0000, Dexuan Cui wrote:
> > From: Greg KH [mailto:gregkh@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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Stephen Hemminger]

On Fri, 8 Aug 2014 06:45:49 -0700
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Fri, Aug 08, 2014 at 08:11:20AM +0000, Dexuan Cui wrote:
> > > From: Greg KH [mailto:gregkh@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 majordomo@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.

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Stephen Hemminger]

On Fri, 8 Aug 2014 06:45:49 -0700
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Fri, Aug 08, 2014 at 08:11:20AM +0000, Dexuan Cui wrote:
> > > From: Greg KH [mailto:gregkh@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 majordomo@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.

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Florian Fainelli]

Le 10/08/2014 20:23, Dexuan Cui a écrit :
>> -----Original Message-----
>> From: Greg KH [mailto:gregkh@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 majordomo@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Florian Fainelli]

Le 10/08/2014 20:23, Dexuan Cui a écrit :
>> -----Original Message-----
>> From: Greg KH [mailto:gregkh@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 majordomo@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Bill Fink]

On Sun, 10 Aug 2014, Florian Fainelli wrote:

> Le 10/08/2014 20:23, Dexuan Cui a écrit :
> >> -----Original Message-----
> >> From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Bill Fink]

On Sun, 10 Aug 2014, Florian Fainelli wrote:

> Le 10/08/2014 20:23, Dexuan Cui a écrit :
> >> -----Original Message-----
> >> From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Bill Fink]

On Sun, 10 Aug 2014, Florian Fainelli wrote:

> Le 10/08/2014 20:23, Dexuan Cui a écrit :
> >> -----Original Message-----
> >> From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Mon, Aug 11, 2014 at 5:23 AM, Dexuan Cui <decui@microsoft.com> wrote:
>> -----Original Message-----
>> From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Mon, Aug 11, 2014 at 5:23 AM, Dexuan Cui <decui@microsoft.com> wrote:
>> -----Original Message-----
>> From: Greg KH [mailto:gregkh@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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Mon, Aug 11, 2014 at 5:23 AM, Dexuan Cui <decui@microsoft.com> wrote:
>> -----Original Message-----
>> From: Greg KH [mailto:gregkh@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

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

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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> 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]: <info> (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]: <info> (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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> 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]: <info> (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]: <info> (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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> 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]: <info> (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]: <info> (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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> 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]: <info> (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]: <info> (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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Tom Gundersen]

On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> 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]: <info> (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]: <info> (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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dan Williams]

On Wed, 2014-08-13 at 15:15 +0200, Tom Gundersen wrote:
> On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> 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]: <info> (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]: <info> (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 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
> --
> 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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dan Williams]

On Wed, 2014-08-13 at 15:15 +0200, Tom Gundersen wrote:
> On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> 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]: <info> (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]: <info> (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 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
> --
> 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

Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dan Williams]

On Wed, 2014-08-13 at 15:15 +0200, Tom Gundersen wrote:
> On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> 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]: <info> (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]: <info> (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 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
> --
> 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


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

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

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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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]: <info> (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]: <info> (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

RE: [PATCH] Hyperv: Trigger DHCP renew after host hibernation

[Dexuan Cui]

> -----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]: <info> (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]: <info> (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
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel