All of lore.kernel.org
 help / color / mirror / Atom feed
* [IPv6] interface-local multicast escapes the local node
@ 2013-02-06  8:49 Erik Hugne
  2013-02-06 12:12 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 28+ messages in thread
From: Erik Hugne @ 2013-02-06  8:49 UTC (permalink / raw)
  To: netdev

It seems that packets sent to interface-local scoped multicast
addresses (ff01::/16) escape out on the network.
According to RFC4291 (section 2.7):
    Interface-Local scope spans only a single interface on a node
    and is useful only for loopback transmission of multicast.

Example program here:
https://gist.github.com/Hugne/4721151

//E

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06  8:49 [IPv6] interface-local multicast escapes the local node Erik Hugne
@ 2013-02-06 12:12 ` Hannes Frederic Sowa
  2013-02-06 13:07   ` Erik Hugne
  2013-02-06 15:24   ` YOSHIFUJI Hideaki
  0 siblings, 2 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-06 12:12 UTC (permalink / raw)
  To: Erik Hugne; +Cc: netdev, yoshfuji

On Wed, Feb 06, 2013 at 09:49:49AM +0100, Erik Hugne wrote:
> It seems that packets sent to interface-local scoped multicast
> addresses (ff01::/16) escape out on the network.
> According to RFC4291 (section 2.7):
>     Interface-Local scope spans only a single interface on a node
>     and is useful only for loopback transmission of multicast.
> 
> Example program here:
> https://gist.github.com/Hugne/4721151

Fixing the output path should be relatively straightforward, please test
the following patch.

Looking at the input path, I think there is also no input protection
for ff01::/16 addresses from the wire if you bind such an address.

[PATCH net-next] ipv6: don't let node/interface scoped multicast traffic escape on the wire

Reported-by: Erik Hugne <erik.hugne@ericsson.com>
Cc: Erik Hugne <erik.hugne@ericsson.com>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/ip6_output.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 906b7e6..b21ff3d 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -118,6 +118,12 @@ static int ip6_finish_output2(struct sk_buff *skb)
 			}
 		}
 
+		if (IPV6_ADDR_MC_SCOPE(&ipv6_hdr(skb)->daddr) <=
+		    IPV6_ADDR_SCOPE_NODELOCAL) {
+			kfree_skb(skb);
+			return 0;
+		}
+
 		IP6_UPD_PO_STATS(dev_net(dev), idev, IPSTATS_MIB_OUTMCAST,
 				skb->len);
 	}
-- 
1.8.1

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 12:12 ` Hannes Frederic Sowa
@ 2013-02-06 13:07   ` Erik Hugne
  2013-02-06 13:49     ` Hannes Frederic Sowa
  2013-02-06 15:24   ` YOSHIFUJI Hideaki
  1 sibling, 1 reply; 28+ messages in thread
From: Erik Hugne @ 2013-02-06 13:07 UTC (permalink / raw)
  To: netdev, yoshfuji

On Wed, Feb 06, 2013 at 01:12:48PM +0100, Hannes Frederic Sowa wrote:
> Fixing the output path should be relatively straightforward, please test
> the following patch.

Tested OK.

> Looking at the input path, I think there is also no input protection
> for ff01::/16 addresses from the wire if you bind such an address.

Yes, this needs to be filtered on the input side aswell.

//E

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 13:07   ` Erik Hugne
@ 2013-02-06 13:49     ` Hannes Frederic Sowa
  2013-02-06 15:06       ` Erik Hugne
  2013-02-06 15:32       ` YOSHIFUJI Hideaki
  0 siblings, 2 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-06 13:49 UTC (permalink / raw)
  To: Erik Hugne; +Cc: netdev, yoshfuji

On Wed, Feb 06, 2013 at 02:07:39PM +0100, Erik Hugne wrote:
> On Wed, Feb 06, 2013 at 01:12:48PM +0100, Hannes Frederic Sowa wrote:
> > Fixing the output path should be relatively straightforward, please test
> > the following patch.
> 
> Tested OK.
> 
> > Looking at the input path, I think there is also no input protection
> > for ff01::/16 addresses from the wire if you bind such an address.
> 
> Yes, this needs to be filtered on the input side aswell.

This patch should do the trick. Perhaps you could also take it for a test
drive? Thanks!

[PATCH net-next] ipv6: don't accept node local multicast traffic from the wire

Cc: Erik Hugne <erik.hugne@ericsson.com>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/ip6_input.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 4ac5bf3..a2f71d2 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -126,6 +126,16 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
 	if (ipv6_addr_is_multicast(&hdr->saddr))
 		goto err;
 
+	/*
+	 * RFC4291 2.7
+	 * Interface-Local scope spans only a single interface on a node
+         * and is useful only for loopback transmission of multicast.
+	 */
+	if (ipv6_addr_is_multicast(&hdr->daddr) &&
+	    IPV6_ADDR_MC_SCOPE(&hdr->daddr) <= IPV6_ADDR_SCOPE_NODELOCAL &&
+	    skb->pkt_type != PACKET_LOOPBACK)
+		goto err;
+
 	skb->transport_header = skb->network_header + sizeof(*hdr);
 	IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
 
-- 
1.8.1

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 13:49     ` Hannes Frederic Sowa
@ 2013-02-06 15:06       ` Erik Hugne
  2013-02-06 15:12         ` Hannes Frederic Sowa
  2013-02-06 15:32       ` YOSHIFUJI Hideaki
  1 sibling, 1 reply; 28+ messages in thread
From: Erik Hugne @ 2013-02-06 15:06 UTC (permalink / raw)
  To: netdev, yoshfuji

> This patch should do the trick. Perhaps you could also take it for a test
> drive? Thanks!

Verified, ff01::/16 from external hosts is blocked.

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 15:06       ` Erik Hugne
@ 2013-02-06 15:12         ` Hannes Frederic Sowa
  0 siblings, 0 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-06 15:12 UTC (permalink / raw)
  To: Erik Hugne; +Cc: netdev, yoshfuji

On Wed, Feb 06, 2013 at 04:06:15PM +0100, Erik Hugne wrote:
> > This patch should do the trick. Perhaps you could also take it for a test
> > drive? Thanks!
> 
> Verified, ff01::/16 from external hosts is blocked.

Thanks for testing. Will do proper patch submissions later with whitespace
issue fixed.

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 12:12 ` Hannes Frederic Sowa
  2013-02-06 13:07   ` Erik Hugne
@ 2013-02-06 15:24   ` YOSHIFUJI Hideaki
  2013-02-06 16:54     ` Hannes Frederic Sowa
  1 sibling, 1 reply; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-06 15:24 UTC (permalink / raw)
  To: Erik Hugne, netdev, YOSHIFUJI Hideaki, erik.hugne

Hannes Frederic Sowa wrote:
> On Wed, Feb 06, 2013 at 09:49:49AM +0100, Erik Hugne wrote:
>> It seems that packets sent to interface-local scoped multicast
>> addresses (ff01::/16) escape out on the network.
>> According to RFC4291 (section 2.7):
>>     Interface-Local scope spans only a single interface on a node
>>     and is useful only for loopback transmission of multicast.
>>
>> Example program here:
>> https://gist.github.com/Hugne/4721151
> 
> Fixing the output path should be relatively straightforward, please test
> the following patch.
> 
> Looking at the input path, I think there is also no input protection
> for ff01::/16 addresses from the wire if you bind such an address.
> 
> [PATCH net-next] ipv6: don't let node/interface scoped multicast traffic escape on the wire
> 
> Reported-by: Erik Hugne <erik.hugne@ericsson.com>
> Cc: Erik Hugne <erik.hugne@ericsson.com>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/ip6_output.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 906b7e6..b21ff3d 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -118,6 +118,12 @@ static int ip6_finish_output2(struct sk_buff *skb)
>  			}
>  		}
>  
> +		if (IPV6_ADDR_MC_SCOPE(&ipv6_hdr(skb)->daddr) <=
> +		    IPV6_ADDR_SCOPE_NODELOCAL) {
> +			kfree_skb(skb);
> +			return 0;
> +		}
> +
>  		IP6_UPD_PO_STATS(dev_net(dev), idev, IPSTATS_MIB_OUTMCAST,
>  				skb->len);
>  	}
> 

NAK.  I think we should select routes via loopback device here.

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 13:49     ` Hannes Frederic Sowa
  2013-02-06 15:06       ` Erik Hugne
@ 2013-02-06 15:32       ` YOSHIFUJI Hideaki
  2013-02-06 16:04         ` Hannes Frederic Sowa
  1 sibling, 1 reply; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-06 15:32 UTC (permalink / raw)
  To: Erik Hugne, netdev, YOSHIFUJI Hideaki

Hannes Frederic Sowa wrote:
> On Wed, Feb 06, 2013 at 02:07:39PM +0100, Erik Hugne wrote:
>> On Wed, Feb 06, 2013 at 01:12:48PM +0100, Hannes Frederic Sowa wrote:
>>> Fixing the output path should be relatively straightforward, please test
>>> the following patch.
>>
>> Tested OK.
>>
>>> Looking at the input path, I think there is also no input protection
>>> for ff01::/16 addresses from the wire if you bind such an address.
>>
>> Yes, this needs to be filtered on the input side aswell.
> 
> This patch should do the trick. Perhaps you could also take it for a test
> drive? Thanks!
> 
> [PATCH net-next] ipv6: don't accept node local multicast traffic from the wire
> 
> Cc: Erik Hugne <erik.hugne@ericsson.com>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/ip6_input.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index 4ac5bf3..a2f71d2 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -126,6 +126,16 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
>  	if (ipv6_addr_is_multicast(&hdr->saddr))
>  		goto err;
>  
> +	/*
> +	 * RFC4291 2.7
> +	 * Interface-Local scope spans only a single interface on a node
> +         * and is useful only for loopback transmission of multicast.
> +	 */
> +	if (ipv6_addr_is_multicast(&hdr->daddr) &&
> +	    IPV6_ADDR_MC_SCOPE(&hdr->daddr) <= IPV6_ADDR_SCOPE_NODELOCAL &&
> +	    skb->pkt_type != PACKET_LOOPBACK)
> +		goto err;
> +
>  	skb->transport_header = skb->network_header + sizeof(*hdr);
>  	IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);

NAK.

Well, do you have relevant RFC?
RFC4291 says that we should drop ff00::/16, but not ff01::/16.

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 15:32       ` YOSHIFUJI Hideaki
@ 2013-02-06 16:04         ` Hannes Frederic Sowa
  2013-02-06 16:18           ` Eric Dumazet
  2013-02-06 16:49           ` YOSHIFUJI Hideaki
  0 siblings, 2 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-06 16:04 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev

On Thu, Feb 07, 2013 at 12:32:17AM +0900, YOSHIFUJI Hideaki wrote:
> Hannes Frederic Sowa wrote:
> > On Wed, Feb 06, 2013 at 02:07:39PM +0100, Erik Hugne wrote:
> >> On Wed, Feb 06, 2013 at 01:12:48PM +0100, Hannes Frederic Sowa wrote:
> >>> Fixing the output path should be relatively straightforward, please test
> >>> the following patch.
> >>
> >> Tested OK.
> >>
> >>> Looking at the input path, I think there is also no input protection
> >>> for ff01::/16 addresses from the wire if you bind such an address.
> >>
> >> Yes, this needs to be filtered on the input side aswell.
> > 
> > This patch should do the trick. Perhaps you could also take it for a test
> > drive? Thanks!
> > 
> > [PATCH net-next] ipv6: don't accept node local multicast traffic from the wire
> > 
> > Cc: Erik Hugne <erik.hugne@ericsson.com>
> > Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > ---
> >  net/ipv6/ip6_input.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> > index 4ac5bf3..a2f71d2 100644
> > --- a/net/ipv6/ip6_input.c
> > +++ b/net/ipv6/ip6_input.c
> > @@ -126,6 +126,16 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
> >  	if (ipv6_addr_is_multicast(&hdr->saddr))
> >  		goto err;
> >  
> > +	/*
> > +	 * RFC4291 2.7
> > +	 * Interface-Local scope spans only a single interface on a node
> > +         * and is useful only for loopback transmission of multicast.
> > +	 */
> > +	if (ipv6_addr_is_multicast(&hdr->daddr) &&
> > +	    IPV6_ADDR_MC_SCOPE(&hdr->daddr) <= IPV6_ADDR_SCOPE_NODELOCAL &&
> > +	    skb->pkt_type != PACKET_LOOPBACK)
> > +		goto err;
> > +
> >  	skb->transport_header = skb->network_header + sizeof(*hdr);
> >  	IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
> 
> NAK.
> 
> Well, do you have relevant RFC?
> RFC4291 says that we should drop ff00::/16, but not ff01::/16.

I know what you mean, the RFC does not state it directly. Hm, the BSDs seem to
drop such destination addresses, too, if they don't originate from a loopback
interface. Or did you mean that there is a flaw in the skb->pkt_type !=
PACKET_LOOPBACK condition?

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 16:04         ` Hannes Frederic Sowa
@ 2013-02-06 16:18           ` Eric Dumazet
  2013-02-06 16:25             ` Hannes Frederic Sowa
  2013-02-06 16:49           ` YOSHIFUJI Hideaki
  1 sibling, 1 reply; 28+ messages in thread
From: Eric Dumazet @ 2013-02-06 16:18 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: YOSHIFUJI Hideaki, Erik Hugne, netdev

On Wed, 2013-02-06 at 17:04 +0100, Hannes Frederic Sowa wrote:
> On Thu, Feb 07, 2013 at 12:32:17AM +0900, YOSHIFUJI Hideaki wrote:

> > Well, do you have relevant RFC?
> > RFC4291 says that we should drop ff00::/16, but not ff01::/16.
> 
> I know what you mean, the RFC does not state it directly. Hm, the BSDs seem to
> drop such destination addresses, too, if they don't originate from a loopback
> interface. Or did you mean that there is a flaw in the skb->pkt_type !=
> PACKET_LOOPBACK condition?

RFC 4291 states on page 13 :

 Interface-Local scope spans only a single interface on a node
 and is useful only for loopback transmission of multicast.

ff01::/16 are Interface-Local, or am I missing something ?

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 16:18           ` Eric Dumazet
@ 2013-02-06 16:25             ` Hannes Frederic Sowa
  0 siblings, 0 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-06 16:25 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: YOSHIFUJI Hideaki, Erik Hugne, netdev

On Wed, Feb 06, 2013 at 08:18:47AM -0800, Eric Dumazet wrote:
> On Wed, 2013-02-06 at 17:04 +0100, Hannes Frederic Sowa wrote:
> > On Thu, Feb 07, 2013 at 12:32:17AM +0900, YOSHIFUJI Hideaki wrote:
> 
> > > Well, do you have relevant RFC?
> > > RFC4291 says that we should drop ff00::/16, but not ff01::/16.
> > 
> > I know what you mean, the RFC does not state it directly. Hm, the BSDs seem to
> > drop such destination addresses, too, if they don't originate from a loopback
> > interface. Or did you mean that there is a flaw in the skb->pkt_type !=
> > PACKET_LOOPBACK condition?
> 
> RFC 4291 states on page 13 :
> 
>  Interface-Local scope spans only a single interface on a node
>  and is useful only for loopback transmission of multicast.
> 
> ff01::/16 are Interface-Local, or am I missing something ?

No, that's correct.

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 16:04         ` Hannes Frederic Sowa
  2013-02-06 16:18           ` Eric Dumazet
@ 2013-02-06 16:49           ` YOSHIFUJI Hideaki
  2013-02-07  1:00             ` Hannes Frederic Sowa
  1 sibling, 1 reply; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-06 16:49 UTC (permalink / raw)
  To: Erik Hugne, netdev, YOSHIFUJI Hideaki

(2013年02月07日 01:04), Hannes Frederic Sowa wrote:
> On Thu, Feb 07, 2013 at 12:32:17AM +0900, YOSHIFUJI Hideaki wrote:
>> Hannes Frederic Sowa wrote:
>>> On Wed, Feb 06, 2013 at 02:07:39PM +0100, Erik Hugne wrote:
>>>> On Wed, Feb 06, 2013 at 01:12:48PM +0100, Hannes Frederic Sowa wrote:
>>>>> Fixing the output path should be relatively straightforward, please test
>>>>> the following patch.
>>>>
>>>> Tested OK.
>>>>
>>>>> Looking at the input path, I think there is also no input protection
>>>>> for ff01::/16 addresses from the wire if you bind such an address.
>>>>
>>>> Yes, this needs to be filtered on the input side aswell.
>>>
>>> This patch should do the trick. Perhaps you could also take it for a test
>>> drive? Thanks!
>>>
>>> [PATCH net-next] ipv6: don't accept node local multicast traffic from the wire
>>>
>>> Cc: Erik Hugne <erik.hugne@ericsson.com>
>>> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>> ---
>>>  net/ipv6/ip6_input.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
>>> index 4ac5bf3..a2f71d2 100644
>>> --- a/net/ipv6/ip6_input.c
>>> +++ b/net/ipv6/ip6_input.c
>>> @@ -126,6 +126,16 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
>>>  	if (ipv6_addr_is_multicast(&hdr->saddr))
>>>  		goto err;
>>>  
>>> +	/*
>>> +	 * RFC4291 2.7
>>> +	 * Interface-Local scope spans only a single interface on a node
>>> +         * and is useful only for loopback transmission of multicast.
>>> +	 */
>>> +	if (ipv6_addr_is_multicast(&hdr->daddr) &&
>>> +	    IPV6_ADDR_MC_SCOPE(&hdr->daddr) <= IPV6_ADDR_SCOPE_NODELOCAL &&
>>> +	    skb->pkt_type != PACKET_LOOPBACK)
>>> +		goto err;
>>> +
>>>  	skb->transport_header = skb->network_header + sizeof(*hdr);
>>>  	IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
>>
>> NAK.
>>
>> Well, do you have relevant RFC?
>> RFC4291 says that we should drop ff00::/16, but not ff01::/16.
> 
> I know what you mean, the RFC does not state it directly. Hm, the BSDs seem to
> drop such destination addresses, too, if they don't originate from a loopback
> interface. Or did you mean that there is a flaw in the skb->pkt_type !=
> PACKET_LOOPBACK condition?

We do not drop ff01::/16, because RFC is silent about it.

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 15:24   ` YOSHIFUJI Hideaki
@ 2013-02-06 16:54     ` Hannes Frederic Sowa
  2013-02-09 12:10       ` Hannes Frederic Sowa
  0 siblings, 1 reply; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-06 16:54 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev

On Thu, Feb 07, 2013 at 12:24:14AM +0900, YOSHIFUJI Hideaki wrote:
> NAK.  I think we should select routes via loopback device here.

Will try your idea, thanks.

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 16:49           ` YOSHIFUJI Hideaki
@ 2013-02-07  1:00             ` Hannes Frederic Sowa
  2013-02-07  7:28               ` Erik Hugne
  0 siblings, 1 reply; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-07  1:00 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev

On Thu, Feb 07, 2013 at 01:49:09AM +0900, YOSHIFUJI Hideaki wrote:
> >>
> >> NAK.
> >>
> >> Well, do you have relevant RFC?
> >> RFC4291 says that we should drop ff00::/16, but not ff01::/16.
> > 
> > I know what you mean, the RFC does not state it directly. Hm, the BSDs seem to
> > drop such destination addresses, too, if they don't originate from a loopback
> > interface. Or did you mean that there is a flaw in the skb->pkt_type !=
> > PACKET_LOOPBACK condition?
> 
> We do not drop ff01::/16, because RFC is silent about it.

I just did a little bit of research on this topic and found this:

RFC4541 3. IPv6 Considerations:

   MLD messages are also not sent regarding groups with addresses in the
   range FF00::/15 (which encompasses both the reserved FF00::/16 and
   node-local FF01::/16 IPv6 address spaces).  These addresses should
   never appear in packets on the link.

It gives a strong indication that we should drop these packets. What do you
think?

Thanks,

  Hannes

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-07  1:00             ` Hannes Frederic Sowa
@ 2013-02-07  7:28               ` Erik Hugne
  2013-02-07 15:41                 ` YOSHIFUJI Hideaki
  0 siblings, 1 reply; 28+ messages in thread
From: Erik Hugne @ 2013-02-07  7:28 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki, netdev

On Thu, Feb 07, 2013 at 02:00:10AM +0100, Hannes Frederic Sowa wrote:
> RFC4541 3. IPv6 Considerations:
> 
>    MLD messages are also not sent regarding groups with addresses in the
>    range FF00::/15 (which encompasses both the reserved FF00::/16 and
>    node-local FF01::/16 IPv6 address spaces).  These addresses should
>    never appear in packets on the link.
> 
> It gives a strong indication that we should drop these packets. What do you
> think?

That was my understanding aswell. I asked for some clarifications on the ietf list:
    <quote>
    > The key is that the packets should be dropped without generating
    > any errors or traffic.
    Yep, you are right - I assumed that implicitly, but
    I should have mentioned that explicitly :-)
    </quote>
http://www.ietf.org/mail-archive/web/ipv6/current/msg17126.html

//E 

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-07  7:28               ` Erik Hugne
@ 2013-02-07 15:41                 ` YOSHIFUJI Hideaki
  0 siblings, 0 replies; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-07 15:41 UTC (permalink / raw)
  To: Erik Hugne; +Cc: netdev, YOSHIFUJI Hideaki

Erik Hugne wrote:
> On Thu, Feb 07, 2013 at 02:00:10AM +0100, Hannes Frederic Sowa wrote:
>> RFC4541 3. IPv6 Considerations:
>>
>>    MLD messages are also not sent regarding groups with addresses in the
>>    range FF00::/15 (which encompasses both the reserved FF00::/16 and
>>    node-local FF01::/16 IPv6 address spaces).  These addresses should
>>    never appear in packets on the link.
>>
>> It gives a strong indication that we should drop these packets. What do you
>> think?
> 
> That was my understanding aswell. I asked for some clarifications on the ietf list:
>     <quote>
>     > The key is that the packets should be dropped without generating
>     > any errors or traffic.
>     Yep, you are right - I assumed that implicitly, but
>     I should have mentioned that explicitly :-)
>     </quote>
> http://www.ietf.org/mail-archive/web/ipv6/current/msg17126.html

Please file errata on RFC4291.  Thanks.

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-06 16:54     ` Hannes Frederic Sowa
@ 2013-02-09 12:10       ` Hannes Frederic Sowa
  2013-02-09 14:12         ` YOSHIFUJI Hideaki
  2013-02-09 15:50         ` [IPv6] interface-local multicast escapes the local node YOSHIFUJI Hideaki
  0 siblings, 2 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-09 12:10 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki, Erik Hugne, netdev

On Wed, Feb 06, 2013 at 05:54:15PM +0100, Hannes Frederic Sowa wrote:
> On Thu, Feb 07, 2013 at 12:24:14AM +0900, YOSHIFUJI Hideaki wrote:
> > NAK.  I think we should select routes via loopback device here.
> 
> Will try your idea, thanks.

Does this patch look reasonable? Btw. i am pleased to see this kind of
things work out as expected most of the time (addrtype checking etc. all
in place). :)

I have a few questions:

a) Do we have a way to lock down routes in the kernel that they are
untouchable from userspace?

b) We don't set IFF_MULTICAST in the loopback interface driver by default.
Is there a reason we couldn't do so? This patch tries to also bind the
interface local all router multicast address on lo, but will only do so
if multicast is enabled on lo before changing the sysctl.

c) I will send a patch as soon as the errata thing is resolved to drop
ff00::/15 from entering the host from the wire. In case of outgoing
traffic to ff00::/16, should this be stopped with a net-unreachable route?

Btw, I saw that the errata on RFC 4291 is already submitted:
http://www.rfc-editor.org/errata_search.php?rfc=4291
Thanks, Erik!

[PATCH net-next RFC] ipv6: let loopback join ff01::{1,2} and create a loopback route to ff00::/16

This patch ensures that traffic to ff01::/16 is directed to the loopback
interface by default and does not leave the host. The interface-local
all nodes and all routers (in case forwarding is enabled) are now joined
by default, too.

Reported-by: Erik Hugne <erik.hugne@ericsson.com>
Cc: Erik Hugne <erik.hugne@ericsson.com>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/linux/in6.h |  6 ++++++
 net/ipv6/addrconf.c | 37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/include/linux/in6.h b/include/linux/in6.h
index a16e193..fd0e86d 100644
--- a/include/linux/in6.h
+++ b/include/linux/in6.h
@@ -36,4 +36,10 @@ extern const struct in6_addr in6addr_linklocal_allnodes;
 extern const struct in6_addr in6addr_linklocal_allrouters;
 #define IN6ADDR_LINKLOCAL_ALLROUTERS_INIT \
 		{ { { 0xff,2,0,0,0,0,0,0,0,0,0,0,0,0,0,2 } } }
+extern const struct in6_addr in6addr_interfacelocal_allnodes;
+#define IN6ADR_INTERFACELOCAL_ALLNODES_INIT \
+		{ { { 0xff,1,0,0,0,0,0,0,0,0,0,0,0,0,0,1 } } }
+extern const struct in6_addr in6addr_interfacelocal_allrouters;
+#define IN6ADR_INTERFACELOCAL_ALLROUTERS_INIT \
+		{ { { 0xff,1,0,0,0,0,0,0,0,0,0,0,0,0,0,2 } } }
 #endif
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index bd9f936..c15b98f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -244,6 +244,8 @@ const struct in6_addr in6addr_any = IN6ADDR_ANY_INIT;
 const struct in6_addr in6addr_loopback = IN6ADDR_LOOPBACK_INIT;
 const struct in6_addr in6addr_linklocal_allnodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
 const struct in6_addr in6addr_linklocal_allrouters = IN6ADDR_LINKLOCAL_ALLROUTERS_INIT;
+const struct in6_addr in6addr_interfacelocal_allnodes = IN6ADR_INTERFACELOCAL_ALLNODES_INIT;
+const struct in6_addr in6addr_interfacelocal_allrouters = IN6ADR_INTERFACELOCAL_ALLROUTERS_INIT;
 
 /* Check if a valid qdisc is available */
 static inline bool addrconf_qdisc_ok(const struct net_device *dev)
@@ -611,10 +613,17 @@ static void dev_forward_change(struct inet6_dev *idev)
 	if (idev->cnf.forwarding)
 		dev_disable_lro(dev);
 	if (dev->flags & IFF_MULTICAST) {
-		if (idev->cnf.forwarding)
+		if (idev->cnf.forwarding) {
 			ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
-		else
+			if (dev->flags & IFF_LOOPBACK)
+				ipv6_dev_mc_inc(dev,
+						&in6addr_interfacelocal_allrouters);
+		} else {
 			ipv6_dev_mc_dec(dev, &in6addr_linklocal_allrouters);
+			if (dev->flags & IFF_LOOPBACK)
+				ipv6_dev_mc_dec(dev,
+						&in6addr_interfacelocal_allrouters);
+		}
 	}
 
 	list_for_each_entry(ifa, &idev->addr_list, if_list) {
@@ -2518,6 +2527,29 @@ static void sit_add_v4_addrs(struct inet6_dev *idev)
 }
 #endif
 
+static void init_loopback_mc(struct net_device *dev)
+{
+	struct fib6_config cfg = {
+		.fc_table = RT6_TABLE_LOCAL,
+		.fc_metric = IP6_RT_PRIO_ADDRCONF,
+		.fc_dst_len = 16,
+		.fc_ifindex = dev->ifindex,
+		.fc_flags = RTF_UP,
+		.fc_protocol = RTPROT_KERNEL,
+		.fc_type = RTN_MULTICAST,
+		.fc_nlinfo.nl_net = dev_net(dev),
+	};
+
+	ipv6_addr_set(&cfg.fc_dst, htonl(0xFF010000), 0, 0, 0);
+	if (ip6_route_add(&cfg))
+		pr_debug("%s: ip6_route_add failed for node local multicast\n",
+			 __func__);
+
+	if (ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes))
+		pr_debug("%s: failed to join interface-local all nodes mc\n",
+			 __func__);
+}
+
 static void init_loopback(struct net_device *dev)
 {
 	struct inet6_dev  *idev;
@@ -2532,6 +2564,7 @@ static void init_loopback(struct net_device *dev)
 	}
 
 	add_addr(idev, &in6addr_loopback, 128, IFA_HOST);
+	init_loopback_mc(dev);
 }
 
 static void addrconf_add_linklocal(struct inet6_dev *idev, const struct in6_addr *addr)
-- 
1.8.1.2

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-09 12:10       ` Hannes Frederic Sowa
@ 2013-02-09 14:12         ` YOSHIFUJI Hideaki
  2013-02-09 23:08           ` Hannes Frederic Sowa
                             ` (2 more replies)
  2013-02-09 15:50         ` [IPv6] interface-local multicast escapes the local node YOSHIFUJI Hideaki
  1 sibling, 3 replies; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-09 14:12 UTC (permalink / raw)
  To: hannes; +Cc: Erik Hugne, netdev, YOSHIFUJI Hideaki

Hi,

Hannes Frederic Sowa wrote:
> On Wed, Feb 06, 2013 at 05:54:15PM +0100, Hannes Frederic Sowa wrote:
>> On Thu, Feb 07, 2013 at 12:24:14AM +0900, YOSHIFUJI Hideaki wrote:
>>> NAK.  I think we should select routes via loopback device here.
>>
>> Will try your idea, thanks.
> 
> Does this patch look reasonable? Btw. i am pleased to see this kind of
> things work out as expected most of the time (addrtype checking etc. all
> in place). :)
> 

Well, I rethink of what "interface-local" means.

It seems applications will join ff01::/16%eth0 instead of ff01::/16%lo.
If so, your original patch seems better.  My bad, sorry.

Would you update original one, with minor modification that defers
kfree_skb() after incrementing MIB, please?

If you think we should join ff01::{1,2} by default, you can send another
patch for it.  (BTW, why don't we join ff05::2, then? ;-))

Thanks.

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-09 12:10       ` Hannes Frederic Sowa
  2013-02-09 14:12         ` YOSHIFUJI Hideaki
@ 2013-02-09 15:50         ` YOSHIFUJI Hideaki
  1 sibling, 0 replies; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-09 15:50 UTC (permalink / raw)
  To: hannes, YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev

Hannes Frederic Sowa wrote:

> c) I will send a patch as soon as the errata thing is resolved to drop
> ff00::/15 from entering the host from the wire. In case of outgoing
> traffic to ff00::/16, should this be stopped with a net-unreachable route?

No, check should placed around ipv6_addr_loopback(), like you have
posted.  Would you cook a patch for ffx0::/16 anyway, please?

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-09 14:12         ` YOSHIFUJI Hideaki
@ 2013-02-09 23:08           ` Hannes Frederic Sowa
  2013-02-10  9:59           ` Hannes Frederic Sowa
  2013-02-10 15:32           ` Hannes Frederic Sowa
  2 siblings, 0 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-09 23:08 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev, YOSHIFUJI Hideaki

On Sat, Feb 09, 2013 at 11:12:46PM +0900, YOSHIFUJI Hideaki wrote:
> Hi,
> 
> Hannes Frederic Sowa wrote:
> > On Wed, Feb 06, 2013 at 05:54:15PM +0100, Hannes Frederic Sowa wrote:
> >> On Thu, Feb 07, 2013 at 12:24:14AM +0900, YOSHIFUJI Hideaki wrote:
> >>> NAK.  I think we should select routes via loopback device here.
> >>
> >> Will try your idea, thanks.
> > 
> > Does this patch look reasonable? Btw. i am pleased to see this kind of
> > things work out as expected most of the time (addrtype checking etc. all
> > in place). :)
> > 
> 
> Well, I rethink of what "interface-local" means.
> 
> It seems applications will join ff01::/16%eth0 instead of ff01::/16%lo.
> If so, your original patch seems better.  My bad, sorry.

No problem, will do. I have not checked carefully, but this would mean we have
to do changes to glibc to have a correct behaving getaddrinfo?

> Would you update original one, with minor modification that defers
> kfree_skb() after incrementing MIB, please?

Yes. Will send the patch tomorrow at the latest.
> 
> If you think we should join ff01::{1,2} by default, you can send another
> patch for it.  (BTW, why don't we join ff05::2, then? ;-))

Ok, I'll split it up. You are right about ff05::2, seems like the right thing
to do, somehow. I hope this won't impact any multicast routing daemons. 

Thanks,

  Hannes

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-09 14:12         ` YOSHIFUJI Hideaki
  2013-02-09 23:08           ` Hannes Frederic Sowa
@ 2013-02-10  9:59           ` Hannes Frederic Sowa
  2013-02-10 10:57             ` YOSHIFUJI Hideaki
  2013-02-10 15:32           ` Hannes Frederic Sowa
  2 siblings, 1 reply; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-10  9:59 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev, YOSHIFUJI Hideaki

On Sat, Feb 09, 2013 at 11:12:46PM +0900, YOSHIFUJI Hideaki wrote:
> It seems applications will join ff01::/16%eth0 instead of ff01::/16%lo.
> If so, your original patch seems better.  My bad, sorry.
> 
> Would you update original one, with minor modification that defers
> kfree_skb() after incrementing MIB, please?

I would add another constraint to the if "&& !(dev->flags & IFF_LOOPBACK)", so
it becomes:

                if (IPV6_ADDR_MC_SCOPE(&ipv6_hdr(skb)->daddr) <=
                    IPV6_ADDR_SCOPE_NODELOCAL &&
                    !(dev->flags & IFF_LOOPBACK))
                        kfree_skb(skb);
                        return 0;
                }


Otherwise ff01::/16%lo would not work because the multicast mirroring through
dev_loopback_xmit won't be taken and the packet would be dropped after that.

Can you confirm? Thanks.

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-10  9:59           ` Hannes Frederic Sowa
@ 2013-02-10 10:57             ` YOSHIFUJI Hideaki
  0 siblings, 0 replies; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-10 10:57 UTC (permalink / raw)
  To: Erik Hugne, netdev, YOSHIFUJI Hideaki, YOSHIFUJI Hideaki

Hannes Frederic Sowa wrote:
> On Sat, Feb 09, 2013 at 11:12:46PM +0900, YOSHIFUJI Hideaki wrote:
>> It seems applications will join ff01::/16%eth0 instead of ff01::/16%lo.
>> If so, your original patch seems better.  My bad, sorry.
>>
>> Would you update original one, with minor modification that defers
>> kfree_skb() after incrementing MIB, please?
> 
> I would add another constraint to the if "&& !(dev->flags & IFF_LOOPBACK)", so
> it becomes:
> 
>                 if (IPV6_ADDR_MC_SCOPE(&ipv6_hdr(skb)->daddr) <=
>                     IPV6_ADDR_SCOPE_NODELOCAL &&
>                     !(dev->flags & IFF_LOOPBACK))
>                         kfree_skb(skb);
>                         return 0;
>                 }
> 
> 
> Otherwise ff01::/16%lo would not work because the multicast mirroring through
> dev_loopback_xmit won't be taken and the packet would be dropped after that.
> 
> Can you confirm? Thanks.

Ack.

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-09 14:12         ` YOSHIFUJI Hideaki
  2013-02-09 23:08           ` Hannes Frederic Sowa
  2013-02-10  9:59           ` Hannes Frederic Sowa
@ 2013-02-10 15:32           ` Hannes Frederic Sowa
  2013-02-10 18:42             ` YOSHIFUJI Hideaki
  2 siblings, 1 reply; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-10 15:32 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev, YOSHIFUJI Hideaki

On Sat, Feb 09, 2013 at 11:12:46PM +0900, YOSHIFUJI Hideaki wrote:
> Hi,
> 
> Hannes Frederic Sowa wrote:
> > On Wed, Feb 06, 2013 at 05:54:15PM +0100, Hannes Frederic Sowa wrote:
> >> On Thu, Feb 07, 2013 at 12:24:14AM +0900, YOSHIFUJI Hideaki wrote:
> >>> NAK.  I think we should select routes via loopback device here.
> >>
> >> Will try your idea, thanks.
> > 
> > Does this patch look reasonable? Btw. i am pleased to see this kind of
> > things work out as expected most of the time (addrtype checking etc. all
> > in place). :)
> > 
> 
> Well, I rethink of what "interface-local" means.
> 
> It seems applications will join ff01::/16%eth0 instead of ff01::/16%lo.
> If so, your original patch seems better.  My bad, sorry.

I was looking at getpeername et. al. where we should report the scope
back to the user. A common pattern is:

                        if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
                                sin->sin6_scope_id = IP6CB(skb)->iif;

I propose to introduce something like 'bool ipv6_addr_intf_scoped(in6_addr)'
and let it check for ll addresses and interface scoped addresses.

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-10 15:32           ` Hannes Frederic Sowa
@ 2013-02-10 18:42             ` YOSHIFUJI Hideaki
  2013-02-10 18:49               ` Hannes Frederic Sowa
  0 siblings, 1 reply; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-10 18:42 UTC (permalink / raw)
  To: hannes; +Cc: Erik Hugne, netdev, YOSHIFUJI Hideaki

Hannes Frederic Sowa wrote:
> On Sat, Feb 09, 2013 at 11:12:46PM +0900, YOSHIFUJI Hideaki wrote:
>> Hi,
>>
>> Hannes Frederic Sowa wrote:
>>> On Wed, Feb 06, 2013 at 05:54:15PM +0100, Hannes Frederic Sowa wrote:
>>>> On Thu, Feb 07, 2013 at 12:24:14AM +0900, YOSHIFUJI Hideaki wrote:
>>>>> NAK.  I think we should select routes via loopback device here.
>>>>
>>>> Will try your idea, thanks.
>>>
>>> Does this patch look reasonable? Btw. i am pleased to see this kind of
>>> things work out as expected most of the time (addrtype checking etc. all
>>> in place). :)
>>>
>>
>> Well, I rethink of what "interface-local" means.
>>
>> It seems applications will join ff01::/16%eth0 instead of ff01::/16%lo.
>> If so, your original patch seems better.  My bad, sorry.
> 
> I was looking at getpeername et. al. where we should report the scope
> back to the user. A common pattern is:
> 
>                         if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
>                                 sin->sin6_scope_id = IP6CB(skb)->iif;
> 
> I propose to introduce something like 'bool ipv6_addr_intf_scoped(in6_addr)'
> and let it check for ll addresses and interface scoped addresses.

Hmm, maybe, we might want to say:

__u32 __ipv6_iface_scope_id(int type, unsigned int iface)
{
	if (type == IPV6_ADDR_ANY ||
	    type & IPV6_ADDR_LOOPBACK ||
	    __ipv6_addr_src_scope(type) > IPV6_ADDR_SCOPE_LINKLOCAL)
		return 0;
	return iface;
}

__u32 ipv6_iface_scope_id(const struct in6_addr *addr, unsigned int iface)
{
	return __ipv6_iface_scope_id(__ipv6_addr_type(addr), iface);
}

And then,
	sin->sin6_scope_id = __ipv6_iface_scope_id(__ipv6_addr_type(&sin->sin6_addr),
						   IP6CB(skb)->iif);
or
	sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr, IP6CB(skb)->iif);

--yoshfuji

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

* Re: [IPv6] interface-local multicast escapes the local node
  2013-02-10 18:42             ` YOSHIFUJI Hideaki
@ 2013-02-10 18:49               ` Hannes Frederic Sowa
  2013-02-11 11:13                 ` [PATCH RFC 1/2] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions Hannes Frederic Sowa
  2013-02-11 11:13                 ` [PATCH RFC 2/2] ipv6: use newly introduced helper functions __ipv6_addr_needs_scope_id and ipv6_iface_scope_id Hannes Frederic Sowa
  0 siblings, 2 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-10 18:49 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: Erik Hugne, netdev

On Mon, Feb 11, 2013 at 03:42:53AM +0900, YOSHIFUJI Hideaki wrote:
> Hannes Frederic Sowa wrote:
> > I was looking at getpeername et. al. where we should report the scope
> > back to the user. A common pattern is:
> > 
> >                         if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> >                                 sin->sin6_scope_id = IP6CB(skb)->iif;
> > 
> > I propose to introduce something like 'bool ipv6_addr_intf_scoped(in6_addr)'
> > and let it check for ll addresses and interface scoped addresses.
> 
> Hmm, maybe, we might want to say:
> 
> __u32 __ipv6_iface_scope_id(int type, unsigned int iface)
> {
> 	if (type == IPV6_ADDR_ANY ||
> 	    type & IPV6_ADDR_LOOPBACK ||
> 	    __ipv6_addr_src_scope(type) > IPV6_ADDR_SCOPE_LINKLOCAL)
> 		return 0;
> 	return iface;
> }
> 
> __u32 ipv6_iface_scope_id(const struct in6_addr *addr, unsigned int iface)
> {
> 	return __ipv6_iface_scope_id(__ipv6_addr_type(addr), iface);
> }
> 
> And then,
> 	sin->sin6_scope_id = __ipv6_iface_scope_id(__ipv6_addr_type(&sin->sin6_addr),
> 						   IP6CB(skb)->iif);
> or
> 	sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr, IP6CB(skb)->iif);

I like it. Will try to convert some checks and let you know how it turned out.

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

* [PATCH RFC 1/2] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions
  2013-02-10 18:49               ` Hannes Frederic Sowa
@ 2013-02-11 11:13                 ` Hannes Frederic Sowa
  2013-02-11 13:36                   ` YOSHIFUJI Hideaki
  2013-02-11 11:13                 ` [PATCH RFC 2/2] ipv6: use newly introduced helper functions __ipv6_addr_needs_scope_id and ipv6_iface_scope_id Hannes Frederic Sowa
  1 sibling, 1 reply; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-11 11:13 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki, Erik Hugne, netdev

This strictly a RFC only (I also left out my signed-off). This patch
introduces helper functions to set and check for the need of scope ids.

Suggested-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ipv6.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 851d541..1f61dc9 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -319,6 +319,19 @@ static inline int ipv6_addr_src_scope(const struct in6_addr *addr)
 	return __ipv6_addr_src_scope(__ipv6_addr_type(addr));
 }
 
+static inline bool __ipv6_addr_needs_scope_id(int type)
+{
+	return type != IPV6_ADDR_ANY &&
+	       !(type & IPV6_ADDR_LOOPBACK && type & IPV6_ADDR_UNICAST) &&
+	       __ipv6_addr_src_scope(type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
+}
+
+static inline __u32 ipv6_iface_scope_id(const struct in6_addr *addr,
+					unsigned int iface)
+{
+	return __ipv6_addr_needs_scope_id(__ipv6_addr_type(addr)) ? iface : 0;
+}
+
 static inline int ipv6_addr_cmp(const struct in6_addr *a1, const struct in6_addr *a2)
 {
 	return memcmp(a1, a2, sizeof(struct in6_addr));
-- 
1.8.1.2

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

* [PATCH RFC 2/2] ipv6: use newly introduced helper functions __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
  2013-02-10 18:49               ` Hannes Frederic Sowa
  2013-02-11 11:13                 ` [PATCH RFC 1/2] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions Hannes Frederic Sowa
@ 2013-02-11 11:13                 ` Hannes Frederic Sowa
  1 sibling, 0 replies; 28+ messages in thread
From: Hannes Frederic Sowa @ 2013-02-11 11:13 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki, Erik Hugne, netdev

This patch is strictly an RFC (I also left out my signed-off). I intended
post it just to show the usage of the new helper functions.

With this patch applied my test programs reported that interface scoped
multicast finally works over loopback. :)

Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/af_inet6.c              |  6 +++---
 net/ipv6/datagram.c              | 12 ++++++------
 net/ipv6/icmp.c                  |  2 +-
 net/ipv6/inet6_connection_sock.c |  6 ++----
 net/ipv6/ip6_input.c             |  2 +-
 net/ipv6/udp.c                   |  6 +++---
 6 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 6b793bf..f56277f 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 			struct net_device *dev = NULL;
 
 			rcu_read_lock();
-			if (addr_type & IPV6_ADDR_LINKLOCAL) {
+			if (__ipv6_addr_needs_scope_id(addr_type)) {
 				if (addr_len >= sizeof(struct sockaddr_in6) &&
 				    addr->sin6_scope_id) {
 					/* Override any existing binding, if another one
@@ -471,8 +471,8 @@ int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
 
 		sin->sin6_port = inet->inet_sport;
 	}
-	if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		sin->sin6_scope_id = sk->sk_bound_dev_if;
+	sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr,
+						 sk->sk_bound_dev_if);
 	*uaddr_len = sizeof(*sin);
 	return 0;
 }
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index f5a5478..c05a2c9 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -124,7 +124,7 @@ ipv4_connected:
 		goto out;
 	}
 
-	if (addr_type&IPV6_ADDR_LINKLOCAL) {
+	if (__ipv6_addr_needs_scope_id(addr_type)) {
 		if (addr_len >= sizeof(struct sockaddr_in6) &&
 		    usin->sin6_scope_id) {
 			if (sk->sk_bound_dev_if &&
@@ -362,8 +362,8 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
 			sin->sin6_addr = ip6h->daddr;
 			if (np->sndflow)
 				sin->sin6_flowinfo = ip6_flowinfo(ip6h);
-			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-				sin->sin6_scope_id = IP6CB(skb)->iif;
+			sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr,
+								 IP6CB(skb)->iif);
 		} else {
 			ipv6_addr_set_v4mapped(*(__be32 *)(nh + serr->addr_offset),
 					       &sin->sin6_addr);
@@ -381,8 +381,8 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
 			sin->sin6_addr = ipv6_hdr(skb)->saddr;
 			if (np->rxopt.all)
 				ip6_datagram_recv_ctl(sk, msg, skb);
-			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-				sin->sin6_scope_id = IP6CB(skb)->iif;
+			sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr,
+								 IP6CB(skb)->iif);
 		} else {
 			struct inet_sock *inet = inet_sk(sk);
 
@@ -653,7 +653,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 					rcu_read_unlock();
 					return -ENODEV;
 				}
-			} else if (addr_type & IPV6_ADDR_LINKLOCAL) {
+			} else if (__ipv6_addr_needs_scope_id(addr_type)) {
 				rcu_read_unlock();
 				return -EINVAL;
 			}
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index fff5bdd..71b900c 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -434,7 +434,7 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
 	 *	Source addr check
 	 */
 
-	if (addr_type & IPV6_ADDR_LINKLOCAL)
+	if (__ipv6_addr_needs_scope_id(addr_type))
 		iif = skb->dev->ifindex;
 
 	/*
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index b386a2c..9f1020b 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -170,10 +170,8 @@ void inet6_csk_addr2sockaddr(struct sock *sk, struct sockaddr * uaddr)
 	sin6->sin6_port	= inet_sk(sk)->inet_dport;
 	/* We do not store received flowlabel for TCP */
 	sin6->sin6_flowinfo = 0;
-	sin6->sin6_scope_id = 0;
-	if (sk->sk_bound_dev_if &&
-	    ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		sin6->sin6_scope_id = sk->sk_bound_dev_if;
+	sin6->sin6_scope_id = ipv6_iface_scope_id(&sin6->sin6_addr,
+						  sk->sk_bound_dev_if);
 }
 
 EXPORT_SYMBOL_GPL(inet6_csk_addr2sockaddr);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 521d9fd..f10bd9f 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -279,7 +279,7 @@ int ip6_mc_input(struct sk_buff *skb)
 	 *      IPv6 multicast router mode is now supported ;)
 	 */
 	if (dev_net(skb->dev)->ipv6.devconf_all->mc_forwarding &&
-	    !(ipv6_addr_type(&hdr->daddr) & IPV6_ADDR_LINKLOCAL) &&
+	    !__ipv6_addr_needs_scope_id(__ipv6_addr_type(&hdr->daddr)) &&
 	    likely(!(IP6CB(skb)->flags & IP6SKB_FORWARDED))) {
 		/*
 		 * Okay, we try to forward - split and duplicate
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 599e1ba6..97d5ad0 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -457,8 +457,8 @@ try_again:
 					       &sin6->sin6_addr);
 		else {
 			sin6->sin6_addr = ipv6_hdr(skb)->saddr;
-			if (ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-				sin6->sin6_scope_id = IP6CB(skb)->iif;
+			sin6->sin6_scope_id = ipv6_iface_scope_id(&sin6->sin6_addr,
+								  IP6CB(skb)->iif);
 		}
 
 	}
@@ -1118,7 +1118,7 @@ do_udp_sendmsg:
 
 		if (addr_len >= sizeof(struct sockaddr_in6) &&
 		    sin6->sin6_scope_id &&
-		    ipv6_addr_type(daddr)&IPV6_ADDR_LINKLOCAL)
+		    __ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)))
 			fl6.flowi6_oif = sin6->sin6_scope_id;
 	} else {
 		if (sk->sk_state != TCP_ESTABLISHED)
-- 
1.8.1.2

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

* Re: [PATCH RFC 1/2] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions
  2013-02-11 11:13                 ` [PATCH RFC 1/2] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions Hannes Frederic Sowa
@ 2013-02-11 13:36                   ` YOSHIFUJI Hideaki
  0 siblings, 0 replies; 28+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-02-11 13:36 UTC (permalink / raw)
  To: netdev, hannes; +Cc: Erik Hugne, YOSHIFUJI Hideaki

Hannes Frederic Sowa wrote:
> This strictly a RFC only (I also left out my signed-off). This patch
> introduces helper functions to set and check for the need of scope ids.
> 
> Suggested-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>  include/net/ipv6.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 851d541..1f61dc9 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -319,6 +319,19 @@ static inline int ipv6_addr_src_scope(const struct in6_addr *addr)
>  	return __ipv6_addr_src_scope(__ipv6_addr_type(addr));
>  }
>  
> +static inline bool __ipv6_addr_needs_scope_id(int type)
> +{
> +	return type != IPV6_ADDR_ANY &&
> +	       !(type & IPV6_ADDR_LOOPBACK && type & IPV6_ADDR_UNICAST) &&
> +	       __ipv6_addr_src_scope(type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
> +}
> +
> +static inline __u32 ipv6_iface_scope_id(const struct in6_addr *addr,
> +					unsigned int iface)
> +{
> +	return __ipv6_addr_needs_scope_id(__ipv6_addr_type(addr)) ? iface : 0;
> +}
> +
>  static inline int ipv6_addr_cmp(const struct in6_addr *a1, const struct in6_addr *a2)
>  {
>  	return memcmp(a1, a2, sizeof(struct in6_addr));
> 

Yes, something like that.    

--yoshfuji

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

end of thread, other threads:[~2013-02-11 13:36 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-06  8:49 [IPv6] interface-local multicast escapes the local node Erik Hugne
2013-02-06 12:12 ` Hannes Frederic Sowa
2013-02-06 13:07   ` Erik Hugne
2013-02-06 13:49     ` Hannes Frederic Sowa
2013-02-06 15:06       ` Erik Hugne
2013-02-06 15:12         ` Hannes Frederic Sowa
2013-02-06 15:32       ` YOSHIFUJI Hideaki
2013-02-06 16:04         ` Hannes Frederic Sowa
2013-02-06 16:18           ` Eric Dumazet
2013-02-06 16:25             ` Hannes Frederic Sowa
2013-02-06 16:49           ` YOSHIFUJI Hideaki
2013-02-07  1:00             ` Hannes Frederic Sowa
2013-02-07  7:28               ` Erik Hugne
2013-02-07 15:41                 ` YOSHIFUJI Hideaki
2013-02-06 15:24   ` YOSHIFUJI Hideaki
2013-02-06 16:54     ` Hannes Frederic Sowa
2013-02-09 12:10       ` Hannes Frederic Sowa
2013-02-09 14:12         ` YOSHIFUJI Hideaki
2013-02-09 23:08           ` Hannes Frederic Sowa
2013-02-10  9:59           ` Hannes Frederic Sowa
2013-02-10 10:57             ` YOSHIFUJI Hideaki
2013-02-10 15:32           ` Hannes Frederic Sowa
2013-02-10 18:42             ` YOSHIFUJI Hideaki
2013-02-10 18:49               ` Hannes Frederic Sowa
2013-02-11 11:13                 ` [PATCH RFC 1/2] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions Hannes Frederic Sowa
2013-02-11 13:36                   ` YOSHIFUJI Hideaki
2013-02-11 11:13                 ` [PATCH RFC 2/2] ipv6: use newly introduced helper functions __ipv6_addr_needs_scope_id and ipv6_iface_scope_id Hannes Frederic Sowa
2013-02-09 15:50         ` [IPv6] interface-local multicast escapes the local node YOSHIFUJI Hideaki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.