[PATCH] ipconfig wait for carrier

[PATCH] ipconfig wait for carrier

[Micha Nelissen]

Currently the ip auto configuration has a hardcoded delay of 1 second.
When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
nfs root may not be found.

Remove the hardcoded delay, and wait for carrier on at least one network
device.

Index: atom-linux/net/ipv4/ipconfig.c
===================================================================
--- atom-linux/net/ipv4/ipconfig.c	(revision 1445)
+++ atom-linux/net/ipv4/ipconfig.c	(working copy)
@@ -86,8 +86,8 @@
 #endif
 
 /* Define the friendly delay before and after opening net devices */
-#define CONF_PRE_OPEN		500	/* Before opening: 1/2 second */
-#define CONF_POST_OPEN		1	/* After opening: 1 second */
+#define CONF_POST_OPEN		10	/* After opening: 10 msecs */
+#define CONF_CARRIER_TIMEOUT	120000	/* Wait for carrier timeout */
 
 /* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
 #define CONF_OPEN_RETRIES 	2	/* (Re)open devices twice */
@@ -187,11 +187,22 @@
 static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
 static struct net_device *ic_dev __initdata = NULL;	/* Selected device */
 
+static int __init ic_is_init_dev(struct net_device *dev)
+{
+	if (dev->flags & IFF_LOOPBACK)
+		return 0;
+	return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+	    (!(dev->flags & IFF_LOOPBACK) &&
+	     (dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
+	     strncmp(dev->name, "dummy", 5));
+}
+
 static int __init ic_open_devs(void)
 {
 	struct ic_device *d, **last;
 	struct net_device *dev;
 	unsigned short oflags;
+	unsigned long start;
 
 	last = &ic_first_dev;
 	rtnl_lock();
@@ -205,12 +216,7 @@
 	}
 
 	for_each_netdev(&init_net, dev) {
-		if (dev->flags & IFF_LOOPBACK)
-			continue;
-		if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
-		    (!(dev->flags & IFF_LOOPBACK) &&
-		     (dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
-		     strncmp(dev->name, "dummy", 5))) {
+		if (ic_is_init_dev(dev)) {
 			int able = 0;
 			if (dev->mtu >= 364)
 				able |= IC_BOOTP;
@@ -244,6 +250,17 @@
 				dev->name, able, d->xid));
 		}
 	}
+
+	/* wait for a carrier on at least one device */
+	start = jiffies;
+	while (jiffies - start < msecs_to_jiffies(CONF_CARRIER_TIMEOUT)) {
+		for_each_netdev(&init_net, dev)
+			if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
+				goto have_carrier;
+
+		msleep(1);
+	}
+have_carrier:
 	rtnl_unlock();
 
 	*last = NULL;
@@ -1325,15 +1342,12 @@
 #ifdef IPCONFIG_DYNAMIC
  try_try_again:
 #endif
-	/* Give hardware a chance to settle */
-	msleep(CONF_PRE_OPEN);
-
 	/* Setup all network devices */
 	if (ic_open_devs() < 0)
 		return -1;
 
 	/* Give drivers a chance to settle */
-	ssleep(CONF_POST_OPEN);
+	msleep(CONF_POST_OPEN);
 
 	/*
 	 * If the config information is insufficient (e.g., our IP address or

Re: [PATCH] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Sat, 14 May 2011 22:36:26 +0200

> Currently the ip auto configuration has a hardcoded delay of 1 second.
> When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
> nfs root may not be found.
> 
> Remove the hardcoded delay, and wait for carrier on at least one network
> device.

Again, you are missing a proper "Signed-off-by: " line, also:

> 
> Index: atom-linux/net/ipv4/ipconfig.c
> ===================================================================
> --- atom-linux/net/ipv4/ipconfig.c	(revision 1445)
> +++ atom-linux/net/ipv4/ipconfig.c	(working copy)

I'm not taking patches which are against some random atom-linux tree,
you have to make your patches against the current upstream sources
only.

[PATCH v2] ipconfig wait for carrier

[Micha Nelissen]

Currently the ip auto configuration has a hardcoded delay of 1 second.
When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
nfs root may not be found.

Remove the hardcoded delay, and wait for carrier on at least one network
device.

Signed-off-by: Micha Nelissen <micha@neli.hopto.org>
Cc: David Miller <davem@davemloft.net>
---
 net/ipv4/ipconfig.c |   32 +++++++++++++++++++++-----------
 1 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..953808f 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -87,8 +87,8 @@
 #endif
 
 /* Define the friendly delay before and after opening net devices */
-#define CONF_PRE_OPEN		500	/* Before opening: 1/2 second */
-#define CONF_POST_OPEN		1	/* After opening: 1 second */
+#define CONF_POST_OPEN		10	/* After opening: 10 msecs */
+#define CONF_CARRIER_TIMEOUT	120000	/* Wait for carrier timeout */
 
 /* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
 #define CONF_OPEN_RETRIES 	2	/* (Re)open devices twice */
@@ -188,14 +188,14 @@ struct ic_device {
 static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
 static struct net_device *ic_dev __initdata = NULL;	/* Selected device */
 
-static bool __init ic_device_match(struct net_device *dev)
+static bool __init ic_is_init_dev(struct net_device *dev)
 {
-	if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+	if (dev->flags & IFF_LOOPBACK)
+		return 0;
+	return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
 	    (!(dev->flags & IFF_LOOPBACK) &&
 	     (dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
-	     strncmp(dev->name, "dummy", 5)))
-		return true;
-	return false;
+	     strncmp(dev->name, "dummy", 5));
 }
 
 static int __init ic_open_devs(void)
@@ -203,6 +203,7 @@ static int __init ic_open_devs(void)
 	struct ic_device *d, **last;
 	struct net_device *dev;
 	unsigned short oflags;
+	unsigned long start;
 
 	last = &ic_first_dev;
 	rtnl_lock();
@@ -216,9 +217,7 @@ static int __init ic_open_devs(void)
 	}
 
 	for_each_netdev(&init_net, dev) {
-		if (dev->flags & IFF_LOOPBACK)
-			continue;
-		if (ic_device_match(dev)) {
+		if (ic_is_init_dev(dev)) {
 			int able = 0;
 			if (dev->mtu >= 364)
 				able |= IC_BOOTP;
@@ -252,6 +251,17 @@ static int __init ic_open_devs(void)
 				dev->name, able, d->xid));
 		}
 	}
+
+	/* wait for a carrier on at least one device */
+	start = jiffies;
+	while (jiffies - start < msecs_to_jiffies(CONF_CARRIER_TIMEOUT)) {
+		for_each_netdev(&init_net, dev)
+			if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
+				goto have_carrier;
+
+		msleep(1);
+	}
+have_carrier:
 	rtnl_unlock();
 
 	*last = NULL;
@@ -1378,7 +1388,7 @@ static int __init ip_auto_config(void)
 		return err;
 
 	/* Give drivers a chance to settle */
-	ssleep(CONF_POST_OPEN);
+	msleep(CONF_POST_OPEN);
 
 	/*
 	 * If the config information is insufficient (e.g., our IP address or
-- 
1.7.4.4

[PATCH v3] ipconfig wait for carrier

[Micha Nelissen]

David, forgive my clumsiness, the previous patch did not compile. This
one applies against 2.6.38 and does compile.

Currently the ip auto configuration has a hardcoded delay of 1 second.
When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
nfs root may not be found.

Remove the hardcoded delay, and wait for carrier on at least one network
device.

Signed-off-by: Micha Nelissen <micha@neli.hopto.org>
Cc: David Miller <davem@davemloft.net>
---
 net/ipv4/ipconfig.c |   35 ++++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..4225f3f 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -87,8 +87,8 @@
 #endif
 
 /* Define the friendly delay before and after opening net devices */
-#define CONF_PRE_OPEN		500	/* Before opening: 1/2 second */
-#define CONF_POST_OPEN		1	/* After opening: 1 second */
+#define CONF_POST_OPEN		10	/* After opening: 10 msecs */
+#define CONF_CARRIER_TIMEOUT	120000	/* Wait for carrier timeout */
 
 /* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
 #define CONF_OPEN_RETRIES 	2	/* (Re)open devices twice */
@@ -188,14 +188,14 @@ struct ic_device {
 static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
 static struct net_device *ic_dev __initdata = NULL;	/* Selected device */
 
-static bool __init ic_device_match(struct net_device *dev)
+static bool __init ic_is_init_dev(struct net_device *dev)
 {
-	if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+	if (dev->flags & IFF_LOOPBACK)
+		return 0;
+	return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
 	    (!(dev->flags & IFF_LOOPBACK) &&
 	     (dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
-	     strncmp(dev->name, "dummy", 5)))
-		return true;
-	return false;
+	     strncmp(dev->name, "dummy", 5));
 }
 
 static int __init ic_open_devs(void)
@@ -203,6 +203,7 @@ static int __init ic_open_devs(void)
 	struct ic_device *d, **last;
 	struct net_device *dev;
 	unsigned short oflags;
+	unsigned long start;
 
 	last = &ic_first_dev;
 	rtnl_lock();
@@ -216,9 +217,7 @@ static int __init ic_open_devs(void)
 	}
 
 	for_each_netdev(&init_net, dev) {
-		if (dev->flags & IFF_LOOPBACK)
-			continue;
-		if (ic_device_match(dev)) {
+		if (ic_is_init_dev(dev)) {
 			int able = 0;
 			if (dev->mtu >= 364)
 				able |= IC_BOOTP;
@@ -252,6 +251,17 @@ static int __init ic_open_devs(void)
 				dev->name, able, d->xid));
 		}
 	}
+
+	/* wait for a carrier on at least one device */
+	start = jiffies;
+	while (jiffies - start < msecs_to_jiffies(CONF_CARRIER_TIMEOUT)) {
+		for_each_netdev(&init_net, dev)
+			if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
+				goto have_carrier;
+
+		msleep(1);
+	}
+have_carrier:
 	rtnl_unlock();
 
 	*last = NULL;
@@ -1324,14 +1334,13 @@ static int __init wait_for_devices(void)
 {
 	int i;
 
-	msleep(CONF_PRE_OPEN);
 	for (i = 0; i < DEVICE_WAIT_MAX; i++) {
 		struct net_device *dev;
 		int found = 0;
 
 		rtnl_lock();
 		for_each_netdev(&init_net, dev) {
-			if (ic_device_match(dev)) {
+			if (ic_is_init_dev(dev)) {
 				found = 1;
 				break;
 			}
@@ -1378,7 +1387,7 @@ static int __init ip_auto_config(void)
 		return err;
 
 	/* Give drivers a chance to settle */
-	ssleep(CONF_POST_OPEN);
+	msleep(CONF_POST_OPEN);
 
 	/*
 	 * If the config information is insufficient (e.g., our IP address or
-- 
1.7.4.4

Re: [PATCH v3] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Wed, 18 May 2011 07:22:41 +0200

> David, forgive my clumsiness, the previous patch did not compile. This
> one applies against 2.6.38 and does compile.

Please send patches against the current code, not some arbitrary older
source tree.

Thanks.

Re: [PATCH v3] ipconfig wait for carrier

[Micha Nelissen]

Op 2011-05-18 8:07, David Miller schreef:
> From: Micha Nelissen<micha@neli.hopto.org>
> Date: Wed, 18 May 2011 07:22:41 +0200
>
>> David, forgive my clumsiness, the previous patch did not compile. This
>> one applies against 2.6.38 and does compile.
>
> Please send patches against the current code, not some arbitrary older
> source tree.

I'm confused. Against which tree/commit do you want it then?

Micha

Re: [PATCH v3] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Wed, 18 May 2011 08:32:35 +0200

> Op 2011-05-18 8:07, David Miller schreef:
>> From: Micha Nelissen<micha@neli.hopto.org>
>> Date: Wed, 18 May 2011 07:22:41 +0200
>>
>>> David, forgive my clumsiness, the previous patch did not compile. This
>>> one applies against 2.6.38 and does compile.
>>
>> Please send patches against the current code, not some arbitrary older
>> source tree.
> 
> I'm confused. Against which tree/commit do you want it then?

Linus's current tree would be fine as would:

	git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git

Re: [PATCH v3] ipconfig wait for carrier

[Micha Nelissen]

Op 2011-05-18 8:37, David Miller schreef:
> From: Micha Nelissen<micha@neli.hopto.org>
> Date: Wed, 18 May 2011 08:32:35 +0200
>
>> I'm confused. Against which tree/commit do you want it then?
>
> Linus's current tree would be fine as would:
>
> 	git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git

Ok I see, thanks. The patch should apply just fine to your tree, there 
is only a spelling change since 2.6.38 which does not conflict.

Micha

Re: [PATCH v3] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Wed, 18 May 2011 08:59:32 +0200

> Op 2011-05-18 8:37, David Miller schreef:
>> From: Micha Nelissen<micha@neli.hopto.org>
>> Date: Wed, 18 May 2011 08:32:35 +0200
>>
>>> I'm confused. Against which tree/commit do you want it then?
>>
>> Linus's current tree would be fine as would:
>>
>> 	git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
> 
> Ok I see, thanks. The patch should apply just fine to your tree, there
> is only a spelling change since 2.6.38 which does not conflict.

Please fix ic_is_init_dev() to return a proper boolean "false" instead
of "0" when IFF_LOOPBACK is set.

Re: [PATCH v3] ipconfig wait for carrier

[Dan Williams]

On Wed, 2011-05-18 at 18:14 -0400, David Miller wrote:
> From: Micha Nelissen <micha@neli.hopto.org>
> Date: Wed, 18 May 2011 08:59:32 +0200
> 
> > Op 2011-05-18 8:37, David Miller schreef:
> >> From: Micha Nelissen<micha@neli.hopto.org>
> >> Date: Wed, 18 May 2011 08:32:35 +0200
> >>
> >>> I'm confused. Against which tree/commit do you want it then?
> >>
> >> Linus's current tree would be fine as would:
> >>
> >> 	git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
> > 
> > Ok I see, thanks. The patch should apply just fine to your tree, there
> > is only a spelling change since 2.6.38 which does not conflict.
> 
> Please fix ic_is_init_dev() to return a proper boolean "false" instead
> of "0" when IFF_LOOPBACK is set.

Shouldn't the code still wait at *least* one second?  Not all drivers
support carrier detect, and those that don't set the carrier always-on.
Thus older devices that used to have 1s to get carrier in line (even if
they don't report it) now have only 10ms.

I think it should wait at least one second like the code currently does,
and then if the carrier still isn't up, wait longer.

Dan

Re: [PATCH v3] ipconfig wait for carrier

[Micha Nelissen]

Dan Williams wrote:
> On Wed, 2011-05-18 at 18:14 -0400, David Miller wrote:
>> Please fix ic_is_init_dev() to return a proper boolean "false" instead
>> of "0" when IFF_LOOPBACK is set.

Ok. Had an int before, but boolean is better.

> Shouldn't the code still wait at *least* one second?  Not all drivers
> support carrier detect, and those that don't set the carrier always-on.
> Thus older devices that used to have 1s to get carrier in line (even if
> they don't report it) now have only 10ms.
> 
> I think it should wait at least one second like the code currently does,
> and then if the carrier still isn't up, wait longer.

What is the 1 second based on?

If a driver does not support carrier detect, then this code will wait
for the timeout period. Or do those older drivers set carrier detect
immediately when device is probed?

Micha

Re: [PATCH v3] ipconfig wait for carrier

[David Miller]

From: Dan Williams <dcbw@redhat.com>
Date: Thu, 19 May 2011 10:31:58 -0500

> On Wed, 2011-05-18 at 18:14 -0400, David Miller wrote:
>> From: Micha Nelissen <micha@neli.hopto.org>
>> Date: Wed, 18 May 2011 08:59:32 +0200
>> 
>> > Op 2011-05-18 8:37, David Miller schreef:
>> >> From: Micha Nelissen<micha@neli.hopto.org>
>> >> Date: Wed, 18 May 2011 08:32:35 +0200
>> >>
>> >>> I'm confused. Against which tree/commit do you want it then?
>> >>
>> >> Linus's current tree would be fine as would:
>> >>
>> >> 	git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
>> > 
>> > Ok I see, thanks. The patch should apply just fine to your tree, there
>> > is only a spelling change since 2.6.38 which does not conflict.
>> 
>> Please fix ic_is_init_dev() to return a proper boolean "false" instead
>> of "0" when IFF_LOOPBACK is set.
> 
> Shouldn't the code still wait at *least* one second?  Not all drivers
> support carrier detect, and those that don't set the carrier always-on.
> Thus older devices that used to have 1s to get carrier in line (even if
> they don't report it) now have only 10ms.
> 
> I think it should wait at least one second like the code currently does,
> and then if the carrier still isn't up, wait longer.

Agreed.

Re: [PATCH v3] ipconfig wait for carrier

[Micha Nelissen]

Dan Williams wrote:
> Shouldn't the code still wait at *least* one second?  Not all drivers
> support carrier detect, and those that don't set the carrier always-on.
> Thus older devices that used to have 1s to get carrier in line (even if
> they don't report it) now have only 10ms.

Btw, it does not matter much, there are 2 cases:
1) DHCP: dhcp will retry every few seconds, so if link is not up, then a
later try will succeed
2) Static IP: an ARP request is performed every second, so the second
request will be answered instead of the first.

Even if link is "fake up" by driver and not actually up after 10 msecs,
things will continue to work (eventually), after a second, just like now.

Micha

Re: [PATCH v3] ipconfig wait for carrier

[Dan Williams]

On Thu, 2011-05-19 at 20:52 +0200, Micha Nelissen wrote:
> Dan Williams wrote:
> > On Wed, 2011-05-18 at 18:14 -0400, David Miller wrote:
> >> Please fix ic_is_init_dev() to return a proper boolean "false" instead
> >> of "0" when IFF_LOOPBACK is set.
> 
> Ok. Had an int before, but boolean is better.
> 
> > Shouldn't the code still wait at *least* one second?  Not all drivers
> > support carrier detect, and those that don't set the carrier always-on.
> > Thus older devices that used to have 1s to get carrier in line (even if
> > they don't report it) now have only 10ms.
> > 
> > I think it should wait at least one second like the code currently does,
> > and then if the carrier still isn't up, wait longer.
> 
> What is the 1 second based on?
> 
> If a driver does not support carrier detect, then this code will wait
> for the timeout period. Or do those older drivers set carrier detect
> immediately when device is probed?

Older devices that do not support carrier detect have the carrier always
on, ie IFF_RUNNING is always set, and netif_carrier_ok() always returns
1.  There is currently no way to determine whether a device supports
carrier detection or not, since drivers do not set a flag anywhere to
that effect (though I'd like it if they did).

I think the bit we're talking about is the change to CONF_POST_OPEN and
the corresponding change from ssleep(1) -> msleep(10).  For drivers that
don't support carrier detect, the patch would effectively decrease the
time that these drivers have to establish a carrier (even if they don't
report it to the kernel!) from 1 second to 10ms.  Then the code in
ic_open_devs() will immediately fall down to have_carrier because these
drivers have the carrier always one.

Net result: previously there was a 1 second window for this older
hardware to establish a link, now there is a 10ms window.  That's a
behavior change that could break stuff that used to work.

Dan

> Micha
> --
> 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 v3] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Thu, 19 May 2011 21:24:42 +0200

> Dan Williams wrote:
>> Shouldn't the code still wait at *least* one second?  Not all drivers
>> support carrier detect, and those that don't set the carrier always-on.
>> Thus older devices that used to have 1s to get carrier in line (even if
>> they don't report it) now have only 10ms.
> 
> Btw, it does not matter much, there are 2 cases:
> 1) DHCP: dhcp will retry every few seconds, so if link is not up, then a
> later try will succeed
> 2) Static IP: an ARP request is performed every second, so the second
> request will be answered instead of the first.
> 
> Even if link is "fake up" by driver and not actually up after 10 msecs,
> things will continue to work (eventually), after a second, just like now.

But this eats one of the CONF_SEND_RETRIES attempts, which is only 6.

Re: [PATCH v3] ipconfig wait for carrier

[Dan Williams]

On Thu, 2011-05-19 at 21:24 +0200, Micha Nelissen wrote:
> Dan Williams wrote:
> > Shouldn't the code still wait at *least* one second?  Not all drivers
> > support carrier detect, and those that don't set the carrier always-on.
> > Thus older devices that used to have 1s to get carrier in line (even if
> > they don't report it) now have only 10ms.
> 
> Btw, it does not matter much, there are 2 cases:
> 1) DHCP: dhcp will retry every few seconds, so if link is not up, then a
> later try will succeed
> 2) Static IP: an ARP request is performed every second, so the second
> request will be answered instead of the first.
> 
> Even if link is "fake up" by driver and not actually up after 10 msecs,
> things will continue to work (eventually), after a second, just like now.

I don't particularly care what happens here, I was simply pointing out
that previous assumptions about older driver behavior are broken by this
patch, and this can cause a change in behavior.  The simplest thing to
do here is to revert only the hunks that change CONF_POST_OPEN, ie set
CONF_POST_OPEN back to 1, and revert the ssleep() -> msleep() bit.  The
rest of it looks fine to me.

But if davem wants to take the patch anyway, that's fine with me too,
since I believe all drivers that don't support carrier detect should be
put out of their misery by a quick bullet to the back of the head at the
end of a dark alley anyway.

Dan

Re: [PATCH v3] ipconfig wait for carrier

[Micha Nelissen]

David Miller wrote:
> From: Micha Nelissen <micha@neli.hopto.org>
> Date: Thu, 19 May 2011 21:24:42 +0200
> 
>> Even if link is "fake up" by driver and not actually up after 10 msecs,
>> things will continue to work (eventually), after a second, just like now.
> 
> But this eats one of the CONF_SEND_RETRIES attempts, which is only 6.

Right. So an alternative could be to increase this to 7, or do you
prefer leaving the 1 sec timeout untouched?

Micha

Re: [PATCH v3] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Thu, 19 May 2011 21:39:25 +0200

> David Miller wrote:
>> From: Micha Nelissen <micha@neli.hopto.org>
>> Date: Thu, 19 May 2011 21:24:42 +0200
>> 
>>> Even if link is "fake up" by driver and not actually up after 10 msecs,
>>> things will continue to work (eventually), after a second, just like now.
>> 
>> But this eats one of the CONF_SEND_RETRIES attempts, which is only 6.
> 
> Right. So an alternative could be to increase this to 7, or do you
> prefer leaving the 1 sec timeout untouched?

I'm not so sure at the moment.

Why don't you post your patch with the "bool" thing fixed and I'll
think about it?

Thanks.

[PATCH v4] ipconfig wait for carrier

[Micha Nelissen]

v3 -> v4: fix return boolean false instead of 0 for ic_is_init_dev

Currently the ip auto configuration has a hardcoded delay of 1 second.
When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
nfs root may not be found.

Remove the hardcoded delay, and wait for carrier on at least one network
device.

Signed-off-by: Micha Nelissen <micha@neli.hopto.org>
Cc: David Miller <davem@davemloft.net>
---
 net/ipv4/ipconfig.c |   35 ++++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..d2e0bf4 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -87,8 +87,8 @@
 #endif
 
 /* Define the friendly delay before and after opening net devices */
-#define CONF_PRE_OPEN		500	/* Before opening: 1/2 second */
-#define CONF_POST_OPEN		1	/* After opening: 1 second */
+#define CONF_POST_OPEN		10	/* After opening: 10 msecs */
+#define CONF_CARRIER_TIMEOUT	120000	/* Wait for carrier timeout */
 
 /* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
 #define CONF_OPEN_RETRIES 	2	/* (Re)open devices twice */
@@ -188,14 +188,14 @@ struct ic_device {
 static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
 static struct net_device *ic_dev __initdata = NULL;	/* Selected device */
 
-static bool __init ic_device_match(struct net_device *dev)
+static bool __init ic_is_init_dev(struct net_device *dev)
 {
-	if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+	if (dev->flags & IFF_LOOPBACK)
+		return false;
+	return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
 	    (!(dev->flags & IFF_LOOPBACK) &&
 	     (dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
-	     strncmp(dev->name, "dummy", 5)))
-		return true;
-	return false;
+	     strncmp(dev->name, "dummy", 5));
 }
 
 static int __init ic_open_devs(void)
@@ -203,6 +203,7 @@ static int __init ic_open_devs(void)
 	struct ic_device *d, **last;
 	struct net_device *dev;
 	unsigned short oflags;
+	unsigned long start;
 
 	last = &ic_first_dev;
 	rtnl_lock();
@@ -216,9 +217,7 @@ static int __init ic_open_devs(void)
 	}
 
 	for_each_netdev(&init_net, dev) {
-		if (dev->flags & IFF_LOOPBACK)
-			continue;
-		if (ic_device_match(dev)) {
+		if (ic_is_init_dev(dev)) {
 			int able = 0;
 			if (dev->mtu >= 364)
 				able |= IC_BOOTP;
@@ -252,6 +251,17 @@ static int __init ic_open_devs(void)
 				dev->name, able, d->xid));
 		}
 	}
+
+	/* wait for a carrier on at least one device */
+	start = jiffies;
+	while (jiffies - start < msecs_to_jiffies(CONF_CARRIER_TIMEOUT)) {
+		for_each_netdev(&init_net, dev)
+			if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
+				goto have_carrier;
+
+		msleep(1);
+	}
+have_carrier:
 	rtnl_unlock();
 
 	*last = NULL;
@@ -1324,14 +1334,13 @@ static int __init wait_for_devices(void)
 {
 	int i;
 
-	msleep(CONF_PRE_OPEN);
 	for (i = 0; i < DEVICE_WAIT_MAX; i++) {
 		struct net_device *dev;
 		int found = 0;
 
 		rtnl_lock();
 		for_each_netdev(&init_net, dev) {
-			if (ic_device_match(dev)) {
+			if (ic_is_init_dev(dev)) {
 				found = 1;
 				break;
 			}
@@ -1378,7 +1387,7 @@ static int __init ip_auto_config(void)
 		return err;
 
 	/* Give drivers a chance to settle */
-	ssleep(CONF_POST_OPEN);
+	msleep(CONF_POST_OPEN);
 
 	/*
 	 * If the config information is insufficient (e.g., our IP address or
-- 
1.7.4.4

Re: [PATCH v4] ipconfig wait for carrier

[David Miller]

From: Micha Nelissen <micha@neli.hopto.org>
Date: Thu, 19 May 2011 22:14:06 +0200

> v3 -> v4: fix return boolean false instead of 0 for ic_is_init_dev
> 
> Currently the ip auto configuration has a hardcoded delay of 1 second.
> When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
> nfs root may not be found.
> 
> Remove the hardcoded delay, and wait for carrier on at least one network
> device.
> 
> Signed-off-by: Micha Nelissen <micha@neli.hopto.org>
> Cc: David Miller <davem@davemloft.net>

Ok, I've applied this, let's see what happens.