gre: transparent ethernet bridging

RFC In review Philip Craig 3 participants 17 messages Jul 31, 2006 – Aug 10, 2006

Subsystems (5)

Networking Network drivers netfilter IPv4/IPv6 +1 more subsystems

lore

Share & tools

lore

Philip Craig

author

2006-07-31 10:06 UTC #0 lore raw

RFC 1/1 gre: transparent ethernet bridging

This patch implements transparent ethernet bridging for gre tunnels. There are a few outstanding issues.

There is no way for userspace to select the type of gre tunnel. The #if 0 near the top of the patch forces all gre tunnels to be bridges. The problem is that userspace uses an IPPROTO_ to select the type of tunnel, but both types of gre tunnel are IPPROTO_GRE. I can't see anything else in struct ip_tunnel_parm that could be used to select this. One approach that I've seen mentioned in the archives is to add a netlink interface to replace the tunnel ioctls.

Network loops are bad. See the comments at the top of ip_gre.c for a description of how gre tunnels handle this normally. But for gre bridges, we don't want to copy the ttl (it breaks routing protocols), and we don't want to force DF (we want to bridge 1500 byte packets). I couldn't think of any solution for this.

Some routers set LLC_SAP_BSPAN in the gre protocol field, and then give the bpdu packet without any other ethernet/llc header. This patch currently tries to fake the ethernet/llc header before passing the packet up, but it is buggy (mac addresses are wrong at least). Maybe a better approach is to call directly into the bridging code. I didn't try that at first because it isn't modular, and may break other things that want to see the packet.

net/ipv4/ip_gre.c +130 ~~-44~~

@@ -30,6 +30,8 @@
 #include <linux/igmp.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/if_ether.h>
+#include <linux/etherdevice.h>
+#include <linux/llc.h>
 
 #include <net/sock.h>
 #include <net/ip.h>

@@ -41,6 +43,8 @@
 #include <net/dsfield.h>
 #include <net/inet_ecn.h>
 #include <net/xfrm.h>
+#include <net/llc.h>
+#include <net/llc_pdu.h>
 
 #ifdef CONFIG_IPV6
 #include <net/ipv6.h>

@@ -119,6 +123,7 @@
 
 static int ipgre_tunnel_init(struct net_device *dev);
 static void ipgre_tunnel_setup(struct net_device *dev);
+static void ipgre_ether_tunnel_setup(struct net_device *dev);
 
 /* Fallback tunnel: no source, no destination, no key, no options */

@@ -274,7 +279,11 @@ static struct ip_tunnel * ipgre_tunnel_l
 			goto failed;
 	}
 
+#if 0
 	dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
+#else
+	dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
+#endif
 	if (!dev)
 	  return NULL;

@@ -550,6 +559,68 @@ ipgre_ecn_encapsulate(u8 tos, struct iph
 	return INET_ECN_encapsulate(tos, inner);
 }
 
+__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
+{
+	u8     *h = skb->data;
+	__be16 flags = *(__be16*)h;
+	__be16 proto = *(__be16*)(h + 2);
+
+	/* WCCP version 1 and 2 protocol decoding.
+	 * - Change protocol to IP
+	 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+	 */
+	if (flags == 0 &&
+	    proto == __constant_htons(ETH_P_WCCP)) {
+		proto = __constant_htons(ETH_P_IP);
+		if ((*(h + offset) & 0xF0) != 0x40)
+			offset += 4;
+	}
+
+	skb->mac.raw = skb->nh.raw;
+	skb->nh.raw = __pskb_pull(skb, offset);
+	skb_postpull_rcsum(skb, skb->h.raw, offset);
+#ifdef CONFIG_NET_IPGRE_BROADCAST
+	if (MULTICAST(iph->daddr)) {
+		/* Looped back packet, drop it! */
+		if (((struct rtable*)skb->dst)->fl.iif == 0)
+			return 0;
+		/* tunnel->stat.multicast++; */
+		skb->pkt_type = PACKET_BROADCAST;
+	}
+#endif
+
+	return proto;
+}
+
+extern const u8 br_group_address[ETH_ALEN];
+
+__be16 ipgre_ether_type_trans(struct sk_buff *skb, struct net_device *dev,
+			      int offset)
+{
+	u8     *h = skb->data;
+	__be16 proto = *(__be16*)(h + 2);
+
+	if (proto == htons(ETH_P_BRIDGE)) {
+		if (!pskb_may_pull(skb, offset + ETH_HLEN))
+			return 0;
+		skb_pull_rcsum(skb, offset);
+		return eth_type_trans(skb, dev);
+	} else if (proto == htons(LLC_SAP_BSPAN)) {
+		skb_pull_rcsum(skb, offset);
+
+		llc_pdu_header_init(skb, LLC_PDU_TYPE_U, LLC_SAP_BSPAN,
+				    LLC_SAP_BSPAN, LLC_PDU_CMD);
+		llc_pdu_init_as_ui_cmd(skb);
+
+		llc_mac_hdr_init(skb, dev->dev_addr, dev->dev_addr);
+		skb_pull(skb, ETH_HLEN);
+
+		return htons(ETH_P_802_2);
+	}
+
+	return 0;
+}
+
 static int ipgre_rcv(struct sk_buff *skb)
 {
 	struct iphdr *iph;

@@ -603,32 +674,8 @@ static int ipgre_rcv(struct sk_buff *skb
 	if ((tunnel = ipgre_tunnel_lookup(iph->saddr, iph->daddr, key)) != NULL) {
 		secpath_reset(skb);
 
-		skb->protocol = *(u16*)(h + 2);
-		/* WCCP version 1 and 2 protocol decoding.
-		 * - Change protocol to IP
-		 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
-		 */
-		if (flags == 0 &&
-		    skb->protocol == __constant_htons(ETH_P_WCCP)) {
-			skb->protocol = __constant_htons(ETH_P_IP);
-			if ((*(h + offset) & 0xF0) != 0x40)
-				offset += 4;
-		}
-
-		skb->mac.raw = skb->nh.raw;
-		skb->nh.raw = __pskb_pull(skb, offset);
-		skb_postpull_rcsum(skb, skb->h.raw, offset);
 		memset(&(IPCB(skb)->opt), 0, sizeof(struct ip_options));
 		skb->pkt_type = PACKET_HOST;
-#ifdef CONFIG_NET_IPGRE_BROADCAST
-		if (MULTICAST(iph->daddr)) {
-			/* Looped back packet, drop it! */
-			if (((struct rtable*)skb->dst)->fl.iif == 0)
-				goto drop;
-			tunnel->stat.multicast++;
-			skb->pkt_type = PACKET_BROADCAST;
-		}
-#endif
 
 		if (((flags&GRE_CSUM) && csum) ||
 		    (!(flags&GRE_CSUM) && tunnel->parms.i_flags&GRE_CSUM)) {

@@ -645,6 +692,15 @@ static int ipgre_rcv(struct sk_buff *skb
 			}
 			tunnel->i_seqno = seqno + 1;
 		}
+		if (tunnel->dev->type == ARPHRD_ETHER)
+			skb->protocol = ipgre_ether_type_trans(skb, tunnel->dev,
+							       offset);
+		else
+			skb->protocol = ipgre_type_trans(skb, offset);
+		if (!skb->protocol) {
+			tunnel->stat.rx_errors++;
+			goto drop;
+		}
 		tunnel->stat.rx_packets++;
 		tunnel->stat.rx_bytes += skb->len;
 		skb->dev = tunnel->dev;

@@ -686,7 +742,10 @@ static int ipgre_tunnel_xmit(struct sk_b
 		goto tx_error;
 	}
 
-	if (dev->hard_header) {
+	if (dev->type == ARPHRD_ETHER) {
+		gre_hlen = tunnel->hlen - ETH_HLEN;
+		tiph = &tunnel->parms.iph;
+	} else if (dev->hard_header) {
 		gre_hlen = 0;
 		tiph = (struct iphdr*)skb->data;
 	} else {

@@ -767,7 +826,7 @@ static int ipgre_tunnel_xmit(struct sk_b
 	else
 		mtu = skb->dst ? dst_mtu(skb->dst) : dev->mtu;
 
-	if (skb->dst)
+	if (skb->dst && skb->dst->ops)
 		skb->dst->ops->update_pmtu(skb->dst, mtu);
 
 	if (skb->protocol == htons(ETH_P_IP)) {

@@ -849,7 +908,9 @@ static int ipgre_tunnel_xmit(struct sk_b
 	iph->saddr		=	rt->rt_src;
 
 	if ((iph->ttl = tiph->ttl) == 0) {
-		if (skb->protocol == htons(ETH_P_IP))
+		if (dev->type == ARPHRD_ETHER)
+			iph->ttl = dst_metric(&rt->u.dst, RTAX_HOPLIMIT);
+		else if (skb->protocol == htons(ETH_P_IP))
 			iph->ttl = old_iph->ttl;
 #ifdef CONFIG_IPV6
 		else if (skb->protocol == htons(ETH_P_IPV6))

@@ -860,7 +921,10 @@ static int ipgre_tunnel_xmit(struct sk_b
 	}
 
 	((u16*)(iph+1))[0] = tunnel->parms.o_flags;
-	((u16*)(iph+1))[1] = skb->protocol;
+	if (dev->type == ARPHRD_ETHER)
+		((__be16*)(iph+1))[1] = htons(ETH_P_BRIDGE);
+	else
+		((__be16*)(iph+1))[1] = skb->protocol;
 
 	if (tunnel->parms.o_flags&(GRE_KEY|GRE_CSUM|GRE_SEQ)) {
 		u32 *ptr = (u32*)(((u8*)iph) + tunnel->hlen - 4);

@@ -956,7 +1020,9 @@ ipgre_tunnel_ioctl (struct net_device *d
 
 				t = netdev_priv(dev);
 
-				if (MULTICAST(p.iph.daddr))
+				if (t->dev->type == ARPHRD_ETHER)
+					nflags = IFF_BROADCAST;
+				else if (MULTICAST(p.iph.daddr))
 					nflags = IFF_BROADCAST;
 				else if (p.iph.daddr)
 					nflags = IFF_POINTOPOINT;

@@ -1147,6 +1213,18 @@ static void ipgre_tunnel_setup(struct ne
 	dev->addr_len		= 4;
 }
 
+static void ipgre_ether_tunnel_setup(struct net_device *dev)
+{
+	ether_setup(dev);
+
+	SET_MODULE_OWNER(dev);
+	dev->uninit		= ipgre_tunnel_uninit;
+	dev->destructor 	= free_netdev;
+	dev->hard_start_xmit	= ipgre_tunnel_xmit;
+	dev->get_stats		= ipgre_tunnel_get_stats;
+	dev->do_ioctl		= ipgre_tunnel_ioctl;
+}
+
 static int ipgre_tunnel_init(struct net_device *dev)
 {
 	struct net_device *tdev = NULL;

@@ -1162,8 +1240,27 @@ static int ipgre_tunnel_init(struct net_
 	tunnel->dev = dev;
 	strcpy(tunnel->parms.name, dev->name);
 
-	memcpy(dev->dev_addr, &tunnel->parms.iph.saddr, 4);
-	memcpy(dev->broadcast, &tunnel->parms.iph.daddr, 4);
+	if (dev->type == ARPHRD_ETHER)
+		random_ether_addr(dev->dev_addr);
+	else {
+		memcpy(dev->dev_addr, &tunnel->parms.iph.saddr, 4);
+		memcpy(dev->broadcast, &tunnel->parms.iph.daddr, 4);
+	}
+
+	if (dev->type == ARPHRD_ETHER)
+		dev->flags |= IFF_BROADCAST;
+#ifdef CONFIG_NET_IPGRE_BROADCAST
+	else if (MULTICAST(iph->daddr)) {
+		if (!iph->saddr)
+			return -EINVAL;
+		dev->flags = IFF_BROADCAST;
+		dev->hard_header = ipgre_header;
+		dev->open = ipgre_open;
+		dev->stop = ipgre_close;
+	}
+#endif
+	else if (iph->daddr)
+		dev->flags |= IFF_POINTOPOINT;
 
 	/* Guess output device to choose reasonable mtu and hard_header_len */

@@ -1179,19 +1276,6 @@ static int ipgre_tunnel_init(struct net_
 			tdev = rt->u.dst.dev;
 			ip_rt_put(rt);
 		}
-
-		dev->flags |= IFF_POINTOPOINT;
-
-#ifdef CONFIG_NET_IPGRE_BROADCAST
-		if (MULTICAST(iph->daddr)) {
-			if (!iph->saddr)
-				return -EINVAL;
-			dev->flags = IFF_BROADCAST;
-			dev->hard_header = ipgre_header;
-			dev->open = ipgre_open;
-			dev->stop = ipgre_close;
-		}
-#endif
 	}
 
 	if (!tdev && tunnel->parms.link)

@@ -1212,6 +1296,8 @@ static int ipgre_tunnel_init(struct net_
 		if (tunnel->parms.o_flags&GRE_SEQ)
 			addend += 4;
 	}
+	if (dev->type == ARPHRD_ETHER)
+		addend += ETH_HLEN;
 	dev->hard_header_len = hlen + addend;
 	dev->mtu = mtu - addend;
 	tunnel->hlen = addend;

include/linux/if_ether.h +1 -0

@@ -55,6 +55,7 @@
 #define ETH_P_DIAG      0x6005          /* DEC Diagnostics              */
 #define ETH_P_CUST      0x6006          /* DEC Customer use             */
 #define ETH_P_SCA       0x6007          /* DEC Systems Comms Arch       */
+#define ETH_P_BRIDGE    0x6558          /* Transparent Ethernet Bridging */
 #define ETH_P_RARP      0x8035		/* Reverse Addr Res packet	*/
 #define ETH_P_ATALK	0x809B		/* Appletalk DDP		*/
 #define ETH_P_AARP	0x80F3		/* Appletalk AARP		*/

Stephen Hemminger

2006-07-31 16:14 UTC #1 lore raw

Quoted 2 lines · On Mon, 31 Jul 2006 20:06:41 +1000 Philip Craig <[email protected]> wrote:

This patch implements transparent ethernet bridging for gre tunnels. There are a few outstanding issues.

Why not use existing bridge code?

Quoted 14 lines

There is no way for userspace to select the type of gre tunnel. The #if 0 near the top of the patch forces all gre tunnels to be bridges. The problem is that userspace uses an IPPROTO_ to select the type of tunnel, but both types of gre tunnel are IPPROTO_GRE. I can't see anything else in struct ip_tunnel_parm that could be used to select this. One approach that I've seen mentioned in the archives is to add a netlink interface to replace the tunnel ioctls.
Network loops are bad. See the comments at the top of ip_gre.c for a description of how gre tunnels handle this normally. But for gre bridges, we don't want to copy the ttl (it breaks routing protocols), and we don't want to force DF (we want to bridge 1500 byte packets). I couldn't think of any solution for this.

Quoted 7 lines

Some routers set LLC_SAP_BSPAN in the gre protocol field, and then give the bpdu packet without any other ethernet/llc header. This patch currently tries to fake the ethernet/llc header before passing the packet up, but it is buggy (mac addresses are wrong at least). Maybe a better approach is to call directly into the bridging code. I didn't try that at first because it isn't modular, and may break other things that want to see the packet.

Existing bridge code already has spanning tree.

Quoted 29 lines

net/ipv4/ip_gre.c +5 -0

@@ -30,6 +30,8 @@
 #include <linux/igmp.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/if_ether.h>
+#include <linux/etherdevice.h>
+#include <linux/llc.h>
 
 #include <net/sock.h>
 #include <net/ip.h>

@@ -41,6 +43,8 @@
 #include <net/dsfield.h>
 #include <net/inet_ecn.h>
 #include <net/xfrm.h>
+#include <net/llc.h>
+#include <net/llc_pdu.h>
 
 #ifdef CONFIG_IPV6
 #include <net/ipv6.h>

@@ -119,6 +123,7 @@
 
 static int ipgre_tunnel_init(struct net_device *dev);
 static void ipgre_tunnel_setup(struct net_device *dev);
+static void ipgre_ether_tunnel_setup(struct net_device *dev);
 
 /* Fallback tunnel: no source, no destination, no key, no options */

net/ipv4/ip_gre.c static struct ip_tunnel * ipgre_tunnel_l · line 286

Quoted 8 lines

net/ipv4/ip_gre.c +4 -0

@@ -274,7 +279,11 @@ static struct ip_tunnel * ipgre_tunnel_l
 			goto failed;
 	}
 
+#if 0
 	dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
+#else
+	dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
+#endif

"Do, or do not there is no try"

Quoted 6 lines

net/ipv4/ip_gre.c +6 -0

+__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
+{
+	u8     *h = skb->data;
+	__be16 flags = *(__be16*)h;
+	__be16 proto = *(__be16*)(h + 2);
+

net/ipv4/ip_gre.c ipgre_ecn_encapsulate(u8 tos, struct iph · line 577

Quoted 10 lines

net/ipv4/ip_gre.c +10 -0

+	/* WCCP version 1 and 2 protocol decoding.
+	 * - Change protocol to IP
+	 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+	 */
+	if (flags == 0 &&
+	    proto == __constant_htons(ETH_P_WCCP)) {
+		proto = __constant_htons(ETH_P_IP);
+		if ((*(h + offset) & 0xF0) != 0x40)
+			offset += 4;
+	}

Don't use __constant_htons() except in initializers and switch cases (where gcc is too stupid to optimize the macro).

Signature · 2 lines

Stephen Hemminger <[email protected]>
"And in the Packet there writ down that doome"

Philip Craig

author

2006-08-01 01:15 UTC #2 lore raw

Quoted 7 lines · Stephen Hemminger

Quoted 2 lines · On Mon, 31 Jul 2006 20:06:41 +1000 Philip Craig <[email protected]> wrote:
This patch implements transparent ethernet bridging for gre tunnels. There are a few outstanding issues.
Why not use existing bridge code?

It does use the existing bridge code. Perhaps the name is misleading. All it does is encapsulate the full ethernet header in a gre packet, rather than only layer 3. That is, currently gre uses ARPHRD_IPGRE, but bridging requires ARPHRD_ETHER.

Quoted 9 lines

Quoted 7 lines
Some routers set LLC_SAP_BSPAN in the gre protocol field, and then give the bpdu packet without any other ethernet/llc header. This patch currently tries to fake the ethernet/llc header before passing the packet up, but it is buggy (mac addresses are wrong at least). Maybe a better approach is to call directly into the bridging code. I didn't try that at first because it isn't modular, and may break other things that want to see the packet.
Existing bridge code already has spanning tree.

Yes, and I want to use that. But this packet is a bit strange in
that it does not have the ethernet header on it. So what is the
best way to pass it to existing code? Either fake the ethernet header, or pass it directly?

Quoted 7 lines

net/ipv4/ip_gre.c static struct ip_tunnel * ipgre_tunnel_l · line 286
Quoted 5 lines
net/ipv4/ip_gre.c +4 -0
+#if 0
 	dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
+#else
+	dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
+#endif
"Do, or do not there is no try"

I am looking for comments as to whether adding a netlink interface to control this is appropriate.

Quoted 11 lines

net/ipv4/ip_gre.c +11 -0

+__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
+{
+	u8     *h = skb->data;
+	__be16 flags = *(__be16*)h;
+	__be16 proto = *(__be16*)(h + 2);
+
+	/* WCCP version 1 and 2 protocol decoding.
+	 * - Change protocol to IP
+	 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+	 */
+	if (flags == 0 &&

Quoted 9 lines

net/ipv4/ip_gre.c ipgre_ecn_encapsulate(u8 tos, struct iph · line 577
Quoted 5 lines
net/ipv4/ip_gre.c +5 -0
+	    proto == __constant_htons(ETH_P_WCCP)) {
+		proto = __constant_htons(ETH_P_IP);
+		if ((*(h + offset) & 0xF0) != 0x40)
+			offset += 4;
+	}
Don't use __constant_htons() except in initializers and switch cases (where gcc is too stupid to optimize the macro).

This is a problem in the existing code, which I am simply moving around. Should I fix it at the same time?

Stephen Hemminger

2006-08-01 05:08 UTC #3 lore raw

Quoted 7 lines · On Tue, 01 Aug 2006 11:15:29 +1000 Philip Craig <[email protected]> wrote:

Quoted 6 lines · Stephen Hemminger
Quoted 2 lines · On Mon, 31 Jul 2006 20:06:41 +1000 Philip Craig <[email protected]> wrote:
This patch implements transparent ethernet bridging for gre tunnels. There are a few outstanding issues.

Quoted 7 lines

Quoted 1 line
Why not use existing bridge code?
It does use the existing bridge code. Perhaps the name is misleading. All it does is encapsulate the full ethernet header in a gre packet, rather than only layer 3. That is, currently gre uses ARPHRD_IPGRE, but bridging requires ARPHRD_ETHER.

I am not against making the bridge code smarter to handle other encapsulation.

Quoted 8 lines

Quoted 8 lines
Quoted 7 lines
Some routers set LLC_SAP_BSPAN in the gre protocol field, and then give the bpdu packet without any other ethernet/llc header. This patch currently tries to fake the ethernet/llc header before passing the packet up, but it is buggy (mac addresses are wrong at least). Maybe a better approach is to call directly into the bridging code. I didn't try that at first because it isn't modular, and may break other things that want to see the packet.

Quoted 6 lines

Quoted 1 line
Existing bridge code already has spanning tree.
Yes, and I want to use that. But this packet is a bit strange in
that it does not have the ethernet header on it. So what is the
best way to pass it to existing code? Either fake the ethernet header, or pass it directly?

Likewise if the bridge STP bpdu input code was smarter, it could deal with it maybe?

Quoted 11 lines

Quoted 8 lines
net/ipv4/ip_gre.c static struct ip_tunnel * ipgre_tunnel_l · line 286
Quoted 6 lines
net/ipv4/ip_gre.c +4 -0
+#if 0
 	dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
+#else
+	dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
+#endif
"Do, or do not there is no try"
I am looking for comments as to whether adding a netlink interface to control this is appropriate.

If we make bridge code type aware, then the ipgre tunnel wouldn't have to change.

Quoted 21 lines

Quoted 11 lines
Quoted 11 lines
net/ipv4/ip_gre.c +11 -0
+__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
+{
+	u8     *h = skb->data;
+	__be16 flags = *(__be16*)h;
+	__be16 proto = *(__be16*)(h + 2);
+
+	/* WCCP version 1 and 2 protocol decoding.
+	 * - Change protocol to IP
+	 * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+	 */
+	if (flags == 0 &&
Quoted 9 lines
net/ipv4/ip_gre.c ipgre_ecn_encapsulate(u8 tos, struct iph · line 577
Quoted 5 lines
net/ipv4/ip_gre.c +5 -0
+	    proto == __constant_htons(ETH_P_WCCP)) {
+		proto = __constant_htons(ETH_P_IP);
+		if ((*(h + offset) & 0xF0) != 0x40)
+			offset += 4;
+	}
Don't use __constant_htons() except in initializers and switch cases (where gcc is too stupid to optimize the macro).

Quoted 2 lines

This is a problem in the existing code, which I am simply moving around. Should I fix it at the same time?

Usually if a diff touches some code, I try to make it use current practice.

Philip Craig

author

2006-08-01 09:29 UTC #4 lore raw

Quoted 2 lines · Stephen Hemminger

I am not against making the bridge code smarter to handle other encapsulation.

Do you mean something like this patch?

The only drawback I see for this approach is that it means you can only encapsulate the ethernet header if the gre interface is bridged. That's not too bad a restriction though.

This patch only works for local packets so far, and doesn't handle the LLC_SAP_BSPAN packets.

Also, if the gre interface is the only port on the bridge, then we have no mac address.

net/bridge/br_device.c +4 -1

@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_arp.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"

@@ -95,7 +96,9 @@ static int br_set_mac_address(struct net
 
 	spin_lock_bh(&br->lock);
 	list_for_each_entry(port, &br->port_list, list) {
-		if (!compare_ether_addr(port->dev->dev_addr, addr->sa_data)) {
+		if (port->dev->type == ARPHRD_ETHER &&
+				!compare_ether_addr(port->dev->dev_addr,
+						    addr->sa_data)) {
 			br_stp_change_bridge_id(br, addr->sa_data);
 			err = 0;
 			break;

net/bridge/br_fdb.c +3 -0

@@ -20,6 +20,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/jhash.h>
+#include <linux/if_arp.h>
 #include <asm/atomic.h>
 #include "br_private.h"

@@ -86,6 +87,7 @@ void br_fdb_changeaddr(struct net_bridge
 				struct net_bridge_port *op;
 				list_for_each_entry(op, &br->port_list, list) {
 					if (op != p &&
+					    op->dev->type == ARPHRD_ETHER &&
 					    !compare_ether_addr(op->dev->dev_addr,
 								f->addr.addr)) {
 						f->dst = op;

@@ -151,6 +153,7 @@ void br_fdb_delete_by_port(struct net_br
 				struct net_bridge_port *op;
 				list_for_each_entry(op, &br->port_list, list) {
 					if (op != p &&
+					    op->dev->type == ARPHRD_ETHER &&
 					    !compare_ether_addr(op->dev->dev_addr,
 								f->addr.addr)) {
 						f->dst = op;

net/bridge/br_forward.c +3 -0

@@ -18,6 +18,7 @@
 #include <linux/skbuff.h>
 #include <linux/if_vlan.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_arp.h>
 #include "br_private.h"
 
 static inline int should_deliver(const struct net_bridge_port *p,

@@ -46,6 +47,8 @@ int br_dev_queue_push_xmit(struct sk_buf
 		nf_bridge_maybe_copy_header(skb);
 #endif
 		skb_push(skb, ETH_HLEN);
+		if (skb->dev->type == ARPHRD_IPGRE)
+			skb->protocol = htons(ETH_P_BRIDGE);
 
 		dev_queue_xmit(skb);
 	}

net/bridge/br_if.c +9 -4

@@ -391,7 +391,10 @@ 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)
+	if (dev->flags & IFF_LOOPBACK)
+		return -EINVAL;
+
+	if (dev->type != ARPHRD_ETHER && dev->type != ARPHRD_IPGRE)
 		return -EINVAL;
 
 	if (dev->hard_start_xmit == br_dev_xmit)

@@ -408,9 +411,11 @@ int br_add_if(struct net_bridge *br, str
 	if (err)
 		goto err0;
 
- 	err = br_fdb_insert(br, p, dev->dev_addr);
-	if (err)
-		goto err1;
+	if (dev->type == ARPHRD_ETHER) {
+ 		err = br_fdb_insert(br, p, dev->dev_addr);
+		if (err)
+			goto err1;
+	}
 
 	err = br_sysfs_addif(p);
 	if (err)

net/bridge/br_input.c +13 -1

@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_arp.h>
 #include "br_private.h"
 
 /* Bridge group multicast address 802.1d (pg 51). */

@@ -124,11 +125,22 @@ static inline int is_link_local(const un
 int br_handle_frame(struct net_bridge_port *p, struct sk_buff **pskb)
 {
 	struct sk_buff *skb = *pskb;
-	const unsigned char *dest = eth_hdr(skb)->h_dest;
+	const unsigned char *dest;
+
+	if (skb->dev->type == ARPHRD_IPGRE) {
+		if (skb->protocol != htons(ETH_P_BRIDGE))
+			return 0;
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			goto err;
+		skb->protocol = eth_type_trans(skb, p->br->dev);
+		skb_postpull_rcsum(skb, skb->mac.raw, ETH_HLEN);
+		skb->nh.raw += ETH_HLEN;
+	}
 
 	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
 		goto err;
 
+	dest = eth_hdr(skb)->h_dest;
 	if (unlikely(is_link_local(dest))) {
 		skb->pkt_type = PACKET_HOST;
 		return NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,

net/bridge/br_notify.c +5 -2

@@ -14,6 +14,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/if_arp.h>
 
 #include "br_private.h"

@@ -48,8 +49,10 @@ static int br_device_event(struct notifi
 		break;
 
 	case NETDEV_CHANGEADDR:
-		br_fdb_changeaddr(p, dev->dev_addr);
-		br_stp_recalculate_bridge_id(br);
+		if (dev->type == ARPHRD_ETHER) {
+			br_fdb_changeaddr(p, dev->dev_addr);
+			br_stp_recalculate_bridge_id(br);
+		}
 		break;
 
 	case NETDEV_CHANGE:

include/linux/if_ether.h +1 -0

@@ -55,6 +55,7 @@
 #define ETH_P_DIAG      0x6005          /* DEC Diagnostics              */
 #define ETH_P_CUST      0x6006          /* DEC Customer use             */
 #define ETH_P_SCA       0x6007          /* DEC Systems Comms Arch       */
+#define ETH_P_BRIDGE    0x6558          /* Transparent Ethernet Bridging */
 #define ETH_P_RARP      0x8035		/* Reverse Addr Res packet	*/
 #define ETH_P_ATALK	0x809B		/* Appletalk DDP		*/
 #define ETH_P_AARP	0x80F3		/* Appletalk AARP		*/

Philip Craig

author

2006-08-02 06:17 UTC #5 lore raw

Quoted 3 lines

Quoted 2 lines · Stephen Hemminger
I am not against making the bridge code smarter to handle other encapsulation.

Here's an updated patch that fixes all issues I am aware of.

It generates a random mac address for gre ports, and also stores a copy of the mac address for ethernet ports, rather than checking dev->type everywhere.

The LLC_SAP_BSPAN packets are handled by simply registering that protocol with dev_add_pack(). This would have worked for my original patch too.

I had to release __fake_rtable as part of br_nf_dev_queue_xmit(), otherwise ip_gre.c paniced trying to call skb->dst->ops->update_ptmu.

net/bridge/br.c +8 -0

@@ -26,6 +26,11 @@
 
 int (*br_should_route_hook) (struct sk_buff **pskb) = NULL;
 
+static struct packet_type br_stp_packet_type = {
+	.type = __constant_htons(LLC_SAP_BSPAN),
+	.func = br_stp_packet_rcv,
+};
+
 static struct llc_sap *br_stp_sap;
 
 static int __init br_init(void)

@@ -36,6 +41,8 @@ static int __init br_init(void)
 		return -EBUSY;
 	}
 
+	dev_add_pack(&br_stp_packet_type);
+	
 	br_fdb_init();
 
 #ifdef CONFIG_BRIDGE_NETFILTER

@@ -56,6 +63,7 @@ static int __init br_init(void)
 static void __exit br_deinit(void)
 {
 	rcu_assign_pointer(br_stp_sap->rcv_func, NULL);
+	dev_remove_pack(&br_stp_packet_type);
 
 #ifdef CONFIG_BRIDGE_NETFILTER
 	br_netfilter_fini();

net/bridge/br_device.c +1 -1

@@ -95,7 +95,7 @@ static int br_set_mac_address(struct net
 
 	spin_lock_bh(&br->lock);
 	list_for_each_entry(port, &br->port_list, list) {
-		if (!compare_ether_addr(port->dev->dev_addr, addr->sa_data)) {
+		if (!compare_ether_addr(port->addr.addr, addr->sa_data)) {
 			br_stp_change_bridge_id(br, addr->sa_data);
 			err = 0;
 			break;

net/bridge/br_fdb.c +9 ~~-11~~

@@ -24,8 +24,7 @@
 #include "br_private.h"
 
 static kmem_cache_t *br_fdb_cache __read_mostly;
-static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
-		      const unsigned char *addr);
+static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source);
 
 void __init br_fdb_init(void)
 {

@@ -67,7 +66,7 @@ static __inline__ void fdb_delete(struct
 	br_fdb_put(f);
 }
 
-void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
+void br_fdb_changeaddr(struct net_bridge_port *p)
 {
 	struct net_bridge *br = p->br;
 	int i;

@@ -86,7 +85,7 @@ void br_fdb_changeaddr(struct net_bridge
 				struct net_bridge_port *op;
 				list_for_each_entry(op, &br->port_list, list) {
 					if (op != p &&
-					    !compare_ether_addr(op->dev->dev_addr,
+					    !compare_ether_addr(op->addr.addr,
 								f->addr.addr)) {
 						f->dst = op;
 						goto insert;

@@ -101,7 +100,7 @@ void br_fdb_changeaddr(struct net_bridge
 	}
  insert:
 	/* insert new address,  may fail if invalid address or dup. */
-	fdb_insert(br, p, newaddr);
+	fdb_insert(br, p);
 
 	spin_unlock_bh(&br->hash_lock);
 }

@@ -151,7 +150,7 @@ void br_fdb_delete_by_port(struct net_br
 				struct net_bridge_port *op;
 				list_for_each_entry(op, &br->port_list, list) {
 					if (op != p &&
-					    !compare_ether_addr(op->dev->dev_addr,
+					    !compare_ether_addr(op->addr.addr,
 								f->addr.addr)) {
 						f->dst = op;
 						goto skip_delete;

@@ -291,9 +290,9 @@ static struct net_bridge_fdb_entry *fdb_
 	return fdb;
 }
 
-static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
-		  const unsigned char *addr)
+static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source)
 {
+	const unsigned char *addr = source->addr.addr;
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;

@@ -320,13 +319,12 @@ static int fdb_insert(struct net_bridge
 	return 0;
 }
 
-int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
-		  const unsigned char *addr)
+int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source)
 {
 	int ret;
 
 	spin_lock_bh(&br->hash_lock);
-	ret = fdb_insert(br, source, addr);
+	ret = fdb_insert(br, source);
 	spin_unlock_bh(&br->hash_lock);
 	return ret;
 }

net/bridge/br_forward.c +3 -0

@@ -18,6 +18,7 @@
 #include <linux/skbuff.h>
 #include <linux/if_vlan.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_arp.h>
 #include "br_private.h"
 
 static inline int should_deliver(const struct net_bridge_port *p,

@@ -46,6 +47,8 @@ int br_dev_queue_push_xmit(struct sk_buf
 		nf_bridge_maybe_copy_header(skb);
 #endif
 		skb_push(skb, ETH_HLEN);
+		if (skb->dev->type == ARPHRD_IPGRE)
+			skb->protocol = htons(ETH_P_BRIDGE);
 
 		dev_queue_xmit(skb);
 	}

net/bridge/br_if.c +11 -2

@@ -21,6 +21,7 @@
 #include <linux/init.h>
 #include <linux/rtnetlink.h>
 #include <linux/if_ether.h>
+#include <linux/etherdevice.h>
 #include <net/sock.h>
 
 #include "br_private.h"

@@ -391,7 +392,10 @@ 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)
+	if (dev->flags & IFF_LOOPBACK)
+		return -EINVAL;
+
+	if (dev->type != ARPHRD_ETHER && dev->type != ARPHRD_IPGRE)
 		return -EINVAL;
 
 	if (dev->hard_start_xmit == br_dev_xmit)

@@ -408,7 +412,12 @@ int br_add_if(struct net_bridge *br, str
 	if (err)
 		goto err0;
 
- 	err = br_fdb_insert(br, p, dev->dev_addr);
+	if (dev->type == ARPHRD_ETHER)
+		memcpy(p->addr.addr, dev->dev_addr, ETH_ALEN);
+	else
+		random_ether_addr(p->addr.addr);
+
+	err = br_fdb_insert(br, p);
 	if (err)
 		goto err1;

net/bridge/br_input.c +13 -1

@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_arp.h>
 #include "br_private.h"
 
 /* Bridge group multicast address 802.1d (pg 51). */

@@ -124,11 +125,22 @@ static inline int is_link_local(const un
 int br_handle_frame(struct net_bridge_port *p, struct sk_buff **pskb)
 {
 	struct sk_buff *skb = *pskb;
-	const unsigned char *dest = eth_hdr(skb)->h_dest;
+	const unsigned char *dest;
+
+	if (skb->dev->type == ARPHRD_IPGRE) {
+		if (skb->protocol != htons(ETH_P_BRIDGE))
+			return 0;
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			goto err;
+		skb->protocol = eth_type_trans(skb, p->br->dev);
+		skb_postpull_rcsum(skb, skb->mac.raw, ETH_HLEN);
+		skb->nh.raw += ETH_HLEN;
+	}
 
 	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
 		goto err;
 
+	dest = eth_hdr(skb)->h_dest;
 	if (unlikely(is_link_local(dest))) {
 		skb->pkt_type = PACKET_HOST;
 		return NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,

net/bridge/br_netfilter.c +12 -2

@@ -765,14 +765,24 @@ out:
 	return NF_STOLEN;
 }
 
+static int __br_nf_dev_queue_xmit(struct sk_buff *skb)
+{
+	if (skb->dst == (struct dst_entry *)&__fake_rtable) {
+		dst_release(skb->dst);
+		skb->dst = NULL;
+	}
+
+	return br_dev_queue_push_xmit(skb);
+}
+
 static int br_nf_dev_queue_xmit(struct sk_buff *skb)
 {
 	if (skb->protocol == htons(ETH_P_IP) &&
 	    skb->len > skb->dev->mtu &&
 	    !(skb_shinfo(skb)->ufo_size || skb_shinfo(skb)->tso_size))
-		return ip_fragment(skb, br_dev_queue_push_xmit);
+		return ip_fragment(skb, __br_nf_dev_queue_xmit);
 	else
-		return br_dev_queue_push_xmit(skb);
+		return __br_nf_dev_queue_xmit(skb);
 }
 
 /* PF_BRIDGE/POST_ROUTING ********************************************/

net/bridge/br_notify.c +6 -2

@@ -14,6 +14,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/if_arp.h>
 
 #include "br_private.h"

@@ -48,8 +49,11 @@ static int br_device_event(struct notifi
 		break;
 
 	case NETDEV_CHANGEADDR:
-		br_fdb_changeaddr(p, dev->dev_addr);
-		br_stp_recalculate_bridge_id(br);
+		if (dev->type == ARPHRD_ETHER) {
+			memcpy(p->addr.addr, dev->dev_addr, ETH_ALEN);
+			br_fdb_changeaddr(p);
+			br_stp_recalculate_bridge_id(br);
+		}
 		break;
 
 	case NETDEV_CHANGE:

net/bridge/br_private.h +6 -4

@@ -77,6 +77,7 @@ struct net_bridge_port
 	bridge_id			designated_bridge;
 	u32				path_cost;
 	u32				designated_cost;
+	mac_addr			addr;
 
 	struct timer_list		forward_delay_timer;
 	struct timer_list		hold_timer;

@@ -139,8 +140,7 @@ extern int br_dev_xmit(struct sk_buff *s
 /* br_fdb.c */
 extern void br_fdb_init(void);
 extern void br_fdb_fini(void);
-extern void br_fdb_changeaddr(struct net_bridge_port *p,
-			      const unsigned char *newaddr);
+extern void br_fdb_changeaddr(struct net_bridge_port *p);
 extern void br_fdb_cleanup(unsigned long arg);
 extern void br_fdb_delete_by_port(struct net_bridge *br,
 			   struct net_bridge_port *p);

@@ -152,8 +152,7 @@ extern void br_fdb_put(struct net_bridge
 extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
 			  unsigned long count, unsigned long off);
 extern int br_fdb_insert(struct net_bridge *br,
-			 struct net_bridge_port *source,
-			 const unsigned char *addr);
+			 struct net_bridge_port *source);
 extern void br_fdb_update(struct net_bridge *br,
 			  struct net_bridge_port *source,
 			  const unsigned char *addr);

@@ -220,6 +219,9 @@ extern ssize_t br_show_bridge_id(char *b
 /* br_stp_bpdu.c */
 extern int br_stp_rcv(struct sk_buff *skb, struct net_device *dev,
 		      struct packet_type *pt, struct net_device *orig_dev);
+extern int br_stp_packet_rcv(struct sk_buff *skb, struct net_device *dev,
+			     struct packet_type *pt,
+			     struct net_device *orig_dev);
 
 /* br_stp_timer.c */
 extern void br_stp_timer_init(struct net_bridge *br);

net/bridge/br_stp_bpdu.c +39 ~~-19~~

95 lines not on this page · raw message

net/bridge/br_stp_if.c +2 -2

@@ -155,8 +155,8 @@ void br_stp_recalculate_bridge_id(struct
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-		    memcmp(p->dev->dev_addr, addr, ETH_ALEN) < 0)
-			addr = p->dev->dev_addr;
+		    memcmp(p->addr.addr, addr, ETH_ALEN) < 0)
+			addr = p->addr.addr;
 
 	}

include/linux/if_ether.h +1 -0

@@ -55,6 +55,7 @@
 #define ETH_P_DIAG      0x6005          /* DEC Diagnostics              */
 #define ETH_P_CUST      0x6006          /* DEC Customer use             */
 #define ETH_P_SCA       0x6007          /* DEC Systems Comms Arch       */
+#define ETH_P_BRIDGE    0x6558          /* Transparent Ethernet Bridging */
 #define ETH_P_RARP      0x8035		/* Reverse Addr Res packet	*/
 #define ETH_P_ATALK	0x809B		/* Appletalk DDP		*/
 #define ETH_P_AARP	0x80F3		/* Appletalk AARP		*/

Stephen Hemminger

2006-08-02 17:23 UTC #6 lore raw

Quoted 9 lines · On Wed, 02 Aug 2006 16:17:42 +1000 Philip Craig <[email protected]> wrote:

Quoted 3 lines
Quoted 2 lines · Stephen Hemminger
I am not against making the bridge code smarter to handle other encapsulation.
Here's an updated patch that fixes all issues I am aware of.
It generates a random mac address for gre ports, and also stores a copy of the mac address for ethernet ports, rather than checking dev->type everywhere.

That looks cleaner. I wonder if using a fixed OUI would be better than random addresses but then choosing an OUI would be a problem.

You probably should add a comment about what this function is doing, and why.

Quoted 10 lines

static int __br_nf_dev_queue_xmit(struct sk_buff *skb)

net/bridge/br_netfilter.c +9 -0

+{
+	if (skb->dst == (struct dst_entry *)&__fake_rtable) {
+		dst_release(skb->dst);
+		skb->dst = NULL;
+	}
+
+	return br_dev_queue_push_xmit(skb);
+}
+

Philip Craig

author

2006-08-03 01:08 UTC #7 lore raw

Quoted 8 lines · Stephen Hemminger

Quoted 3 lines · On Wed, 02 Aug 2006 16:17:42 +1000 Philip Craig <[email protected]> wrote:
It generates a random mac address for gre ports, and also stores a copy of the mac address for ethernet ports, rather than checking dev->type everywhere.
That looks cleaner. I wonder if using a fixed OUI would be better than random addresses but then choosing an OUI would be a problem.

random_ether_addr() sets the local assignment bit. This is what various other virtual devices do (including tap devices, which can also be bridged).

Quoted 2 lines

You probably should add a comment about what this function is doing, and why.

Okay.

Lennert Buytenhek

2006-08-02 07:42 UTC #8 lore raw

In reply to Stephen Hemminger

Quoted 9 lines · Stephen Hemminger

Quoted 6 lines
Quoted 1 line
Why not use existing bridge code?
It does use the existing bridge code. Perhaps the name is misleading. All it does is encapsulate the full ethernet header in a gre packet, rather than only layer 3. That is, currently gre uses ARPHRD_IPGRE, but bridging requires ARPHRD_ETHER.
I am not against making the bridge code smarter to handle other encapsulation.

What if you want to run ethernet directly over a GRE tunnel, without using bridging?

Philip Craig

author

2006-08-03 01:33 UTC #9 lore raw

Quoted 7 lines · Lennert Buytenhek

Quoted 5 lines · Stephen Hemminger
Quoted 5 lines
Quoted 1 line
Why not use existing bridge code?
It does use the existing bridge code. Perhaps the name is misleading. All it does is encapsulate the full ethernet header in a gre packet, rather than only layer 3. That is, currently gre uses ARPHRD_IPGRE, but bridging requires ARPHRD_ETHER.

Quoted 5 lines

Quoted 2 lines
I am not against making the bridge code smarter to handle other encapsulation.
What if you want to run ethernet directly over a GRE tunnel, without using bridging?

But on the other hand, this method allows you to send both ethernet and non-ethernet traffic over the same GRE tunnel. Is that useful? Actually, this feature is what makes the handling of the LLC_SAP_BSPAN packets simple.

The patch to bridging is a lot cleaner than the patch to GRE, and it also sidesteps the userspace configuration issues, so I don't want to go back to modifying the GRE device.

Both could be achieved by creating a new virtual device that sits between GRE and bridging.

Lennert Buytenhek

2006-08-03 07:33 UTC #10 lore raw

Quoted 10 lines · Philip Craig

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
All it does is encapsulate the full ethernet header in a gre packet, rather than only layer 3. That is, currently gre uses ARPHRD_IPGRE, but bridging requires ARPHRD_ETHER.
I am not against making the bridge code smarter to handle other encapsulation.
What if you want to run ethernet directly over a GRE tunnel, without using bridging?

Quoted 8 lines

But on the other hand, this method allows you to send both ethernet and non-ethernet traffic over the same GRE tunnel. Is that useful? Actually, this feature is what makes the handling of the LLC_SAP_BSPAN packets simple.
The patch to bridging is a lot cleaner than the patch to GRE, and it also sidesteps the userspace configuration issues, so I don't want to go back to modifying the GRE device.

So now you _need_ bridging in the middle to send ethernet traffic over a GRE tunnel? Ugh.

If you really want to send ethernet and non-ethernet traffic over the same tunnel, can't you make multiple devices?

If GRE generally transmits BPDUs without ethernet header, the handling for that ought to be in GRE, IMHO.

cheers,
Lennert

Philip Craig

author

2006-08-03 09:15 UTC #11 lore raw

Quoted 2 lines · Lennert Buytenhek

So now you _need_ bridging in the middle to send ethernet traffic over a GRE tunnel? Ugh.

Agreed that would not be nice. What is the usage scenario for this? At least one end of the tunnel will be bridged?

Quoted 2 lines

If you really want to send ethernet and non-ethernet traffic over the same tunnel, can't you make multiple devices?

Do you mean make multiple GRE devices, where one has an ethernet mode set? That would require more extensive changes to GRE than my original patch. I don't think being able to send both is important enough to justify that. Better to just use my original patch if that is acceptable. There are parts of it I can clean up more, but nothing major. And it still needs some userspace modifications.

Or create a new virtual device to sit between gre and bridging that simply adds the ethernet header? This is the most modular approach, but it seems like overkill for such a simple thing.

Quoted 2 lines

If GRE generally transmits BPDUs without ethernet header, the handling for that ought to be in GRE, IMHO.

I don't think this is normal behaviour. At least, not all routers do it. More likely some routers have done it as an optimisation. It does appear to be specific to GRE.

Lennert Buytenhek

2006-08-03 19:40 UTC #12 lore raw

Quoted 5 lines · Philip Craig

Quoted 2 lines
So now you _need_ bridging in the middle to send ethernet traffic over a GRE tunnel? Ugh.
Agreed that would not be nice. What is the usage scenario for this? At least one end of the tunnel will be bridged?

For example for VPN purposes, I could imagine that you wouldn't want to use bridging.

Quoted 4 lines

Quoted 2 lines
If you really want to send ethernet and non-ethernet traffic over the same tunnel, can't you make multiple devices?
Do you mean make multiple GRE devices, where one has an ethernet mode set?

For example. If you want to send ethernet-encapsulated and other-protocol- encapsulated traffic over the same GRE tunnel, that would seem like the cleanest solution to me.

Quoted 6 lines

Quoted 2 lines
If GRE generally transmits BPDUs without ethernet header, the handling for that ought to be in GRE, IMHO.
I don't think this is normal behaviour. At least, not all routers do it. More likely some routers have done it as an optimisation. It does appear to be specific to GRE.

Hmm, so it depends on the device whether BPDUs are sent with or without an ethernet header? Do the devices that send BPDUs without ethernet header at least accept a BPDU with an ethernet header (and vice versa), or aren't the two modes compatible at all?

cheers,
Lennert

Philip Craig

author

2006-08-04 01:01 UTC #13 lore raw

Quoted 9 lines · Lennert Buytenhek

Quoted 4 lines · Philip Craig
Quoted 2 lines
So now you _need_ bridging in the middle to send ethernet traffic over a GRE tunnel? Ugh.
Agreed that would not be nice. What is the usage scenario for this? At least one end of the tunnel will be bridged?
For example for VPN purposes, I could imagine that you wouldn't want to use bridging.

What is the purpose of including the ethernet header if it only exists on the tunnel? I have used GRE tunnels for VPNs, but they have always needed bridging. If I don't need bridging, then the VPN is only passing IP traffic and I just send that without GRE at all.

Quoted 7 lines

Quoted 3 lines
Quoted 2 lines
If you really want to send ethernet and non-ethernet traffic over the same tunnel, can't you make multiple devices?
Do you mean make multiple GRE devices, where one has an ethernet mode set?
For example. If you want to send ethernet-encapsulated and other-protocol- encapsulated traffic over the same GRE tunnel, that would seem like the cleanest solution to me.

It is fine for sending, but when receiving, which packets go to which device? Does a packet appear on only one device when sending, but both devices when receiving? Or does the operation of the non-ethernet device change based on whether an ethernet device exists? Modifying GRE to handle multiple GRE tunnels that are actually the same does not seem clean.

This seems cleaner to me:
The GRE tunnel receives a packet of any protocol (ethernet included), and adds the IP/GRE header. Anything else is done above GRE. Whatever needs the ethernet header should add it.

And then the decision is, how do we add the ethernet header?
- an option to GRE to always add the ethernet header
- a new virtual device
- as part of bridging

Any of these will work for me.

Quoted 4 lines

Hmm, so it depends on the device whether BPDUs are sent with or without an ethernet header? Do the devices that send BPDUs without ethernet header at least accept a BPDU with an ethernet header (and vice versa), or aren't the two modes compatible at all?

I don't have any to test with currently, but we only had complaints about us not receiving them, and only at some sites. So I assume that devices can accept either format.

Lennert Buytenhek

2006-08-04 08:02 UTC #14 lore raw

Quoted 10 lines · Philip Craig

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
So now you _need_ bridging in the middle to send ethernet traffic over a GRE tunnel? Ugh.
Agreed that would not be nice. What is the usage scenario for this? At least one end of the tunnel will be bridged?
For example for VPN purposes, I could imagine that you wouldn't want to use bridging.
What is the purpose of including the ethernet header if it only exists on the tunnel?

I have one machine at home that appears to be on my employer's network via such a tunnel. I don't use bridging, because I don't need any other machine at home to access this tunnel. I do want bridging, and not proxy ARP, because it allows me to run arpwatch, and doesn't require me to reconfigure something at the remote end if I, for example, want to add another IP address to my home box.

Quoted 11 lines

Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
If you really want to send ethernet and non-ethernet traffic over the same tunnel, can't you make multiple devices?
Do you mean make multiple GRE devices, where one has an ethernet mode set?
For example. If you want to send ethernet-encapsulated and other-protocol- encapsulated traffic over the same GRE tunnel, that would seem like the cleanest solution to me.
It is fine for sending, but when receiving, which packets go to which device? Does a packet appear on only one device when sending, but both devices when receiving?

No, just send it to the device that matches the lower-level protocol. So if you receive an ethernet packet, make it be received on the ethernet sub-device, if it's IP, make it be received on the IP sub-device, etc.

Note that I'm not saying that this is necessarily a good idea, it's just what I would suggest if you want to run both 'bare IP' and ethernet encapsulated traffic over the same GRE tunnel at the same time. (Not sure why you'd want that.)

Quoted 7 lines

This seems cleaner to me:
The GRE tunnel receives a packet of any protocol (ethernet included), and adds the IP/GRE header. Anything else is done above GRE. Whatever needs the ethernet header should add it.
And then the decision is, how do we add the ethernet header?
- an option to GRE to always add the ethernet header

I think I lost you here. You surely can't just 'make up' any ethernet header?

cheers,
Lennert

Philip Craig

author

2006-08-07 01:55 UTC #15 lore raw

Quoted 6 lines · Lennert Buytenhek

I have one machine at home that appears to be on my employer's network via such a tunnel. I don't use bridging, because I don't need any other machine at home to access this tunnel. I do want bridging, and not proxy ARP, because it allows me to run arpwatch, and doesn't require me to reconfigure something at the remote end if I, for example, want to add another IP address to my home box.

Okay.
If this is using Linux, do you have a patch that does this already?

Quoted 8 lines

No, just send it to the device that matches the lower-level protocol. So if you receive an ethernet packet, make it be received on the ethernet sub-device, if it's IP, make it be received on the IP sub-device, etc.
Note that I'm not saying that this is necessarily a good idea, it's just what I would suggest if you want to run both 'bare IP' and ethernet encapsulated traffic over the same GRE tunnel at the same time. (Not sure why you'd want that.)

I still don't understand what the IP sub-device is, or whether this is different from the options I list below.
But I don't have a good reason for wanting this either. So lets forget about this and talk about the three options below.

Quoted 10 lines

Quoted 7 lines
This seems cleaner to me:
The GRE tunnel receives a packet of any protocol (ethernet included), and adds the IP/GRE header. Anything else is done above GRE. Whatever needs the ethernet header should add it.
And then the decision is, how do we add the ethernet header?
- an option to GRE to always add the ethernet header
I think I lost you here. You surely can't just 'make up' any ethernet header?

No, you need a device that looks like an ethernet device.
We can modify the GRE device to look like ethernet (as an option). Or we can create a new ethernet device stacked on top of the GRE device. Or we can use the bridge device, which already looks like ethernet.

My first patch in this thread did it by modifying the GRE device. Look at that patch if my explanation still doesn't make sense. The option part is controlled by the "#if 0" in that patch (which needs fixing of course).

You have a valid use that doesn't need bridging, so IMO we can rule out doing it in the bridging code.

Lennert Buytenhek

2006-08-10 13:09 UTC #16 lore raw

Quoted 9 lines · Philip Craig

Quoted 6 lines
I have one machine at home that appears to be on my employer's network via such a tunnel. I don't use bridging, because I don't need any other machine at home to access this tunnel. I do want bridging, and not proxy ARP, because it allows me to run arpwatch, and doesn't require me to reconfigure something at the remote end if I, for example, want to add another IP address to my home box.
Okay.
If this is using Linux, do you have a patch that does this already?

I use vtun:

http://vtun.sourceforge.net/

But I would prefer using some in-kernel ethernet tunneling method with ipsec instead.