All of lore.kernel.org
 help / color / mirror / Atom feed
* Multicast Fails Over Multipoint GRE Tunnel
@ 2011-03-14 23:34 Doug Kehn
  2011-03-15 15:34 ` Eric Dumazet
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Kehn @ 2011-03-14 23:34 UTC (permalink / raw)
  To: netdev

Hi All,

I'm running kernel version 2.6.36 on ARM XSCALE (big-endian) and multicast over a multipoint GRE tunnel isn't working.  For my architecture, this worked on 2.6.26.8.  For x86, multicast over a multipoint GRE tunnel worked with kernel version 2.6.31 but failed with version 2.6.35.  Multicast over a multipoint GRE tunnel fails because ipgre_header() fails the 'if (iph->daddr)' check and reutrns -t->hlen.  ipgre_header() is being called, from neigh_connected_output(), with a non-null daddr; the contents of daddr is zero.

Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2 resolves the problem.  (Reviewing the HEAD of net-next-2.6 it appears that ipgre_header() remains unchanged from 2.6.36.)

The configuration used to discover/diagnose the problem:

ip tunnel add tun1 mode gre key 11223344 ttl 64 csum remote any
ip link set dev tun1 up
ip link set dev tun1 multicast on
ip addr flush dev tun1
ip addr add 10.40.92.114/24 broadcast 10.40.92.255 dev tun1

12: tun1: <MULTICAST,NOARP,UP,10000> mtu 1468 qdisc noqueue
    link/gre 0.0.0.0 brd 0.0.0.0
    inet 10.40.92.114/24 brd 10.40.92.255 scope global tun1

Then attempt:
ping -I tun1 224.0.0.9

Are additional configuration steps now required for multicast over multipoint GRE tunnel or is ipgre_header() in error?

Thanks,
...doug



      

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-14 23:34 Multicast Fails Over Multipoint GRE Tunnel Doug Kehn
@ 2011-03-15 15:34 ` Eric Dumazet
  2011-03-15 16:36   ` Timo Teräs
  2011-03-15 21:24   ` Multicast Fails Over Multipoint GRE Tunnel Doug Kehn
  0 siblings, 2 replies; 15+ messages in thread
From: Eric Dumazet @ 2011-03-15 15:34 UTC (permalink / raw)
  To: Doug Kehn; +Cc: netdev, Timo Teras

Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a écrit :
> Hi All,
> 
> I'm running kernel version 2.6.36 on ARM XSCALE (big-endian) and multicast over a multipoint GRE tunnel isn't working.  For my architecture, this worked on 2.6.26.8.  For x86, multicast over a multipoint GRE tunnel worked with kernel version 2.6.31 but failed with version 2.6.35.  Multicast over a multipoint GRE tunnel fails because ipgre_header() fails the 'if (iph->daddr)' check and reutrns -t->hlen.  ipgre_header() is being called, from neigh_connected_output(), with a non-null daddr; the contents of daddr is zero.
> 
> Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2 resolves the problem.  (Reviewing the HEAD of net-next-2.6 it appears that ipgre_header() remains unchanged from 2.6.36.)
> 
> The configuration used to discover/diagnose the problem:
> 
> ip tunnel add tun1 mode gre key 11223344 ttl 64 csum remote any
> ip link set dev tun1 up
> ip link set dev tun1 multicast on
> ip addr flush dev tun1
> ip addr add 10.40.92.114/24 broadcast 10.40.92.255 dev tun1
> 
> 12: tun1: <MULTICAST,NOARP,UP,10000> mtu 1468 qdisc noqueue
>     link/gre 0.0.0.0 brd 0.0.0.0
>     inet 10.40.92.114/24 brd 10.40.92.255 scope global tun1
> 
> Then attempt:
> ping -I tun1 224.0.0.9
> 
> Are additional configuration steps now required for multicast over multipoint GRE tunnel or is ipgre_header() in error?

Hi Doug

CC Timo Teras <timo.teras@iki.fi>

I would do a partial revert of Timo patch, but this means initial
concern should be addressed ?

(Timo mentioned : 
	If the NOARP packets are not dropped, ipgre_tunnel_xmit() will
	take rt->rt_gateway (= NBMA IP) and use that for route
	look up (and may lead to bogus xfrm acquires).)


Is the following works for you ?

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index da5941f..47844fa2 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1170,8 +1170,10 @@ static int ipgre_header(struct sk_buff *skb, struct net_device *dev,
 
 	if (saddr)
 		memcpy(&iph->saddr, saddr, 4);
-	if (daddr)
+	if (daddr) {
 		memcpy(&iph->daddr, daddr, 4);
+		return t->hlen;
+	}
 	if (iph->daddr)
 		return t->hlen;
 



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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-15 15:34 ` Eric Dumazet
@ 2011-03-15 16:36   ` Timo Teräs
  2011-03-15 18:28     ` Timo Teräs
                       ` (2 more replies)
  2011-03-15 21:24   ` Multicast Fails Over Multipoint GRE Tunnel Doug Kehn
  1 sibling, 3 replies; 15+ messages in thread
From: Timo Teräs @ 2011-03-15 16:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Doug Kehn, netdev

On 03/15/2011 05:34 PM, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a écrit :
>> I'm running kernel version 2.6.36 on ARM XSCALE (big-endian) and multicast over a multipoint GRE tunnel isn't working.  For my architecture, this worked on 2.6.26.8.  For x86, multicast over a multipoint GRE tunnel worked with kernel version 2.6.31 but failed with version 2.6.35.  Multicast over a multipoint GRE tunnel fails because ipgre_header() fails the 'if (iph->daddr)' check and reutrns -t->hlen.  ipgre_header() is being called, from neigh_connected_output(), with a non-null daddr; the contents of daddr is zero.
>>
>> Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2 resolves the problem.  (Reviewing the HEAD of net-next-2.6 it appears that ipgre_header() remains unchanged from 2.6.36.)
>>
>> The configuration used to discover/diagnose the problem:
>>
>> ip tunnel add tun1 mode gre key 11223344 ttl 64 csum remote any
>> ip link set dev tun1 up
>> ip link set dev tun1 multicast on
>> ip addr flush dev tun1
>> ip addr add 10.40.92.114/24 broadcast 10.40.92.255 dev tun1
>>
>> 12: tun1: <MULTICAST,NOARP,UP,10000> mtu 1468 qdisc noqueue
>>     link/gre 0.0.0.0 brd 0.0.0.0
>>     inet 10.40.92.114/24 brd 10.40.92.255 scope global tun1
>>
>> Then attempt:
>> ping -I tun1 224.0.0.9
>>
>> Are additional configuration steps now required for multicast over multipoint GRE tunnel or is ipgre_header() in error?
> 
> Hi Doug
> 
> CC Timo Teras <timo.teras@iki.fi>
> 
> I would do a partial revert of Timo patch, but this means initial
> concern should be addressed ?
> 
> (Timo mentioned : 
> 	If the NOARP packets are not dropped, ipgre_tunnel_xmit() will
> 	take rt->rt_gateway (= NBMA IP) and use that for route
> 	look up (and may lead to bogus xfrm acquires).)
> 
> 
> Is the following works for you ?

I have memory that _header() is called with daddr being valid pointer,
but pointing to zero memory. So basically my situation would break with
this.

The above configuration would be fixable by setting broadcast to tun1
interface explicitly. But I'm not sure if the above configuration is
somehow different that it'd need to work.

Basically how things work on send path is:
 1. arp_constructor maps multicast address to NUD_NOARP
 2. arp_mc_map in turn copies dev->broadcast to haddr
    (so you get the ip packet pointing to zero ip)
    - i assumed normally gre tunnels would have the
      broadcast address set
 3. ipgre_tunnel_xmit checks for daddr==0 and uses the
    rt_gateway then, which would map to the multicast
    address

So I assume that the above ping command not working would end up sending
gre encapsulated packet where both the inner and outer IP address is the
same multicast address?

Looks like my patch also broke the default behaviour that if one has
such GRE tunnel, and you send unicast packets, it would default the link
destination to be same as the inner destination. Not sure if this would
be useful.

So basically my situation is undistinguishable from the above one. The
only difference is that the above problems only with multicast, and I'm
having with unicast.

I think the fundamental problem is that arp_mc_map maps the multicast
address to zeroes (due to device broadcast being zero).

Could we instead maybe do something like:

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7927589..372448a 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -215,6 +215,12 @@ int arp_mc_map(__be32 addr, u8 *haddr, struct
net_device *dev, int dir)
        case ARPHRD_INFINIBAND:
                ip_ib_mc_map(addr, dev->broadcast, haddr);
                return 0;
+       case ARPHRD_IPGRE:
+               if (dev->broadcast)
+                       memcpy(haddr, dev->broadcast, dev->addr_len);
+               else
+                       memcpy(haddr, &addr, sizeof(addr));
+               return 0;
        default:
                if (dir) {
                        memcpy(haddr, dev->broadcast, dev->addr_len);



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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-15 16:36   ` Timo Teräs
@ 2011-03-15 18:28     ` Timo Teräs
  2011-03-15 21:33     ` Doug Kehn
  2011-03-15 21:35     ` Doug Kehn
  2 siblings, 0 replies; 15+ messages in thread
From: Timo Teräs @ 2011-03-15 18:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Doug Kehn, netdev

On 03/15/2011 06:36 PM, Timo Teräs wrote:
> On 03/15/2011 05:34 PM, Eric Dumazet wrote:
>> (Timo mentioned : 
>> 	If the NOARP packets are not dropped, ipgre_tunnel_xmit() will
>> 	take rt->rt_gateway (= NBMA IP) and use that for route
>> 	look up (and may lead to bogus xfrm acquires).)
>>
>> Is the following works for you ?
> 
> I have memory that _header() is called with daddr being valid pointer,
> but pointing to zero memory. So basically my situation would break with
> this.

Ok, I've gone through now the code paths. And I believe I made
originally the assumption that ipgre_tunnel_xmit would should not ever
get tiph->daddr == 0 if we got ipgre_header() call.

However, what actually happens is for (NOARP interfaces) in arp.c:
 - unicast traffic gets NOARP entries mapped to dev->dev_addr (in gre
case it's the tunnel 'local' address)
 - multicast gets mapped to dev->broadcast

And if we create gre tunnel without local or remote address we end up
getting the NOARP entries with hwaddr 0.0.0.0.

Now, for unicast traffic it's mostly pointless. If the tunnel was
locally bound the packets would never leave: they'd get NOARP entry for
local address. And if it's locally unbound, the packets get rt_gateway,
which is pretty confusing routing wise (it apparently assumes your link
device has same ipv4 subnet as the gre device).

On multicast side it makes a bit more sense to map multicast groups. And
this happened implicitly.

IMHO, we should fix the arp code in ipv4 and ipv6 to do proper
ARPHRD_IPGRE mappings so that the _header() gets called with proper
data. I think the multicast-to-same-multicast group mapping makes sense.
But do not really know what to do with unicast packets sent to gre
interface with NOARP and no link broadcast IP address.

Actually this was my problem: the unicast packets for gre interface with
NOARP flag resulted in trying to send packets out. So I could probably
just fix this by creating my gre interface *with* the ARP flag in the
first place.

But is there any sensible thing to do with the unicast packets in above
case? I think those should be just dropped. Or does someone think that
it'd ever make sense to take the inner unicast address and use it as the
outer address too? If so, my patch should be just reverted.

My honest thought is to keep the ip_gre header check as it is currently
and fix arp code in ipv4 / neighbour code in ipv6 to do the proper NOARP
mappings as needed. We might be able to get rid of the huge protocol
dependent "tiph->daddr == 0" check in xmit path this way, and make sure
that the header is set properly.

This would also allow us to see proper NOARP entries when doing "ip
neigh show nud noarp". Now it will just show 0.0.0.0 entries with gre
devices without telling where to the packets are actually being sent to.

Any thoughts?

Cheers,
  Timo

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-15 15:34 ` Eric Dumazet
  2011-03-15 16:36   ` Timo Teräs
@ 2011-03-15 21:24   ` Doug Kehn
  1 sibling, 0 replies; 15+ messages in thread
From: Doug Kehn @ 2011-03-15 21:24 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Timo Teras

Hi Eric,

--- On Tue, 3/15/11, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Doug Kehn" <rdkehn@yahoo.com>
> Cc: netdev@vger.kernel.org, "Timo Teras" <timo.teras@iki.fi>
> Date: Tuesday, March 15, 2011, 11:34 AM
> Le lundi 14 mars 2011 à 16:34 -0700,
> Doug Kehn a écrit :
> > Hi All,
> > 
> > I'm running kernel version 2.6.36 on ARM XSCALE
> (big-endian) and multicast over a multipoint GRE tunnel
> isn't working.  For my architecture, this worked on
> 2.6.26.8.  For x86, multicast over a multipoint GRE
> tunnel worked with kernel version 2.6.31 but failed with
> version 2.6.35.  Multicast over a multipoint GRE tunnel
> fails because ipgre_header() fails the 'if (iph->daddr)'
> check and reutrns -t->hlen.  ipgre_header() is being
> called, from neigh_connected_output(), with a non-null
> daddr; the contents of daddr is zero.
> > 
> > Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2
> resolves the problem.  (Reviewing the HEAD of
> net-next-2.6 it appears that ipgre_header() remains
> unchanged from 2.6.36.)
> > 
> > The configuration used to discover/diagnose the
> problem:
> > 
> > ip tunnel add tun1 mode gre key 11223344 ttl 64 csum
> remote any
> > ip link set dev tun1 up
> > ip link set dev tun1 multicast on
> > ip addr flush dev tun1
> > ip addr add 10.40.92.114/24 broadcast 10.40.92.255 dev
> tun1
> > 
> > 12: tun1: <MULTICAST,NOARP,UP,10000> mtu 1468
> qdisc noqueue
> >     link/gre 0.0.0.0 brd 0.0.0.0
> >     inet 10.40.92.114/24 brd
> 10.40.92.255 scope global tun1
> > 
> > Then attempt:
> > ping -I tun1 224.0.0.9
> > 
> > Are additional configuration steps now required for
> multicast over multipoint GRE tunnel or is ipgre_header() in
> error?
> 
> Hi Doug
> 
> CC Timo Teras <timo.teras@iki.fi>
> 
> I would do a partial revert of Timo patch, but this means
> initial
> concern should be addressed ?
> 
> (Timo mentioned : 
>     If the NOARP packets are not dropped,
> ipgre_tunnel_xmit() will
>     take rt->rt_gateway (= NBMA IP) and
> use that for route
>     look up (and may lead to bogus xfrm
> acquires).)
> 
> 
> Is the following works for you ?
> 
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index da5941f..47844fa2 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1170,8 +1170,10 @@ static int ipgre_header(struct
> sk_buff *skb, struct net_device *dev,
>  
>      if (saddr)
>         
> memcpy(&iph->saddr, saddr, 4);
> -    if (daddr)
> +    if (daddr) {
>         
> memcpy(&iph->daddr, daddr, 4);
> +        return t->hlen;
> +    }
>      if (iph->daddr)
>          return t->hlen;
>  

Yes, the partial revert does work.

Regards,
...doug



      

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-15 16:36   ` Timo Teräs
  2011-03-15 18:28     ` Timo Teräs
@ 2011-03-15 21:33     ` Doug Kehn
  2011-03-15 21:35     ` Doug Kehn
  2 siblings, 0 replies; 15+ messages in thread
From: Doug Kehn @ 2011-03-15 21:33 UTC (permalink / raw)
  To: Eric Dumazet, Timo Teräs; +Cc: netdev

Hi Timo,


--- On Tue, 3/15/11, Timo Teräs <timo.teras@iki.fi> wrote:

> From: Timo Teräs <timo.teras@iki.fi>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Eric Dumazet" <eric.dumazet@gmail.com>
> Cc: "Doug Kehn" <rdkehn@yahoo.com>, netdev@vger.kernel.org
> Date: Tuesday, March 15, 2011, 12:36 PM
> On 03/15/2011 05:34 PM, Eric Dumazet
> wrote:
> > Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a
> écrit :
> >> I'm running kernel version 2.6.36 on ARM XSCALE
> (big-endian) and multicast over a multipoint GRE tunnel
> isn't working.  For my architecture, this worked on
> 2.6.26.8.  For x86, multicast over a multipoint GRE
> tunnel worked with kernel version 2.6.31 but failed with
> version 2.6.35.  Multicast over a multipoint GRE tunnel
> fails because ipgre_header() fails the 'if (iph->daddr)'
> check and reutrns -t->hlen.  ipgre_header() is being
> called, from neigh_connected_output(), with a non-null
> daddr; the contents of daddr is zero.
> >>
> >> Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2
> resolves the problem.  (Reviewing the HEAD of
> net-next-2.6 it appears that ipgre_header() remains
> unchanged from 2.6.36.)
> >>
> >> The configuration used to discover/diagnose the
> problem:
> >>
> >> ip tunnel add tun1 mode gre key 11223344 ttl 64
> csum remote any
> >> ip link set dev tun1 up
> >> ip link set dev tun1 multicast on
> >> ip addr flush dev tun1
> >> ip addr add 10.40.92.114/24 broadcast 10.40.92.255
> dev tun1
> >>
> >> 12: tun1: <MULTICAST,NOARP,UP,10000> mtu
> 1468 qdisc noqueue
> >>     link/gre 0.0.0.0 brd
> 0.0.0.0
> >>     inet 10.40.92.114/24 brd
> 10.40.92.255 scope global tun1
> >>
> >> Then attempt:
> >> ping -I tun1 224.0.0.9
> >>
> >> Are additional configuration steps now required
> for multicast over multipoint GRE tunnel or is
> ipgre_header() in error?
> > 
> > Hi Doug
> > 
> > CC Timo Teras <timo.teras@iki.fi>
> > 
> > I would do a partial revert of Timo patch, but this
> means initial
> > concern should be addressed ?
> > 
> > (Timo mentioned : 
> >     If the NOARP packets are not
> dropped, ipgre_tunnel_xmit() will
> >     take rt->rt_gateway (= NBMA IP)
> and use that for route
> >     look up (and may lead to bogus xfrm
> acquires).)
> > 
> > 
> > Is the following works for you ?
> 
> I have memory that _header() is called with daddr being
> valid pointer,
> but pointing to zero memory. So basically my situation
> would break with
> this.
> 
> The above configuration would be fixable by setting
> broadcast to tun1
> interface explicitly. But I'm not sure if the above
> configuration is
> somehow different that it'd need to work.
> 
> Basically how things work on send path is:
>  1. arp_constructor maps multicast address to NUD_NOARP
>  2. arp_mc_map in turn copies dev->broadcast to haddr
>     (so you get the ip packet pointing to zero
> ip)
>     - i assumed normally gre tunnels would have
> the
>       broadcast address set
>  3. ipgre_tunnel_xmit checks for daddr==0 and uses the
>     rt_gateway then, which would map to the
> multicast
>     address
> 
> So I assume that the above ping command not working would
> end up sending
> gre encapsulated packet where both the inner and outer IP
> address is the
> same multicast address?
> 
> Looks like my patch also broke the default behaviour that
> if one has
> such GRE tunnel, and you send unicast packets, it would
> default the link
> destination to be same as the inner destination. Not sure
> if this would
> be useful.
> 
> So basically my situation is undistinguishable from the
> above one. The
> only difference is that the above problems only with
> multicast, and I'm
> having with unicast.
> 
> I think the fundamental problem is that arp_mc_map maps the
> multicast
> address to zeroes (due to device broadcast being zero).
> 
> Could we instead maybe do something like:
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7927589..372448a 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -215,6 +215,12 @@ int arp_mc_map(__be32 addr, u8 *haddr,
> struct
> net_device *dev, int dir)
>         case ARPHRD_INFINIBAND:
>                
> ip_ib_mc_map(addr, dev->broadcast, haddr);
>                
> return 0;
> +       case ARPHRD_IPGRE:
> +           
>    if (dev->broadcast)
> +               
>        memcpy(haddr,
> dev->broadcast, dev->addr_len);
> +           
>    else
> +               
>        memcpy(haddr, &addr,
> sizeof(addr));
> +           
>    return 0;
>         default:
>                 if
> (dir) {
>                
>         memcpy(haddr, dev->broadcast,
> dev->addr_len);
> 


I wasn't sure if the above patch was in addition too or in lieu of the partial revert of ipgre_header() suggested by Eric; both cases were attempted.

The multicast scenario described does not work if only the arp_mc_map() patch is applied.

The multicast scenario described does work if the partial revert of ipgre_header(), suggested by Eric, and the arp_mc_map patch are applied.

Regards,
...doug



      

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-15 16:36   ` Timo Teräs
  2011-03-15 18:28     ` Timo Teräs
  2011-03-15 21:33     ` Doug Kehn
@ 2011-03-15 21:35     ` Doug Kehn
  2011-03-16  6:01       ` Timo Teräs
  2 siblings, 1 reply; 15+ messages in thread
From: Doug Kehn @ 2011-03-15 21:35 UTC (permalink / raw)
  To: Eric Dumazet, Timo Teräs; +Cc: netdev

Hi Timo,


--- On Tue, 3/15/11, Timo Teräs <timo.teras@iki.fi> wrote:

> From: Timo Teräs <timo.teras@iki.fi>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Eric Dumazet" <eric.dumazet@gmail.com>
> Cc: "Doug Kehn" <rdkehn@yahoo.com>, netdev@vger.kernel.org
> Date: Tuesday, March 15, 2011, 12:36 PM
> On 03/15/2011 05:34 PM, Eric Dumazet
> wrote:
> > Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a
> écrit :
> >> I'm running kernel version 2.6.36 on ARM XSCALE
> (big-endian) and multicast over a multipoint GRE tunnel
> isn't working.  For my architecture, this worked on
> 2.6.26.8.  For x86, multicast over a multipoint GRE
> tunnel worked with kernel version 2.6.31 but failed with
> version 2.6.35.  Multicast over a multipoint GRE tunnel
> fails because ipgre_header() fails the 'if (iph->daddr)'
> check and reutrns -t->hlen.  ipgre_header() is being
> called, from neigh_connected_output(), with a non-null
> daddr; the contents of daddr is zero.
> >>
> >> Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2
> resolves the problem.  (Reviewing the HEAD of
> net-next-2.6 it appears that ipgre_header() remains
> unchanged from 2.6.36.)
> >>
> >> The configuration used to discover/diagnose the
> problem:
> >>
> >> ip tunnel add tun1 mode gre key 11223344 ttl 64
> csum remote any
> >> ip link set dev tun1 up
> >> ip link set dev tun1 multicast on
> >> ip addr flush dev tun1
> >> ip addr add 10.40.92.114/24 broadcast 10.40.92.255
> dev tun1
> >>
> >> 12: tun1: <MULTICAST,NOARP,UP,10000> mtu
> 1468 qdisc noqueue
> >>     link/gre 0.0.0.0 brd
> 0.0.0.0
> >>     inet 10.40.92.114/24 brd
> 10.40.92.255 scope global tun1
> >>
> >> Then attempt:
> >> ping -I tun1 224.0.0.9
> >>
> >> Are additional configuration steps now required
> for multicast over multipoint GRE tunnel or is
> ipgre_header() in error?
> > 
> > Hi Doug
> > 
> > CC Timo Teras <timo.teras@iki.fi>
> > 
> > I would do a partial revert of Timo patch, but this
> means initial
> > concern should be addressed ?
> > 
> > (Timo mentioned : 
> >     If the NOARP packets are not
> dropped, ipgre_tunnel_xmit() will
> >     take rt->rt_gateway (= NBMA IP)
> and use that for route
> >     look up (and may lead to bogus xfrm
> acquires).)
> > 
> > 
> > Is the following works for you ?
> 
> I have memory that _header() is called with daddr being
> valid pointer,
> but pointing to zero memory. So basically my situation
> would break with
> this.
> 
> The above configuration would be fixable by setting
> broadcast to tun1
> interface explicitly. But I'm not sure if the above
> configuration is
> somehow different that it'd need to work.
> 
> Basically how things work on send path is:
>  1. arp_constructor maps multicast address to NUD_NOARP
>  2. arp_mc_map in turn copies dev->broadcast to haddr
>     (so you get the ip packet pointing to zero
> ip)
>     - i assumed normally gre tunnels would have
> the
>       broadcast address set
>  3. ipgre_tunnel_xmit checks for daddr==0 and uses the
>     rt_gateway then, which would map to the
> multicast
>     address
> 
> So I assume that the above ping command not working would
> end up sending
> gre encapsulated packet where both the inner and outer IP
> address is the
> same multicast address?
> 
> Looks like my patch also broke the default behaviour that
> if one has
> such GRE tunnel, and you send unicast packets, it would
> default the link
> destination to be same as the inner destination. Not sure
> if this would
> be useful.
> 
> So basically my situation is undistinguishable from the
> above one. The
> only difference is that the above problems only with
> multicast, and I'm
> having with unicast.
> 
> I think the fundamental problem is that arp_mc_map maps the
> multicast
> address to zeroes (due to device broadcast being zero).
> 
> Could we instead maybe do something like:
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7927589..372448a 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -215,6 +215,12 @@ int arp_mc_map(__be32 addr, u8 *haddr,
> struct
> net_device *dev, int dir)
>         case ARPHRD_INFINIBAND:
>                
> ip_ib_mc_map(addr, dev->broadcast, haddr);
>                
> return 0;
> +       case ARPHRD_IPGRE:
> +           
>    if (dev->broadcast)
> +               
>        memcpy(haddr,
> dev->broadcast, dev->addr_len);
> +           
>    else
> +               
>        memcpy(haddr, &addr,
> sizeof(addr));
> +           
>    return 0;
>         default:
>                 if
> (dir) {
>                
>         memcpy(haddr, dev->broadcast,
> dev->addr_len);
> 


I wasn't sure if the above patch was in addition too or in lieu of the partial revert of ipgre_header() suggested by Eric; both cases were attempted.

The multicast scenario described does not work if only the arp_mc_map() patch is applied.

The multicast scenario described does work if the partial revert of ipgre_header(), suggested by Eric, and the arp_mc_map patch are applied.

Regards,
...doug



      

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-15 21:35     ` Doug Kehn
@ 2011-03-16  6:01       ` Timo Teräs
  2011-03-16 20:02         ` Doug Kehn
  0 siblings, 1 reply; 15+ messages in thread
From: Timo Teräs @ 2011-03-16  6:01 UTC (permalink / raw)
  To: Doug Kehn; +Cc: Eric Dumazet, netdev

On 03/15/2011 11:35 PM, Doug Kehn wrote:
>> From: Timo Teräs <timo.teras@iki.fi>
>> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
>> To: "Eric Dumazet" <eric.dumazet@gmail.com>
>> Cc: "Doug Kehn" <rdkehn@yahoo.com>, netdev@vger.kernel.org
>> Date: Tuesday, March 15, 2011, 12:36 PM
>> On 03/15/2011 05:34 PM, Eric Dumazet
>> wrote:
>>> Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a
>> écrit :
>>>> I'm running kernel version 2.6.36 on ARM XSCALE
>> (big-endian) and multicast over a multipoint GRE tunnel
>> isn't working.  For my architecture, this worked on
>> 2.6.26.8.  For x86, multicast over a multipoint GRE
>> tunnel worked with kernel version 2.6.31 but failed with
>> version 2.6.35.  Multicast over a multipoint GRE tunnel
>> fails because ipgre_header() fails the 'if (iph->daddr)'
>> check and reutrns -t->hlen.  ipgre_header() is being
>> called, from neigh_connected_output(), with a non-null
>> daddr; the contents of daddr is zero.
>
> I wasn't sure if the above patch was in addition too or in lieu of the partial revert of ipgre_header() suggested by Eric; both cases were attempted.

It was intended without the ipgre_header revert. The partial revert does
not differ from full revert related to my problem.

> The multicast scenario described does not work if only the arp_mc_map() patch is applied.

Uh. Right, the if test had wrong condition. The intention was to show
the idea that we create the multicast-to-multicast GRE NOARP mappings in
arp code where it should've been done in the first place (IMHO).

Could you try the below patch? That should work better. And the
ipgre_header should not be touched.

- Timo

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7927589..8c24845 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -215,6 +215,13 @@ int arp_mc_map(__be32 addr, u8 *haddr, struct
net_device *dev, int dir)
 	case ARPHRD_INFINIBAND:
 		ip_ib_mc_map(addr, dev->broadcast, haddr);
 		return 0;
+	case ARPHRD_IPGRE:
+		if (dev->addr_len == 4 &&
+		    get_unaligned_be32(dev->broadcast) != INADDR_ANY)
+			memcpy(haddr, dev->broadcast, dev->addr_len);
+		else
+			memcpy(haddr, &addr, sizeof(addr));
+		return 0;
 	default:
 		if (dir) {
 			memcpy(haddr, dev->broadcast, dev->addr_len);

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-16  6:01       ` Timo Teräs
@ 2011-03-16 20:02         ` Doug Kehn
  2011-03-27 16:17           ` Timo Teräs
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Kehn @ 2011-03-16 20:02 UTC (permalink / raw)
  To: Timo Teräs; +Cc: Eric Dumazet, netdev

Hi Timo,


--- On Wed, 3/16/11, Timo Teräs <timo.teras@iki.fi> wrote:

> From: Timo Teräs <timo.teras@iki.fi>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Doug Kehn" <rdkehn@yahoo.com>
> Cc: "Eric Dumazet" <eric.dumazet@gmail.com>, netdev@vger.kernel.org
> Date: Wednesday, March 16, 2011, 2:01 AM
> On 03/15/2011 11:35 PM, Doug Kehn
> wrote:
> >> From: Timo Teräs <timo.teras@iki.fi>
> >> Subject: Re: Multicast Fails Over Multipoint GRE
> Tunnel
> >> To: "Eric Dumazet" <eric.dumazet@gmail.com>
> >> Cc: "Doug Kehn" <rdkehn@yahoo.com>,
> netdev@vger.kernel.org
> >> Date: Tuesday, March 15, 2011, 12:36 PM
> >> On 03/15/2011 05:34 PM, Eric Dumazet
> >> wrote:
> >>> Le lundi 14 mars 2011 à 16:34 -0700, Doug
> Kehn a
> >> écrit :
> >>>> I'm running kernel version 2.6.36 on ARM
> XSCALE
> >> (big-endian) and multicast over a multipoint GRE
> tunnel
> >> isn't working.  For my architecture, this
> worked on
> >> 2.6.26.8.  For x86, multicast over a
> multipoint GRE
> >> tunnel worked with kernel version 2.6.31 but
> failed with
> >> version 2.6.35.  Multicast over a multipoint
> GRE tunnel
> >> fails because ipgre_header() fails the 'if
> (iph->daddr)'
> >> check and reutrns -t->hlen. 
> ipgre_header() is being
> >> called, from neigh_connected_output(), with a
> non-null
> >> daddr; the contents of daddr is zero.
> >
> > I wasn't sure if the above patch was in addition too
> or in lieu of the partial revert of ipgre_header() suggested
> by Eric; both cases were attempted.
> 
> It was intended without the ipgre_header revert. The
> partial revert does
> not differ from full revert related to my problem.
> 
> > The multicast scenario described does not work if only
> the arp_mc_map() patch is applied.
> 
> Uh. Right, the if test had wrong condition. The intention
> was to show
> the idea that we create the multicast-to-multicast GRE
> NOARP mappings in
> arp code where it should've been done in the first place
> (IMHO).
> 
> Could you try the below patch? That should work better. And
> the
> ipgre_header should not be touched.
> 
> - Timo
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7927589..8c24845 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -215,6 +215,13 @@ int arp_mc_map(__be32 addr, u8 *haddr,
> struct
> net_device *dev, int dir)
>      case ARPHRD_INFINIBAND:
>          ip_ib_mc_map(addr,
> dev->broadcast, haddr);
>          return 0;
> +    case ARPHRD_IPGRE:
> +        if (dev->addr_len
> == 4 &&
> +           
> get_unaligned_be32(dev->broadcast) != INADDR_ANY)
> +           
> memcpy(haddr, dev->broadcast, dev->addr_len);
> +        else
> +           
> memcpy(haddr, &addr, sizeof(addr));
> +        return 0;
>      default:
>          if (dir) {
>             
> memcpy(haddr, dev->broadcast, dev->addr_len);
> --

It does!  With this patch my configuration works.

Thanks,
...doug



      

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

* Re: Multicast Fails Over Multipoint GRE Tunnel
  2011-03-16 20:02         ` Doug Kehn
@ 2011-03-27 16:17           ` Timo Teräs
  2011-03-29  8:40             ` [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6 Timo Teräs
  0 siblings, 1 reply; 15+ messages in thread
From: Timo Teräs @ 2011-03-27 16:17 UTC (permalink / raw)
  To: Doug Kehn; +Cc: Eric Dumazet, David Miller, netdev

On 03/16/2011 10:02 PM, Doug Kehn wrote:
>> From: Timo Teräs <timo.teras@iki.fi>
>> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
>> To: "Doug Kehn" <rdkehn@yahoo.com>
>> Cc: "Eric Dumazet" <eric.dumazet@gmail.com>, netdev@vger.kernel.org
>> Date: Wednesday, March 16, 2011, 2:01 AM
>> On 03/15/2011 11:35 PM, Doug Kehn
>> wrote:
>>>> From: Timo Teräs <timo.teras@iki.fi>
>>>> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
>>>> To: "Eric Dumazet" <eric.dumazet@gmail.com>
>>>> Cc: "Doug Kehn" <rdkehn@yahoo.com>, netdev@vger.kernel.org
>>>> Date: Tuesday, March 15, 2011, 12:36 PM
>>>> On 03/15/2011 05:34 PM, Eric Dumazet wrote:
>>>> Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a écrit :
>>>> I'm running kernel version 2.6.36 on ARM XSCALE
>>>> (big-endian) and multicast over a multipoint GRE tunnel
>>>> isn't working.  For my architecture, this worked on
>>>> 2.6.26.8.  For x86, multicast over a multipoint GRE
>>>> tunnel worked with kernel version 2.6.31 but failed with
>>>> version 2.6.35.  Multicast over a multipoint GRE tunnel
>>>> fails because ipgre_header() fails the 'if (iph->daddr)'
>>>> check and reutrns -t->hlen. ipgre_header() is being
>>>> called, from neigh_connected_output(), with a non-null
>>>> daddr; the contents of daddr is zero.
>>>
>>> I wasn't sure if the above patch was in addition too
>>> or in lieu of the partial revert of ipgre_header() suggested
>>> by Eric; both cases were attempted.
>>
>> It was intended without the ipgre_header revert. The
>> partial revert does
>> not differ from full revert related to my problem.
>>
>>> The multicast scenario described does not work if only
>> the arp_mc_map() patch is applied.
>>
>> Uh. Right, the if test had wrong condition. The intention
>> was to show the idea that we create the multicast-to-multicast
>> GRE NOARP mappings in arp code where it should've been done in
>> the first place (IMHO).
>>
>> Could you try the below patch? That should work better. And
>> the ipgre_header should not be touched.
> 
> It does!  With this patch my configuration works.

I've been trying to think what would be the proper way to handle
multicast in ip_gre. I would also hope to get the ipmr code integrated
with opennhrp and ip_gre (see my earlier mail from couple of years ago:
http://lists.openwall.net/netdev/2009/08/11/27).

I believe we should just create the noarp multicast mappings as my patch
suggests. Both for IPv4 and IPv6 we map the inner multicast group to the
IPv4 header of GRE. And I believe we should drop the support of
auto-mapping unicast addresses to unicast address. Does anyone think
it's useful if sending to 1.2.3.4 in gre1, would result in automapping
the outer IP also to 1.2.3.4 in case the interface was in NOARP mode? I
believe the user should just insert manually the ARP entries in this case.

Also, I noticed that ipmr got support for multiple tables, that fixes
one of the problem I stated along with the old patch. So I'm planning to
reimplement at some point the patch from the above link. It'd probably
involve also hooking link-local multicast traffic through the ipmr
engine when using the NBMA mode. Obviously the ipmr when configured
would override the static NOARP multicast mappings.

If no one has objections I'll write up an official-looking patch that
adds the IPv4 multicast mappings, and try to fix up the same for IPv6
side. When this is done, it should be possible the get rid of the "if
((dst = tiph->daddr) == 0) {" block in ipgre_tunnel_xmit(), since we are
then creating all the mappings from the ARP code where it should be done
anyway.

Any comments for this plan? Eric? Dave?

Cheers,
  Timo

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

* [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
  2011-03-27 16:17           ` Timo Teräs
@ 2011-03-29  8:40             ` Timo Teräs
  2011-03-29  9:11               ` Eric Dumazet
  2011-03-29 20:26               ` Doug Kehn
  0 siblings, 2 replies; 15+ messages in thread
From: Timo Teräs @ 2011-03-29  8:40 UTC (permalink / raw)
  To: netdev; +Cc: Doug Kehn, Timo Teräs

My commit 6d55cb91a0020ac0 (gre: fix hard header destination
address checking) broke multicast.

The reason is that ip_gre used to get ipgre_header() calls with
zero destination if we have NOARP or multicast destination. Instead
the actual target was decided at ipgre_tunnel_xmit() time based on
per-protocol dissection.

Instead of allowing the "abuse" of ->header() calls with invalid
destination, this creates multicast mappings for ip_gre. This also
fixes "ip neigh show nud noarp" to display the proper multicast
mappings used by the gre device.

Reported-by: Doug Kehn <rdkehn@yahoo.com>
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
Compile tested only. Doug tested IPv4 side with the earlier patch.

The IPv6 side needs a review. I'm not sure if mapped IPv4 multicast
addresses are intrepreted as multicast addresses by ndisc code or
if we could map real IPv6 multicast addresses to IPv4 multicast
addresses.

 include/net/if_inet6.h |   16 ++++++++++++++++
 include/net/ip.h       |    8 ++++++++
 net/ipv4/arp.c         |    3 +++
 net/ipv6/ndisc.c       |    2 ++
 4 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 04977ee..fccc218 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -286,5 +286,21 @@ static inline void ipv6_ib_mc_map(const struct in6_addr *addr,
 	buf[9]  = broadcast[9];
 	memcpy(buf + 10, addr->s6_addr + 6, 10);
 }
+
+static inline int ipv6_ipgre_mc_map(const struct in6_addr *addr,
+				    const unsigned char *broadcast, char *buf)
+{
+	if ((broadcast[0] | broadcast[1] | broadcast[2] | broadcast[3]) != 0) {
+		memcpy(buf, broadcast, 4);
+	} else {
+		/* v4mapped? */
+		if ((addr->s6_addr32[0] | addr->s6_addr32[1] |
+		     (addr->s6_addr32[2] ^ htonl(0x0000ffff))) != 0)
+			return -EINVAL;
+		memcpy(buf, &addr->s6_addr32[3], 4);
+	}
+	return 0;
+}
+
 #endif
 #endif
diff --git a/include/net/ip.h b/include/net/ip.h
index a4f6311..7c41658 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -339,6 +339,14 @@ static inline void ip_ib_mc_map(__be32 naddr, const unsigned char *broadcast, ch
 	buf[16] = addr & 0x0f;
 }
 
+static inline void ip_ipgre_mc_map(__be32 naddr, const unsigned char *broadcast, char *buf)
+{
+	if ((broadcast[0] | broadcast[1] | broadcast[2] | broadcast[3]) != 0)
+		memcpy(buf, broadcast, 4);
+	else
+		memcpy(buf, &naddr, sizeof(naddr));
+}
+
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 #include <linux/ipv6.h>
 #endif
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 090d273..1b74d3b 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -215,6 +215,9 @@ int arp_mc_map(__be32 addr, u8 *haddr, struct net_device *dev, int dir)
 	case ARPHRD_INFINIBAND:
 		ip_ib_mc_map(addr, dev->broadcast, haddr);
 		return 0;
+	case ARPHRD_IPGRE:
+		ip_ipgre_mc_map(addr, dev->broadcast, haddr);
+		return 0;
 	default:
 		if (dir) {
 			memcpy(haddr, dev->broadcast, dev->addr_len);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 0e49c9d..92f952d 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -341,6 +341,8 @@ int ndisc_mc_map(struct in6_addr *addr, char *buf, struct net_device *dev, int d
 	case ARPHRD_INFINIBAND:
 		ipv6_ib_mc_map(addr, dev->broadcast, buf);
 		return 0;
+	case ARPHRD_IPGRE:
+		return ipv6_ipgre_mc_map(addr, dev->broadcast, buf);
 	default:
 		if (dir) {
 			memcpy(buf, dev->broadcast, dev->addr_len);
-- 
1.7.1


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

* Re: [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
  2011-03-29  8:40             ` [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6 Timo Teräs
@ 2011-03-29  9:11               ` Eric Dumazet
  2011-03-29 10:00                 ` Timo Teräs
  2011-03-29 20:26               ` Doug Kehn
  1 sibling, 1 reply; 15+ messages in thread
From: Eric Dumazet @ 2011-03-29  9:11 UTC (permalink / raw)
  To: Timo Teräs; +Cc: netdev, Doug Kehn

Le mardi 29 mars 2011 à 11:40 +0300, Timo Teräs a écrit :
> My commit 6d55cb91a0020ac0 (gre: fix hard header destination
> address checking) broke multicast.
> 
> The reason is that ip_gre used to get ipgre_header() calls with
> zero destination if we have NOARP or multicast destination. Instead
> the actual target was decided at ipgre_tunnel_xmit() time based on
> per-protocol dissection.
> 
> Instead of allowing the "abuse" of ->header() calls with invalid
> destination, this creates multicast mappings for ip_gre. This also
> fixes "ip neigh show nud noarp" to display the proper multicast
> mappings used by the gre device.
> 
> Reported-by: Doug Kehn <rdkehn@yahoo.com>
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
> ---
> Compile tested only. Doug tested IPv4 side with the earlier patch.


> +static inline int ipv6_ipgre_mc_map(const struct in6_addr *addr,
> +				    const unsigned char *broadcast, char *buf)
> +{
> +	if ((broadcast[0] | broadcast[1] | broadcast[2] | broadcast[3]) != 0) {
> +		memcpy(buf, broadcast, 4);
> +	} else {
> +		/* v4mapped? */
> +		if ((addr->s6_addr32[0] | addr->s6_addr32[1] |
> +		     (addr->s6_addr32[2] ^ htonl(0x0000ffff))) != 0)
> +			return -EINVAL;

		if (ipv6_addr_v4mapped(addr))


> +		memcpy(buf, &addr->s6_addr32[3], 4);
> +	}
> +	return 0;
> +}



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

* Re: [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
  2011-03-29  9:11               ` Eric Dumazet
@ 2011-03-29 10:00                 ` Timo Teräs
  0 siblings, 0 replies; 15+ messages in thread
From: Timo Teräs @ 2011-03-29 10:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Doug Kehn

On 03/29/2011 12:11 PM, Eric Dumazet wrote:
> Le mardi 29 mars 2011 à 11:40 +0300, Timo Teräs a écrit :
>> My commit 6d55cb91a0020ac0 (gre: fix hard header destination
>> address checking) broke multicast.
>>
>> The reason is that ip_gre used to get ipgre_header() calls with
>> zero destination if we have NOARP or multicast destination. Instead
>> the actual target was decided at ipgre_tunnel_xmit() time based on
>> per-protocol dissection.
>>
>> Instead of allowing the "abuse" of ->header() calls with invalid
>> destination, this creates multicast mappings for ip_gre. This also
>> fixes "ip neigh show nud noarp" to display the proper multicast
>> mappings used by the gre device.
>>
>> Reported-by: Doug Kehn <rdkehn@yahoo.com>
>> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
>> ---
>> Compile tested only. Doug tested IPv4 side with the earlier patch.
> 
> 
>> +static inline int ipv6_ipgre_mc_map(const struct in6_addr *addr,
>> +				    const unsigned char *broadcast, char *buf)
>> +{
>> +	if ((broadcast[0] | broadcast[1] | broadcast[2] | broadcast[3]) != 0) {
>> +		memcpy(buf, broadcast, 4);
>> +	} else {
>> +		/* v4mapped? */
>> +		if ((addr->s6_addr32[0] | addr->s6_addr32[1] |
>> +		     (addr->s6_addr32[2] ^ htonl(0x0000ffff))) != 0)
>> +			return -EINVAL;
> 
> 		if (ipv6_addr_v4mapped(addr))
> 
> 
>> +		memcpy(buf, &addr->s6_addr32[3], 4);
>> +	}
>> +	return 0;
>> +}

I wanted to put the function same header as all other similar ones:
net/if_inet6.h. However, ipv6_addr_v4mapped() is defined in net/ipv6.h
which includes net/if_inet6.h. So I can't really use that function there.


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

* Re: [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
  2011-03-29  8:40             ` [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6 Timo Teräs
  2011-03-29  9:11               ` Eric Dumazet
@ 2011-03-29 20:26               ` Doug Kehn
  2011-03-30  7:11                 ` David Miller
  1 sibling, 1 reply; 15+ messages in thread
From: Doug Kehn @ 2011-03-29 20:26 UTC (permalink / raw)
  To: netdev, Timo Teräs; +Cc: Timo Teräs


--- On Tue, 3/29/11, Timo Teräs <timo.teras@iki.fi> wrote:

> From: Timo Teräs <timo.teras@iki.fi>
> Subject: [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
> To: netdev@vger.kernel.org
> Cc: "Doug Kehn" <rdkehn@yahoo.com>, "Timo Teräs" <timo.teras@iki.fi>
> Date: Tuesday, March 29, 2011, 4:40 AM
> My commit 6d55cb91a0020ac0 (gre: fix
> hard header destination
> address checking) broke multicast.
> 
> The reason is that ip_gre used to get ipgre_header() calls
> with
> zero destination if we have NOARP or multicast destination.
> Instead
> the actual target was decided at ipgre_tunnel_xmit() time
> based on
> per-protocol dissection.
> 
> Instead of allowing the "abuse" of ->header() calls with
> invalid
> destination, this creates multicast mappings for ip_gre.
> This also
> fixes "ip neigh show nud noarp" to display the proper
> multicast
> mappings used by the gre device.
> 
> Reported-by: Doug Kehn <rdkehn@yahoo.com>
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>

Acked-by: Doug Kehn <rdkehn@yahoo.com>

> ---
> Compile tested only. Doug tested IPv4 side with the earlier
> patch.

This patch set [still] works with IPv4.

> 
> The IPv6 side needs a review. I'm not sure if mapped IPv4
> multicast
> addresses are intrepreted as multicast addresses by ndisc
> code or
> if we could map real IPv6 multicast addresses to IPv4
> multicast
> addresses.
> 
>  include/net/if_inet6.h |   16
> ++++++++++++++++
>  include/net/ip.h       | 
>   8 ++++++++
>  net/ipv4/arp.c     
>    |    3 +++
>  net/ipv6/ndisc.c       | 
>   2 ++
>  4 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/if_inet6.h
> b/include/net/if_inet6.h
> index 04977ee..fccc218 100644
> --- a/include/net/if_inet6.h
> +++ b/include/net/if_inet6.h
> @@ -286,5 +286,21 @@ static inline void
> ipv6_ib_mc_map(const struct in6_addr *addr,
>      buf[9]  = broadcast[9];
>      memcpy(buf + 10, addr->s6_addr + 6,
> 10);
>  }
> +
> +static inline int ipv6_ipgre_mc_map(const struct in6_addr
> *addr,
> +           
>         const unsigned char
> *broadcast, char *buf)
> +{
> +    if ((broadcast[0] | broadcast[1] |
> broadcast[2] | broadcast[3]) != 0) {
> +        memcpy(buf,
> broadcast, 4);
> +    } else {
> +        /* v4mapped? */
> +        if
> ((addr->s6_addr32[0] | addr->s6_addr32[1] |
> +         
>    (addr->s6_addr32[2] ^
> htonl(0x0000ffff))) != 0)
> +           
> return -EINVAL;
> +        memcpy(buf,
> &addr->s6_addr32[3], 4);
> +    }
> +    return 0;
> +}
> +
>  #endif
>  #endif
> diff --git a/include/net/ip.h b/include/net/ip.h
> index a4f6311..7c41658 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -339,6 +339,14 @@ static inline void ip_ib_mc_map(__be32
> naddr, const unsigned char *broadcast, ch
>      buf[16] = addr & 0x0f;
>  }
>  
> +static inline void ip_ipgre_mc_map(__be32 naddr, const
> unsigned char *broadcast, char *buf)
> +{
> +    if ((broadcast[0] | broadcast[1] |
> broadcast[2] | broadcast[3]) != 0)
> +        memcpy(buf,
> broadcast, 4);
> +    else
> +        memcpy(buf,
> &naddr, sizeof(naddr));
> +}
> +
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>  #include <linux/ipv6.h>
>  #endif
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 090d273..1b74d3b 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -215,6 +215,9 @@ int arp_mc_map(__be32 addr, u8 *haddr,
> struct net_device *dev, int dir)
>      case ARPHRD_INFINIBAND:
>          ip_ib_mc_map(addr,
> dev->broadcast, haddr);
>          return 0;
> +    case ARPHRD_IPGRE:
> +       
> ip_ipgre_mc_map(addr, dev->broadcast, haddr);
> +        return 0;
>      default:
>          if (dir) {
>             
> memcpy(haddr, dev->broadcast, dev->addr_len);
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 0e49c9d..92f952d 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -341,6 +341,8 @@ int ndisc_mc_map(struct in6_addr *addr,
> char *buf, struct net_device *dev, int d
>      case ARPHRD_INFINIBAND:
>          ipv6_ib_mc_map(addr,
> dev->broadcast, buf);
>          return 0;
> +    case ARPHRD_IPGRE:
> +        return
> ipv6_ipgre_mc_map(addr, dev->broadcast, buf);
>      default:
>          if (dir) {
>             
> memcpy(buf, dev->broadcast, dev->addr_len);
> -- 
> 1.7.1
> 
> --
> 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
> 

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

* Re: [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
  2011-03-29 20:26               ` Doug Kehn
@ 2011-03-30  7:11                 ` David Miller
  0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2011-03-30  7:11 UTC (permalink / raw)
  To: rdkehn; +Cc: netdev, timo.teras

From: Doug Kehn <rdkehn@yahoo.com>
Date: Tue, 29 Mar 2011 13:26:02 -0700 (PDT)

> 
> --- On Tue, 3/29/11, Timo Teräs <timo.teras@iki.fi> wrote:
> 
>> From: Timo Teräs <timo.teras@iki.fi>
>> Subject: [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6
>> To: netdev@vger.kernel.org
>> Cc: "Doug Kehn" <rdkehn@yahoo.com>, "Timo Teräs" <timo.teras@iki.fi>
>> Date: Tuesday, March 29, 2011, 4:40 AM
>> My commit 6d55cb91a0020ac0 (gre: fix
>> hard header destination
>> address checking) broke multicast.
>> 
>> The reason is that ip_gre used to get ipgre_header() calls
>> with
>> zero destination if we have NOARP or multicast destination.
>> Instead
>> the actual target was decided at ipgre_tunnel_xmit() time
>> based on
>> per-protocol dissection.
>> 
>> Instead of allowing the "abuse" of ->header() calls with
>> invalid
>> destination, this creates multicast mappings for ip_gre.
>> This also
>> fixes "ip neigh show nud noarp" to display the proper
>> multicast
>> mappings used by the gre device.
>> 
>> Reported-by: Doug Kehn <rdkehn@yahoo.com>
>> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
> 
> Acked-by: Doug Kehn <rdkehn@yahoo.com>

Applied.

Timo, if updates are necessary for the ipv4 mapped handling in ipv6,
please send that as a follow-up fix.

Thanks.

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

end of thread, other threads:[~2011-03-30  7:12 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-14 23:34 Multicast Fails Over Multipoint GRE Tunnel Doug Kehn
2011-03-15 15:34 ` Eric Dumazet
2011-03-15 16:36   ` Timo Teräs
2011-03-15 18:28     ` Timo Teräs
2011-03-15 21:33     ` Doug Kehn
2011-03-15 21:35     ` Doug Kehn
2011-03-16  6:01       ` Timo Teräs
2011-03-16 20:02         ` Doug Kehn
2011-03-27 16:17           ` Timo Teräs
2011-03-29  8:40             ` [PATCH] net: gre: provide multicast mappings for ipv4 and ipv6 Timo Teräs
2011-03-29  9:11               ` Eric Dumazet
2011-03-29 10:00                 ` Timo Teräs
2011-03-29 20:26               ` Doug Kehn
2011-03-30  7:11                 ` David Miller
2011-03-15 21:24   ` Multicast Fails Over Multipoint GRE Tunnel Doug Kehn

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.