* 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: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: [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: 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 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 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: 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