Remarks and comments about ipconfig behavior

Remarks and comments about ipconfig behavior

[Erwan Velu]

Hey Fellows !

I've been figuring a strange behavior today and I'd like to share with 
you both experience and remarks.

On my system that runs a 3.2.29 but that's also applicable with 
linux-next and all other releases.

As shown here : 
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=net/ipv4/ipconfig.c;h=67e8a6b086ea7a0d2c4cc986ed6ed0e6b4414c6a;hb=HEAD#l262 
, if you specify an ip= option on the cmdline, the kernel is expecting 
the carrier to be present unless it will make a loop up to 2mn as per 
CONF_CARRIER_TIMEOUT value.

That lever for me two points :
- why is this timeout setup for so long ? Even with a spantree 
configuration, not having a carrier for 2mn is *waow* ... Does a 30sec 
could not be enough ? What is the need of waiting so long time ?

- Until we get the carrier, the kernel just stops and the boot process 
is totally locked but there isn't any message shown to the user. The 
system really look like frozen/dead for 2 minutes.

I spent a complete day trying to understand why my box was unable to 
boot while the network cable was removed.
I just discover this behavior by comparing log files to understand that 
was the reason.


So my suggestion would be the following and I can offer patches if you 
agree on thoses points :


- reducing the timeout to something smaller like 30sec
- display a message every second to inform the user we are waiting the 
carrier to satisfy the ipconfig option

What are you thoughts on that ?

Thanks you,
Erwan Velu

Re: Remarks and comments about ipconfig behavior

[David Miller]

From: Erwan Velu <erwanaliasr1@gmail.com>
Date: Thu, 13 Sep 2012 22:21:15 +0200

> That lever for me two points :
> - why is this timeout setup for so long ? Even with a spantree
> - configuration, not having a carrier for 2mn is *waow* ... Does a 30sec
> - could not be enough ? What is the need of waiting so long time ?

I've seen PHY/switch/hub combinations that take longer than 30 seconds
to fully negotiate the link.

There is really no upper limit to the link speed/duplex/etc.
negoatiation process.

Even if the actual negoatiation protocol had an upper limit on
negoatiation time, hardware implementations do things like try
sampling the quality of the cable signal and may choose to
down-rev the advertised features and restart the negoatiation.

Re: Remarks and comments about ipconfig behavior

[Erwan Velu]

Le 13/09/2012 22:31, David Miller a écrit :
> From: Erwan Velu<erwanaliasr1@gmail.com>
> Date: Thu, 13 Sep 2012 22:21:15 +0200
>
>> That lever for me two points :
>> - why is this timeout setup for so long ? Even with a spantree
>> - configuration, not having a carrier for 2mn is *waow* ... Does a 30sec
>> - could not be enough ? What is the need of waiting so long time ?
> I've seen PHY/switch/hub combinations that take longer than 30 seconds
> to fully negotiate the link.
>
> There is really no upper limit to the link speed/duplex/etc.
> negoatiation process.
>
> Even if the actual negoatiation protocol had an upper limit on
> negoatiation time, hardware implementations do things like try
> sampling the quality of the cable signal and may choose to
> down-rev the advertised features and restart the negoatiation.
Ok but shouldn't we display some message to the user trying to explain 
why the kernel is stopped and perfectly silent during its booting 
process ? 2 minutes, that's a pretty long time with an almost frozen 
kernel isn't it ?

Re: Remarks and comments about ipconfig behavior

[David Miller]

From: Erwan Velu <erwanaliasr1@gmail.com>
Date: Thu, 13 Sep 2012 22:37:45 +0200

> Ok but shouldn't we display some message to the user trying to explain
> why the kernel is stopped and perfectly silent during its booting
> process ? 2 minutes, that's a pretty long time with an almost frozen
> kernel isn't it ?

Patches welcome.

[PATCH] ipconfig: Inform user if carrier is not ready

[Erwan Velu]

From: Erwan Velu <erwanaliasr1@gmail.com>

While using the ip= option at the cmdline, the kernel can hold the boot
process for 2 minutes (CONF_CARRIER_TIMEOUT) if the carrier is not
present.

While waiting the carrier, user is not informed about this situation and
so could think the kernel is frozen.

This patch is just adding a simple message every second telling we are
waiting the carrier to come up.
---
  net/ipv4/ipconfig.c |    8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 67e8a6b..d9f34b7 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -205,6 +205,7 @@ static int __init ic_open_devs(void)
      struct net_device *dev;
      unsigned short oflags;
      unsigned long start;
+    unsigned int loops=0;

      last = &ic_first_dev;
      rtnl_lock();
@@ -266,6 +267,13 @@ static int __init ic_open_devs(void)
              if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
                  goto have_carrier;

+        loops++;
+        /* This loop is blocking the boot process until we get the 
carrier or reach the timeout.
+         * We have to inform the user about the situation as it could 
look like a kernel freeze.
+         * Every second, we display a short message indicating we wait 
the carrier */
+        if ((loops % 1000) == 0) {
+            pr_info("IP-Config: Waiting Carrier (%d/%d):\n",loops / 
1000, CONF_CARRIER_TIMEOUT / 1000);
+        }
          msleep(1);
      }
  have_carrier:
-- 
1.7.10

Re: [PATCH] ipconfig: Inform user if carrier is not ready

[Francois Romieu]

Erwan Velu <erwanaliasr1@gmail.com> :
[...]
> This patch is just adding a simple message every second telling we are
> waiting the carrier to come up.
> ---
>  net/ipv4/ipconfig.c |    8 ++++++++
>  1 file changed, 8 insertions(+)

The Signed-off-by: line is missing.

> diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
> index 67e8a6b..d9f34b7 100644
> --- a/net/ipv4/ipconfig.c
> +++ b/net/ipv4/ipconfig.c
> @@ -205,6 +205,7 @@ static int __init ic_open_devs(void)
>      struct net_device *dev;
>      unsigned short oflags;
>      unsigned long start;
> +    unsigned int loops=0;

(nit)
	unsigned int loops = 0;

>      last = &ic_first_dev;
>      rtnl_lock();
> @@ -266,6 +267,13 @@ static int __init ic_open_devs(void)
>              if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
>                  goto have_carrier;
> 
> +        loops++;
> +        /* This loop is blocking the boot process until we get the
> carrier or reach the timeout.

Please split it into 80 cols max lines.

[...]
> +         * Every second, we display a short message indicating we
> wait the carrier */

(you can remove this part of the comment)

-- 
Ueimor

[PATCH V2] ipconfig: Inform user if carrier is not ready

[Erwan Velu]

From: Erwan Velu <erwanaliasr1@gmail.com>

While using the ip= option at the cmdline, the kernel can hold the boot
process for 2 minutes (CONF_CARRIER_TIMEOUT) if the carrier is not
present.

While waiting the carrier, user is not informed about this situation and
so could think the kernel is frozen.

If we don't get the carrier after some seconds, let's display a message to
inform the user about the remaining time before reaching the timeout.

Signed-off-by: Erwan Velu <erwanaliasr1@gmail.com>
---
  net/ipv4/ipconfig.c |   12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 67e8a6b..413d22d 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -90,6 +90,7 @@
  /* Define the friendly delay before and after opening net devices */
  #define CONF_POST_OPEN        10    /* After opening: 10 msecs */
  #define CONF_CARRIER_TIMEOUT    120000    /* Wait for carrier timeout */
+#define CONF_WARN_CARRIER_TIMEOUT 5000    /* Time before showing a 
warning message  */

  /* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
  #define CONF_OPEN_RETRIES     2    /* (Re)open devices twice */
@@ -205,6 +206,7 @@ static int __init ic_open_devs(void)
      struct net_device *dev;
      unsigned short oflags;
      unsigned long start;
+    unsigned int loops=0;

      last = &ic_first_dev;
      rtnl_lock();
@@ -266,6 +268,16 @@ static int __init ic_open_devs(void)
              if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
                  goto have_carrier;

+        /* This loop is blocking the boot process until
+         * we get the carrier or reach the timeout.
+         * We have to inform the user about the situation as
+         * it could look like a kernel freeze.
+         * After CONF_WARN_CARRIER_TIMEOUT milliseconds,
+         * we display the remaing time before reaching the timeout. */
+        if (++loops == CONF_WARN_CARRIER_TIMEOUT) {
+            pr_info("IP-Config: Waiting up to %d seconds for carrier on 
interface\n",
+                (CONF_CARRIER_TIMEOUT - CONF_WARN_CARRIER_TIMEOUT)/ 1000);
+        }
          msleep(1);
      }
  have_carrier:
-- 
1.7.10

Re: [PATCH V2] ipconfig: Inform user if carrier is not ready

[David Miller]

From: Erwan Velu <erwanaliasr1@gmail.com>
Date: Fri, 14 Sep 2012 21:08:16 +0200

> +        /* This loop is blocking the boot process until
> +         * we get the carrier or reach the timeout.
> +         * We have to inform the user about the situation as
> +         * it could look like a kernel freeze.
> +         * After CONF_WARN_CARRIER_TIMEOUT milliseconds,
> + * we display the remaing time before reaching the timeout. */

This is badly formatted, either because you explicitly did so
or because your email client corrupted the patch.

Either way you have to fix this before we can seriously consider
your patch.

Re: [PATCH V2] ipconfig: Inform user if carrier is not ready

[Rick Jones]

On 09/14/2012 12:08 PM, Erwan Velu wrote:
> From: Erwan Velu <erwanaliasr1@gmail.com>
>
> While using the ip= option at the cmdline, the kernel can hold the boot
> process for 2 minutes (CONF_CARRIER_TIMEOUT) if the carrier is not
> present.
>
> While waiting the carrier, user is not informed about this situation and
> so could think the kernel is frozen.
>
> If we don't get the carrier after some seconds, let's display a message to
> inform the user about the remaining time before reaching the timeout.

That should be much better than upwards of 120 messages.

rick jones

[PATCH V3] ipconfig: Inform user if carrier is not ready

[Erwan Velu]

From: Erwan Velu <erwanaliasr1@gmail.com>

While using the ip= option at the cmdline, the kernel can hold the boot
process for 2 minutes (CONF_CARRIER_TIMEOUT) if the carrier is not
present.

While waiting the carrier, user is not informed about this situation and
so could think the kernel is frozen.

If we don't get the carrier after some seconds, let's display a message to
inform the user about the remaining time before reaching the timeout.

Signed-off-by: Erwan Velu <erwanaliasr1@gmail.com>
---
  net/ipv4/ipconfig.c |   12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 67e8a6b..b79dca6 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -90,6 +90,7 @@
  /* Define the friendly delay before and after opening net devices */
  #define CONF_POST_OPEN        10    /* After opening: 10 msecs */
  #define CONF_CARRIER_TIMEOUT    120000    /* Wait for carrier timeout */
+#define CONF_WARN_CARRIER_TIMEOUT 5000    /* Time before showing a 
warning message  */

  /* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
  #define CONF_OPEN_RETRIES     2    /* (Re)open devices twice */
@@ -205,6 +206,7 @@ static int __init ic_open_devs(void)
      struct net_device *dev;
      unsigned short oflags;
      unsigned long start;
+    unsigned int loops=0;

      last = &ic_first_dev;
      rtnl_lock();
@@ -266,6 +268,16 @@ static int __init ic_open_devs(void)
              if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
                  goto have_carrier;

+        /* This loop is blocking the boot process until
+           we get the carrier or reach the timeout.
+           We have to inform the user about the situation as
+           it could look like a kernel freeze.
+           After CONF_WARN_CARRIER_TIMEOUT milliseconds,
+           we display the remaing time before reaching the timeout.*/
+        if (++loops == CONF_WARN_CARRIER_TIMEOUT) {
+            pr_info("IP-Config: Waiting up to %d seconds for carrier on 
interface\n",
+                (CONF_CARRIER_TIMEOUT - CONF_WARN_CARRIER_TIMEOUT)/ 1000);
+        }
          msleep(1);
      }
  have_carrier:
-- 
1.7.10

Re: [PATCH V2] ipconfig: Inform user if carrier is not ready

[Erwan Velu]

Le 14/09/2012 21:17, David Miller a écrit :
> This is badly formatted, either because you explicitly did so or 
> because your email client corrupted the patch. Either way you have to 
> fix this before we can seriously consider your patch. 
All apollogies, that was my stupid fault.

That's fixed. Sent as V3.

Thanks for proof-reading this stupid patch :)

Erwan

Re: [PATCH V3] ipconfig: Inform user if carrier is not ready

[David Miller]

Your email client is still corrupting this patch.

Turn off all transformations in your outgoing emails such as
text formatting and such.

In fact, email the patch to yourself, and make sure you can successfully
apply the result.

Also, you have formatted the comment improperly.

Don't format comments:

	/* Like
	   this.  */

but rather, format them:

	/* Like
	 * this.
	 */

You also didn't style the variable declaration and assignment properly,
there must be a space around the "=" sign, like this:

	unsigned int loops = 0;

Thanks.

Re: [PATCH V3] ipconfig: Inform user if carrier is not ready

[Erwan Velu]

Le 14/09/2012 21:42, David Miller a écrit :
> Your email client is still corrupting this patch.
>
> Turn off all transformations in your outgoing emails such as
> text formatting and such.
>
> In fact, email the patch to yourself, and make sure you can successfully
> apply the result
I didn't succeed at making thunderbird not corrupting the patch...
I did put the patch on an http link to insure no corruption will occurs.

http://pubz.free.fr/0001-ipconfig-Inform-user-if-carrier-is-not-ready.patch

Hope this will be acceptable....

Cheers,

Re: [PATCH V3] ipconfig: Inform user if carrier is not ready

[David Miller]

From: Erwan Velu <erwanaliasr1@gmail.com>
Date: Fri, 14 Sep 2012 22:13:00 +0200

> I didn't succeed at making thunderbird not corrupting the patch...

Read Documentation/email-clients.txt in the kernel tree for help.

Re: [PATCH V3] ipconfig: Inform user if carrier is not ready

[Erwan Velu]

Le 14/09/2012 22:23, David Miller a écrit :
> From: Erwan Velu <erwanaliasr1@gmail.com>
> Date: Fri, 14 Sep 2012 22:13:00 +0200
>
>> I didn't succeed at making thunderbird not corrupting the patch...
> Read Documentation/email-clients.txt in the kernel tree for help.
>
Sent with git send-email... this time this will be far better I hope..
Thanks rick for pointing this.

Sorry for this long thread about newbie posting...

Erwan

Re: Remarks and comments about ipconfig behavior

[Erwan Velu]

Le 13/09/2012 22:31, David Miller a écrit :
> I've seen PHY/switch/hub combinations that take longer than 30 seconds to fully negotiate the link. There is really no upper limit to the link speed/duplex/etc. negoatiation process. Even if the actual negoatiation protocol had an upper limit on negoatiation time, hardware implementations do things like try sampling the quality of the cable signal and may choose to down-rev the advertised features and restart the negoatiation. 

I do understand that some might need some longer values while some others like me really need a smaller one.

On my case, this 2mn wait breaks an hardware watchdog, so I did a small patch on my local build to get it reduced as I know  the selected value works fine for this hardware setup. And it works now perfectly.

So could it be valuable to export it as a CONFIG_something instead of patching this #define ?

Cheers,