LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch] ipv4: Update ip command line processing (take II)
@ 2007-12-26  2:59 Simon Horman
  2007-12-26  4:55 ` David Miller
  2008-01-06  3:58 ` [PATCH] Fix regression in ip command line processing Amos Waterland
  0 siblings, 2 replies; 6+ messages in thread
From: Simon Horman @ 2007-12-26  2:59 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: Amos Waterland, David Miller

Recently the documentation in Documentation/nfsroot.txt was
update to note that in fact ip=off and ip=::::::off as the
latter is ignored and the default (on) is used.

This was certainly a step in the direction of reducing confusion.
But it seems to me that the code ought to be fixed up so that
ip=::::::off actually turns off ip autoconfiguration.

This patch also notes more specifically that ip=on (aka ip=::::::on)
is the default.

Cc: Amos Waterland <apw@us.ibm.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

--- 
Fri, 21 Dec 2007 16:48:16 +0900
* Removed check on if (name) in ic_proto_name().
  It should have been if (*name == '\0'), but even then its
  a bit bogus, providing an early exit for a code-path that
  is called approximately once.

Index: net-2.6.25/net/ipv4/ipconfig.c
===================================================================
--- net-2.6.25.orig/net/ipv4/ipconfig.c	2007-12-26 11:56:20.000000000 +0900
+++ net-2.6.25/net/ipv4/ipconfig.c	2007-12-26 11:57:04.000000000 +0900
@@ -1417,6 +1417,10 @@ static int __init ic_proto_name(char *na
 	if (!strcmp(name, "on") || !strcmp(name, "any")) {
 		return 1;
 	}
+	if (!strcmp(name, "off") || !strcmp(name, "none")) {
+		ic_enable = 0;
+		return 1;
+	}
 #ifdef CONFIG_IP_PNP_DHCP
 	else if (!strcmp(name, "dhcp")) {
 		ic_proto_enabled &= ~IC_RARP;
@@ -1451,12 +1455,6 @@ static int __init ip_auto_config_setup(c
 
 	ic_set_manually = 1;
 
-	ic_enable = (*addrs &&
-		(strcmp(addrs, "off") != 0) &&
-		(strcmp(addrs, "none") != 0));
-	if (!ic_enable)
-		return 1;
-
 	if (ic_proto_name(addrs))
 		return 1;
 
Index: net-2.6.25/Documentation/nfsroot.txt
===================================================================
--- net-2.6.25.orig/Documentation/nfsroot.txt	2007-12-26 11:56:20.000000000 +0900
+++ net-2.6.25/Documentation/nfsroot.txt	2007-12-26 11:57:04.000000000 +0900
@@ -97,10 +97,6 @@ ip=<client-ip>:<server-ip>:<gw-ip>:<netm
   autoconfiguration will take place.  The most common way to use this
   is "ip=dhcp".
 
-  Note that "ip=off" is not the same thing as "ip=::::::off", because in
-  the latter autoconfiguration will take place if any of DHCP, BOOTP or RARP
-  are compiled in the kernel.
-
   <client-ip>	IP address of the client.
 
   		Default:  Determined using autoconfiguration.
@@ -150,6 +146,7 @@ ip=<client-ip>:<server-ip>:<gw-ip>:<netm
 
                   off or none: don't use autoconfiguration
 		  on or any:   use any protocol available in the kernel
+			       (default)
 		  dhcp:        use DHCP
 		  bootp:       use BOOTP
 		  rarp:        use RARP

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] ipv4: Update ip command line processing (take II)
  2007-12-26  2:59 [patch] ipv4: Update ip command line processing (take II) Simon Horman
@ 2007-12-26  4:55 ` David Miller
  2007-12-26  5:04   ` Simon Horman
  2008-01-06  3:58 ` [PATCH] Fix regression in ip command line processing Amos Waterland
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2007-12-26  4:55 UTC (permalink / raw)
  To: horms; +Cc: linux-kernel, netdev, apw

From: Simon Horman <horms@verge.net.au>
Date: Wed, 26 Dec 2007 11:59:05 +0900

> Recently the documentation in Documentation/nfsroot.txt was
> update to note that in fact ip=off and ip=::::::off as the
> latter is ignored and the default (on) is used.
> 
> This was certainly a step in the direction of reducing confusion.
> But it seems to me that the code ought to be fixed up so that
> ip=::::::off actually turns off ip autoconfiguration.
> 
> This patch also notes more specifically that ip=on (aka ip=::::::on)
> is the default.
> 
> Cc: Amos Waterland <apw@us.ibm.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>

I applied this to net-2.6 since I think it's a bug fix, and
I also tailored the changelog header to more it clear this
is a fix not just an "Update". :-)

Thanks!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] ipv4: Update ip command line processing (take II)
  2007-12-26  4:55 ` David Miller
@ 2007-12-26  5:04   ` Simon Horman
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2007-12-26  5:04 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, netdev, apw

On Tue, Dec 25, 2007 at 08:55:43PM -0800, David Miller wrote:
> From: Simon Horman <horms@verge.net.au>
> Date: Wed, 26 Dec 2007 11:59:05 +0900
> 
> > Recently the documentation in Documentation/nfsroot.txt was
> > update to note that in fact ip=off and ip=::::::off as the
> > latter is ignored and the default (on) is used.
> > 
> > This was certainly a step in the direction of reducing confusion.
> > But it seems to me that the code ought to be fixed up so that
> > ip=::::::off actually turns off ip autoconfiguration.
> > 
> > This patch also notes more specifically that ip=on (aka ip=::::::on)
> > is the default.
> > 
> > Cc: Amos Waterland <apw@us.ibm.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> I applied this to net-2.6 since I think it's a bug fix, and
> I also tailored the changelog header to more it clear this
> is a fix not just an "Update". :-)

Ok, thanks.

-- 
Horms


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] Fix regression in ip command line processing
  2007-12-26  2:59 [patch] ipv4: Update ip command line processing (take II) Simon Horman
  2007-12-26  4:55 ` David Miller
@ 2008-01-06  3:58 ` Amos Waterland
  2008-01-06  7:20   ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Amos Waterland @ 2008-01-06  3:58 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-kernel, netdev, David Miller, akpm

The recent changes for ip command line processing fixed some problems
but unfortunately broke some common usage scenarios.  In current
2.6.24-rc6 the following command line results in no IP address
assignment, which is surely a regression:

 ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:off

Please find below a patch that works for all cases I can find.

Signed-off-by: Amos Waterland <apw@us.ibm.com>

---

For convenience, here are the test cases phrased as qemu invocations:

ADDRESS ASSIGNED

qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=on"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=dhcp"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=both"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=any"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::on"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::dhcp"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:off"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:on"

ADDRESS NOT ASSIGNED

qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip="
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=off"
qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::off"

---

 Documentation/nfsroot.txt |    1 +
 net/ipv4/ipconfig.c       |   19 +++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/Documentation/nfsroot.txt b/Documentation/nfsroot.txt
index c86dd38..1a4d199 100644
--- a/Documentation/nfsroot.txt
+++ b/Documentation/nfsroot.txt
@@ -145,6 +145,7 @@ ip=<client-ip>:<server-ip>:<gw-ip>:<netmask>:<hostname>:<device>:<autoconf>
 		this option.
 
                   off or none: don't use autoconfiguration
+		      	       (do static IP assignment instead)
 		  on or any:   use any protocol available in the kernel
 			       (default)
 		  dhcp:        use DHCP
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 56a6757..8563b2e 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -1404,8 +1404,7 @@ static int __init ic_proto_name(char *name)
 		return 1;
 	}
 	if (!strcmp(name, "off") || !strcmp(name, "none")) {
-		ic_enable = 0;
-		return 1;
+		return 0;
 	}
 #ifdef CONFIG_IP_PNP_DHCP
 	else if (!strcmp(name, "dhcp")) {
@@ -1442,10 +1441,20 @@ static int __init ip_auto_config_setup(char *addrs)
 	ic_set_manually = 1;
 	ic_enable = 1;
 
+	/*
+	 * If any dhcp, bootp etc options are set, leave autoconfig on
+	 * and skip the below static IP processing.
+	 */
 	if (ic_proto_name(addrs))
 		return 1;
 
-	/* Parse the whole string */
+	/* If no static IP is given, turn off autoconfig and bail.  */
+	if (*addrs == 0 || strcmp(addrs, "off") == 0 || strcmp(addrs, "none") == 0) {
+		ic_enable = 0;
+		return 1;
+	}
+
+	/* Parse string for static IP assignment.  */
 	ip = addrs;
 	while (ip && *ip) {
 		if ((cp = strchr(ip, ':')))
@@ -1483,7 +1492,9 @@ static int __init ip_auto_config_setup(char *addrs)
 				strlcpy(user_dev_name, ip, sizeof(user_dev_name));
 				break;
 			case 6:
-				ic_proto_name(ip);
+				if (ic_proto_name(ip) == 0 && ic_myaddr == NONE) {
+					ic_enable = 0;
+				}
 				break;
 			}
 		}

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix regression in ip command line processing
  2008-01-06  3:58 ` [PATCH] Fix regression in ip command line processing Amos Waterland
@ 2008-01-06  7:20   ` David Miller
  2008-01-06  9:25     ` Simon Horman
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2008-01-06  7:20 UTC (permalink / raw)
  To: apw; +Cc: horms, linux-kernel, netdev, akpm

From: Amos Waterland <apw@us.ibm.com>
Date: Sat, 5 Jan 2008 22:58:16 -0500

> ADDRESS ASSIGNED
> 
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=on"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=dhcp"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=both"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=any"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::on"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::dhcp"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:off"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:on"
> 
> ADDRESS NOT ASSIGNED
> 
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip="
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=off"
> qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::off"

I really wish Simon had tested his original patch as extensively
as you have, this is the second major regression it has caused.
:-/

I'll apply your fix, although I'll break up some super long lines
you've added, thanks a lot.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix regression in ip command line processing
  2008-01-06  7:20   ` David Miller
@ 2008-01-06  9:25     ` Simon Horman
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2008-01-06  9:25 UTC (permalink / raw)
  To: David Miller; +Cc: apw, linux-kernel, netdev, akpm

On Sat, Jan 05, 2008 at 11:20:33PM -0800, David Miller wrote:
> From: Amos Waterland <apw@us.ibm.com>
> Date: Sat, 5 Jan 2008 22:58:16 -0500
> 
> > ADDRESS ASSIGNED
> > 
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=on"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=dhcp"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=both"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=any"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::on"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::dhcp"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:off"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=10.0.2.15::10.0.2.2:255.255.255.0::eth0:on"
> > 
> > ADDRESS NOT ASSIGNED
> > 
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip="
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=off"
> > qemu -kernel x86/arch/i386/boot/bzImage /dev/zero -append "ip=::::::off"
> 
> I really wish Simon had tested his original patch as extensively
> as you have, this is the second major regression it has caused.
> :-/

Ok, this time around its an issue with my understainding of how things
are supposed to work. But sorry none the less.

-- 
Horms


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-01-06  9:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-26  2:59 [patch] ipv4: Update ip command line processing (take II) Simon Horman
2007-12-26  4:55 ` David Miller
2007-12-26  5:04   ` Simon Horman
2008-01-06  3:58 ` [PATCH] Fix regression in ip command line processing Amos Waterland
2008-01-06  7:20   ` David Miller
2008-01-06  9:25     ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).