[RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Denys Dmytriyenko]

Signed-off-by: Denys Dmytriyenko <denis@denix.org>
---
 recipes/netbase/netbase/interfaces |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/recipes/netbase/netbase/interfaces b/recipes/netbase/netbase/interfaces
index c615642..bae03dd 100644
--- a/recipes/netbase/netbase/interfaces
+++ b/recipes/netbase/netbase/interfaces
@@ -52,6 +52,8 @@ iface atml0 inet dhcp
 # Wired or wireless interfaces
 auto eth0
 iface eth0 inet dhcp
+        pre-up /bin/grep -v -e "ip=[0-9]\+\.[0-9]\+\.[0-9]\+\.[0-9]\+" /proc/cmdline > /dev/null
+
 iface eth1 inet dhcp
 
 # Ethernet/RNDIS gadget (g_ether)
-- 
1.6.3.3

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Phil Blundell]

On Mon, 2009-09-21 at 12:07 -0400, Denys Dmytriyenko wrote:
>  iface eth0 inet dhcp
> +        pre-up /bin/grep -v -e "ip=[0-9]\+\.[0-9]\+\.[0-9]\+\.[0-9]\+" /proc/cmdline > /dev/null
> +

This is pretty gruesome.  It will introduce an extra two fork+exec's
worth of overhead to every "ifup eth0" for everybody, and it also fails
to check that the ip= parameter actually corresponded to eth0 (i.e. it
would stop eth0 coming up even if the parameter was specifying an
address for eth1).  

It seems like there must be a better way of solving this problem.  How
about just teaching "ifup -a" to spot interfaces that are already up and
leave them alone?

p.

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Koen Kooi]

On 21-09-09 18:07, Denys Dmytriyenko wrote:
>
> Signed-off-by: Denys Dmytriyenko<denis@denix.org>

Acked-by: Koen Kooi <koen@openembedded.org>


> ---
>   recipes/netbase/netbase/interfaces |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/recipes/netbase/netbase/interfaces b/recipes/netbase/netbase/interfaces
> index c615642..bae03dd 100644
> --- a/recipes/netbase/netbase/interfaces
> +++ b/recipes/netbase/netbase/interfaces
> @@ -52,6 +52,8 @@ iface atml0 inet dhcp
>   # Wired or wireless interfaces
>   auto eth0
>   iface eth0 inet dhcp
> +        pre-up /bin/grep -v -e "ip=[0-9]\+\.[0-9]\+\.[0-9]\+\.[0-9]\+" /proc/cmdline>  /dev/null
> +
>   iface eth1 inet dhcp
>
>   # Ethernet/RNDIS gadget (g_ether)

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Denys Dmytriyenko]

On Mon, Sep 21, 2009 at 08:45:06PM +0100, Phil Blundell wrote:
> On Mon, 2009-09-21 at 12:07 -0400, Denys Dmytriyenko wrote:
> >  iface eth0 inet dhcp
> > +        pre-up /bin/grep -v -e "ip=[0-9]\+\.[0-9]\+\.[0-9]\+\.[0-9]\+" /proc/cmdline > /dev/null
> > +
> 
> This is pretty gruesome.  It will introduce an extra two fork+exec's
> worth of overhead to every "ifup eth0" for everybody, and it also fails
> to check that the ip= parameter actually corresponded to eth0 (i.e. it
> would stop eth0 coming up even if the parameter was specifying an
> address for eth1).  

I would agree it's a quick and dirty fix, that's why it's RFC :)

As in my case we only have eth0, I didn't bother touching eth1 or any other 
interface.

Also, while the proper and complete format for the ip= parameter is:
ip=<ipaddr>:<serverip>:<gatewayip>:<netmask>:<hostname>:<iface>:<state>

Many users just specify the IP address on the command line:
ip=<ipaddr>

In this case the IP would be assigned to the first available interface, so it 
is harder to do it from the /etc/network/interfaces file using pre-up...

> It seems like there must be a better way of solving this problem.  How
> about just teaching "ifup -a" to spot interfaces that are already up and
> leave them alone?

Won't work for the case of kernel-acquired DHCP address, i.e. kernel level 
autoconfig, aka IP_PNP, aka ip=dhcp command line.

From ifup perspective the interface is up and IP is assigned, but we need to 
start udhcpc to re-acquire and keep the lease, as kernel can't do this.

Speaking of which, I am working on a fix for udhcpc to also properly handle 
this case - it needs to be started with -r <ip> to request the same IP that 
kernel received, instead of asking for any available... Some DHCP servers are 
stupid and give a different IP, killing NFS/TCP mounted rootfs...


So, the discussion is still open - please send your comments/suggestions. 
Thanks.

-- 
Denys

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Stanislav Brabec]

Denys Dmytriyenko wrote:
> On Mon, Sep 21, 2009 at 08:45:06PM +0100, Phil Blundell wrote:

> > It seems like there must be a better way of solving this problem.  How
> > about just teaching "ifup -a" to spot interfaces that are already up and
> > leave them alone?
> 
> Won't work for the case of kernel-acquired DHCP address, i.e. kernel level 
> autoconfig, aka IP_PNP, aka ip=dhcp command line.

It was done in past, but removed. With one of new wpa_supplicant
versions, wireless stopped to work. wpa_supplicant turns up any wireless
interface immediately. (Well, WPA wireless still needs debugging. It
works with about 50% probability.)

What about parsing of kernel parameters during the boot and then doing
something.

-- 
Stanislav Brabec
http://www.penguin.cz/~utx/zaurus

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Phil Blundell]

On Mon, 2009-09-21 at 18:39 -0400, Denys Dmytriyenko wrote:
> On Mon, Sep 21, 2009 at 08:45:06PM +0100, Phil Blundell wrote:
> > It seems like there must be a better way of solving this problem.  How
> > about just teaching "ifup -a" to spot interfaces that are already up and
> > leave them alone?
> 
> Won't work for the case of kernel-acquired DHCP address, i.e. kernel level 
> autoconfig, aka IP_PNP, aka ip=dhcp command line.

True, but your original patch won't help with this situation either
(since "ip=dhcp" won't match the regex).  I think this is a different
problem and probably requires a different solution: the ideal thing
would be for the kernel to set a flag on the interface to say that it
needs to be taken over by a DHCP client, or alternatively to invoke the
DHCP client itself via the hotplug mechanism.  Failing that you could
arrange for the startup scripts to poke around at /proc/cmdline and try
to second-guess what the kernel has done, although that would be rather
less satisfactory.

p.

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Denys Dmytriyenko]

On Tue, Sep 22, 2009 at 02:28:32PM +0100, Phil Blundell wrote:
> On Mon, 2009-09-21 at 18:39 -0400, Denys Dmytriyenko wrote:
> > On Mon, Sep 21, 2009 at 08:45:06PM +0100, Phil Blundell wrote:
> > > It seems like there must be a better way of solving this problem.  How
> > > about just teaching "ifup -a" to spot interfaces that are already up and
> > > leave them alone?
> > 
> > Won't work for the case of kernel-acquired DHCP address, i.e. kernel level 
> > autoconfig, aka IP_PNP, aka ip=dhcp command line.
> 
> True, but your original patch won't help with this situation either
> (since "ip=dhcp" won't match the regex).  I think this is a different

Not true. The default is to start udhcpc always. I just cover one case to 
prevent it from starting when ip=x.x.x.x

> problem and probably requires a different solution: the ideal thing
> would be for the kernel to set a flag on the interface to say that it
> needs to be taken over by a DHCP client, or alternatively to invoke the
> DHCP client itself via the hotplug mechanism.  Failing that you could
> arrange for the startup scripts to poke around at /proc/cmdline and try
> to second-guess what the kernel has done, although that would be rather
> less satisfactory.

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Phil Blundell]

On Tue, 2009-09-22 at 12:42 -0400, Denys Dmytriyenko wrote:
> On Tue, Sep 22, 2009 at 02:28:32PM +0100, Phil Blundell wrote:
> > On Mon, 2009-09-21 at 18:39 -0400, Denys Dmytriyenko wrote:
> > > On Mon, Sep 21, 2009 at 08:45:06PM +0100, Phil Blundell wrote:
> > > > It seems like there must be a better way of solving this problem.  How
> > > > about just teaching "ifup -a" to spot interfaces that are already up and
> > > > leave them alone?
> > > 
> > > Won't work for the case of kernel-acquired DHCP address, i.e. kernel level 
> > > autoconfig, aka IP_PNP, aka ip=dhcp command line.
> > 
> > True, but your original patch won't help with this situation either
> > (since "ip=dhcp" won't match the regex).  I think this is a different
> 
> Not true. The default is to start udhcpc always. I just cover one case to 
> prevent it from starting when ip=x.x.x.x

Right, but as you said in your other mail, in the case where the kernel
has already claimed a dhcp lease you want to start udhcpc with different
arguments to the usual ones.  So, although your original patch
admittedly doesn't make that situation any worse, it also doesn't make
it much better either.

I still think that this:

> > problem and probably requires a different solution: the ideal thing
> > would be for the kernel to set a flag on the interface to say that it
> > needs to be taken over by a DHCP client, or alternatively to invoke the
> > DHCP client itself via the hotplug mechanism.  Failing that you could
> > arrange for the startup scripts to poke around at /proc/cmdline and try
> > to second-guess what the kernel has done, although that would be rather
> > less satisfactory.

together with the thing I mentioned earlier, is the right way to solve
this kind of issue in general.  

Poking at /proc/cmdline from the pre-up command has another downside as
well, namely that if you bring the interface down it will then be
impossible to get it back up again because the command line will still
have the "ip=" bits in it even though they are no longer relevant.

p.

Re: [RFC][PATCH] netbase: don't start udhcpc if kernel assigned IP statically

[Denys Dmytriyenko]

On Tue, Sep 22, 2009 at 07:00:12PM +0100, Phil Blundell wrote:
> On Tue, 2009-09-22 at 12:42 -0400, Denys Dmytriyenko wrote:
> > On Tue, Sep 22, 2009 at 02:28:32PM +0100, Phil Blundell wrote:
> > > On Mon, 2009-09-21 at 18:39 -0400, Denys Dmytriyenko wrote:
> > > > On Mon, Sep 21, 2009 at 08:45:06PM +0100, Phil Blundell wrote:
> > > > > It seems like there must be a better way of solving this problem.  How
> > > > > about just teaching "ifup -a" to spot interfaces that are already up and
> > > > > leave them alone?
> > > > 
> > > > Won't work for the case of kernel-acquired DHCP address, i.e. kernel level 
> > > > autoconfig, aka IP_PNP, aka ip=dhcp command line.
> > > 
> > > True, but your original patch won't help with this situation either
> > > (since "ip=dhcp" won't match the regex).  I think this is a different
> > 
> > Not true. The default is to start udhcpc always. I just cover one case to 
> > prevent it from starting when ip=x.x.x.x
> 
> Right, but as you said in your other mail, in the case where the kernel
> has already claimed a dhcp lease you want to start udhcpc with different
> arguments to the usual ones.  So, although your original patch
> admittedly doesn't make that situation any worse, it also doesn't make
> it much better either.

Correct. As I was thinking on adding the check for kernel acquired DHCP lease 
somewhere inside the udhcpc code or script, it appears to be quite independent 
from this patch to pre-up...

> I still think that this:
> 
> > > problem and probably requires a different solution: the ideal thing
> > > would be for the kernel to set a flag on the interface to say that it
> > > needs to be taken over by a DHCP client, or alternatively to invoke the
> > > DHCP client itself via the hotplug mechanism.  Failing that you could
> > > arrange for the startup scripts to poke around at /proc/cmdline and try
> > > to second-guess what the kernel has done, although that would be rather
> > > less satisfactory.
> 
> together with the thing I mentioned earlier, is the right way to solve
> this kind of issue in general.  
> 
> Poking at /proc/cmdline from the pre-up command has another downside as
> well, namely that if you bring the interface down it will then be
> impossible to get it back up again because the command line will still
> have the "ip=" bits in it even though they are no longer relevant.

Fair enough, I will try to figure out a better way of fixing it for all the 
possible cases... Thanks.

-- 
Denys