[PATCH] busybox: udhcpc: fix IPv6 support when using udhcpc

[PATCH] busybox: udhcpc: fix IPv6 support when using udhcpc

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

The udhcpc script calls ip addr flush .. which flushes addresses
of any address family, including IPv6. However, busybox udhcpc is
IPv4 only and should not influence IPv6 addressing. Hence use ip
addr flush with family constrait.

The script particularly broke IPv6 SLAAC: Typically when udhcpc
calls the script the kernel already assigned the IPv6 link-local
address. The flush removes the link-local IPv6 address again and
prohibits proper IPv6 operation such as SLAAC since neighbor
discovery protocol relies on IPv6 link-local addressing.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
---
 meta/recipes-core/busybox/files/simple.script | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-core/busybox/files/simple.script b/meta/recipes-core/busybox/files/simple.script
index 6ed0293525..8b5eb53633 100644
--- a/meta/recipes-core/busybox/files/simple.script
+++ b/meta/recipes-core/busybox/files/simple.script
@@ -28,7 +28,7 @@ case "$1" in
 		fi
 		if ! root_is_nfs ; then
                         if [ $have_bin_ip -eq 1 ]; then
-                                /SBIN_DIR/ip addr flush dev $interface
+                                /SBIN_DIR/ip -4 addr flush dev $interface
                                 /SBIN_DIR/ip link set dev $interface up
                         else
                                 /SBIN_DIR/ifconfig $interface 0.0.0.0
-- 
2.13.6

Re: [PATCH] busybox: udhcpc: fix IPv6 support when using udhcpc

[Stefan Agner]

Hi,

Any comment on this patch? Just stumbled upon that problem today again
:-)

--
Stefan

On 14.05.2018 16:44, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> The udhcpc script calls ip addr flush .. which flushes addresses
> of any address family, including IPv6. However, busybox udhcpc is
> IPv4 only and should not influence IPv6 addressing. Hence use ip
> addr flush with family constrait.
> 
> The script particularly broke IPv6 SLAAC: Typically when udhcpc
> calls the script the kernel already assigned the IPv6 link-local
> address. The flush removes the link-local IPv6 address again and
> prohibits proper IPv6 operation such as SLAAC since neighbor
> discovery protocol relies on IPv6 link-local addressing.

> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
>  meta/recipes-core/busybox/files/simple.script | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-core/busybox/files/simple.script
> b/meta/recipes-core/busybox/files/simple.script
> index 6ed0293525..8b5eb53633 100644
> --- a/meta/recipes-core/busybox/files/simple.script
> +++ b/meta/recipes-core/busybox/files/simple.script
> @@ -28,7 +28,7 @@ case "$1" in
>  		fi
>  		if ! root_is_nfs ; then
>                          if [ $have_bin_ip -eq 1 ]; then
> -                                /SBIN_DIR/ip addr flush dev $interface
> +                                /SBIN_DIR/ip -4 addr flush dev $interface
>                                  /SBIN_DIR/ip link set dev $interface up
>                          else
>                                  /SBIN_DIR/ifconfig $interface 0.0.0.0
> -- 
> 2.13.6

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Quentin Schulz]

Hi all,

On Mon, May 14, 2018 at 04:44:15PM +0200, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> The udhcpc script calls ip addr flush .. which flushes addresses
> of any address family, including IPv6. However, busybox udhcpc is
> IPv4 only and should not influence IPv6 addressing. Hence use ip
> addr flush with family constrait.
> 
> The script particularly broke IPv6 SLAAC: Typically when udhcpc
> calls the script the kernel already assigned the IPv6 link-local
> address. The flush removes the link-local IPv6 address again and
> prohibits proper IPv6 operation such as SLAAC since neighbor
> discovery protocol relies on IPv6 link-local addressing.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
>  meta/recipes-core/busybox/files/simple.script | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-core/busybox/files/simple.script b/meta/recipes-core/busybox/files/simple.script
> index 6ed0293525..8b5eb53633 100644
> --- a/meta/recipes-core/busybox/files/simple.script
> +++ b/meta/recipes-core/busybox/files/simple.script
> @@ -28,7 +28,7 @@ case "$1" in
>  		fi
>  		if ! root_is_nfs ; then
>                          if [ $have_bin_ip -eq 1 ]; then
> -                                /SBIN_DIR/ip addr flush dev $interface
> +                                /SBIN_DIR/ip -4 addr flush dev $interface
>                                  /SBIN_DIR/ip link set dev $interface up
>                          else
>                                  /SBIN_DIR/ifconfig $interface 0.0.0.0

Kindly pinging, happened to us as well many times.

OT: Didn't have the original mail, so hopefully getting the mbox file
and answering to it didn't trigger anything weird somewhere :)

Thanks,
Quentin

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Quentin Schulz]

Hi all,

On Mon, Jan 13, 2020 at 03:57:31PM +0100, Quentin Schulz wrote:
> Hi all,
> 
> On Mon, May 14, 2018 at 04:44:15PM +0200, Stefan Agner wrote:
> > From: Stefan Agner <stefan.agner@toradex.com>
> > 
> > The udhcpc script calls ip addr flush .. which flushes addresses
> > of any address family, including IPv6. However, busybox udhcpc is
> > IPv4 only and should not influence IPv6 addressing. Hence use ip
> > addr flush with family constrait.
> > 
> > The script particularly broke IPv6 SLAAC: Typically when udhcpc
> > calls the script the kernel already assigned the IPv6 link-local
> > address. The flush removes the link-local IPv6 address again and
> > prohibits proper IPv6 operation such as SLAAC since neighbor
> > discovery protocol relies on IPv6 link-local addressing.
> > 
> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> > ---
> >  meta/recipes-core/busybox/files/simple.script | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/recipes-core/busybox/files/simple.script b/meta/recipes-core/busybox/files/simple.script
> > index 6ed0293525..8b5eb53633 100644
> > --- a/meta/recipes-core/busybox/files/simple.script
> > +++ b/meta/recipes-core/busybox/files/simple.script
> > @@ -28,7 +28,7 @@ case "$1" in
> >  		fi
> >  		if ! root_is_nfs ; then
> >                          if [ $have_bin_ip -eq 1 ]; then
> > -                                /SBIN_DIR/ip addr flush dev $interface
> > +                                /SBIN_DIR/ip -4 addr flush dev $interface
> >                                  /SBIN_DIR/ip link set dev $interface up
> >                          else
> >                                  /SBIN_DIR/ifconfig $interface 0.0.0.0
> 
> Kindly pinging, happened to us as well many times.
> 

Kindly pinging.

Quentin

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Stefan Agner]

On 2020-01-20 13:32, Quentin Schulz wrote:
> Hi all,
> 
> On Mon, Jan 13, 2020 at 03:57:31PM +0100, Quentin Schulz wrote:
>> Hi all,
>>
>> On Mon, May 14, 2018 at 04:44:15PM +0200, Stefan Agner wrote:
>> > From: Stefan Agner <stefan.agner@toradex.com>
>> >
>> > The udhcpc script calls ip addr flush .. which flushes addresses
>> > of any address family, including IPv6. However, busybox udhcpc is
>> > IPv4 only and should not influence IPv6 addressing. Hence use ip
>> > addr flush with family constrait.
>> >
>> > The script particularly broke IPv6 SLAAC: Typically when udhcpc
>> > calls the script the kernel already assigned the IPv6 link-local
>> > address. The flush removes the link-local IPv6 address again and
>> > prohibits proper IPv6 operation such as SLAAC since neighbor
>> > discovery protocol relies on IPv6 link-local addressing.
>> >
>> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> > ---
>> >  meta/recipes-core/busybox/files/simple.script | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/meta/recipes-core/busybox/files/simple.script b/meta/recipes-core/busybox/files/simple.script
>> > index 6ed0293525..8b5eb53633 100644
>> > --- a/meta/recipes-core/busybox/files/simple.script
>> > +++ b/meta/recipes-core/busybox/files/simple.script
>> > @@ -28,7 +28,7 @@ case "$1" in
>> >  		fi
>> >  		if ! root_is_nfs ; then
>> >                          if [ $have_bin_ip -eq 1 ]; then
>> > -                                /SBIN_DIR/ip addr flush dev $interface
>> > +                                /SBIN_DIR/ip -4 addr flush dev $interface
>> >                                  /SBIN_DIR/ip link set dev $interface up
>> >                          else
>> >                                  /SBIN_DIR/ifconfig $interface 0.0.0.0
>>
>> Kindly pinging, happened to us as well many times.
>>
> 
> Kindly pinging.

Just checked, we still override that script in our layer, so definitely
would be happy if this gets merged upstream so I can get rid of our
custom script downstream.

--
Stefan

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Khem Raj]

On 1/20/20 4:57 AM, Stefan Agner wrote:
> On 2020-01-20 13:32, Quentin Schulz wrote:
>> Hi all,
>>
>> On Mon, Jan 13, 2020 at 03:57:31PM +0100, Quentin Schulz wrote:
>>> Hi all,
>>>
>>> On Mon, May 14, 2018 at 04:44:15PM +0200, Stefan Agner wrote:
>>>> From: Stefan Agner <stefan.agner@toradex.com>
>>>>
>>>> The udhcpc script calls ip addr flush .. which flushes addresses
>>>> of any address family, including IPv6. However, busybox udhcpc is
>>>> IPv4 only and should not influence IPv6 addressing. Hence use ip
>>>> addr flush with family constrait.
>>>>
>>>> The script particularly broke IPv6 SLAAC: Typically when udhcpc
>>>> calls the script the kernel already assigned the IPv6 link-local
>>>> address. The flush removes the link-local IPv6 address again and
>>>> prohibits proper IPv6 operation such as SLAAC since neighbor
>>>> discovery protocol relies on IPv6 link-local addressing.
>>>>
>>>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>>>> ---
>>>>   meta/recipes-core/busybox/files/simple.script | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/meta/recipes-core/busybox/files/simple.script b/meta/recipes-core/busybox/files/simple.script
>>>> index 6ed0293525..8b5eb53633 100644
>>>> --- a/meta/recipes-core/busybox/files/simple.script
>>>> +++ b/meta/recipes-core/busybox/files/simple.script
>>>> @@ -28,7 +28,7 @@ case "$1" in
>>>>   		fi
>>>>   		if ! root_is_nfs ; then
>>>>                           if [ $have_bin_ip -eq 1 ]; then
>>>> -                                /SBIN_DIR/ip addr flush dev $interface
>>>> +                                /SBIN_DIR/ip -4 addr flush dev $interface
>>>>                                   /SBIN_DIR/ip link set dev $interface up
>>>>                           else
>>>>                                   /SBIN_DIR/ifconfig $interface 0.0.0.0
>>>
>>> Kindly pinging, happened to us as well many times.
>>>
>>
>> Kindly pinging.
> 
> Just checked, we still override that script in our layer, so definitely
> would be happy if this gets merged upstream so I can get rid of our
> custom script downstream.
> 

I think this is good to go got OE-core, but I was wondering if default 
script in busybox also need this and perhaps upstream too

> --
> Stefan
> 

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Quentin Schulz]

Hi Khem,

On Mon, Jan 20, 2020 at 11:26:27AM -0800, Khem Raj wrote:
> > Just checked, we still override that script in our layer, so definitely
> > would be happy if this gets merged upstream so I can get rid of our
> > custom script downstream.
> > 
> 
> I think this is good to go got OE-core, but I was wondering if default
> script in busybox also need this and perhaps upstream too
> 

What do you mean by "default script"?
https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/busybox/files/default.script
?

Upstream could benefit from it, I'd agree. Though, it is technically
just provided as an example.
https://git.busybox.net/busybox/tree/examples/udhcp/simple.script

The reason why I didn't bother to send a patch to busybox before pinging
on this patch was that we're already different from the upstream simple.script
so it didn't make sense to me to add the Upstream-Status: pending or
something on the patch (in some ways, since it's patching the file
directly and not adding a patch in SRC_URI). Anyway, digressing. Do you
want a patch to be sent to busybox ML (or PR or whatever they use)
before taking this patch?

Quentin

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Khem Raj]

On Tue, Jan 21, 2020 at 1:23 AM Quentin Schulz
<quentin.schulz@streamunlimited.com> wrote:
>
> Hi Khem,
>
> On Mon, Jan 20, 2020 at 11:26:27AM -0800, Khem Raj wrote:
> > > Just checked, we still override that script in our layer, so definitely
> > > would be happy if this gets merged upstream so I can get rid of our
> > > custom script downstream.
> > >
> >
> > I think this is good to go got OE-core, but I was wondering if default
> > script in busybox also need this and perhaps upstream too
> >
>
> What do you mean by "default script"?
> https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/busybox/files/default.script
> ?

yes.
>
> Upstream could benefit from it, I'd agree. Though, it is technically
> just provided as an example.
> https://git.busybox.net/busybox/tree/examples/udhcp/simple.script
>

> The reason why I didn't bother to send a patch to busybox before pinging
> on this patch was that we're already different from the upstream simple.script
> so it didn't make sense to me to add the Upstream-Status: pending or
> something on the patch (in some ways, since it's patching the file
> directly and not adding a patch in SRC_URI). Anyway, digressing. Do you
> want a patch to be sent to busybox ML (or PR or whatever they use)
> before taking this patch?
>

I think the problem this patch fixes is generic and somewhere the
script OE has is also derived from
that example, so while the patch in itself is enough for OE, it would
be better if it was in upstream too
perhaps one less thing to worry about when we cherry pick changes from
upstream script in future.

> Quentin

Re: busybox: udhcpc: fix IPv6 support when using udhcpc

[Quentin Schulz]

Hi Khem,

On Tue, Jan 21, 2020 at 10:08:59AM -0800, Khem Raj wrote:
> On Tue, Jan 21, 2020 at 1:23 AM Quentin Schulz
> <quentin.schulz@streamunlimited.com> wrote:
> >
> > Hi Khem,
> >
> > On Mon, Jan 20, 2020 at 11:26:27AM -0800, Khem Raj wrote:
> > > > Just checked, we still override that script in our layer, so definitely
> > > > would be happy if this gets merged upstream so I can get rid of our
> > > > custom script downstream.
> > > >
> > >
> > > I think this is good to go got OE-core, but I was wondering if default
> > > script in busybox also need this and perhaps upstream too
> > >
> >
> > What do you mean by "default script"?
> > https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/busybox/files/default.script
> > ?
> 
> yes.

I'm confused then. Bear with me, I'm new to userspace so I'm sure I'm missing
something rather obvious.
The content of default.script is:

exec run-parts -a "$1" /etc/udhcpc.d

AFAICT from a quick look at busybox.inc, the only things in that
/etc/udhcpc.d directory are simple.script (named 50default actually) and
default.script.

What do you think should be done for that file wrt the original issue?

> >
> > Upstream could benefit from it, I'd agree. Though, it is technically
> > just provided as an example.
> > https://git.busybox.net/busybox/tree/examples/udhcp/simple.script
> >
> 

@all: FYI, patch is merged:
https://git.openembedded.org/openembedded-core/commit/meta/recipes-core/busybox/files/simple.script?id=b77541dbb2f442e51842f9d24c8745a6df2d1478

> > The reason why I didn't bother to send a patch to busybox before pinging
> > on this patch was that we're already different from the upstream simple.script
> > so it didn't make sense to me to add the Upstream-Status: pending or
> > something on the patch (in some ways, since it's patching the file
> > directly and not adding a patch in SRC_URI). Anyway, digressing. Do you
> > want a patch to be sent to busybox ML (or PR or whatever they use)
> > before taking this patch?
> >
> 
> I think the problem this patch fixes is generic and somewhere the
> script OE has is also derived from
> that example, so while the patch in itself is enough for OE, it would
> be better if it was in upstream too
> perhaps one less thing to worry about when we cherry pick changes from
> upstream script in future.
> 

Agreed, I'll put it on my TODO-list. If I do not send an answer on this
mail with the link to the PR or patch in the next week, anyone, please ping me.

Thx,
Quentin

Re: [OE-core] busybox: udhcpc: fix IPv6 support when using udhcpc

[Quentin Schulz]

Hi all,

On Wed, Jan 22, 2020 at 11:06:55AM +0100, Quentin Schulz wrote:
> > > The reason why I didn't bother to send a patch to busybox before pinging
> > > on this patch was that we're already different from the upstream simple.script
> > > so it didn't make sense to me to add the Upstream-Status: pending or
> > > something on the patch (in some ways, since it's patching the file
> > > directly and not adding a patch in SRC_URI). Anyway, digressing. Do you
> > > want a patch to be sent to busybox ML (or PR or whatever they use)
> > > before taking this patch?
> > >
> > 
> > I think the problem this patch fixes is generic and somewhere the
> > script OE has is also derived from
> > that example, so while the patch in itself is enough for OE, it would
> > be better if it was in upstream too
> > perhaps one less thing to worry about when we cherry pick changes from
> > upstream script in future.
> > 
> 
> Agreed, I'll put it on my TODO-list. If I do not send an answer on this
> mail with the link to the PR or patch in the next week, anyone, please ping me.
> 

*cough* Took a bit more *cough* than a week *cough*

http://lists.busybox.net/pipermail/busybox/2020-June/088028.html

Quentin

Re: [OE-core] busybox: udhcpc: fix IPv6 support when using udhcpc

[Stefan Agner]

On 2020-06-16 11:50, Quentin Schulz wrote:
> Hi all,
> 
> On Wed, Jan 22, 2020 at 11:06:55AM +0100, Quentin Schulz wrote:
>> > > The reason why I didn't bother to send a patch to busybox before pinging
>> > > on this patch was that we're already different from the upstream simple.script
>> > > so it didn't make sense to me to add the Upstream-Status: pending or
>> > > something on the patch (in some ways, since it's patching the file
>> > > directly and not adding a patch in SRC_URI). Anyway, digressing. Do you
>> > > want a patch to be sent to busybox ML (or PR or whatever they use)
>> > > before taking this patch?
>> > >
>> >
>> > I think the problem this patch fixes is generic and somewhere the
>> > script OE has is also derived from
>> > that example, so while the patch in itself is enough for OE, it would
>> > be better if it was in upstream too
>> > perhaps one less thing to worry about when we cherry pick changes from
>> > upstream script in future.
>> >
>>
>> Agreed, I'll put it on my TODO-list. If I do not send an answer on this
>> mail with the link to the PR or patch in the next week, anyone, please ping me.
>>
> 
> *cough* Took a bit more *cough* than a week *cough*
> 
> http://lists.busybox.net/pipermail/busybox/2020-June/088028.html

Cool, thx for upstreaming!

--
Stefan