[patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Jiri Pirko]

This is needed in order to implement userspace address configuration,
namely ip6-privacy (rfc4941) in NetworkManager.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/ipv6/addrconf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd3fb30..962c7c9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
 		return -ENODEV;
 
 	/* We ignore other flags so far. */
-	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
+	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
+				      IFA_F_TEMPORARY);
 
 	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
 	if (ifa == NULL) {
-- 
1.8.3.1

[patch iproute2] allow to create temporary addresses

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 ip/ipaddress.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1c3e4da..3ca774d 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1332,6 +1332,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			req.ifa.ifa_flags |= IFA_F_HOMEADDRESS;
 		} else if (strcmp(*argv, "nodad") == 0) {
 			req.ifa.ifa_flags |= IFA_F_NODAD;
+		} else if (strcmp(*argv, "temporary") == 0 ||
+			   strcmp(*argv, "secondary") == 0) {
+			req.ifa.ifa_flags |= IFA_F_SECONDARY;
 		} else {
 			if (strcmp(*argv, "local") == 0) {
 				NEXT_ARG();
-- 
1.8.3.1

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Thu, Oct 24, 2013 at 03:45:55PM +0200, Jiri Pirko wrote:
> This is needed in order to implement userspace address configuration,
> namely ip6-privacy (rfc4941) in NetworkManager.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/ipv6/addrconf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index cd3fb30..962c7c9 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
>  		return -ENODEV;
>  
>  	/* We ignore other flags so far. */
> -	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
> +	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
> +				      IFA_F_TEMPORARY);
>  
>  	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
>  	if (ifa == NULL) {

Hm, the kernel will pick up IFA_F_TEMPORARY marked addresses and do ipv6 address
regeneration (depending on lifetimes). Is this intended?

Btw. I am very interested in this topic as there is currently work in the IETF
to move away from eui-48 generation of addresses:

https://datatracker.ietf.org/doc/draft-ietf-6man-stable-privacy-addresses/

This needs to be done in userspace as we depend on a secret generated at
system install time.

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Jiri Pirko]

Thu, Oct 24, 2013 at 04:02:53PM CEST, hannes@stressinduktion.org wrote:
>On Thu, Oct 24, 2013 at 03:45:55PM +0200, Jiri Pirko wrote:
>> This is needed in order to implement userspace address configuration,
>> namely ip6-privacy (rfc4941) in NetworkManager.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  net/ipv6/addrconf.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index cd3fb30..962c7c9 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
>>  		return -ENODEV;
>>  
>>  	/* We ignore other flags so far. */
>> -	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
>> +	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
>> +				      IFA_F_TEMPORARY);
>>  
>>  	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
>>  	if (ifa == NULL) {
>
>Hm, the kernel will pick up IFA_F_TEMPORARY marked addresses and do ipv6 address
>regeneration (depending on lifetimes). Is this intended?

I think that that behaviour is valid. It is in compliance with valid lft and
preferred lft behaviour.

>
>Btw. I am very interested in this topic as there is currently work in the IETF
>to move away from eui-48 generation of addresses:
>
>https://datatracker.ietf.org/doc/draft-ietf-6man-stable-privacy-addresses/
>
>This needs to be done in userspace as we depend on a secret generated at
>system install time.
>
>Greetings,
>
>  Hannes
>

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Vladislav Yasevich]

On Thu, Oct 24, 2013 at 12:59 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Oct 24, 2013 at 04:02:53PM CEST, hannes@stressinduktion.org wrote:
>>On Thu, Oct 24, 2013 at 03:45:55PM +0200, Jiri Pirko wrote:
>>> This is needed in order to implement userspace address configuration,
>>> namely ip6-privacy (rfc4941) in NetworkManager.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> ---
>>>  net/ipv6/addrconf.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index cd3fb30..962c7c9 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
>>>              return -ENODEV;
>>>
>>>      /* We ignore other flags so far. */
>>> -    ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
>>> +    ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
>>> +                                  IFA_F_TEMPORARY);
>>>
>>>      ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
>>>      if (ifa == NULL) {
>>
>>Hm, the kernel will pick up IFA_F_TEMPORARY marked addresses and do ipv6 address
>>regeneration (depending on lifetimes). Is this intended?
>
> I think that that behaviour is valid. It is in compliance with valid lft and
> preferred lft behaviour.
>

Actually, I don't think it would do the regeneration since ifpub
pointer is not set.

I appears that the temp address management would have to be done in
userspace.

-vlad


>>
>>Btw. I am very interested in this topic as there is currently work in the IETF
>>to move away from eui-48 generation of addresses:
>>
>>https://datatracker.ietf.org/doc/draft-ietf-6man-stable-privacy-addresses/
>>
>>This needs to be done in userspace as we depend on a secret generated at
>>system install time.
>>
>>Greetings,
>>
>>  Hannes
>>
> --
> 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 net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Fri, Oct 25, 2013 at 06:05:40AM -0400, Vladislav Yasevich wrote:
> On Thu, Oct 24, 2013 at 12:59 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> > Thu, Oct 24, 2013 at 04:02:53PM CEST, hannes@stressinduktion.org wrote:
> >>On Thu, Oct 24, 2013 at 03:45:55PM +0200, Jiri Pirko wrote:
> >>> This is needed in order to implement userspace address configuration,
> >>> namely ip6-privacy (rfc4941) in NetworkManager.
> >>>
> >>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >>> ---
> >>>  net/ipv6/addrconf.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> >>> index cd3fb30..962c7c9 100644
> >>> --- a/net/ipv6/addrconf.c
> >>> +++ b/net/ipv6/addrconf.c
> >>> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
> >>>              return -ENODEV;
> >>>
> >>>      /* We ignore other flags so far. */
> >>> -    ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
> >>> +    ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
> >>> +                                  IFA_F_TEMPORARY);
> >>>
> >>>      ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
> >>>      if (ifa == NULL) {
> >>
> >>Hm, the kernel will pick up IFA_F_TEMPORARY marked addresses and do ipv6 address
> >>regeneration (depending on lifetimes). Is this intended?
> >
> > I think that that behaviour is valid. It is in compliance with valid lft and
> > preferred lft behaviour.
> >
> 
> Actually, I don't think it would do the regeneration since ifpub
> pointer is not set.
> 
> I appears that the temp address management would have to be done in
> userspace.

Sorry, yes this is correct. I should have looked at the source and not answer
just from memory.

Thanks,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Vladislav Yasevich]

On Thu, Oct 24, 2013 at 9:45 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> This is needed in order to implement userspace address configuration,
> namely ip6-privacy (rfc4941) in NetworkManager.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/ipv6/addrconf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index cd3fb30..962c7c9 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
>                 return -ENODEV;
>
>         /* We ignore other flags so far. */
> -       ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
> +       ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
> +                                     IFA_F_TEMPORARY);
>
>         ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
>         if (ifa == NULL) {
> --
> 1.8.3.1
>

Jiri

So, is the idea behind this is that all of temp address management
would be done in user space?  If so, then you
may have to verify that no-one sets the lifetime values on the prefix
in your other patch.   I am still trying to figure out
why this would be needed.

Thanks
-vlad

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Jiri Pirko]

Sat, Oct 26, 2013 at 01:05:34AM CEST, vyasevich@gmail.com wrote:
>On Thu, Oct 24, 2013 at 9:45 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> This is needed in order to implement userspace address configuration,
>> namely ip6-privacy (rfc4941) in NetworkManager.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  net/ipv6/addrconf.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index cd3fb30..962c7c9 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
>>                 return -ENODEV;
>>
>>         /* We ignore other flags so far. */
>> -       ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
>> +       ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
>> +                                     IFA_F_TEMPORARY);
>>
>>         ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
>>         if (ifa == NULL) {
>> --
>> 1.8.3.1
>>
>
>Jiri
>
>So, is the idea behind this is that all of temp address management
>would be done in user space?  If so, then you
>may have to verify that no-one sets the lifetime values on the prefix
>in your other patch.   I am still trying to figure out
>why this would be needed.

The idea is to provide possibility to do address configuration not in
kernel but rather in userspace (as it is done for example in NetworkManager)

Maybe I'm missing something, but why is it problem to have the
possibility to set lifetime even for temporary prefix?

>
>Thanks
>-vlad

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

Hi Jiri!

On Sun, Oct 27, 2013 at 02:29:41PM +0100, Jiri Pirko wrote:
> The idea is to provide possibility to do address configuration not in
> kernel but rather in userspace (as it is done for example in NetworkManager)
> 
> Maybe I'm missing something, but why is it problem to have the
> possibility to set lifetime even for temporary prefix?

There is no problem setting the lifetime for a temporary prefix (in
contrary, it needs one) but the code paths designed for IFA_F_TEMPORARY
may not fiddle with it. This needs to be checked.

In this constellation addrconf_verify does not refresh the privacy
address when its preferred lifetime is expired, if you create the
address by only passing IFA_F_TEMPORARY to rtm_newaddr (as Vlad pointed
out). E.g. NetworkManager has to take care about that, then.

A temporary address is also bound to a non-privacy public address so
it's lifetime is determined by its lifetime (e.g. if you switch the
network and don't receive on-link information for that prefix any
more). NetworkManager would have to take care about that, too. It is
just a question of what NetworkManager wants to handle itself or lets
the kernel handle for it.

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Vladislav Yasevich]

On Sun, Oct 27, 2013 at 12:48 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Jiri!
>
> On Sun, Oct 27, 2013 at 02:29:41PM +0100, Jiri Pirko wrote:
>> The idea is to provide possibility to do address configuration not in
>> kernel but rather in userspace (as it is done for example in NetworkManager)
>>
>> Maybe I'm missing something, but why is it problem to have the
>> possibility to set lifetime even for temporary prefix?
>
> There is no problem setting the lifetime for a temporary prefix (in
> contrary, it needs one) but the code paths designed for IFA_F_TEMPORARY
> may not fiddle with it. This needs to be checked.
>
> In this constellation addrconf_verify does not refresh the privacy
> address when its preferred lifetime is expired, if you create the
> address by only passing IFA_F_TEMPORARY to rtm_newaddr (as Vlad pointed
> out). E.g. NetworkManager has to take care about that, then.
>
> A temporary address is also bound to a non-privacy public address so
> it's lifetime is determined by its lifetime (e.g. if you switch the
> network and don't receive on-link information for that prefix any
> more). NetworkManager would have to take care about that, too. It is
> just a question of what NetworkManager wants to handle itself or lets
> the kernel handle for it.
>


Exactly.  If we want to re-use the as much of the kernel implementation as
possible, it might be nice to also add a regen notification in case there
is no public address.  This way, Network Manger (or another application)
can do the re-generation if need be for these user configured temporary
addresses.

-vlad


> Greetings,
>
>   Hannes
>

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Sun, 27 Oct 2013 17:48:35 +0100

> A temporary address is also bound to a non-privacy public address so
> it's lifetime is determined by its lifetime (e.g. if you switch the
> network and don't receive on-link information for that prefix any
> more). NetworkManager would have to take care about that, too. It is
> just a question of what NetworkManager wants to handle itself or lets
> the kernel handle for it.

How much really needs to be in userspace to implement RFC4941?

I don't like the idea that even for a fully up and properly
functioning link, if NetworkManager wedges then critical things like
temporary address (re-)generation, will cease.

All that seems to matter is the secret used to generate the temporary
address sequence, and perhaps things like site specific lifetime
parameters.  These are things userland can send to the kernel in
netlink messages.

Full disclosure that I am saying this from the perspective of someone
who believes that one of the biggest mistakes ever was allowing the
core of DHCP to be done in userspace.

Right now every ipv4 DHCP user ends up with their interface in
promiscuous mode.  The DHCP client implementations are huge non-
trivial bodies of code, and getting them to adopt the changes necessary
to not put the interface in promiscuous mode is harder than pulling
teeth.

If at least the DHCP protocol communications part were in the kernel,
on the other hand, we could remove the problem quite swiftly.

This problem has existed for more than a decade, btw.  There simply
exists no will to fix it properly, even after all this time, because
breaking the ice on those DHCP client code bases in userbase is hard.

So I want to see less fundamental stuff about interface configuration
and address assignment in userland, not more.

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Dan Williams]

On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Sun, 27 Oct 2013 17:48:35 +0100
> 
> > A temporary address is also bound to a non-privacy public address so
> > it's lifetime is determined by its lifetime (e.g. if you switch the
> > network and don't receive on-link information for that prefix any
> > more). NetworkManager would have to take care about that, too. It is
> > just a question of what NetworkManager wants to handle itself or lets
> > the kernel handle for it.
> 
> How much really needs to be in userspace to implement RFC4941?
> 
> I don't like the idea that even for a fully up and properly
> functioning link, if NetworkManager wedges then critical things like
> temporary address (re-)generation, will cease.

Honestly, I'd be completely happy to leave temporary address handling up
to the kernel and *not* do it in userspace; the kernel already has all
the code.  There are two problems with that though, (a) it's tied to
in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
these are solvable.

First off, what's the reasoning behind having IPv6 privacy as a config
option?  It's off-by-default and must be explicitly turned on, so is
there any harm in removing the config?  Or is it just for
smallest-kernel-ever folks?

Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
that for some new static address, that the kernel should create and
manage the temporary privacy addresses associated with its prefix?

Dan

> All that seems to matter is the secret used to generate the temporary
> address sequence, and perhaps things like site specific lifetime
> parameters.  These are things userland can send to the kernel in
> netlink messages.
> 
> Full disclosure that I am saying this from the perspective of someone
> who believes that one of the biggest mistakes ever was allowing the
> core of DHCP to be done in userspace.
> 
> Right now every ipv4 DHCP user ends up with their interface in
> promiscuous mode.  The DHCP client implementations are huge non-
> trivial bodies of code, and getting them to adopt the changes necessary
> to not put the interface in promiscuous mode is harder than pulling
> teeth.
> 
> If at least the DHCP protocol communications part were in the kernel,
> on the other hand, we could remove the problem quite swiftly.
> 
> This problem has existed for more than a decade, btw.  There simply
> exists no will to fix it properly, even after all this time, because
> breaking the ice on those DHCP client code bases in userbase is hard.
> 
> So I want to see less fundamental stuff about interface configuration
> and address assignment in userland, not more.
> --
> 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 net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Dan Williams]

On Mon, 2013-10-28 at 18:16 -0500, Dan Williams wrote:
> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Date: Sun, 27 Oct 2013 17:48:35 +0100
> > 
> > > A temporary address is also bound to a non-privacy public address so
> > > it's lifetime is determined by its lifetime (e.g. if you switch the
> > > network and don't receive on-link information for that prefix any
> > > more). NetworkManager would have to take care about that, too. It is
> > > just a question of what NetworkManager wants to handle itself or lets
> > > the kernel handle for it.
> > 
> > How much really needs to be in userspace to implement RFC4941?
> > 
> > I don't like the idea that even for a fully up and properly
> > functioning link, if NetworkManager wedges then critical things like
> > temporary address (re-)generation, will cease.
> 
> Honestly, I'd be completely happy to leave temporary address handling up
> to the kernel and *not* do it in userspace; the kernel already has all
> the code.  There are two problems with that though, (a) it's tied to
> in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
> these are solvable.
> 
> First off, what's the reasoning behind having IPv6 privacy as a config
> option?  It's off-by-default and must be explicitly turned on, so is
> there any harm in removing the config?  Or is it just for
> smallest-kernel-ever folks?
> 
> Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> that for some new static address, that the kernel should create and
> manage the temporary privacy addresses associated with its prefix?

Except that we've run out of IFA_F_* flags already, since it's a u8 and
there are already 8 flags.  What to do?

Dan

> Dan
> 
> > All that seems to matter is the secret used to generate the temporary
> > address sequence, and perhaps things like site specific lifetime
> > parameters.  These are things userland can send to the kernel in
> > netlink messages.
> > 
> > Full disclosure that I am saying this from the perspective of someone
> > who believes that one of the biggest mistakes ever was allowing the
> > core of DHCP to be done in userspace.
> > 
> > Right now every ipv4 DHCP user ends up with their interface in
> > promiscuous mode.  The DHCP client implementations are huge non-
> > trivial bodies of code, and getting them to adopt the changes necessary
> > to not put the interface in promiscuous mode is harder than pulling
> > teeth.
> > 
> > If at least the DHCP protocol communications part were in the kernel,
> > on the other hand, we could remove the problem quite swiftly.
> > 
> > This problem has existed for more than a decade, btw.  There simply
> > exists no will to fix it properly, even after all this time, because
> > breaking the ice on those DHCP client code bases in userbase is hard.
> > 
> > So I want to see less fundamental stuff about interface configuration
> > and address assignment in userland, not more.
> > --
> > 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
> 
> 
> --
> 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 net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: Dan Williams <dcbw@redhat.com>
Date: Mon, 28 Oct 2013 18:23:03 -0500

> Except that we've run out of IFA_F_* flags already, since it's a u8 and
> there are already 8 flags.  What to do?

That's not a problem, just add a new netlink attribute "extended
flags" that can be passed in with address netlink messages.

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Date: Sun, 27 Oct 2013 17:48:35 +0100
> > 
> > > A temporary address is also bound to a non-privacy public address so
> > > it's lifetime is determined by its lifetime (e.g. if you switch the
> > > network and don't receive on-link information for that prefix any
> > > more). NetworkManager would have to take care about that, too. It is
> > > just a question of what NetworkManager wants to handle itself or lets
> > > the kernel handle for it.
> > 
> > How much really needs to be in userspace to implement RFC4941?
> > 
> > I don't like the idea that even for a fully up and properly
> > functioning link, if NetworkManager wedges then critical things like
> > temporary address (re-)generation, will cease.
> 
> Honestly, I'd be completely happy to leave temporary address handling up
> to the kernel and *not* do it in userspace; the kernel already has all
> the code.  There are two problems with that though, (a) it's tied to
> in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
> these are solvable.

Ah, (a) does complicate things, I agree. But the tieing is essential
currently. So it seems a netlink interface would be needed to tie a new
address to an already installed one, if the kernel should still deal
with the regeneration?

> First off, what's the reasoning behind having IPv6 privacy as a config
> option?  It's off-by-default and must be explicitly turned on, so is
> there any harm in removing the config?  Or is it just for
> smallest-kernel-ever folks?

I don't know about the policy. Does it really matter as distributions
normally switch it on? But I would not like to see the option removed
entirly, maybe the default could be changed.

> Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> that for some new static address, that the kernel should create and
> manage the temporary privacy addresses associated with its prefix?

But this would only be needed if they were managed in user-space, no?

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Dan Williams]

On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
> On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> > On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > > Date: Sun, 27 Oct 2013 17:48:35 +0100
> > > 
> > > > A temporary address is also bound to a non-privacy public address so
> > > > it's lifetime is determined by its lifetime (e.g. if you switch the
> > > > network and don't receive on-link information for that prefix any
> > > > more). NetworkManager would have to take care about that, too. It is
> > > > just a question of what NetworkManager wants to handle itself or lets
> > > > the kernel handle for it.
> > > 
> > > How much really needs to be in userspace to implement RFC4941?
> > > 
> > > I don't like the idea that even for a fully up and properly
> > > functioning link, if NetworkManager wedges then critical things like
> > > temporary address (re-)generation, will cease.
> > 
> > Honestly, I'd be completely happy to leave temporary address handling up
> > to the kernel and *not* do it in userspace; the kernel already has all
> > the code.  There are two problems with that though, (a) it's tied to
> > in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
> > these are solvable.
> 
> Ah, (a) does complicate things, I agree. But the tieing is essential
> currently. So it seems a netlink interface would be needed to tie a new
> address to an already installed one, if the kernel should still deal
> with the regeneration?

I think it's simpler than that.  New flag set when adding the
non-private address that says "create and manage privacy addresses for
this non-private address".  The kernel then adds the privacy addresses
generated off the non-private address/prefixlen, and ties their lifetime
to the non-private address.  If the non-private address is removed, the
privacy addresses could get removed too.

I don't think we need API to tie addresses to already installed ones,
because the kernel already has the privacy address generation code, so
why should userspace generate the privacy address at all?  Just leave
that to the kernel.

> > First off, what's the reasoning behind having IPv6 privacy as a config
> > option?  It's off-by-default and must be explicitly turned on, so is
> > there any harm in removing the config?  Or is it just for
> > smallest-kernel-ever folks?
> 
> I don't know about the policy. Does it really matter as distributions
> normally switch it on? But I would not like to see the option removed
> entirly, maybe the default could be changed.
> 
> > Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> > that for some new static address, that the kernel should create and
> > manage the temporary privacy addresses associated with its prefix?
> 
> But this would only be needed if they were managed in user-space, no?

"if they" == what?  privacy address or static address?  What
NetworkManager is trying to do is handle RAs in userspace with libndp
for various flexibility and behavioral reasons, but we'd really like to
leave all the temporary address stuff up to the kernel.

So NM would handle RA/RS and when it gets a prefix, it would create the
IPv6 non-private address and add it to the interface.  When adding, it
would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
would then handle all the privacy address generation, lifetimes, and
timers.  Basically, break some of the privacy code away from the
in-kernel RA handling so that privacy addresses could be triggered from
userland too.

Would that be workable?

Dan 

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

Hi!

On Tue, Oct 29, 2013 at 09:31:18AM -0500, Dan Williams wrote:
> On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
> > On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> > > On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > > > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > > > Date: Sun, 27 Oct 2013 17:48:35 +0100
> > > > 
> > > > > A temporary address is also bound to a non-privacy public address so
> > > > > it's lifetime is determined by its lifetime (e.g. if you switch the
> > > > > network and don't receive on-link information for that prefix any
> > > > > more). NetworkManager would have to take care about that, too. It is
> > > > > just a question of what NetworkManager wants to handle itself or lets
> > > > > the kernel handle for it.
> > > > 
> > > > How much really needs to be in userspace to implement RFC4941?
> > > > 
> > > > I don't like the idea that even for a fully up and properly
> > > > functioning link, if NetworkManager wedges then critical things like
> > > > temporary address (re-)generation, will cease.
> > > 
> > > Honestly, I'd be completely happy to leave temporary address handling up
> > > to the kernel and *not* do it in userspace; the kernel already has all
> > > the code.  There are two problems with that though, (a) it's tied to
> > > in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
> > > these are solvable.
> > 
> > Ah, (a) does complicate things, I agree. But the tieing is essential
> > currently. So it seems a netlink interface would be needed to tie a new
> > address to an already installed one, if the kernel should still deal
> > with the regeneration?
> 
> I think it's simpler than that.  New flag set when adding the
> non-private address that says "create and manage privacy addresses for
> this non-private address".  The kernel then adds the privacy addresses
> generated off the non-private address/prefixlen, and ties their lifetime
> to the non-private address.  If the non-private address is removed, the
> privacy addresses could get removed too.
> 
> I don't think we need API to tie addresses to already installed ones,
> because the kernel already has the privacy address generation code, so
> why should userspace generate the privacy address at all?  Just leave
> that to the kernel.

Ok.

> > > First off, what's the reasoning behind having IPv6 privacy as a config
> > > option?  It's off-by-default and must be explicitly turned on, so is
> > > there any harm in removing the config?  Or is it just for
> > > smallest-kernel-ever folks?
> > 
> > I don't know about the policy. Does it really matter as distributions
> > normally switch it on? But I would not like to see the option removed
> > entirly, maybe the default could be changed.
> > 
> > > Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> > > that for some new static address, that the kernel should create and
> > > manage the temporary privacy addresses associated with its prefix?
> > 
> > But this would only be needed if they were managed in user-space, no?
> 
> "if they" == what?  privacy address or static address?  What

With "they" I meant privacy addresses.

> NetworkManager is trying to do is handle RAs in userspace with libndp
> for various flexibility and behavioral reasons, but we'd really like to
> leave all the temporary address stuff up to the kernel.

Can you provide me with details why the Kernel RA implementation is not good
enough? I tried to find some bugs, I found some but they were missing details
or were not even correct or outdated.

> So NM would handle RA/RS and when it gets a prefix, it would create the
> IPv6 non-private address and add it to the interface.  When adding, it
> would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> would then handle all the privacy address generation, lifetimes, and
> timers.  Basically, break some of the privacy code away from the
> in-kernel RA handling so that privacy addresses could be triggered from
> userland too.
> 
> Would that be workable?

That sounds like a solid plan for me. I would actually liked to see that NM
would use the kernel implementation but I guess there is no way back any more.
:(

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Dan Williams]

On Tue, 2013-10-29 at 15:38 +0100, Hannes Frederic Sowa wrote:
> Hi!
> 
> On Tue, Oct 29, 2013 at 09:31:18AM -0500, Dan Williams wrote:
> > On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
> > > On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> > > > On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > > > > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > > > > Date: Sun, 27 Oct 2013 17:48:35 +0100
> > > > > 
> > > > > > A temporary address is also bound to a non-privacy public address so
> > > > > > it's lifetime is determined by its lifetime (e.g. if you switch the
> > > > > > network and don't receive on-link information for that prefix any
> > > > > > more). NetworkManager would have to take care about that, too. It is
> > > > > > just a question of what NetworkManager wants to handle itself or lets
> > > > > > the kernel handle for it.
> > > > > 
> > > > > How much really needs to be in userspace to implement RFC4941?
> > > > > 
> > > > > I don't like the idea that even for a fully up and properly
> > > > > functioning link, if NetworkManager wedges then critical things like
> > > > > temporary address (re-)generation, will cease.
> > > > 
> > > > Honestly, I'd be completely happy to leave temporary address handling up
> > > > to the kernel and *not* do it in userspace; the kernel already has all
> > > > the code.  There are two problems with that though, (a) it's tied to
> > > > in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
> > > > these are solvable.
> > > 
> > > Ah, (a) does complicate things, I agree. But the tieing is essential
> > > currently. So it seems a netlink interface would be needed to tie a new
> > > address to an already installed one, if the kernel should still deal
> > > with the regeneration?
> > 
> > I think it's simpler than that.  New flag set when adding the
> > non-private address that says "create and manage privacy addresses for
> > this non-private address".  The kernel then adds the privacy addresses
> > generated off the non-private address/prefixlen, and ties their lifetime
> > to the non-private address.  If the non-private address is removed, the
> > privacy addresses could get removed too.
> > 
> > I don't think we need API to tie addresses to already installed ones,
> > because the kernel already has the privacy address generation code, so
> > why should userspace generate the privacy address at all?  Just leave
> > that to the kernel.
> 
> Ok.
> 
> > > > First off, what's the reasoning behind having IPv6 privacy as a config
> > > > option?  It's off-by-default and must be explicitly turned on, so is
> > > > there any harm in removing the config?  Or is it just for
> > > > smallest-kernel-ever folks?
> > > 
> > > I don't know about the policy. Does it really matter as distributions
> > > normally switch it on? But I would not like to see the option removed
> > > entirly, maybe the default could be changed.
> > > 
> > > > Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> > > > that for some new static address, that the kernel should create and
> > > > manage the temporary privacy addresses associated with its prefix?
> > > 
> > > But this would only be needed if they were managed in user-space, no?
> > 
> > "if they" == what?  privacy address or static address?  What
> 
> With "they" I meant privacy addresses.
> 
> > NetworkManager is trying to do is handle RAs in userspace with libndp
> > for various flexibility and behavioral reasons, but we'd really like to
> > leave all the temporary address stuff up to the kernel.
> 
> Can you provide me with details why the Kernel RA implementation is not good
> enough? I tried to find some bugs, I found some but they were missing details
> or were not even correct or outdated.

First, RA handling is too tightly tied to interface flags.  This is a
problem because interfaces are required to be IFF_UP for various
operations like carrier detection, wireless scanning, reading certain
interface properties, link statistics, etc.  We can play games with
flipping accept_ra, but changing accept_ra doesn't trigger an RS.  At
the moment, only interface flag changes trigger an RS, and that also can
reset a lot of L2 state.

Second, Router Solicitations are required at various times other than
NETDEV_CHANGE/NETDEV_UP.  The router is not guaranteed to send (nor are
we guaranteed to receive) an RA when the RDNSS/DNSSL approach their
lifetimes, so to ensure those values are still still valid, we need to
send out an RS, especially on lossy networks where the RA might get
dropped.

Third, we need more flexibility in reading ND user options like DNSSL
and RDNSS and newer stuff.  Every new option that userspace might want
to process requires some kernel code (ndisc_is_useropt()) to push it out
to userspace, and that means these options are not available on older
kernels.

Fourth, there's no opportunity to override any of the RA-derived
settings with user preferences before the kernel commits them.  Perhaps
the user wishes to ignore a specific prefix (but accept other prefixes
or other RAs), or to ignore the automatically provided routes, or
whatever.

> > So NM would handle RA/RS and when it gets a prefix, it would create the
> > IPv6 non-private address and add it to the interface.  When adding, it
> > would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> > would then handle all the privacy address generation, lifetimes, and
> > timers.  Basically, break some of the privacy code away from the
> > in-kernel RA handling so that privacy addresses could be triggered from
> > userland too.
> > 
> > Would that be workable?
> 
> That sounds like a solid plan for me. I would actually liked to see that NM
> would use the kernel implementation but I guess there is no way back any more.
> :(

Some of the issues mentioned above might be inappropriate to solve
entirely in the kernel.  I have no problem with in-kernel IPv6 for
simple use-cases and it works great for these.  But if we think about
more complex functionality, especially where userland might want an
opportunity to override some of the behavior, and backwards
compatibility with kernels that don't implement these changes, we'd like
to handle some addrconf in userspace.

Dan

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Vlad Yasevich]

On 10/29/2013 10:31 AM, Dan Williams wrote:
> On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
>> On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
>>> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
>>>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>>> Date: Sun, 27 Oct 2013 17:48:35 +0100
>>>>
>>>>> A temporary address is also bound to a non-privacy public address so
>>>>> it's lifetime is determined by its lifetime (e.g. if you switch the
>>>>> network and don't receive on-link information for that prefix any
>>>>> more). NetworkManager would have to take care about that, too. It is
>>>>> just a question of what NetworkManager wants to handle itself or lets
>>>>> the kernel handle for it.
>>>>
>>>> How much really needs to be in userspace to implement RFC4941?
>>>>
>>>> I don't like the idea that even for a fully up and properly
>>>> functioning link, if NetworkManager wedges then critical things like
>>>> temporary address (re-)generation, will cease.
>>>
>>> Honestly, I'd be completely happy to leave temporary address handling up
>>> to the kernel and *not* do it in userspace; the kernel already has all
>>> the code.  There are two problems with that though, (a) it's tied to
>>> in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
>>> these are solvable.
>>
>> Ah, (a) does complicate things, I agree. But the tieing is essential
>> currently. So it seems a netlink interface would be needed to tie a new
>> address to an already installed one, if the kernel should still deal
>> with the regeneration?
>
> I think it's simpler than that.  New flag set when adding the
> non-private address that says "create and manage privacy addresses for
> this non-private address".  The kernel then adds the privacy addresses
> generated off the non-private address/prefixlen, and ties their lifetime
> to the non-private address.  If the non-private address is removed, the
> privacy addresses could get removed too.
>
> I don't think we need API to tie addresses to already installed ones,
> because the kernel already has the privacy address generation code, so
> why should userspace generate the privacy address at all?  Just leave
> that to the kernel.
>
>>> First off, what's the reasoning behind having IPv6 privacy as a config
>>> option?  It's off-by-default and must be explicitly turned on, so is
>>> there any harm in removing the config?  Or is it just for
>>> smallest-kernel-ever folks?
>>
>> I don't know about the policy. Does it really matter as distributions
>> normally switch it on? But I would not like to see the option removed
>> entirly, maybe the default could be changed.
>>
>>> Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
>>> that for some new static address, that the kernel should create and
>>> manage the temporary privacy addresses associated with its prefix?
>>
>> But this would only be needed if they were managed in user-space, no?
>
> "if they" == what?  privacy address or static address?  What
> NetworkManager is trying to do is handle RAs in userspace with libndp
> for various flexibility and behavioral reasons, but we'd really like to
> leave all the temporary address stuff up to the kernel.
>
> So NM would handle RA/RS and when it gets a prefix, it would create the
> IPv6 non-private address and add it to the interface.  When adding, it
> would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> would then handle all the privacy address generation, lifetimes, and
> timers.  Basically, break some of the privacy code away from the
> in-kernel RA handling so that privacy addresses could be triggered from
> userland too.
>
> Would that be workable?

You are still dependent on the NM/user app to do this and what happens
if that apps wedges?

I think we should just do privacy addresses automatically, or based on
some sysconfig setting per interface to give users ability to turn it
off.  But I agree with David, and I speak from experience.
You don't whant address configuration to be done by userspace daemon.
There are too many things that can go wrong.

-vlad

>
> Dan
>

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Dan Williams]

On Tue, 2013-10-29 at 12:58 -0400, Vlad Yasevich wrote:
> On 10/29/2013 10:31 AM, Dan Williams wrote:
> > On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
> >> On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> >>> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> >>>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >>>> Date: Sun, 27 Oct 2013 17:48:35 +0100
> >>>>
> >>>>> A temporary address is also bound to a non-privacy public address so
> >>>>> it's lifetime is determined by its lifetime (e.g. if you switch the
> >>>>> network and don't receive on-link information for that prefix any
> >>>>> more). NetworkManager would have to take care about that, too. It is
> >>>>> just a question of what NetworkManager wants to handle itself or lets
> >>>>> the kernel handle for it.
> >>>>
> >>>> How much really needs to be in userspace to implement RFC4941?
> >>>>
> >>>> I don't like the idea that even for a fully up and properly
> >>>> functioning link, if NetworkManager wedges then critical things like
> >>>> temporary address (re-)generation, will cease.
> >>>
> >>> Honestly, I'd be completely happy to leave temporary address handling up
> >>> to the kernel and *not* do it in userspace; the kernel already has all
> >>> the code.  There are two problems with that though, (a) it's tied to
> >>> in-kernel RA handling, and (b) it's controlled by a CONFIG option.  Both
> >>> these are solvable.
> >>
> >> Ah, (a) does complicate things, I agree. But the tieing is essential
> >> currently. So it seems a netlink interface would be needed to tie a new
> >> address to an already installed one, if the kernel should still deal
> >> with the regeneration?
> >
> > I think it's simpler than that.  New flag set when adding the
> > non-private address that says "create and manage privacy addresses for
> > this non-private address".  The kernel then adds the privacy addresses
> > generated off the non-private address/prefixlen, and ties their lifetime
> > to the non-private address.  If the non-private address is removed, the
> > privacy addresses could get removed too.
> >
> > I don't think we need API to tie addresses to already installed ones,
> > because the kernel already has the privacy address generation code, so
> > why should userspace generate the privacy address at all?  Just leave
> > that to the kernel.
> >
> >>> First off, what's the reasoning behind having IPv6 privacy as a config
> >>> option?  It's off-by-default and must be explicitly turned on, so is
> >>> there any harm in removing the config?  Or is it just for
> >>> smallest-kernel-ever folks?
> >>
> >> I don't know about the policy. Does it really matter as distributions
> >> normally switch it on? But I would not like to see the option removed
> >> entirly, maybe the default could be changed.
> >>
> >>> Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> >>> that for some new static address, that the kernel should create and
> >>> manage the temporary privacy addresses associated with its prefix?
> >>
> >> But this would only be needed if they were managed in user-space, no?
> >
> > "if they" == what?  privacy address or static address?  What
> > NetworkManager is trying to do is handle RAs in userspace with libndp
> > for various flexibility and behavioral reasons, but we'd really like to
> > leave all the temporary address stuff up to the kernel.
> >
> > So NM would handle RA/RS and when it gets a prefix, it would create the
> > IPv6 non-private address and add it to the interface.  When adding, it
> > would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> > would then handle all the privacy address generation, lifetimes, and
> > timers.  Basically, break some of the privacy code away from the
> > in-kernel RA handling so that privacy addresses could be triggered from
> > userland too.
> >
> > Would that be workable?
> 
> You are still dependent on the NM/user app to do this and what happens
> if that apps wedges?

In my proposal, the kernel would still manage the lifetimes of all the
addresses, since user app would add the non-privacy address with the
correct lifetime, and the kernel would generate the privacy addresses
with a corresponding lifetime.  If the app wedges for some reason, then
the kernel will deprecate and eventually remove the non-privacy *and*
privacy addresses since their lifetimes have expired.

What if your dhclient wedges?  What if ovsd or teamd goes down?  Or your
openvpn, vpnc, pptp, pppd, sshd, whatever wedges?  There's a lot of
networking that's controlled by userland these days, and failure of
these things also potentailly wedges your network.  We should be
striving to make the best userland we can instead of trying to stuff
everything into the kernel in the name of reliability.

> I think we should just do privacy addresses automatically, or based on
> some sysconfig setting per interface to give users ability to turn it
> off.  But I agree with David, and I speak from experience.
> You don't whant address configuration to be done by userspace daemon.
> There are too many things that can go wrong.

Should IPv6 should be that different from IPv4?  DHCP is done by a
userspace daemon in all cases (v6 and v4), and other v4 is always done
by userland (static files, avahi-autoipd, other daemons).  You can never
get away from userland here, and we need more flexibility in userland
than the kernel currently provides with its addrconf implementation.

Dan

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: Dan Williams <dcbw@redhat.com>
Date: Mon, 28 Oct 2013 18:16:19 -0500

> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> First off, what's the reasoning behind having IPv6 privacy as a config
> option?  It's off-by-default and must be explicitly turned on, so is
> there any harm in removing the config?  Or is it just for
> smallest-kernel-ever folks?

I think it's for "smallest kernel ever" stuff.  Even every arch
defconfig that mentions it has it enabled :-)

Maybe it was optional initially because the code was new and
experimental'ish.  I don't know.

Regardless of the reason I think it only obfuscates the code with
ifdefs right now and I would be happy to see it disappear.

Any objections to this patch?

====================
[PATCH] ipv6: Remove privacy config option.

The code for privacy extentions is very mature, and making it
configurable only gives marginal memory/code savings in exchange
for obfuscation and hard to read code via CPP ifdef'ery.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/linux/ipv6.h   |  2 --
 include/net/if_inet6.h |  5 +----
 net/ipv6/Kconfig       | 18 ------------------
 net/ipv6/addrconf.c    | 41 +++--------------------------------------
 4 files changed, 4 insertions(+), 62 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index a80a63c..5d89d1b 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -21,13 +21,11 @@ struct ipv6_devconf {
 	__s32		force_mld_version;
 	__s32		mldv1_unsolicited_report_interval;
 	__s32		mldv2_unsolicited_report_interval;
-#ifdef CONFIG_IPV6_PRIVACY
 	__s32		use_tempaddr;
 	__s32		temp_valid_lft;
 	__s32		temp_prefered_lft;
 	__s32		regen_max_retry;
 	__s32		max_desync_factor;
-#endif
 	__s32		max_addresses;
 	__s32		accept_ra_defrtr;
 	__s32		accept_ra_pinfo;
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 02ef772..76d5427 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -66,11 +66,10 @@ struct inet6_ifaddr {
 	struct hlist_node	addr_lst;
 	struct list_head	if_list;
 
-#ifdef CONFIG_IPV6_PRIVACY
 	struct list_head	tmp_list;
 	struct inet6_ifaddr	*ifpub;
 	int			regen_count;
-#endif
+
 	bool			tokenized;
 
 	struct rcu_head		rcu;
@@ -192,11 +191,9 @@ struct inet6_dev {
 	__u32			if_flags;
 	int			dead;
 
-#ifdef CONFIG_IPV6_PRIVACY
 	u8			rndid[8];
 	struct timer_list	regen_timer;
 	struct list_head	tempaddr_list;
-#endif
 
 	struct in6_addr		token;
 
diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index e1a8d90..d92e558 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -21,24 +21,6 @@ menuconfig IPV6
 
 if IPV6
 
-config IPV6_PRIVACY
-	bool "IPv6: Privacy Extensions (RFC 3041) support"
-	---help---
-	  Privacy Extensions for Stateless Address Autoconfiguration in IPv6
-	  support.  With this option, additional periodically-altered
-	  pseudo-random global-scope unicast address(es) will be assigned to
-	  your interface(s).
-	
-	  We use our standard pseudo-random algorithm to generate the
-          randomized interface identifier, instead of one described in RFC 3041.
-
-	  By default the kernel does not generate temporary addresses.
-	  To use temporary addresses, do
-	
-	        echo 2 >/proc/sys/net/ipv6/conf/all/use_tempaddr 
-
-	  See <file:Documentation/networking/ip-sysctl.txt> for details.
-
 config IPV6_ROUTER_PREF
 	bool "IPv6: Router Preference (RFC 4191) support"
 	---help---
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd3fb30..542d095 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -83,11 +83,7 @@
 #include <linux/if_tunnel.h>
 #include <linux/rtnetlink.h>
 #include <linux/netconf.h>
-
-#ifdef CONFIG_IPV6_PRIVACY
 #include <linux/random.h>
-#endif
-
 #include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
@@ -124,11 +120,9 @@ static inline void addrconf_sysctl_unregister(struct inet6_dev *idev)
 }
 #endif
 
-#ifdef CONFIG_IPV6_PRIVACY
 static void __ipv6_regen_rndid(struct inet6_dev *idev);
 static void __ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr);
 static void ipv6_regen_rndid(unsigned long data);
-#endif
 
 static int ipv6_generate_eui64(u8 *eui, struct net_device *dev);
 static int ipv6_count_addresses(struct inet6_dev *idev);
@@ -183,13 +177,11 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
-#ifdef CONFIG_IPV6_PRIVACY
 	.use_tempaddr 		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
 	.temp_prefered_lft	= TEMP_PREFERRED_LIFETIME,
 	.regen_max_retry	= REGEN_MAX_RETRY,
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
-#endif
 	.max_addresses		= IPV6_MAX_ADDRESSES,
 	.accept_ra_defrtr	= 1,
 	.accept_ra_pinfo	= 1,
@@ -221,13 +213,11 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
-#ifdef CONFIG_IPV6_PRIVACY
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
 	.temp_prefered_lft	= TEMP_PREFERRED_LIFETIME,
 	.regen_max_retry	= REGEN_MAX_RETRY,
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
-#endif
 	.max_addresses		= IPV6_MAX_ADDRESSES,
 	.accept_ra_defrtr	= 1,
 	.accept_ra_pinfo	= 1,
@@ -371,7 +361,6 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 	}
 #endif
 
-#ifdef CONFIG_IPV6_PRIVACY
 	INIT_LIST_HEAD(&ndev->tempaddr_list);
 	setup_timer(&ndev->regen_timer, ipv6_regen_rndid, (unsigned long)ndev);
 	if ((dev->flags&IFF_LOOPBACK) ||
@@ -384,7 +373,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 		in6_dev_hold(ndev);
 		ipv6_regen_rndid((unsigned long) ndev);
 	}
-#endif
+
 	ndev->token = in6addr_any;
 
 	if (netif_running(dev) && addrconf_qdisc_ok(dev))
@@ -865,12 +854,10 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 	/* Add to inet6_dev unicast addr list. */
 	ipv6_link_dev_addr(idev, ifa);
 
-#ifdef CONFIG_IPV6_PRIVACY
 	if (ifa->flags&IFA_F_TEMPORARY) {
 		list_add(&ifa->tmp_list, &idev->tempaddr_list);
 		in6_ifa_hold(ifa);
 	}
-#endif
 
 	in6_ifa_hold(ifa);
 	write_unlock(&idev->lock);
@@ -913,7 +900,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 	spin_unlock_bh(&addrconf_hash_lock);
 
 	write_lock_bh(&idev->lock);
-#ifdef CONFIG_IPV6_PRIVACY
+
 	if (ifp->flags&IFA_F_TEMPORARY) {
 		list_del(&ifp->tmp_list);
 		if (ifp->ifpub) {
@@ -922,7 +909,6 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 		}
 		__in6_ifa_put(ifp);
 	}
-#endif
 
 	list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
 		if (ifa == ifp) {
@@ -1013,7 +999,6 @@ out:
 	in6_ifa_put(ifp);
 }
 
-#ifdef CONFIG_IPV6_PRIVACY
 static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *ift)
 {
 	struct inet6_dev *idev = ifp->idev;
@@ -1116,7 +1101,6 @@ retry:
 out:
 	return ret;
 }
-#endif
 
 /*
  *	Choose an appropriate source address (RFC3484)
@@ -1131,9 +1115,7 @@ enum {
 #endif
 	IPV6_SADDR_RULE_OIF,
 	IPV6_SADDR_RULE_LABEL,
-#ifdef CONFIG_IPV6_PRIVACY
 	IPV6_SADDR_RULE_PRIVACY,
-#endif
 	IPV6_SADDR_RULE_ORCHID,
 	IPV6_SADDR_RULE_PREFIX,
 	IPV6_SADDR_RULE_MAX
@@ -1247,7 +1229,6 @@ static int ipv6_get_saddr_eval(struct net *net,
 				      &score->ifa->addr, score->addr_type,
 				      score->ifa->idev->dev->ifindex) == dst->label;
 		break;
-#ifdef CONFIG_IPV6_PRIVACY
 	case IPV6_SADDR_RULE_PRIVACY:
 	    {
 		/* Rule 7: Prefer public address
@@ -1259,7 +1240,6 @@ static int ipv6_get_saddr_eval(struct net *net,
 		ret = (!(score->ifa->flags & IFA_F_TEMPORARY)) ^ preftmp;
 		break;
 	    }
-#endif
 	case IPV6_SADDR_RULE_ORCHID:
 		/* Rule 8-: Prefer ORCHID vs ORCHID or
 		 *	    non-ORCHID vs non-ORCHID
@@ -1588,7 +1568,6 @@ static void addrconf_dad_stop(struct inet6_ifaddr *ifp, int dad_failed)
 		if (dad_failed)
 			ipv6_ifa_notify(0, ifp);
 		in6_ifa_put(ifp);
-#ifdef CONFIG_IPV6_PRIVACY
 	} else if (ifp->flags&IFA_F_TEMPORARY) {
 		struct inet6_ifaddr *ifpub;
 		spin_lock_bh(&ifp->lock);
@@ -1602,7 +1581,6 @@ static void addrconf_dad_stop(struct inet6_ifaddr *ifp, int dad_failed)
 			spin_unlock_bh(&ifp->lock);
 		}
 		ipv6_del_addr(ifp);
-#endif
 	} else
 		ipv6_del_addr(ifp);
 }
@@ -1851,7 +1829,6 @@ static int ipv6_inherit_eui64(u8 *eui, struct inet6_dev *idev)
 	return err;
 }
 
-#ifdef CONFIG_IPV6_PRIVACY
 /* (re)generation of randomized interface identifier (RFC 3041 3.2, 3.5) */
 static void __ipv6_regen_rndid(struct inet6_dev *idev)
 {
@@ -1919,7 +1896,6 @@ static void  __ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmp
 	if (tmpaddr && memcmp(idev->rndid, &tmpaddr->s6_addr[8], 8) == 0)
 		__ipv6_regen_rndid(idev);
 }
-#endif
 
 /*
  *	Add prefix route.
@@ -2207,9 +2183,7 @@ ok:
 		if (ifp) {
 			int flags;
 			unsigned long now;
-#ifdef CONFIG_IPV6_PRIVACY
 			struct inet6_ifaddr *ift;
-#endif
 			u32 stored_lft;
 
 			/* update lifetime (RFC2462 5.5.3 e) */
@@ -2250,7 +2224,6 @@ ok:
 			} else
 				spin_unlock(&ifp->lock);
 
-#ifdef CONFIG_IPV6_PRIVACY
 			read_lock_bh(&in6_dev->lock);
 			/* update all temporary addresses in the list */
 			list_for_each_entry(ift, &in6_dev->tempaddr_list,
@@ -2315,7 +2288,7 @@ ok:
 			} else {
 				read_unlock_bh(&in6_dev->lock);
 			}
-#endif
+
 			in6_ifa_put(ifp);
 			addrconf_verify(0);
 		}
@@ -2995,7 +2968,6 @@ static int addrconf_ifdown(struct net_device *dev, int how)
 	if (!how)
 		idev->if_flags &= ~(IF_RS_SENT|IF_RA_RCVD|IF_READY);
 
-#ifdef CONFIG_IPV6_PRIVACY
 	if (how && del_timer(&idev->regen_timer))
 		in6_dev_put(idev);
 
@@ -3015,7 +2987,6 @@ static int addrconf_ifdown(struct net_device *dev, int how)
 		in6_ifa_put(ifa);
 		write_lock_bh(&idev->lock);
 	}
-#endif
 
 	while (!list_empty(&idev->addr_list)) {
 		ifa = list_first_entry(&idev->addr_list,
@@ -3528,7 +3499,6 @@ restart:
 					in6_ifa_put(ifp);
 					goto restart;
 				}
-#ifdef CONFIG_IPV6_PRIVACY
 			} else if ((ifp->flags&IFA_F_TEMPORARY) &&
 				   !(ifp->flags&IFA_F_TENTATIVE)) {
 				unsigned long regen_advance = ifp->idev->cnf.regen_max_retry *
@@ -3556,7 +3526,6 @@ restart:
 				} else if (time_before(ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ, next))
 					next = ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ;
 				spin_unlock(&ifp->lock);
-#endif
 			} else {
 				/* ifp->prefered_lft <= ifp->valid_lft */
 				if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
@@ -4128,13 +4097,11 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 		jiffies_to_msecs(cnf->mldv1_unsolicited_report_interval);
 	array[DEVCONF_MLDV2_UNSOLICITED_REPORT_INTERVAL] =
 		jiffies_to_msecs(cnf->mldv2_unsolicited_report_interval);
-#ifdef CONFIG_IPV6_PRIVACY
 	array[DEVCONF_USE_TEMPADDR] = cnf->use_tempaddr;
 	array[DEVCONF_TEMP_VALID_LFT] = cnf->temp_valid_lft;
 	array[DEVCONF_TEMP_PREFERED_LFT] = cnf->temp_prefered_lft;
 	array[DEVCONF_REGEN_MAX_RETRY] = cnf->regen_max_retry;
 	array[DEVCONF_MAX_DESYNC_FACTOR] = cnf->max_desync_factor;
-#endif
 	array[DEVCONF_MAX_ADDRESSES] = cnf->max_addresses;
 	array[DEVCONF_ACCEPT_RA_DEFRTR] = cnf->accept_ra_defrtr;
 	array[DEVCONF_ACCEPT_RA_PINFO] = cnf->accept_ra_pinfo;
@@ -4828,7 +4795,6 @@ static struct addrconf_sysctl_table
 			.mode		= 0644,
 			.proc_handler	= proc_dointvec_ms_jiffies,
 		},
-#ifdef CONFIG_IPV6_PRIVACY
 		{
 			.procname	= "use_tempaddr",
 			.data		= &ipv6_devconf.use_tempaddr,
@@ -4864,7 +4830,6 @@ static struct addrconf_sysctl_table
 			.mode		= 0644,
 			.proc_handler	= proc_dointvec,
 		},
-#endif
 		{
 			.procname	= "max_addresses",
 			.data		= &ipv6_devconf.max_addresses,
-- 
1.7.11.7

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Mon, Oct 28, 2013 at 08:08:10PM -0400, David Miller wrote:
> From: Dan Williams <dcbw@redhat.com>
> Date: Mon, 28 Oct 2013 18:16:19 -0500
> 
> > On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > First off, what's the reasoning behind having IPv6 privacy as a config
> > option?  It's off-by-default and must be explicitly turned on, so is
> > there any harm in removing the config?  Or is it just for
> > smallest-kernel-ever folks?
> 
> I think it's for "smallest kernel ever" stuff.  Even every arch
> defconfig that mentions it has it enabled :-)
> 
> Maybe it was optional initially because the code was new and
> experimental'ish.  I don't know.
> 
> Regardless of the reason I think it only obfuscates the code with
> ifdefs right now and I would be happy to see it disappear.
> 
> Any objections to this patch?

Yeah, I changed my mind. The ifdefs are really hideous. Fine for me.

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 29 Oct 2013 01:13:40 +0100

> On Mon, Oct 28, 2013 at 08:08:10PM -0400, David Miller wrote:
>> From: Dan Williams <dcbw@redhat.com>
>> Date: Mon, 28 Oct 2013 18:16:19 -0500
>> 
>> > On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
>> > First off, what's the reasoning behind having IPv6 privacy as a config
>> > option?  It's off-by-default and must be explicitly turned on, so is
>> > there any harm in removing the config?  Or is it just for
>> > smallest-kernel-ever folks?
>> 
>> I think it's for "smallest kernel ever" stuff.  Even every arch
>> defconfig that mentions it has it enabled :-)
>> 
>> Maybe it was optional initially because the code was new and
>> experimental'ish.  I don't know.
>> 
>> Regardless of the reason I think it only obfuscates the code with
>> ifdefs right now and I would be happy to see it disappear.
>> 
>> Any objections to this patch?
> 
> Yeah, I changed my mind. The ifdefs are really hideous. Fine for me.

Ok, pushed to net-next.

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Mon, Oct 28, 2013 at 05:17:25PM -0400, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Sun, 27 Oct 2013 17:48:35 +0100
> 
> > A temporary address is also bound to a non-privacy public address so
> > it's lifetime is determined by its lifetime (e.g. if you switch the
> > network and don't receive on-link information for that prefix any
> > more). NetworkManager would have to take care about that, too. It is
> > just a question of what NetworkManager wants to handle itself or lets
> > the kernel handle for it.
> 
> How much really needs to be in userspace to implement RFC4941?

First off, I am not part of the NetworkManager community and don't know
their plans how and what features they want to implement. But I researched
and guess a bit. Maybe Jiri can jump in and provide links and details?

Currently we are pretty fine with our in-kernel implementation of RFC
4941. We may not have some features the RFC requires, like choosing if
we use the privacy address on a per-prefix basis. We only distinguish
per interface. But it is used by people and doing fine, I guess.

> I don't like the idea that even for a fully up and properly
> functioning link, if NetworkManager wedges then critical things like
> temporary address (re-)generation, will cease.

I am totally d'accord. It is essential that regeneration does not stop
under any circumstances.

> All that seems to matter is the secret used to generate the temporary
> address sequence, and perhaps things like site specific lifetime
> parameters.  These are things userland can send to the kernel in
> netlink messages.

RFC 4941 does not dictate per se the use of a stable secret key. It is
currently generated in the kernel. If NM wants to provide a way to have
a stable key, I agree, it should be provided via netlink (maybe Nicolas
work on the netconf api could be extended for that).

I found this bug: <https://bugzilla.gnome.org/show_bug.cgi?id=705170#c2>

It does not seem to mention stable keys, so I guess it is not on
their radar. The comment I referenced talked about fixing the kernel
interface but I did not find any pointers what is wrong with it? There was
discussion that the /all/use_tempaddr should enable privacy extensions
globally on all interfaces even if they were running at that point:
http://patchwork.ozlabs.org/patch/131911/ (it got rejected). If something
like this is needed I guess we could provide such a setting. But as
NM has to have state on all interfaces I don't see a particular reason
they would need this.

I don't follow netdev closely that long so maybe I missed a lot of the
discussions that maybe took place about this.

> Full disclosure that I am saying this from the perspective of someone
> who believes that one of the biggest mistakes ever was allowing the
> core of DHCP to be done in userspace.

*nod*

> Right now every ipv4 DHCP user ends up with their interface in
> promiscuous mode.  The DHCP client implementations are huge non-
> trivial bodies of code, and getting them to adopt the changes necessary
> to not put the interface in promiscuous mode is harder than pulling
> teeth.

> If at least the DHCP protocol communications part were in the kernel,
> on the other hand, we could remove the problem quite swiftly.
> 
> This problem has existed for more than a decade, btw.  There simply
> exists no will to fix it properly, even after all this time, because
> breaking the ice on those DHCP client code bases in userbase is hard.

I wonder why NetworkManager rewrote IPv6 router discovery but not IPv4
DHCP client stuff. It could have been a good moment to introduce something
like PROT_DHCP sockets. Maybe it is not too late... ;)

> So I want to see less fundamental stuff about interface configuration
> and address assignment in userland, not more.

I agree and don't think the kernel is that bad at it. If bugs pop up I'll
certainly will have a look at them. ;-)

Off-topic:

Down the road there may be the deprecation of EUI-64 (ipv6 address generation
based on the MAC address). It will be replaced by an approach to have an
install-time secret key which will used like this:

  RID = F(Prefix, Net_Iface, Network_ID, DAD_Counter, stable_secret_key)

(network_id is something like the essid of the wlan etc.; F some hashing
function)

It should be used for link-local addresses as well as SLAAC:
<http://tools.ietf.org/html/draft-ietf-6man-stable-privacy-addresses-14>

So we should design the interfaces generic enough to cope with that. This
will have some implications: we need link-local address early in boot-up
because of the bind() of early boot-up daemons or even network booting,
mld etc.

My current idea to handle this, is that a kernel boot parameter is
provided to stop the generation of link-local addresses and that we kick
off address configuration from user-space at early as the secret key is
provided, which may well be from the initramfs. I'll send a RFC as soon
as I can find some time to clean it up and have it actually tested.

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 29 Oct 2013 00:31:58 +0100

> I wonder why NetworkManager rewrote IPv6 router discovery but not IPv4
> DHCP client stuff. It could have been a good moment to introduce something
> like PROT_DHCP sockets. Maybe it is not too late... ;)

Note that you don't even need to put the DHCP protocol core into the
kernel to fix the promiscuous problem.  You just have to use the
current kernel interfaces correctly.

It used to be the case a very long time ago that you couldn't even
receive broadcast UDP datagrams on a socket until an address was
configured on it.

So everyone turns on promiscuous mode and uses RAW sockets or
AF_PACKET.

Stupid?  yes.

But now the kernel will receive broadcast UDP datagrams just fine even
if there are no ipv4 addresses assigned yet.

I did a mock-up simple ipv4 DHCP client implementation just to test it
out, and it did work just fine.  Unfortunately, I can't find it at the
moment, I hope I didn't just delete it in frustration. :-)

> My current idea to handle this, is that a kernel boot parameter is
> provided to stop the generation of link-local addresses and that we kick
> off address configuration from user-space at early as the secret key is
> provided, which may well be from the initramfs. I'll send a RFC as soon
> as I can find some time to clean it up and have it actually tested.

I guess I'm ok with this, as I can't come up with any reasonable
alternative scheme.

RE: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Laight]

> Note that you don't even need to put the DHCP protocol core into the
> kernel to fix the promiscuous problem.  You just have to use the
> current kernel interfaces correctly.
> 
> It used to be the case a very long time ago that you couldn't even
> receive broadcast UDP datagrams on a socket until an address was
> configured on it.
> 
> So everyone turns on promiscuous mode and uses RAW sockets or
> AF_PACKET.
> 
> Stupid?  yes.

Not only that, but the dhcp client could use a normal UDP socket
to keep the lease renewed - I suspect it has only ever needed
to use the BPF interface (I didn't think it set promiscuous)
when acquiring the initial lease.

	David
 

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Tue, Oct 29, 2013 at 09:37:06AM -0000, David Laight wrote:
> > Note that you don't even need to put the DHCP protocol core into the
> > kernel to fix the promiscuous problem.  You just have to use the
> > current kernel interfaces correctly.
> > 
> > It used to be the case a very long time ago that you couldn't even
> > receive broadcast UDP datagrams on a socket until an address was
> > configured on it.
> > 
> > So everyone turns on promiscuous mode and uses RAW sockets or
> > AF_PACKET.
> > 
> > Stupid?  yes.
> 
> Not only that, but the dhcp client could use a normal UDP socket
> to keep the lease renewed - I suspect it has only ever needed
> to use the BPF interface (I didn't think it set promiscuous)
> when acquiring the initial lease.

Yes, this is a very unfortunate situation. From my experience it is not that
easy to get a patch merged into isc-dhcp.

It seems not that invasive to switch from af_packet to an udp socket with
SO_BROADCAST set.

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Eric Dumazet]

On Tue, 2013-10-29 at 13:40 +0100, Hannes Frederic Sowa wrote:

> Yes, this is a very unfortunate situation. From my experience it is not that
> easy to get a patch merged into isc-dhcp.
> 
> It seems not that invasive to switch from af_packet to an udp socket with
> SO_BROADCAST set.

Has anyone cooked and submitted such a patch ?

I can visit ISC with a hammer if needed ;)

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Tue, Oct 29, 2013 at 06:09:23AM -0700, Eric Dumazet wrote:
> On Tue, 2013-10-29 at 13:40 +0100, Hannes Frederic Sowa wrote:
> 
> > Yes, this is a very unfortunate situation. From my experience it is not that
> > easy to get a patch merged into isc-dhcp.
> > 
> > It seems not that invasive to switch from af_packet to an udp socket with
> > SO_BROADCAST set.
> 
> Has anyone cooked and submitted such a patch ?

Not me, my patch was intended for the server side (dhcrelay correctly
choosing source addresses).

> I can visit ISC with a hammer if needed ;)

Actually that is a very big motivation for me to try to come up with a
patch at the weekend. ;-)

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 29 Oct 2013 13:40:10 +0100

> It seems not that invasive to switch from af_packet to an udp socket
> with SO_BROADCAST set.

Precisely!

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Tue, Oct 29, 2013 at 03:58:09PM -0400, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Tue, 29 Oct 2013 13:40:10 +0100
> 
> > It seems not that invasive to switch from af_packet to an udp socket
> > with SO_BROADCAST set.
> 
> Precisely!

dhclient compiled with --enable-use-sockets worked out of the box, I merely
had to fix a small compile error (most non-intrusive version):
https://github.com/hannes/isc-dhcp/commit/55c3b7d80541b38389244f67f7f5bdb16ad02474

# lsof -p $(pidof dhclient)
COMMAND   PID USER   FD   TYPE             DEVICE SIZE/OFF   NODE NAME
dhclient 1247 root  cwd    DIR              252,2     4096      2 /
dhclient 1247 root  rtd    DIR              252,2     4096      2 /
dhclient 1247 root  txt    REG              252,2  6683635 145354 /home/hannes/isc-dhcp/client/dhclient
dhclient 1247 root  mem    REG              252,2   162472  30914 /usr/lib64/ld-2.17.so
dhclient 1247 root  mem    REG              252,2  2108632  30915 /usr/lib64/libc-2.17.so
dhclient 1247 root  mem    REG              252,2    62368   2205 /usr/lib64/libnss_files-2.17.so
dhclient 1247 root    0u   CHR                1,3      0t0   1028 /dev/null
dhclient 1247 root    1u   CHR                1,3      0t0   1028 /dev/null
dhclient 1247 root    2u   CHR                1,3      0t0   1028 /dev/null
dhclient 1247 root    3u  unix 0xffff880114b3c200      0t0  20634 socket
dhclient 1247 root    4w   REG              252,2      802 539097 /var/db/dhclient.leases
dhclient 1247 root    5u  IPv4              20658      0t0    UDP *:bootpc 
dhclient 1247 root   20u  IPv4              20635      0t0    UDP *:43148 
dhclient 1247 root   21u  IPv6              20636      0t0    UDP *:37190 


Broadcasts get dropped when rp_filter is activated and no ip address
is bound for that interface. Do we want to relax the restriction for
broadcast so dhcp with sockets can be shipped by default?

Greetings,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Nicolas Dichtel]

Le 29/10/2013 13:40, Hannes Frederic Sowa a écrit :
> On Tue, Oct 29, 2013 at 09:37:06AM -0000, David Laight wrote:
>>> Note that you don't even need to put the DHCP protocol core into the
>>> kernel to fix the promiscuous problem.  You just have to use the
>>> current kernel interfaces correctly.
>>>
>>> It used to be the case a very long time ago that you couldn't even
>>> receive broadcast UDP datagrams on a socket until an address was
>>> configured on it.
>>>
>>> So everyone turns on promiscuous mode and uses RAW sockets or
>>> AF_PACKET.
>>>
>>> Stupid?  yes.
>>
>> Not only that, but the dhcp client could use a normal UDP socket
>> to keep the lease renewed - I suspect it has only ever needed
>> to use the BPF interface (I didn't think it set promiscuous)
>> when acquiring the initial lease.
>
> Yes, this is a very unfortunate situation. From my experience it is not that
> easy to get a patch merged into isc-dhcp.
>
> It seems not that invasive to switch from af_packet to an udp socket with
> SO_BROADCAST set.
If I remember well, another problem is to be able to send these packets with
0.0.0.0 when another IP address is available on the system:

RFC2131
4.1 Constructing and sending DHCP messages
...
    DHCP messages broadcast by a client prior to that client obtaining
    its IP address must have the source address field in the IP header
    set to 0.

We made a patch (never proposed upstream) to add a socket option to keep
this 0.0.0.0 address.
If people are interested, I can try to port it on net-next.

Nicolas

RE: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Laight]

> From: Nicolas Dichtel [mailto:nicolas.dichtel@6wind.com]
...
> If I remember well, another problem is to be able to send these packets with
> 0.0.0.0 when another IP address is available on the system:
> 
> RFC2131
> 4.1 Constructing and sending DHCP messages
> ...
>     DHCP messages broadcast by a client prior to that client obtaining
>     its IP address must have the source address field in the IP header
>     set to 0.
> 
> We made a patch (never proposed upstream) to add a socket option to keep
> this 0.0.0.0 address.
> If people are interested, I can try to port it on net-next.

Really, what dhcp does before the interface has an address doesn't
really matter - using bpf isn't a problem.

What it needs to do is switch to using a normal socket for the renewals.

Maybe I'll try asking Roy about it on the NetBSD lists.

	David

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Tue, Nov 05, 2013 at 05:12:46PM -0000, David Laight wrote:
> > From: Nicolas Dichtel [mailto:nicolas.dichtel@6wind.com]
> ...
> > If I remember well, another problem is to be able to send these packets with
> > 0.0.0.0 when another IP address is available on the system:
> > 
> > RFC2131
> > 4.1 Constructing and sending DHCP messages
> > ...
> >     DHCP messages broadcast by a client prior to that client obtaining
> >     its IP address must have the source address field in the IP header
> >     set to 0.
> > 
> > We made a patch (never proposed upstream) to add a socket option to keep
> > this 0.0.0.0 address.
> > If people are interested, I can try to port it on net-next.
> 
> Really, what dhcp does before the interface has an address doesn't
> really matter - using bpf isn't a problem.
> 
> What it needs to do is switch to using a normal socket for the renewals.
> 
> Maybe I'll try asking Roy about it on the NetBSD lists.

I would do the full switch to UDP sockets. It is much easier to accomplish
in the isc-dhcp codebase. It is e.g. already implemented and used e.g. by
Solaris. I would just have to wire up Nicolas INADDR_ANY source patch.

By the way:
Although dhclient uses an AF_PACKET socket, it never does request
additional multicast addresses or promisc mode. So the only overhead is
that we have to run the socket filter on each incoming packet.

Greetings,

  Hannes

RE: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Laight]

> By the way:
> Although dhclient uses an AF_PACKET socket, it never does request
> additional multicast addresses or promisc mode. So the only overhead is
> that we have to run the socket filter on each incoming packet.

Just creating the AF_PACKET socket requires that every receive
frame be duplicated.
On Linux this is a reference counted but the cost is still non-zero.
On some other OS (I first saw this with VxWorks) it is a full data copy.

	David

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Wed, Nov 06, 2013 at 09:23:05AM -0000, David Laight wrote:
> > By the way:
> > Although dhclient uses an AF_PACKET socket, it never does request
> > additional multicast addresses or promisc mode. So the only overhead is
> > that we have to run the socket filter on each incoming packet.
> 
> Just creating the AF_PACKET socket requires that every receive
> frame be duplicated.
> On Linux this is a reference counted but the cost is still non-zero.
> On some other OS (I first saw this with VxWorks) it is a full data copy.

Yes, I saw that, so my guess was that running the filter on every incoming UDP
packet would be the highes cost factor (it still does get filtered by protocol
number early in the stack).

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Hannes Frederic Sowa]

On Tue, Nov 05, 2013 at 06:02:17PM +0100, Nicolas Dichtel wrote:
> Le 29/10/2013 13:40, Hannes Frederic Sowa a écrit :
> >On Tue, Oct 29, 2013 at 09:37:06AM -0000, David Laight wrote:
> >>>Note that you don't even need to put the DHCP protocol core into the
> >>>kernel to fix the promiscuous problem.  You just have to use the
> >>>current kernel interfaces correctly.
> >>>
> >>>It used to be the case a very long time ago that you couldn't even
> >>>receive broadcast UDP datagrams on a socket until an address was
> >>>configured on it.
> >>>
> >>>So everyone turns on promiscuous mode and uses RAW sockets or
> >>>AF_PACKET.
> >>>
> >>>Stupid?  yes.
> >>
> >>Not only that, but the dhcp client could use a normal UDP socket
> >>to keep the lease renewed - I suspect it has only ever needed
> >>to use the BPF interface (I didn't think it set promiscuous)
> >>when acquiring the initial lease.
> >
> >Yes, this is a very unfortunate situation. From my experience it is not 
> >that
> >easy to get a patch merged into isc-dhcp.
> >
> >It seems not that invasive to switch from af_packet to an udp socket with
> >SO_BROADCAST set.
> If I remember well, another problem is to be able to send these packets with
> 0.0.0.0 when another IP address is available on the system:
> 
> RFC2131
> 4.1 Constructing and sending DHCP messages
> ...
>    DHCP messages broadcast by a client prior to that client obtaining
>    its IP address must have the source address field in the IP header
>    set to 0.
> 
> We made a patch (never proposed upstream) to add a socket option to keep
> this 0.0.0.0 address.
> If people are interested, I can try to port it on net-next.

Yes, this indeed is a problem for a socket only dhcp client. I would make the
appropriate changes to isc-dhcp if you submit such a patch.

Thanks,

  Hannes

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[Nicolas Dichtel]

Le 05/11/2013 21:57, Hannes Frederic Sowa a écrit :
> On Tue, Nov 05, 2013 at 06:02:17PM +0100, Nicolas Dichtel wrote:
>> Le 29/10/2013 13:40, Hannes Frederic Sowa a écrit :
>>> On Tue, Oct 29, 2013 at 09:37:06AM -0000, David Laight wrote:
>>>>> Note that you don't even need to put the DHCP protocol core into the
>>>>> kernel to fix the promiscuous problem.  You just have to use the
>>>>> current kernel interfaces correctly.
>>>>>
>>>>> It used to be the case a very long time ago that you couldn't even
>>>>> receive broadcast UDP datagrams on a socket until an address was
>>>>> configured on it.
>>>>>
>>>>> So everyone turns on promiscuous mode and uses RAW sockets or
>>>>> AF_PACKET.
>>>>>
>>>>> Stupid?  yes.
>>>>
>>>> Not only that, but the dhcp client could use a normal UDP socket
>>>> to keep the lease renewed - I suspect it has only ever needed
>>>> to use the BPF interface (I didn't think it set promiscuous)
>>>> when acquiring the initial lease.
>>>
>>> Yes, this is a very unfortunate situation. From my experience it is not
>>> that
>>> easy to get a patch merged into isc-dhcp.
>>>
>>> It seems not that invasive to switch from af_packet to an udp socket with
>>> SO_BROADCAST set.
>> If I remember well, another problem is to be able to send these packets with
>> 0.0.0.0 when another IP address is available on the system:
>>
>> RFC2131
>> 4.1 Constructing and sending DHCP messages
>> ...
>>     DHCP messages broadcast by a client prior to that client obtaining
>>     its IP address must have the source address field in the IP header
>>     set to 0.
>>
>> We made a patch (never proposed upstream) to add a socket option to keep
>> this 0.0.0.0 address.
>> If people are interested, I can try to port it on net-next.
>
> Yes, this indeed is a problem for a socket only dhcp client. I would make the
> appropriate changes to isc-dhcp if you submit such a patch.
Ok, I will try to do this for the end of the week.


Nicolas

[RFC PATCH net-next 1/2] ipv4: fix wildcard search with inet_confirm_addr()

[Nicolas Dichtel]

Help of this function says: "in_dev: only on this interface, 0=any interface",
but in fact the code supposes that it will never be NULL.

Note that this function was never called with in_dev == NULL, but it's exported
and may be used by an external module.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---

This patch has been submitted formally for net (some rework are needed), but the
next one is based on it, it's why I send it here.

 drivers/net/bonding/bonding.h | 10 +++-------
 include/linux/inetdevice.h    |  4 ++--
 net/ipv4/arp.c                |  3 ++-
 net/ipv4/devinet.c            |  8 ++++----
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 77a07a12e77f..203deaf30f83 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -377,15 +377,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
 
 static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local)
 {
-	struct in_device *in_dev;
-	__be32 addr = 0;
+	__be32 addr;
 
 	rcu_read_lock();
-	in_dev = __in_dev_get_rcu(dev);
-
-	if (in_dev)
-		addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
-
+	addr = inet_confirm_addr(dev_net(dev), __in_dev_get_rcu(dev), dst,
+				 local, RT_SCOPE_HOST);
 	rcu_read_unlock();
 	return addr;
 }
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 0d678aefe69d..f188e7598edd 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -164,8 +164,8 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
 void devinet_init(void);
 struct in_device *inetdev_by_index(struct net *, int);
 __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
-__be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local,
-			 int scope);
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
+			 __be32 local, int scope);
 struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix,
 				    __be32 mask);
 
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7808093cede6..fc5ebc7e1962 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -408,7 +408,8 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
 	default:
 		return 0;
 	}
-	return !inet_confirm_addr(in_dev, sip, tip, scope);
+	return !inet_confirm_addr(dev_net(in_dev->dev), in_dev, sip, tip,
+				  scope);
 }
 
 static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a1b5bcbd04ae..47cdb035f817 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1236,22 +1236,22 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
 
 /*
  * Confirm that local IP address exists using wildcards:
+ * - net: netns to check, cannot be NULL
  * - in_dev: only on this interface, 0=any interface
  * - dst: only in the same subnet as dst, 0=any dst
  * - local: address, 0=autoselect the local address
  * - scope: maximum allowed scope value for the local address
  */
-__be32 inet_confirm_addr(struct in_device *in_dev,
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
 			 __be32 dst, __be32 local, int scope)
 {
 	__be32 addr = 0;
 	struct net_device *dev;
-	struct net *net;
 
 	if (scope != RT_SCOPE_LINK)
-		return confirm_addr_indev(in_dev, dst, local, scope);
+		return in_dev ? confirm_addr_indev(in_dev, dst, local, scope) :
+				0;
 
-	net = dev_net(in_dev->dev);
 	rcu_read_lock();
 	for_each_netdev_rcu(net, dev) {
 		in_dev = __in_dev_get_rcu(dev);
-- 
1.8.4.1

[RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[Nicolas Dichtel]

This feature allows to a send packets with address source set to 0.0.0.0 even if
an ip address is available on another interface.

It's useful for DHCP client, to allow them to use UDP sockets and be compliant
with the RFC2131, Section 4.1:

4.1 Constructing and sending DHCP messages
...
   DHCP messages broadcast by a client prior to that client obtaining
   its IP address must have the source address field in the IP header
   set to 0.

Based on a previous work from
Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/linux/udp.h      |  3 ++-
 include/uapi/linux/udp.h |  1 +
 net/ipv4/udp.c           | 21 +++++++++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/linux/udp.h b/include/linux/udp.h
index 42278bbf7a88..ca947206f0fe 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -63,7 +63,8 @@ struct udp_sock {
 #define UDPLITE_SEND_CC  0x2  		/* set via udplite setsockopt         */
 #define UDPLITE_RECV_CC  0x4		/* set via udplite setsocktopt        */
 	__u8		 pcflag;        /* marks socket as UDP-Lite if > 0    */
-	__u8		 unused[3];
+	__u8		 src_any:1;	/* saddr to 0 when no IP is available */
+	__u8		 unused[2];
 	/*
 	 * For encapsulation sockets.
 	 */
diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
index e2bcfd75a30d..daf4a48face1 100644
--- a/include/uapi/linux/udp.h
+++ b/include/uapi/linux/udp.h
@@ -29,6 +29,7 @@ struct udphdr {
 /* UDP socket options */
 #define UDP_CORK	1	/* Never send partially complete segments */
 #define UDP_ENCAP	100	/* Set the socket to accept encapsulated packets */
+#define UDP_SRC_ANY	101	/* Set src addr to 0 when no IP is available */
 
 /* UDP encapsulation types */
 #define UDP_ENCAP_ESPINUDP_NON_IKE	1 /* draft-ietf-ipsec-nat-t-ike-00/01 */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 89909dd730dd..f58945187dbd 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -111,6 +111,7 @@
 #include <linux/static_key.h>
 #include <trace/events/skb.h>
 #include <net/busy_poll.h>
+#include <linux/inetdevice.h>
 #include "udp_impl.h"
 
 struct udp_table udp_table __read_mostly;
@@ -1015,6 +1016,18 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		goto do_confirm;
 back_from_confirm:
 
+	if (up->src_any && sk->sk_bound_dev_if) {
+		struct net_device *dev;
+		struct in_device *in_dev;
+
+		rcu_read_lock();
+		dev = dev_get_by_index_rcu(sock_net(sk), sk->sk_bound_dev_if);
+		in_dev = dev ? __in_dev_get_rcu(dev) : NULL;
+		if (!inet_confirm_addr(sock_net(sk), in_dev, 0, 0,
+				       RT_SCOPE_HOST))
+			fl4->saddr = 0;
+		rcu_read_unlock();
+	}
 	saddr = fl4->saddr;
 	if (!ipc.addr)
 		daddr = ipc.addr = fl4->daddr;
@@ -2045,6 +2058,10 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 		up->pcflag |= UDPLITE_RECV_CC;
 		break;
 
+	case UDP_SRC_ANY:
+		up->src_any = val ? 1 : 0;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -2107,6 +2124,10 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
 		val = up->pcrlen;
 		break;
 
+	case UDP_SRC_ANY:
+		val = up->src_any;
+		break;
+
 	default:
 		return -ENOPROTOOPT;
 	}
-- 
1.8.4.1

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[Julian Anastasov]

	Hello,

On Sat, 9 Nov 2013, Nicolas Dichtel wrote:

> This feature allows to a send packets with address source set to 0.0.0.0 even if
> an ip address is available on another interface.
> 
> It's useful for DHCP client, to allow them to use UDP sockets and be compliant
> with the RFC2131, Section 4.1:
> 
> 4.1 Constructing and sending DHCP messages
> ...
>    DHCP messages broadcast by a client prior to that client obtaining
>    its IP address must have the source address field in the IP header
>    set to 0.
> 
> Based on a previous work from
> Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

...

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 89909dd730dd..f58945187dbd 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c

...

> +	if (up->src_any && sk->sk_bound_dev_if) {
> +		struct net_device *dev;
> +		struct in_device *in_dev;
> +
> +		rcu_read_lock();
> +		dev = dev_get_by_index_rcu(sock_net(sk), sk->sk_bound_dev_if);
> +		in_dev = dev ? __in_dev_get_rcu(dev) : NULL;
> +		if (!inet_confirm_addr(sock_net(sk), in_dev, 0, 0,
> +				       RT_SCOPE_HOST))

	I don't have an opinion about UDP_SRC_ANY, just some
comments...

	Can a simple !in_dev->ifa_list check replace the
!inet_confirm_addr call? Looking at __inet_insert_ifa()
it seems only 0.0.0.0 does not add an ifa. Long ago
adding 0.0.0.0 was a way to create in_dev for dev but
now in_dev is created on device registration, i.e. even
before addresses are added.

	For the first patch, may be it is not needed.
We have two choices:

1. Do not change args and just fix comments. Of course,
it is tricky to use this function by using scope instead
of in_dev as a key for device-specific matching because
such interface is confusing.

2. Add 'net' arg and use in_dev as explained in my
previous email. Not sure if changing args of exported
function is acceptable.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[Nicolas Dichtel]

Le 09/11/2013 15:46, Julian Anastasov a écrit :
>
> 	Hello,
>
> On Sat, 9 Nov 2013, Nicolas Dichtel wrote:
>
>> This feature allows to a send packets with address source set to 0.0.0.0 even if
>> an ip address is available on another interface.
>>
>> It's useful for DHCP client, to allow them to use UDP sockets and be compliant
>> with the RFC2131, Section 4.1:
>>
>> 4.1 Constructing and sending DHCP messages
>> ...
>>     DHCP messages broadcast by a client prior to that client obtaining
>>     its IP address must have the source address field in the IP header
>>     set to 0.
>>
>> Based on a previous work from
>> Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> ...
>
>> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
>> index 89909dd730dd..f58945187dbd 100644
>> --- a/net/ipv4/udp.c
>> +++ b/net/ipv4/udp.c
>
> ...
>
>> +	if (up->src_any && sk->sk_bound_dev_if) {
>> +		struct net_device *dev;
>> +		struct in_device *in_dev;
>> +
>> +		rcu_read_lock();
>> +		dev = dev_get_by_index_rcu(sock_net(sk), sk->sk_bound_dev_if);
>> +		in_dev = dev ? __in_dev_get_rcu(dev) : NULL;
>> +		if (!inet_confirm_addr(sock_net(sk), in_dev, 0, 0,
>> +				       RT_SCOPE_HOST))
>
> 	I don't have an opinion about UDP_SRC_ANY, just some
> comments...
>
> 	Can a simple !in_dev->ifa_list check replace the
> !inet_confirm_addr call? Looking at __inet_insert_ifa()
> it seems only 0.0.0.0 does not add an ifa. Long ago
> adding 0.0.0.0 was a way to create in_dev for dev but
> now in_dev is created on device registration, i.e. even
> before addresses are added.
>
> 	For the first patch, may be it is not needed.
> We have two choices:
>
> 1. Do not change args and just fix comments. Of course,
> it is tricky to use this function by using scope instead
> of in_dev as a key for device-specific matching because
> such interface is confusing.
I hesitated to take this choice, but I think that keeping the
original behavior is better.

>
> 2. Add 'net' arg and use in_dev as explained in my
> previous email. Not sure if changing args of exported
> function is acceptable.
FWIK, it's not a problem.


Regards,
Nicolas

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Sat,  9 Nov 2013 01:54:34 +0100

> This feature allows to a send packets with address source set to 0.0.0.0 even if
> an ip address is available on another interface.
> 
> It's useful for DHCP client, to allow them to use UDP sockets and be compliant
> with the RFC2131, Section 4.1:
> 
> 4.1 Constructing and sending DHCP messages
> ...
>    DHCP messages broadcast by a client prior to that client obtaining
>    its IP address must have the source address field in the IP header
>    set to 0.
> 
> Based on a previous work from
> Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

This requirement of the RFC is inconsistent with a host based
addressing model, that which Linux employs, it assumes an interface
based one.

The wording here is also very ambiguous.

This RFC fails to even remotely consider what the right behavior
should be in a host based addressing environment at all, and anyone
reading this RFC should just accept that.

Furthermore, the fact that you're implementing _addressing_ policy in
the UDP code makes this change even more unreasonable.

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[Nicolas Dichtel]

Le 11/11/2013 06:18, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Sat,  9 Nov 2013 01:54:34 +0100
>
>> This feature allows to a send packets with address source set to 0.0.0.0 even if
>> an ip address is available on another interface.
>>
>> It's useful for DHCP client, to allow them to use UDP sockets and be compliant
>> with the RFC2131, Section 4.1:
>>
>> 4.1 Constructing and sending DHCP messages
>> ...
>>     DHCP messages broadcast by a client prior to that client obtaining
>>     its IP address must have the source address field in the IP header
>>     set to 0.
>>
>> Based on a previous work from
>> Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> This requirement of the RFC is inconsistent with a host based
> addressing model, that which Linux employs, it assumes an interface
> based one.
There are some exceptions, for example when user tunes arp_ignore sysctl ;-)

>
> The wording here is also very ambiguous.
>
> This RFC fails to even remotely consider what the right behavior
> should be in a host based addressing environment at all, and anyone
> reading this RFC should just accept that.
I agree that this is ambiguous. And it's a 'must', not a 'MUST', which
is not the same for an RFC ;-)

>
> Furthermore, the fact that you're implementing _addressing_ policy in
> the UDP code makes this change even more unreasonable.
>
I made this choice because using 0.0.0.0, for TCP for example, seems a
non-sense.

But fair enough, let's drop this patch.


Thank you,
Nicolas

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 14 Nov 2013 14:05:00 +0100

> Le 11/11/2013 06:18, David Miller a écrit :
>> Furthermore, the fact that you're implementing _addressing_ policy in
>> the UDP code makes this change even more unreasonable.
>>
> I made this choice because using 0.0.0.0, for TCP for example, seems a
> non-sense.

It's at the very least something applicable to all datagram sockets.

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[Nicolas Dichtel]

Le 14/11/2013 20:57, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Thu, 14 Nov 2013 14:05:00 +0100
>
>> Le 11/11/2013 06:18, David Miller a écrit :
>>> Furthermore, the fact that you're implementing _addressing_ policy in
>>> the UDP code makes this change even more unreasonable.
>>>
>> I made this choice because using 0.0.0.0, for TCP for example, seems a
>> non-sense.
>
> It's at the very least something applicable to all datagram sockets.
Yes, right.

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[Hannes Frederic Sowa]

On Mon, Nov 11, 2013 at 12:18:03AM -0500, David Miller wrote:
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Sat,  9 Nov 2013 01:54:34 +0100
> 
> > This feature allows to a send packets with address source set to 0.0.0.0 even if
> > an ip address is available on another interface.
> > 
> > It's useful for DHCP client, to allow them to use UDP sockets and be compliant
> > with the RFC2131, Section 4.1:
> > 
> > 4.1 Constructing and sending DHCP messages
> > ...
> >    DHCP messages broadcast by a client prior to that client obtaining
> >    its IP address must have the source address field in the IP header
> >    set to 0.
> > 
> > Based on a previous work from
> > Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.
> > 
> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> 
> This requirement of the RFC is inconsistent with a host based
> addressing model, that which Linux employs, it assumes an interface
> based one.
> 
> The wording here is also very ambiguous.
> 
> This RFC fails to even remotely consider what the right behavior
> should be in a host based addressing environment at all, and anyone
> reading this RFC should just accept that.
> 
> Furthermore, the fact that you're implementing _addressing_ policy in
> the UDP code makes this change even more unreasonable.

I agree on this and will check how other operating systems which only
use sockets do deal with that.

Do you have an idea how to deal with the rp_filter issue if no ip address is
set? Should we relax it in such cases?

Greetings,

  Hannes

Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 14 Nov 2013 15:31:54 +0100

> Do you have an idea how to deal with the rp_filter issue if no ip
> address is set? Should we relax it in such cases?

I think it is kind of pointless to protect ourselves from locally
generated packets, for which the flow ID was explicitly asked for
by the application.

So yes I think relaxing rp_filter in such cases makes sense.

Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag

[David Miller]

From: "David Laight" <David.Laight@ACULAB.COM>
Date: Tue, 29 Oct 2013 09:37:06 -0000

>> Note that you don't even need to put the DHCP protocol core into the
>> kernel to fix the promiscuous problem.  You just have to use the
>> current kernel interfaces correctly.
>> 
>> It used to be the case a very long time ago that you couldn't even
>> receive broadcast UDP datagrams on a socket until an address was
>> configured on it.
>> 
>> So everyone turns on promiscuous mode and uses RAW sockets or
>> AF_PACKET.
>> 
>> Stupid?  yes.
> 
> Not only that, but the dhcp client could use a normal UDP socket
> to keep the lease renewed - I suspect it has only ever needed
> to use the BPF interface (I didn't think it set promiscuous)
> when acquiring the initial lease.

You don't even need it for that, you can use a normal vanilla UDP
socket for the entire set of transactions.

When we initially seek a lease, we get our response in a broadcast
packet and the kernel is perfectly willing to receive that broadcast
packet, as stated, even when no IP address is assigned to the
interface.

The BPF program is only necessary if you don't use UDP sockets and use
RAW ones instead.  Look at what it's used for, it's purely matching
protocol UDP, port N.