All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: How to use gretap with bridge?
       [not found] ` <20091029170631.GA29405@gondor.apana.org.au>
@ 2009-10-29 18:39   ` Neulinger, Nathan
  2009-10-29 19:01   ` Neulinger, Nathan
  1 sibling, 0 replies; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-29 18:39 UTC (permalink / raw)
  To: netdev

I've been able to reproduce this with a upstream kernel (2.6.32-rc5) -
symptom appears to be specific to the IP addresses specified on the ip
command, but not in any clear way. I assume that remote should be the ip
of the host at the remote end of the tunnel, and local should be an IP
address of a real interface on the this machine?

Am I missing something obvious here? At the time of the below commands,
br0 exists, but has no members, and eth0 is configured and up with ip
131.151.0.36/255.255.254.0. All other interfaces are down. 

[root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
local 131.151.0.36
[root@bridge-rol ~]# brctl addif br0 gre3
can't add gre3 to bridge br0: Invalid argument

[root@bridge-rol ~]# ip link del gre3
[root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
local 131.151.0.35
[root@bridge-rol ~]# brctl addif br0 gre3
can't add gre3 to bridge br0: Invalid argument

[root@bridge-rol ~]# ip link del gre3
[root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
local 10.151.0.35
[root@bridge-rol ~]# brctl addif br0 gre3

[root@bridge-rol ~]# ip link del gre3
[root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
local 131.1.1.1
[root@bridge-rol ~]# brctl addif br0 gre3
can't add gre3 to bridge br0: Invalid argument



-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
> Sent: Thursday, October 29, 2009 12:07 PM
> To: Neulinger, Nathan
> Subject: Re: How to use gretap with bridge?
> 
> On Thu, Oct 29, 2009 at 10:41:18AM -0500, Neulinger, Nathan wrote:
> > Is there some trick I'm missing to adding a gretap interface to a
> > bridge?
> >
> > ip link add gre1 type gretap remote 131.151.0.36 local 131.151.0.35
> > ip link set gre1 up
> > brctl addbr br0
> > brctl addif br0 gre1
> >
> > This results in an Invalid argument error when issuing the addif.
> > Testing with latest fc12 2.6.31.5-96 kernel.
> >
> > Any suggestions?
> 
> I can't reproduce this here.  Can you please try the latest
> upstream kernel? If it still does the same thing, please post
> to netdev@vger.kernel.org.
> 
> Thanks!
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* RE: How to use gretap with bridge?
       [not found] ` <20091029170631.GA29405@gondor.apana.org.au>
  2009-10-29 18:39   ` How to use gretap with bridge? Neulinger, Nathan
@ 2009-10-29 19:01   ` Neulinger, Nathan
  2009-10-29 20:00     ` Stephen Hemminger
  1 sibling, 1 reply; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-29 19:01 UTC (permalink / raw)
  To: netdev

Further testing - if the leading octet of the 'local' address is even,
it allows it to be added to bridge, if it's odd, it won't.

Any ideas?

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Neulinger, Nathan
> Sent: Thursday, October 29, 2009 1:39 PM
> To: 'netdev@vger.kernel.org'
> Subject: RE: How to use gretap with bridge?
> 
> I've been able to reproduce this with a upstream kernel (2.6.32-rc5) -
> symptom appears to be specific to the IP addresses specified on the ip
> command, but not in any clear way. I assume that remote should be the
> ip of the host at the remote end of the tunnel, and local should be an
> IP address of a real interface on the this machine?
> 
> Am I missing something obvious here? At the time of the below
commands,
> br0 exists, but has no members, and eth0 is configured and up with ip
> 131.151.0.36/255.255.254.0. All other interfaces are down.
> 
> [root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
> local 131.151.0.36
> [root@bridge-rol ~]# brctl addif br0 gre3
> can't add gre3 to bridge br0: Invalid argument
> 
> [root@bridge-rol ~]# ip link del gre3
> [root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
> local 131.151.0.35
> [root@bridge-rol ~]# brctl addif br0 gre3
> can't add gre3 to bridge br0: Invalid argument
> 
> [root@bridge-rol ~]# ip link del gre3
> [root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
> local 10.151.0.35
> [root@bridge-rol ~]# brctl addif br0 gre3
> 
> [root@bridge-rol ~]# ip link del gre3
> [root@bridge-rol ~]# ip link add gre3 type gretap remote 131.151.35.35
> local 131.1.1.1
> [root@bridge-rol ~]# brctl addif br0 gre3
> can't add gre3 to bridge br0: Invalid argument
> 
> 
> 
> -- Nathan
> 
> ------------------------------------------------------------
> Nathan Neulinger                       nneul@mst.edu
> Missouri S&T Information Technology    (573) 612-1412
> System Administrator - Principal       KD0DMH
> 
> 
> > -----Original Message-----
> > From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
> > Sent: Thursday, October 29, 2009 12:07 PM
> > To: Neulinger, Nathan
> > Subject: Re: How to use gretap with bridge?
> >
> > On Thu, Oct 29, 2009 at 10:41:18AM -0500, Neulinger, Nathan wrote:
> > > Is there some trick I'm missing to adding a gretap interface to a
> > > bridge?
> > >
> > > ip link add gre1 type gretap remote 131.151.0.36 local
131.151.0.35
> > > ip link set gre1 up
> > > brctl addbr br0
> > > brctl addif br0 gre1
> > >
> > > This results in an Invalid argument error when issuing the addif.
> > > Testing with latest fc12 2.6.31.5-96 kernel.
> > >
> > > Any suggestions?
> >
> > I can't reproduce this here.  Can you please try the latest
> > upstream kernel? If it still does the same thing, please post
> > to netdev@vger.kernel.org.
> >
> > Thanks!
> > --
> > Visit Openswan at http://www.openswan.org/
> > Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> > Home Page: http://gondor.apana.org.au/~herbert/
> > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: How to use gretap with bridge?
  2009-10-29 19:01   ` Neulinger, Nathan
@ 2009-10-29 20:00     ` Stephen Hemminger
  2009-10-29 20:22       ` Neulinger, Nathan
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Stephen Hemminger @ 2009-10-29 20:00 UTC (permalink / raw)
  To: Neulinger, Nathan; +Cc: netdev

On Thu, 29 Oct 2009 14:01:31 -0500
"Neulinger, Nathan" <nneul@mst.edu> wrote:

> Further testing - if the leading octet of the 'local' address is even,
> it allows it to be added to bridge, if it's odd, it won't.
> 
> Any ideas?
> 

If leading octet of MAC address is odd, then bridge thinks it
is not a valid ethernet for bridging because it is a multicast
address.


-- 

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

* RE: How to use gretap with bridge?
  2009-10-29 20:00     ` Stephen Hemminger
@ 2009-10-29 20:22       ` Neulinger, Nathan
  2009-10-29 22:12         ` Stephen Hemminger
  2009-10-29 20:59       ` How to use gretap with bridge? Neulinger, Nathan
  2009-10-29 22:04       ` Neulinger, Nathan
  2 siblings, 1 reply; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-29 20:22 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

I was referring to the local IP in the "ip link add ... remote x.z.z.z
local y.z.z.z" command specifying the endpoints of the tunnel. It lets
it be added to the bridge if y is even, but not if y is odd. Why should
it care what the IP of the tunnel endpoints are?

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Thursday, October 29, 2009 3:01 PM
> To: Neulinger, Nathan
> Cc: netdev@vger.kernel.org
> Subject: Re: How to use gretap with bridge?
> 
> On Thu, 29 Oct 2009 14:01:31 -0500
> "Neulinger, Nathan" <nneul@mst.edu> wrote:
> 
> > Further testing - if the leading octet of the 'local' address is
> even,
> > it allows it to be added to bridge, if it's odd, it won't.
> >
> > Any ideas?
> >
> 
> If leading octet of MAC address is odd, then bridge thinks it
> is not a valid ethernet for bridging because it is a multicast
> address.
> 
> 
> --

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

* RE: How to use gretap with bridge?
  2009-10-29 20:00     ` Stephen Hemminger
  2009-10-29 20:22       ` Neulinger, Nathan
@ 2009-10-29 20:59       ` Neulinger, Nathan
  2009-10-29 22:04       ` Neulinger, Nathan
  2 siblings, 0 replies; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-29 20:59 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev

As a note - the bridging/tunneling is working perfectly once I force it
to use a bogus IP range that starts with an even number, but
unfortunately, that's not going to work so good given that our primary
address space is 131.151.x.x. 

Any ideas on what is up with the even/odd error?

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Neulinger, Nathan
> Sent: Thursday, October 29, 2009 3:22 PM
> To: 'Stephen Hemminger'
> Cc: netdev@vger.kernel.org
> Subject: RE: How to use gretap with bridge?
> 
> I was referring to the local IP in the "ip link add ... remote x.z.z.z
> local y.z.z.z" command specifying the endpoints of the tunnel. It lets
> it be added to the bridge if y is even, but not if y is odd. Why
should
> it care what the IP of the tunnel endpoints are?
> 
> -- Nathan
> 
> ------------------------------------------------------------
> Nathan Neulinger                       nneul@mst.edu
> Missouri S&T Information Technology    (573) 612-1412
> System Administrator - Principal       KD0DMH
> 
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: Thursday, October 29, 2009 3:01 PM
> > To: Neulinger, Nathan
> > Cc: netdev@vger.kernel.org
> > Subject: Re: How to use gretap with bridge?
> >
> > On Thu, 29 Oct 2009 14:01:31 -0500
> > "Neulinger, Nathan" <nneul@mst.edu> wrote:
> >
> > > Further testing - if the leading octet of the 'local' address is
> > even,
> > > it allows it to be added to bridge, if it's odd, it won't.
> > >
> > > Any ideas?
> > >
> >
> > If leading octet of MAC address is odd, then bridge thinks it
> > is not a valid ethernet for bridging because it is a multicast
> > address.
> >
> >
> > --

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

* RE: How to use gretap with bridge?
  2009-10-29 20:00     ` Stephen Hemminger
  2009-10-29 20:22       ` Neulinger, Nathan
  2009-10-29 20:59       ` How to use gretap with bridge? Neulinger, Nathan
@ 2009-10-29 22:04       ` Neulinger, Nathan
  2009-10-29 23:47         ` Herbert Xu
  2 siblings, 1 reply; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-29 22:04 UTC (permalink / raw)
  To: Herbert Xu, Stephen Hemminger; +Cc: netdev

Now I see it - Stephen actually had it right on - the problem is that
the gre tunnel is creating a MAC address on the fly based on the tunnel
endpoint ip address, so if the tunnel endpoint address starts with an
odd number, it hits the multicast check in the bridging code. (I'm sure
that's what he meant and I just missed it entirely.)

Simplest option would probably be to just mask off the first octet with
0xFD or using the ip as the last four octets of the mac instead of the
first four.

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Neulinger, Nathan
> Sent: Thursday, October 29, 2009 3:59 PM
> To: 'Herbert Xu'
> Cc: 'netdev@vger.kernel.org'
> Subject: RE: How to use gretap with bridge?
> 
> As a note - the bridging/tunneling is working perfectly once I force
it
> to use a bogus IP range that starts with an even number, but
> unfortunately, that's not going to work so good given that our primary
> address space is 131.151.x.x.
> 
> Any ideas on what is up with the even/odd error?
> 
> -- Nathan
> 
> ------------------------------------------------------------
> Nathan Neulinger                       nneul@mst.edu
> Missouri S&T Information Technology    (573) 612-1412
> System Administrator - Principal       KD0DMH
> 
> 
> > -----Original Message-----
> > From: Neulinger, Nathan
> > Sent: Thursday, October 29, 2009 3:22 PM
> > To: 'Stephen Hemminger'
> > Cc: netdev@vger.kernel.org
> > Subject: RE: How to use gretap with bridge?
> >
> > I was referring to the local IP in the "ip link add ... remote
> x.z.z.z
> > local y.z.z.z" command specifying the endpoints of the tunnel. It
> lets
> > it be added to the bridge if y is even, but not if y is odd. Why
> should
> > it care what the IP of the tunnel endpoints are?
> >
> > -- Nathan
> >
> > ------------------------------------------------------------
> > Nathan Neulinger                       nneul@mst.edu
> > Missouri S&T Information Technology    (573) 612-1412
> > System Administrator - Principal       KD0DMH
> >
> >
> > > -----Original Message-----
> > > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > > Sent: Thursday, October 29, 2009 3:01 PM
> > > To: Neulinger, Nathan
> > > Cc: netdev@vger.kernel.org
> > > Subject: Re: How to use gretap with bridge?
> > >
> > > On Thu, 29 Oct 2009 14:01:31 -0500
> > > "Neulinger, Nathan" <nneul@mst.edu> wrote:
> > >
> > > > Further testing - if the leading octet of the 'local' address is
> > > even,
> > > > it allows it to be added to bridge, if it's odd, it won't.
> > > >
> > > > Any ideas?
> > > >
> > >
> > > If leading octet of MAC address is odd, then bridge thinks it
> > > is not a valid ethernet for bridging because it is a multicast
> > > address.
> > >
> > >
> > > --

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

* Re: How to use gretap with bridge?
  2009-10-29 20:22       ` Neulinger, Nathan
@ 2009-10-29 22:12         ` Stephen Hemminger
  2009-10-29 22:24           ` [RFC] bridge: check address size Stephen Hemminger
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2009-10-29 22:12 UTC (permalink / raw)
  To: Neulinger, Nathan; +Cc: netdev

On Thu, 29 Oct 2009 15:22:19 -0500
"Neulinger, Nathan" <nneul@mst.edu> wrote:

> I was referring to the local IP in the "ip link add ... remote x.z.z.z
> local y.z.z.z" command specifying the endpoints of the tunnel. It lets
> it be added to the bridge if y is even, but not if y is odd. Why should
> it care what the IP of the tunnel endpoints are?
> 
> -- Nathan
> 

It looks like a GRE driver bug. It uses IP address for dev->dev_addr
when it really should be generating a Ethernet address when using
Ethernet wrapper mode.

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

* [RFC] bridge: check address size
  2009-10-29 22:12         ` Stephen Hemminger
@ 2009-10-29 22:24           ` Stephen Hemminger
  2009-11-02  8:07             ` David Miller
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2009-10-29 22:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Neulinger, Nathan, netdev

Check the address size of underlying device because the bridge assumes
the underlying device has ethernet address format. See forwarding table
and STP, for places where this true.

Also, add some comments to explain errors.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/bridge/br_if.c	2009-10-29 15:18:48.363916679 -0700
+++ b/net/bridge/br_if.c	2009-10-29 15:21:38.142667043 -0700
@@ -377,12 +377,17 @@ int br_add_if(struct net_bridge *br, str
 	struct net_bridge_port *p;
 	int err = 0;
 
-	if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
+	/* Don't allow bridging non ethernet like devices */
+	if (dev->flags & IFF_LOOPBACK
+	    || dev->type != ARPHRD_ETHER
+	    || dev->addr_len != ETH_ALEN)
 		return -EINVAL;
 
+	/* No bridging of bridges */
 	if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit)
 		return -ELOOP;
 
+	/* Device is already being bridged */
 	if (dev->br_port != NULL)
 		return -EBUSY;
 


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

* Re: How to use gretap with bridge?
  2009-10-29 22:04       ` Neulinger, Nathan
@ 2009-10-29 23:47         ` Herbert Xu
  2009-10-29 23:51           ` Neulinger, Nathan
  0 siblings, 1 reply; 18+ messages in thread
From: Herbert Xu @ 2009-10-29 23:47 UTC (permalink / raw)
  To: Neulinger, Nathan; +Cc: Stephen Hemminger, netdev

On Thu, Oct 29, 2009 at 05:04:02PM -0500, Neulinger, Nathan wrote:
> Now I see it - Stephen actually had it right on - the problem is that
> the gre tunnel is creating a MAC address on the fly based on the tunnel
> endpoint ip address, so if the tunnel endpoint address starts with an
> odd number, it hits the multicast check in the bridging code. (I'm sure
> that's what he meant and I just missed it entirely.)
> 
> Simplest option would probably be to just mask off the first octet with
> 0xFD or using the ip as the last four octets of the mac instead of the
> first four.

This looks like a bug in either iproute or the kernel.  It's
not supposed to set a MAC address unless the user specifically
gives one.  If one is not given the kernel will generate a valid
MAC address.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* RE: How to use gretap with bridge?
  2009-10-29 23:47         ` Herbert Xu
@ 2009-10-29 23:51           ` Neulinger, Nathan
  2009-10-30 15:51             ` Herbert Xu
  0 siblings, 1 reply; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-29 23:51 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Stephen Hemminger, netdev

Actually, it looks like it's broke here in ipgre_tunnel_init:

--- net/ipv4/ip_gre.c.orig      2009-10-29 18:45:29.335723326 -0500
+++ net/ipv4/ip_gre.c   2009-10-29 18:45:13.069697015 -0500
@@ -1240,7 +1240,8 @@
        tunnel->dev = dev;
        strcpy(tunnel->parms.name, dev->name);
 
-       memcpy(dev->dev_addr, &tunnel->parms.iph.saddr, 4);
+       /* assign random mac addr on init */
+       random_ether_addr(dev->dev_addr);
        memcpy(dev->broadcast, &tunnel->parms.iph.daddr, 4);
 
        if (iph->daddr) {

The above change fixes it for me, but I'm no expert on this chunk of
code. (Perhaps it it shouldn't set dev_addr at all?)

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
> Sent: Thursday, October 29, 2009 6:48 PM
> To: Neulinger, Nathan
> Cc: Stephen Hemminger; netdev@vger.kernel.org
> Subject: Re: How to use gretap with bridge?
> 
> On Thu, Oct 29, 2009 at 05:04:02PM -0500, Neulinger, Nathan wrote:
> > Now I see it - Stephen actually had it right on - the problem is
that
> > the gre tunnel is creating a MAC address on the fly based on the
> tunnel
> > endpoint ip address, so if the tunnel endpoint address starts with
an
> > odd number, it hits the multicast check in the bridging code. (I'm
> sure
> > that's what he meant and I just missed it entirely.)
> >
> > Simplest option would probably be to just mask off the first octet
> with
> > 0xFD or using the ip as the last four octets of the mac instead of
> the
> > first four.
> 
> This looks like a bug in either iproute or the kernel.  It's
> not supposed to set a MAC address unless the user specifically
> gives one.  If one is not given the kernel will generate a valid
> MAC address.
> 
> Cheers,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: How to use gretap with bridge?
  2009-10-29 23:51           ` Neulinger, Nathan
@ 2009-10-30 15:51             ` Herbert Xu
  2009-10-30 16:39               ` Stephen Hemminger
  2009-10-30 17:08               ` Neulinger, Nathan
  0 siblings, 2 replies; 18+ messages in thread
From: Herbert Xu @ 2009-10-30 15:51 UTC (permalink / raw)
  To: Neulinger, Nathan, David S. Miller; +Cc: shemminger, netdev

Neulinger, Nathan <nneul@mst.edu> wrote:
>
> The above change fixes it for me, but I'm no expert on this chunk of
> code. (Perhaps it it shouldn't set dev_addr at all?)

OK, it was a stupid mistake on my part.  I added a netdev ops
struct for tap but didn't actually use it!  Please let us know
whether this patch fixes the problem.

gre: Fix dev_addr clobbering for gretap

Nathan Neulinger noticed that gretap devices get their MAC address
from the local IP address, which results in invalid MAC addresses
half of the time.

This is because gretap is still using the tunnel netdev ops rather
than the correct tap netdev ops struct.

This patch also fixes changelink to not clobber the MAC address
for the gretap case.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 41ada99..1433338 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1464,7 +1464,7 @@ static void ipgre_tap_setup(struct net_device *dev)
 
 	ether_setup(dev);
 
-	dev->netdev_ops		= &ipgre_netdev_ops;
+	dev->netdev_ops		= &ipgre_tap_netdev_ops;
 	dev->destructor 	= free_netdev;
 
 	dev->iflink		= 0;
@@ -1525,25 +1525,29 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
 		if (t->dev != dev)
 			return -EEXIST;
 	} else {
-		unsigned nflags = 0;
-
 		t = nt;
 
-		if (ipv4_is_multicast(p.iph.daddr))
-			nflags = IFF_BROADCAST;
-		else if (p.iph.daddr)
-			nflags = IFF_POINTOPOINT;
+		if (dev->type != ARPHRD_ETHER) {
+			unsigned nflags = 0;
 
-		if ((dev->flags ^ nflags) &
-		    (IFF_POINTOPOINT | IFF_BROADCAST))
-			return -EINVAL;
+			if (ipv4_is_multicast(p.iph.daddr))
+				nflags = IFF_BROADCAST;
+			else if (p.iph.daddr)
+				nflags = IFF_POINTOPOINT;
+
+			if ((dev->flags ^ nflags) &
+			    (IFF_POINTOPOINT | IFF_BROADCAST))
+				return -EINVAL;
+		}
 
 		ipgre_tunnel_unlink(ign, t);
 		t->parms.iph.saddr = p.iph.saddr;
 		t->parms.iph.daddr = p.iph.daddr;
 		t->parms.i_key = p.i_key;
-		memcpy(dev->dev_addr, &p.iph.saddr, 4);
-		memcpy(dev->broadcast, &p.iph.daddr, 4);
+		if (dev->type != ARPHRD_ETHER) {
+			memcpy(dev->dev_addr, &p.iph.saddr, 4);
+			memcpy(dev->broadcast, &p.iph.daddr, 4);
+		}
 		ipgre_tunnel_link(ign, t);
 		netdev_state_change(dev);
 	}

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

* Re: How to use gretap with bridge?
  2009-10-30 15:51             ` Herbert Xu
@ 2009-10-30 16:39               ` Stephen Hemminger
  2009-10-30 19:27                 ` David Miller
  2009-10-30 17:08               ` Neulinger, Nathan
  1 sibling, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2009-10-30 16:39 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Neulinger, Nathan, David S. Miller, netdev

On Fri, 30 Oct 2009 11:51:48 -0400
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Neulinger, Nathan <nneul@mst.edu> wrote:
> >
> > The above change fixes it for me, but I'm no expert on this chunk of
> > code. (Perhaps it it shouldn't set dev_addr at all?)
> 
> OK, it was a stupid mistake on my part.  I added a netdev ops
> struct for tap but didn't actually use it!  Please let us know
> whether this patch fixes the problem.
> 
> gre: Fix dev_addr clobbering for gretap
> 
> Nathan Neulinger noticed that gretap devices get their MAC address
> from the local IP address, which results in invalid MAC addresses
> half of the time.
> 
> This is because gretap is still using the tunnel netdev ops rather
> than the correct tap netdev ops struct.
> 
> This patch also fixes changelink to not clobber the MAC address
> for the gretap case.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Thanks,

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

-- 

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

* RE: How to use gretap with bridge?
  2009-10-30 15:51             ` Herbert Xu
  2009-10-30 16:39               ` Stephen Hemminger
@ 2009-10-30 17:08               ` Neulinger, Nathan
  1 sibling, 0 replies; 18+ messages in thread
From: Neulinger, Nathan @ 2009-10-30 17:08 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller; +Cc: shemminger, netdev

Confirmed, this fixes the problem for me and even now allows changing
the mac access by request. 

Now I'm on to figuring out MTU issues. Thanks!

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       nneul@mst.edu
Missouri S&T Information Technology    (573) 612-1412
System Administrator - Principal       KD0DMH


> -----Original Message-----
> From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
> Sent: Friday, October 30, 2009 10:52 AM
> To: Neulinger, Nathan; David S. Miller
> Cc: shemminger@vyatta.com; netdev@vger.kernel.org
> Subject: Re: How to use gretap with bridge?
> 
> Neulinger, Nathan <nneul@mst.edu> wrote:
> >
> > The above change fixes it for me, but I'm no expert on this chunk of
> > code. (Perhaps it it shouldn't set dev_addr at all?)
> 
> OK, it was a stupid mistake on my part.  I added a netdev ops
> struct for tap but didn't actually use it!  Please let us know
> whether this patch fixes the problem.
> 
> gre: Fix dev_addr clobbering for gretap
> 
> Nathan Neulinger noticed that gretap devices get their MAC address
> from the local IP address, which results in invalid MAC addresses
> half of the time.
> 
> This is because gretap is still using the tunnel netdev ops rather
> than the correct tap netdev ops struct.
> 
> This patch also fixes changelink to not clobber the MAC address
> for the gretap case.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Thanks,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 41ada99..1433338 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1464,7 +1464,7 @@ static void ipgre_tap_setup(struct net_device
> *dev)
> 
>  	ether_setup(dev);
> 
> -	dev->netdev_ops		= &ipgre_netdev_ops;
> +	dev->netdev_ops		= &ipgre_tap_netdev_ops;
>  	dev->destructor 	= free_netdev;
> 
>  	dev->iflink		= 0;
> @@ -1525,25 +1525,29 @@ static int ipgre_changelink(struct net_device
> *dev, struct nlattr *tb[],
>  		if (t->dev != dev)
>  			return -EEXIST;
>  	} else {
> -		unsigned nflags = 0;
> -
>  		t = nt;
> 
> -		if (ipv4_is_multicast(p.iph.daddr))
> -			nflags = IFF_BROADCAST;
> -		else if (p.iph.daddr)
> -			nflags = IFF_POINTOPOINT;
> +		if (dev->type != ARPHRD_ETHER) {
> +			unsigned nflags = 0;
> 
> -		if ((dev->flags ^ nflags) &
> -		    (IFF_POINTOPOINT | IFF_BROADCAST))
> -			return -EINVAL;
> +			if (ipv4_is_multicast(p.iph.daddr))
> +				nflags = IFF_BROADCAST;
> +			else if (p.iph.daddr)
> +				nflags = IFF_POINTOPOINT;
> +
> +			if ((dev->flags ^ nflags) &
> +			    (IFF_POINTOPOINT | IFF_BROADCAST))
> +				return -EINVAL;
> +		}
> 
>  		ipgre_tunnel_unlink(ign, t);
>  		t->parms.iph.saddr = p.iph.saddr;
>  		t->parms.iph.daddr = p.iph.daddr;
>  		t->parms.i_key = p.i_key;
> -		memcpy(dev->dev_addr, &p.iph.saddr, 4);
> -		memcpy(dev->broadcast, &p.iph.daddr, 4);
> +		if (dev->type != ARPHRD_ETHER) {
> +			memcpy(dev->dev_addr, &p.iph.saddr, 4);
> +			memcpy(dev->broadcast, &p.iph.daddr, 4);
> +		}
>  		ipgre_tunnel_link(ign, t);
>  		netdev_state_change(dev);
>  	}

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

* Re: How to use gretap with bridge?
  2009-10-30 16:39               ` Stephen Hemminger
@ 2009-10-30 19:27                 ` David Miller
  0 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2009-10-30 19:27 UTC (permalink / raw)
  To: shemminger; +Cc: herbert, nneul, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 30 Oct 2009 09:39:57 -0700

> On Fri, 30 Oct 2009 11:51:48 -0400
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
>> gre: Fix dev_addr clobbering for gretap
>> 
>> Nathan Neulinger noticed that gretap devices get their MAC address
>> from the local IP address, which results in invalid MAC addresses
>> half of the time.
>> 
>> This is because gretap is still using the tunnel netdev ops rather
>> than the correct tap netdev ops struct.
>> 
>> This patch also fixes changelink to not clobber the MAC address
>> for the gretap case.
>> 
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>> 
>> Thanks,
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Applied to net-2.6, thanks!

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

* Re: [RFC] bridge: check address size
  2009-10-29 22:24           ` [RFC] bridge: check address size Stephen Hemminger
@ 2009-11-02  8:07             ` David Miller
  2009-11-02 16:54               ` Stephen Hemminger
  2009-11-04 17:47               ` [PATCH] bridge: prevent bridging wrong device Stephen Hemminger
  0 siblings, 2 replies; 18+ messages in thread
From: David Miller @ 2009-11-02  8:07 UTC (permalink / raw)
  To: shemminger; +Cc: nneul, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 29 Oct 2009 15:24:08 -0700

> -	if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
> +	/* Don't allow bridging non ethernet like devices */
> +	if (dev->flags & IFF_LOOPBACK
> +	    || dev->type != ARPHRD_ETHER
> +	    || dev->addr_len != ETH_ALEN)

Please format this as:

> +	if (dev->flags & IFF_LOOPBACK ||
> +	    dev->type != ARPHRD_ETHER ||
> +	    dev->addr_len != ETH_ALEN)

What you're doing in the patch follows the GNU coding standards, no
the kernel ones. :-)

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

* Re: [RFC] bridge: check address size
  2009-11-02  8:07             ` David Miller
@ 2009-11-02 16:54               ` Stephen Hemminger
  2009-11-04 17:47               ` [PATCH] bridge: prevent bridging wrong device Stephen Hemminger
  1 sibling, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2009-11-02 16:54 UTC (permalink / raw)
  To: David Miller; +Cc: nneul, netdev

On Mon, 02 Nov 2009 00:07:56 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 29 Oct 2009 15:24:08 -0700
> 
> > -	if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
> > +	/* Don't allow bridging non ethernet like devices */
> > +	if (dev->flags & IFF_LOOPBACK
> > +	    || dev->type != ARPHRD_ETHER
> > +	    || dev->addr_len != ETH_ALEN)
> 
> Please format this as:
> 
> > +	if (dev->flags & IFF_LOOPBACK ||
> > +	    dev->type != ARPHRD_ETHER ||
> > +	    dev->addr_len != ETH_ALEN)
> 
> What you're doing in the patch follows the GNU coding standards, no
> the kernel ones. :-)

Sure, I never saw which side to put conditionals as part of coding standard document.

-- 

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

* [PATCH]  bridge: prevent bridging wrong device
  2009-11-02  8:07             ` David Miller
  2009-11-02 16:54               ` Stephen Hemminger
@ 2009-11-04 17:47               ` Stephen Hemminger
  2009-11-06  5:08                 ` David Miller
  1 sibling, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2009-11-04 17:47 UTC (permalink / raw)
  To: David Miller; +Cc: nneul, netdev


The bridge code assumes ethernet addressing, so be more strict in
the what is allowed. This showed up when GRE had a bug and was not
using correct address format.

Add some more comments for increased clarity.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/bridge/br_if.c	2009-10-29 15:18:48.363916679 -0700
+++ b/net/bridge/br_if.c	2009-11-03 19:39:17.733252912 -0800
@@ -377,12 +377,16 @@ int br_add_if(struct net_bridge *br, str
 	struct net_bridge_port *p;
 	int err = 0;
 
-	if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
+	/* Don't allow bridging non-ethernet like devices */
+	if ((dev->flags & IFF_LOOPBACK) ||
+	    dev->type != ARPHRD_ETHER || dev->addr_len != ETH_ALEN)
 		return -EINVAL;
 
+	/* No bridging of bridges */
 	if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit)
 		return -ELOOP;
 
+	/* Device is already being bridged */
 	if (dev->br_port != NULL)
 		return -EBUSY;
 

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

* Re: [PATCH] bridge: prevent bridging wrong device
  2009-11-04 17:47               ` [PATCH] bridge: prevent bridging wrong device Stephen Hemminger
@ 2009-11-06  5:08                 ` David Miller
  0 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2009-11-06  5:08 UTC (permalink / raw)
  To: shemminger; +Cc: nneul, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 4 Nov 2009 09:47:13 -0800

> The bridge code assumes ethernet addressing, so be more strict in
> the what is allowed. This showed up when GRE had a bug and was not
> using correct address format.
> 
> Add some more comments for increased clarity.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied to net-2.6, thanks.

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

end of thread, other threads:[~2009-11-06  5:07 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <846C5B546E47494CBBD796CA8CA1617EA3B414@MST-VMAIL1.srv.mst.edu>
     [not found] ` <20091029170631.GA29405@gondor.apana.org.au>
2009-10-29 18:39   ` How to use gretap with bridge? Neulinger, Nathan
2009-10-29 19:01   ` Neulinger, Nathan
2009-10-29 20:00     ` Stephen Hemminger
2009-10-29 20:22       ` Neulinger, Nathan
2009-10-29 22:12         ` Stephen Hemminger
2009-10-29 22:24           ` [RFC] bridge: check address size Stephen Hemminger
2009-11-02  8:07             ` David Miller
2009-11-02 16:54               ` Stephen Hemminger
2009-11-04 17:47               ` [PATCH] bridge: prevent bridging wrong device Stephen Hemminger
2009-11-06  5:08                 ` David Miller
2009-10-29 20:59       ` How to use gretap with bridge? Neulinger, Nathan
2009-10-29 22:04       ` Neulinger, Nathan
2009-10-29 23:47         ` Herbert Xu
2009-10-29 23:51           ` Neulinger, Nathan
2009-10-30 15:51             ` Herbert Xu
2009-10-30 16:39               ` Stephen Hemminger
2009-10-30 19:27                 ` David Miller
2009-10-30 17:08               ` Neulinger, Nathan

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.