[PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Francois-Xavier Le Bail]

This change allows to follow a recommandation of RFC4942.

- Add "anycast_src_echo_reply" sysctl to control the use of anycast addresses
  as source addresses for ICMPv6 echo reply. This sysctl is false by default
  to preserve existing behavior.
- Use it in icmpv6_echo_reply().

Reference:
RFC4942 - IPv6 Transition/Coexistence Security Considerations
   (http://tools.ietf.org/html/rfc4942#section-2.1.6)

2.1.6. Anycast Traffic Identification and Security

   [...]
   To avoid exposing knowledge about the internal structure of the
   network, it is recommended that anycast servers now take advantage of
   the ability to return responses with the anycast address as the
   source address if possible.

Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
---
v4: update Subject and Documentation, this work also with anycast addresses
    created via API, not just with Subnet-Router anycast addresses.

 Documentation/networking/ip-sysctl.txt |    7 +++++++
 include/net/netns/ipv6.h               |    1 +
 net/ipv6/icmp.c                        |    4 +++-
 net/ipv6/sysctl_net_ipv6.c             |    8 ++++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index d71afa8..7373115 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1094,6 +1094,13 @@ bindv6only - BOOLEAN
 
 	Default: FALSE (as specified in RFC3493)
 
+anycast_src_echo_reply - BOOLEAN
+	Controls the use of anycast addresses as source addresses for ICMPv6
+	echo reply
+	TRUE:  enabled
+	FALSE: disabled
+	Default: FALSE
+
 IPv6 Fragmentation:
 
 ip6frag_high_thresh - INTEGER
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 0fb2401..76fc7d1 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -73,6 +73,7 @@ struct netns_ipv6 {
 #endif
 	atomic_t		dev_addr_genid;
 	atomic_t		rt_genid;
+	int			anycast_src_echo_reply;
 };
 
 #if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 5d42009..65c8619 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 
 	saddr = &ipv6_hdr(skb)->daddr;
 
-	if (!ipv6_unicast_destination(skb))
+	if (!ipv6_unicast_destination(skb) &&
+	    !(net->ipv6.anycast_src_echo_reply &&
+	      ipv6_chk_acast_addr(net, NULL, saddr)))
 		saddr = NULL;
 
 	memcpy(&tmp_hdr, icmph, sizeof(tmp_hdr));
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 107b2f1..6b6a2c8 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -24,6 +24,13 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "anycast_src_echo_reply",
+		.data		= &init_net.ipv6.anycast_src_echo_reply,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
 	{ }
 };
 
@@ -51,6 +58,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
 	if (!ipv6_table)
 		goto out;
 	ipv6_table[0].data = &net->ipv6.sysctl.bindv6only;
+	ipv6_table[1].data = &net->ipv6.anycast_src_echo_reply;
 
 	ipv6_route_table = ipv6_route_sysctl_init(net);
 	if (!ipv6_route_table)

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Hannes Frederic Sowa]

On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index 5d42009..65c8619 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
>  
>  	saddr = &ipv6_hdr(skb)->daddr;
>  
> -	if (!ipv6_unicast_destination(skb))
> +	if (!ipv6_unicast_destination(skb) &&
> +	    !(net->ipv6.anycast_src_echo_reply &&
> +	      ipv6_chk_acast_addr(net, NULL, saddr)))
>  		saddr = NULL;

I am not sure why you left out the device at ipv6_chk_acast_addr?

IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
scope global but need to check the interface for scope link.

It is already possible via setsockopt JOIN_ANYCAST that an ll address is
anycast on another interface which may not be checked here.

Greetings,

  Hannes

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[François-Xavier Le Bail]

On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:

> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > index 5d42009..65c8619 100644
> > --- a/net/ipv6/icmp.c
> > +++ b/net/ipv6/icmp.c
> > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> >  
> >  	saddr = &ipv6_hdr(skb)->daddr;
> >  
> > -	if (!ipv6_unicast_destination(skb))
> > +	if (!ipv6_unicast_destination(skb) &&
> > +	    !(net->ipv6.anycast_src_echo_reply &&
> > +	      ipv6_chk_acast_addr(net, NULL, saddr)))
> >  		saddr = NULL;

> I am not sure why you left out the device at ipv6_chk_acast_addr?

> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> scope global but need to check the interface for scope link.

> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> anycast on another interface which may not be checked here.

In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.

Do you find some problem testing my patch ?

BR

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Hannes Frederic Sowa]

On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote:
> On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 
> > On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > > index 5d42009..65c8619 100644
> > > --- a/net/ipv6/icmp.c
> > > +++ b/net/ipv6/icmp.c
> > > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> > >  
> > >  	saddr = &ipv6_hdr(skb)->daddr;
> > >  
> > > -	if (!ipv6_unicast_destination(skb))
> > > +	if (!ipv6_unicast_destination(skb) &&
> > > +	    !(net->ipv6.anycast_src_echo_reply &&
> > > +	      ipv6_chk_acast_addr(net, NULL, saddr)))
> > >  		saddr = NULL;
> 
> > I am not sure why you left out the device at ipv6_chk_acast_addr?
> 
> > IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> > scope global but need to check the interface for scope link.
> 
> > It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> > anycast on another interface which may not be checked here.
> 
> In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.

A counter-example would be NOARP interfaces where we also don't speak ndisc.

> Do you find some problem testing my patch ?

A quick test I did yesterday looked good.

Greetings,

  Hannes

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[François-Xavier Le Bail]

On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:

> On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote:
> > On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> > 
> > > On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > > > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > > > index 5d42009..65c8619 100644
> > > > --- a/net/ipv6/icmp.c
> > > > +++ b/net/ipv6/icmp.c
> > > > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> > > >  
> > > >  	saddr = &ipv6_hdr(skb)->daddr;
> > > >  
> > > > -	if (!ipv6_unicast_destination(skb))
> > > > +	if (!ipv6_unicast_destination(skb) &&
> > > > +	    !(net->ipv6.anycast_src_echo_reply &&
> > > > +	      ipv6_chk_acast_addr(net, NULL, saddr)))
> > > >  		saddr = NULL;
> > 
> > > I am not sure why you left out the device at ipv6_chk_acast_addr?
> > 
> > > IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> > > scope global but need to check the interface for scope link.
> > 
> > > It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> > > anycast on another interface which may not be checked here.
> > 
> > In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.
> 
> A counter-example would be NOARP interfaces where we also don't speak ndisc.

Right. But if an echo request arrive on an interface which do not listen at this LL anycast, is a reply can occur ?

> > Do you find some problem testing my patch ?
> 
> A quick test I did yesterday looked good.

Thanks for the test.

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Hannes Frederic Sowa]

On Mon, Jan 06, 2014 at 05:48:16AM -0800, François-Xavier Le Bail wrote:
> On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 
> > On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote:
> > > On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> > > 
> > > > On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > > > > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > > > > index 5d42009..65c8619 100644
> > > > > --- a/net/ipv6/icmp.c
> > > > > +++ b/net/ipv6/icmp.c
> > > > > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> > > > >  
> > > > >  	saddr = &ipv6_hdr(skb)->daddr;
> > > > >  
> > > > > -	if (!ipv6_unicast_destination(skb))
> > > > > +	if (!ipv6_unicast_destination(skb) &&
> > > > > +	    !(net->ipv6.anycast_src_echo_reply &&
> > > > > +	      ipv6_chk_acast_addr(net, NULL, saddr)))
> > > > >  		saddr = NULL;
> > > 
> > > > I am not sure why you left out the device at ipv6_chk_acast_addr?
> > > 
> > > > IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> > > > scope global but need to check the interface for scope link.
> > > 
> > > > It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> > > > anycast on another interface which may not be checked here.
> > > 
> > > In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.
> > 
> > A counter-example would be NOARP interfaces where we also don't speak ndisc.
> 
> Right. But if an echo request arrive on an interface which do not listen at this LL anycast, is a reply can occur ?

I thought quite the opposite with anycast + forwarding enabled, that
we would process the packet. But a quick test with this snippet showed no
problems on a tunnel router. Local input is ok, too.

#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <net/if.h>

int main(int argc, char **argv)
{
  struct ipv6_mreq mreq = {0};
  int sockfd=socket(AF_INET6, SOCK_DGRAM,0);
  inet_pton(AF_INET6, "fe80::AAAA", &mreq.ipv6mr_multiaddr);
  mreq.ipv6mr_interface = if_nametoindex("dummy0");
  setsockopt(sockfd, IPPROTO_IPV6, IPV6_JOIN_ANYCAST, &mreq, sizeof(mreq));
  pause();
}

I guess, I would have added skb->dev for the sake of defensive style
(and maybe performance, if tables get large). I have not tested that
and it's your choice (or Davids, of course). But I currently don't see
a reason why it should break. ;)

Greetings,

  Hannes

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 6 Jan 2014 00:05:05 +0100

> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>> index 5d42009..65c8619 100644
>> --- a/net/ipv6/icmp.c
>> +++ b/net/ipv6/icmp.c
>> @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
>>  
>>  	saddr = &ipv6_hdr(skb)->daddr;
>>  
>> -	if (!ipv6_unicast_destination(skb))
>> +	if (!ipv6_unicast_destination(skb) &&
>> +	    !(net->ipv6.anycast_src_echo_reply &&
>> +	      ipv6_chk_acast_addr(net, NULL, saddr)))
>>  		saddr = NULL;
> 
> I am not sure why you left out the device at ipv6_chk_acast_addr?
> 
> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> scope global but need to check the interface for scope link.
> 
> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> anycast on another interface which may not be checked here.

I think we should pass a valid device in unless it breaks something
obvious.

Thanks.

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[François-Xavier Le Bail]

On Tue, 1/7/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:

>> Anyway, although I think that this solution is valid, I
>> am testing another way to do this change.
 
> Ok, thanks for explaining, I see now, of course.
 
> Maybe we could just do 
> (ipv6_addr_type(addr) & IPV6_ADDR_LINKLOCAL) ? skb->dev : NULL?
 
> I guess the NULL solution would be ok now, too. You can
> decide. I just  think we can be a bit more defensive here with no additional
> cost. Routing table behaviour is pretty complicated and maybe can change
> in future.
 
I see.
first solution : I add this "defensive" update.
second solution : the v5 patch I will send soon.

Thanks for the review.
François-Xavier

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Hannes Frederic Sowa]

On Tue, Jan 07, 2014 at 04:38:38AM -0800, François-Xavier Le Bail wrote:
> On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 
> >>But if an echo request arrive on an interface
> >> which do not listen at this LL anycast, is a reply can occur ?
>  
> > I thought quite the opposite with anycast + forwarding
> > enabled, that  we would process the packet.
> 
> yes, for a global scope anycast.
> But a routers must not forward any packets with link-local source or destination addresses to other links, thus also ll anycast.

Of course, but in this case I hesitated because I haven't dealt a lot
with anycast recently in routing code and it seemed like RTF_ANYCAST is
exclusive-or with RTF_LOCAL. So I was really unsure if this protection
was really in place. We already had some illegal forwarding/input paths
in the past. ;)

Greetings,

  Hannes

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Hannes Frederic Sowa]

On Tue, Jan 07, 2014 at 04:38:22AM -0800, François-Xavier Le Bail wrote:
> On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 
> 
> > > > I think we should pass a valid device in unless it
> > > > breaks something obvious.
> 
> > > The problem is that "saddr" is not necessarily an address on "skb->dev"
> > > in icmpv6_echo_reply(). It may be an address on another interface.
>  
> > Maybe you're right, but then I don't get it. Could you make
> > an example?
> 
> Yes :
> box with eth1, eth2, forwarding enable.
> 
> echo request arrives on eth2
> -------------------------------> eth2 (2a01:3::1)  {forwarding enable}   eth1 (2a01:2::1)
> 
> if dest == 2a01:3::
> Jan  7 10:36:36 localhost kernel: [   59.155376] icmpv6_echo_reply: saddr == 2a01:3::
> Jan  7 10:36:36 localhost kernel: [   59.155395] icmpv6_echo_reply: skb->dev->name == eth2
> Jan  7 10:36:36 localhost kernel: [   59.155398] icmpv6_echo_reply: ipv6_chk_acast_addr(net, skb->dev, saddr) == 1
> Jan  7 10:36:36 localhost kernel: [   59.155400] icmpv6_echo_reply: ipv6_chk_acast_addr(net, NULL, saddr) == 1
> if dest == 2a01:2::
> Jan  7 10:36:46 localhost kernel: [   68.807565] icmpv6_echo_reply: saddr == 2a01:2::
> Jan  7 10:36:46 localhost kernel: [   68.807580] icmpv6_echo_reply: skb->dev->name == eth2
> Jan  7 10:36:46 localhost kernel: [   68.807583] icmpv6_echo_reply: ipv6_chk_acast_addr(net, skb->dev, saddr) == 0
> Jan  7 10:36:46 localhost kernel: [   68.807586] icmpv6_echo_reply: ipv6_chk_acast_addr(net, NULL, saddr) == 1
> 
> So, as 2a01:2:: is a address on eth1, ipv6_chk_acast_addr(net, skb->dev, saddr) with dev == eth2 return 0.
> 
> It is the reason why I use dev == NULL.
> 
> Do your tests show something different ?

I haven't tested this particular setup.

> 
> Anyway, although I think that this solution is valid, I am testing another way to do this change.

Ok, thanks for explaining, I see now, of course.

Maybe we could just do (ipv6_addr_type(addr) & IPV6_ADDR_LINKLOCAL) ? skb->dev
: NULL?

I guess the NULL solution would be ok now, too. You can decide. I just
think we can be a bit more defensive here with no additional cost. Routing
table behaviour is pretty complicated and maybe can change in future.

Thank you,

  Hannes

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[François-Xavier Le Bail]

On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:

>>But if an echo request arrive on an interface
>> which do not listen at this LL anycast, is a reply can occur ?
 
> I thought quite the opposite with anycast + forwarding
> enabled, that  we would process the packet.

yes, for a global scope anycast.
But a routers must not forward any packets with link-local source or destination addresses to other links, thus also ll anycast.

BR
François-Xavier

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[François-Xavier Le Bail]

On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:


> > > I think we should pass a valid device in unless it
> > > breaks something obvious.

> > The problem is that "saddr" is not necessarily an address on "skb->dev"
> > in icmpv6_echo_reply(). It may be an address on another interface.
 
> Maybe you're right, but then I don't get it. Could you make
> an example?

Yes :
box with eth1, eth2, forwarding enable.

echo request arrives on eth2
-------------------------------> eth2 (2a01:3::1)  {forwarding enable}   eth1 (2a01:2::1)

if dest == 2a01:3::
Jan  7 10:36:36 localhost kernel: [   59.155376] icmpv6_echo_reply: saddr == 2a01:3::
Jan  7 10:36:36 localhost kernel: [   59.155395] icmpv6_echo_reply: skb->dev->name == eth2
Jan  7 10:36:36 localhost kernel: [   59.155398] icmpv6_echo_reply: ipv6_chk_acast_addr(net, skb->dev, saddr) == 1
Jan  7 10:36:36 localhost kernel: [   59.155400] icmpv6_echo_reply: ipv6_chk_acast_addr(net, NULL, saddr) == 1
if dest == 2a01:2::
Jan  7 10:36:46 localhost kernel: [   68.807565] icmpv6_echo_reply: saddr == 2a01:2::
Jan  7 10:36:46 localhost kernel: [   68.807580] icmpv6_echo_reply: skb->dev->name == eth2
Jan  7 10:36:46 localhost kernel: [   68.807583] icmpv6_echo_reply: ipv6_chk_acast_addr(net, skb->dev, saddr) == 0
Jan  7 10:36:46 localhost kernel: [   68.807586] icmpv6_echo_reply: ipv6_chk_acast_addr(net, NULL, saddr) == 1

So, as 2a01:2:: is a address on eth1, ipv6_chk_acast_addr(net, skb->dev, saddr) with dev == eth2 return 0.

It is the reason why I use dev == NULL.

Do your tests show something different ?

Anyway, although I think that this solution is valid, I am testing another way to do this change.

BR,
Francois-Xavier

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[Hannes Frederic Sowa]

On Mon, Jan 06, 2014 at 01:53:49PM -0800, François-Xavier Le Bail wrote:
> On Mon, 1/6/14, David Miller <davem@davemloft.net> wrote:
> 
> > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Date: Mon, 6 Jan 2014 00:05:05 +0100
> 
> >> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> >>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> >>> index 5d42009..65c8619 100644
> >>> --- a/net/ipv6/icmp.c
> >>> +++ b/net/ipv6/icmp.c
> >>> @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> >>>  
> >>>      saddr = &ipv6_hdr(skb)->daddr;
> >>>  
> >>> -    if (!ipv6_unicast_destination(skb))
> >>> +    if (!ipv6_unicast_destination(skb) &&
> >>> +        !(net->ipv6.anycast_src_echo_reply &&
> >>> +          ipv6_chk_acast_addr(net, NULL, saddr)))
> >>>          saddr = NULL;
> >> 
> >> I am not sure why you left out the device at ipv6_chk_acast_addr?
> >> 
> >> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> >> scope global but need to check the interface for scope link.
> >> 
> >> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> >> anycast on another interface which may not be checked here.
> 
> > I think we should pass a valid device in unless it breaks something
> > obvious.
> 
> The problem is that "saddr" is not necessarily an address on "skb->dev"
> in icmpv6_echo_reply(). It may be an address on another interface.

Maybe you're right, but then I don't get it. Could you make an example?

Thanks,

  Hannes

Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply

[François-Xavier Le Bail]

On Mon, 1/6/14, David Miller <davem@davemloft.net> wrote:

> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Mon, 6 Jan 2014 00:05:05 +0100

>> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
>>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>>> index 5d42009..65c8619 100644
>>> --- a/net/ipv6/icmp.c
>>> +++ b/net/ipv6/icmp.c
>>> @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
>>>  
>>>      saddr = &ipv6_hdr(skb)->daddr;
>>>  
>>> -    if (!ipv6_unicast_destination(skb))
>>> +    if (!ipv6_unicast_destination(skb) &&
>>> +        !(net->ipv6.anycast_src_echo_reply &&
>>> +          ipv6_chk_acast_addr(net, NULL, saddr)))
>>>          saddr = NULL;
>> 
>> I am not sure why you left out the device at ipv6_chk_acast_addr?
>> 
>> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
>> scope global but need to check the interface for scope link.
>> 
>> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
>> anycast on another interface which may not be checked here.

> I think we should pass a valid device in unless it breaks something
> obvious.

The problem is that "saddr" is not necessarily an address on "skb->dev"
in icmpv6_echo_reply(). It may be an address on another interface.

If dev arg is NULL, ipv6_chk_acast_addr() searches all devices for "saddr", that what we need.

It works for global scope and link-local anycast address.

So passing a valid device to ipv6_chk_acast_addr() breaks the goal of the patch.

Thanks.