Patchwork How to use gretap with bridge?

login
register
mail settings
Submitter Herbert Xu
Date Oct. 30, 2009, 3:51 p.m.
Message ID <20091030155148.GA8283@gondor.apana.org.au>
Download mbox | patch
Permalink /patch/37311/
State Accepted
Delegated to: David Miller
Headers show

Comments

Herbert Xu - Oct. 30, 2009, 3:51 p.m.
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,
stephen hemminger - Oct. 30, 2009, 4:39 p.m.
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>
Neulinger, Nathan - Oct. 30, 2009, 5:08 p.m.
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);
>  	}
--
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
David Miller - Oct. 30, 2009, 7:27 p.m.
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!
--
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

Patch

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);
 	}