[Bridge] [PATCH] Add vlan id to bridge forward database

[Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

[-- Attachment #1: vlan.diff --]
[-- Type: text/plain, Size: 10017 bytes --]

Hi!

This makes forwarding table aware of 802.1Q vlan ids and stores
id with MACs in the table.

It solves problems when having same MAC on diffent pairs
(vlan, port). Current code gets confused at that situation.

Local MACs are managed as present on every vlan.

Signed-off-by: Jaime Medrano <jaime.medrano@gmail.com>
---
 include/linux/if_bridge.h |    3 ++-
 net/atm/lec.c             |    2 +-
 net/bridge/br_device.c    |   13 +++++++++----
 net/bridge/br_fdb.c       |   32 ++++++++++++++++++++------------
 net/bridge/br_input.c     |   15 ++++++++++++---
 net/bridge/br_private.h   |   13 +++++++++----
 6 files changed, 53 insertions(+), 25 deletions(-)

Index: linux-2.6.23/net/bridge/br_private.h
===================================================================
--- linux-2.6.23.orig/net/bridge/br_private.h	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_private.h	2007-12-17 11:55:17.000000000 +0100
@@ -55,6 +55,7 @@
 	atomic_t			use_count;
 	unsigned long			ageing_timer;
 	mac_addr			addr;
+	unsigned short			vlan_id;
 	unsigned char			is_local;
 	unsigned char			is_static;
 };
@@ -150,9 +151,11 @@
 extern void br_fdb_delete_by_port(struct net_bridge *br,
 				  const struct net_bridge_port *p, int do_all);
 extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-						 const unsigned char *addr);
+						 const unsigned char *addr,
+					         unsigned short vlan_id);
 extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					       unsigned char *addr);
+					       unsigned char *addr,
+					       unsigned short vlan_id);
 extern void br_fdb_put(struct net_bridge_fdb_entry *ent);
 extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
 			  unsigned long count, unsigned long off);
@@ -161,7 +164,8 @@
 			 const unsigned char *addr);
 extern void br_fdb_update(struct net_bridge *br,
 			  struct net_bridge_port *source,
-			  const unsigned char *addr);
+			  const unsigned char *addr,
+			  unsigned short vlan_id);
 
 /* br_forward.c */
 extern void br_deliver(const struct net_bridge_port *to,
@@ -237,7 +241,8 @@
 
 /* br.c */
 extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
-						       unsigned char *addr);
+						       unsigned char *addr,
+						       unsigned short vlan_id);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
 
 
Index: linux-2.6.23/net/bridge/br_device.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_device.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_device.c	2007-12-17 11:55:17.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -42,10 +43,14 @@
 
 	if (dest[0] & 1)
 		br_flood_deliver(br, skb);
-	else if ((dst = __br_fdb_get(br, dest)) != NULL)
-		br_deliver(dst->dst, skb);
-	else
-		br_flood_deliver(br, skb);
+	else {
+		unsigned short vlan_id = 0;
+		vlan_get_tag(skb, &vlan_id);
+		if ((dst = __br_fdb_get(br, dest, vlan_id)) != NULL)
+			br_deliver(dst->dst, skb);
+		else
+			br_flood_deliver(br, skb);
+	}
 
 	return 0;
 }
Index: linux-2.6.23/net/bridge/br_fdb.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_fdb.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_fdb.c	2007-12-17 12:00:28.000000000 +0100
@@ -211,13 +211,15 @@
 
 /* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
 struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-					  const unsigned char *addr)
+					  const unsigned char *addr,
+					  unsigned short vlan_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr)) {
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || fdb->vlan_id == vlan_id)) {
 			if (unlikely(has_expired(br, fdb)))
 				break;
 			return fdb;
@@ -229,12 +231,13 @@
 
 /* Interface used by ATM hook that keeps a ref count */
 struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					unsigned char *addr)
+					unsigned char *addr,
+					unsigned short vlan_id)
 {
 	struct net_bridge_fdb_entry *fdb;
 
 	rcu_read_lock();
-	fdb = __br_fdb_get(br, addr);
+	fdb = __br_fdb_get(br, addr, vlan_id);
 	if (fdb && !atomic_inc_not_zero(&fdb->use_count))
 		fdb = NULL;
 	rcu_read_unlock();
@@ -289,6 +292,7 @@
 			fe->is_local = f->is_local;
 			if (!f->is_static)
 				fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->ageing_timer);
+			fe->vlan_id = f->vlan_id;
 			++fe;
 			++num;
 		}
@@ -301,13 +305,15 @@
 }
 
 static inline struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
-						    const unsigned char *addr)
+						    const unsigned char *addr,
+						    unsigned short vlan_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, head, hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr))
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || fdb->vlan_id == vlan_id))
 			return fdb;
 	}
 	return NULL;
@@ -316,6 +322,7 @@
 static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
 					       struct net_bridge_port *source,
 					       const unsigned char *addr,
+					       unsigned short vlan_id,
 					       int is_local)
 {
 	struct net_bridge_fdb_entry *fdb;
@@ -323,6 +330,7 @@
 	fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
 	if (fdb) {
 		memcpy(fdb->addr.addr, addr, ETH_ALEN);
+		fdb->vlan_id = vlan_id;
 		atomic_set(&fdb->use_count, 1);
 		hlist_add_head_rcu(&fdb->hlist, head);
 
@@ -343,7 +351,7 @@
 	if (!is_valid_ether_addr(addr))
 		return -EINVAL;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, 0);
 	if (fdb) {
 		/* it is okay to have multiple ports with same
 		 * address, just use the first one.
@@ -357,7 +365,7 @@
 		fdb_delete(fdb);
 	}
 
-	if (!fdb_create(head, source, addr, 1))
+	if (!fdb_create(head, source, addr, 0, 1))
 		return -ENOMEM;
 
 	return 0;
@@ -375,7 +383,7 @@
 }
 
 void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr)
+		   const unsigned char *addr, unsigned short vlan_id)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;
@@ -389,7 +397,7 @@
 	      source->state == BR_STATE_FORWARDING))
 		return;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, vlan_id);
 	if (likely(fdb)) {
 		/* attempt to update an entry for a local interface */
 		if (unlikely(fdb->is_local)) {
@@ -404,8 +412,8 @@
 		}
 	} else {
 		spin_lock(&br->hash_lock);
-		if (!fdb_find(head, addr))
-			fdb_create(head, source, addr, 0);
+		if (!fdb_find(head, addr, vlan_id))
+			fdb_create(head, source, addr, vlan_id, 0);
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
 		 */
Index: linux-2.6.23/net/bridge/br_input.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_input.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_input.c	2007-12-17 11:55:17.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_vlan.h>
 #include "br_private.h"
 
 /* Bridge group multicast address 802.1d (pg 51). */
@@ -44,13 +45,17 @@
 	struct net_bridge *br;
 	struct net_bridge_fdb_entry *dst;
 	struct sk_buff *skb2;
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_id = 0;
 
 	if (!p || p->state == BR_STATE_DISABLED)
 		goto drop;
 
 	/* insert into forwarding database after filtering to avoid spoofing */
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q))
+		vlan_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
 	br = p->br;
-	br_fdb_update(br, p, eth_hdr(skb)->h_source);
+	br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_id);
 
 	if (p->state == BR_STATE_LEARNING)
 		goto drop;
@@ -66,7 +71,7 @@
 	if (is_multicast_ether_addr(dest)) {
 		br->statistics.multicast++;
 		skb2 = skb;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest, vlan_id)) && dst->is_local) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;
@@ -96,9 +101,13 @@
 static int br_handle_local_finish(struct sk_buff *skb)
 {
 	struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_id = 0;
 
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q))
+		vlan_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
 	if (p)
-		br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_id);
 	return 0;	 /* process further */
 }
 
Index: linux-2.6.23/net/atm/lec.c
===================================================================
--- linux-2.6.23.orig/net/atm/lec.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/atm/lec.c	2007-12-17 11:55:17.000000000 +0100
@@ -557,7 +557,7 @@
 				break;
 
 			f = br_fdb_get_hook(dev->br_port->br,
-					    mesg->content.proxy.mac_addr);
+					    mesg->content.proxy.mac_addr, 0);
 			if (f != NULL && f->dst->dev != dev
 			    && f->dst->state == BR_STATE_FORWARDING) {
 				/* hit from bridge table, send LE_ARP_RESPONSE */
Index: linux-2.6.23/include/linux/if_bridge.h
===================================================================
--- linux-2.6.23.orig/include/linux/if_bridge.h	2007-12-17 11:59:46.000000000 +0100
+++ linux-2.6.23/include/linux/if_bridge.h	2007-12-17 12:00:28.000000000 +0100
@@ -97,7 +97,8 @@
 	__u8 port_no;
 	__u8 is_local;
 	__u32 ageing_timer_value;
-	__u32 unused;
+	__u16 vlan_id;
+	__u16 unused;
 };
 
 #ifdef __KERNEL__

-- 

[Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

[-- Attachment #1: vlan.diff --]
[-- Type: text/plain, Size: 10017 bytes --]

Hi!

This makes forwarding table aware of 802.1Q vlan ids and stores
id with MACs in the table.

It solves problems when having same MAC on diffent pairs
(vlan, port). Current code gets confused at that situation.

Local MACs are managed as present on every vlan.

Signed-off-by: Jaime Medrano <jaime.medrano@gmail.com>
---
 include/linux/if_bridge.h |    3 ++-
 net/atm/lec.c             |    2 +-
 net/bridge/br_device.c    |   13 +++++++++----
 net/bridge/br_fdb.c       |   32 ++++++++++++++++++++------------
 net/bridge/br_input.c     |   15 ++++++++++++---
 net/bridge/br_private.h   |   13 +++++++++----
 6 files changed, 53 insertions(+), 25 deletions(-)

Index: linux-2.6.23/net/bridge/br_private.h
===================================================================
--- linux-2.6.23.orig/net/bridge/br_private.h	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_private.h	2007-12-17 11:55:17.000000000 +0100
@@ -55,6 +55,7 @@
 	atomic_t			use_count;
 	unsigned long			ageing_timer;
 	mac_addr			addr;
+	unsigned short			vlan_id;
 	unsigned char			is_local;
 	unsigned char			is_static;
 };
@@ -150,9 +151,11 @@
 extern void br_fdb_delete_by_port(struct net_bridge *br,
 				  const struct net_bridge_port *p, int do_all);
 extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-						 const unsigned char *addr);
+						 const unsigned char *addr,
+					         unsigned short vlan_id);
 extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					       unsigned char *addr);
+					       unsigned char *addr,
+					       unsigned short vlan_id);
 extern void br_fdb_put(struct net_bridge_fdb_entry *ent);
 extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
 			  unsigned long count, unsigned long off);
@@ -161,7 +164,8 @@
 			 const unsigned char *addr);
 extern void br_fdb_update(struct net_bridge *br,
 			  struct net_bridge_port *source,
-			  const unsigned char *addr);
+			  const unsigned char *addr,
+			  unsigned short vlan_id);
 
 /* br_forward.c */
 extern void br_deliver(const struct net_bridge_port *to,
@@ -237,7 +241,8 @@
 
 /* br.c */
 extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
-						       unsigned char *addr);
+						       unsigned char *addr,
+						       unsigned short vlan_id);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
 
 
Index: linux-2.6.23/net/bridge/br_device.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_device.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_device.c	2007-12-17 11:55:17.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -42,10 +43,14 @@
 
 	if (dest[0] & 1)
 		br_flood_deliver(br, skb);
-	else if ((dst = __br_fdb_get(br, dest)) != NULL)
-		br_deliver(dst->dst, skb);
-	else
-		br_flood_deliver(br, skb);
+	else {
+		unsigned short vlan_id = 0;
+		vlan_get_tag(skb, &vlan_id);
+		if ((dst = __br_fdb_get(br, dest, vlan_id)) != NULL)
+			br_deliver(dst->dst, skb);
+		else
+			br_flood_deliver(br, skb);
+	}
 
 	return 0;
 }
Index: linux-2.6.23/net/bridge/br_fdb.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_fdb.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_fdb.c	2007-12-17 12:00:28.000000000 +0100
@@ -211,13 +211,15 @@
 
 /* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
 struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-					  const unsigned char *addr)
+					  const unsigned char *addr,
+					  unsigned short vlan_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr)) {
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || fdb->vlan_id == vlan_id)) {
 			if (unlikely(has_expired(br, fdb)))
 				break;
 			return fdb;
@@ -229,12 +231,13 @@
 
 /* Interface used by ATM hook that keeps a ref count */
 struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					unsigned char *addr)
+					unsigned char *addr,
+					unsigned short vlan_id)
 {
 	struct net_bridge_fdb_entry *fdb;
 
 	rcu_read_lock();
-	fdb = __br_fdb_get(br, addr);
+	fdb = __br_fdb_get(br, addr, vlan_id);
 	if (fdb && !atomic_inc_not_zero(&fdb->use_count))
 		fdb = NULL;
 	rcu_read_unlock();
@@ -289,6 +292,7 @@
 			fe->is_local = f->is_local;
 			if (!f->is_static)
 				fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->ageing_timer);
+			fe->vlan_id = f->vlan_id;
 			++fe;
 			++num;
 		}
@@ -301,13 +305,15 @@
 }
 
 static inline struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
-						    const unsigned char *addr)
+						    const unsigned char *addr,
+						    unsigned short vlan_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, head, hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr))
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || fdb->vlan_id == vlan_id))
 			return fdb;
 	}
 	return NULL;
@@ -316,6 +322,7 @@
 static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
 					       struct net_bridge_port *source,
 					       const unsigned char *addr,
+					       unsigned short vlan_id,
 					       int is_local)
 {
 	struct net_bridge_fdb_entry *fdb;
@@ -323,6 +330,7 @@
 	fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
 	if (fdb) {
 		memcpy(fdb->addr.addr, addr, ETH_ALEN);
+		fdb->vlan_id = vlan_id;
 		atomic_set(&fdb->use_count, 1);
 		hlist_add_head_rcu(&fdb->hlist, head);
 
@@ -343,7 +351,7 @@
 	if (!is_valid_ether_addr(addr))
 		return -EINVAL;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, 0);
 	if (fdb) {
 		/* it is okay to have multiple ports with same
 		 * address, just use the first one.
@@ -357,7 +365,7 @@
 		fdb_delete(fdb);
 	}
 
-	if (!fdb_create(head, source, addr, 1))
+	if (!fdb_create(head, source, addr, 0, 1))
 		return -ENOMEM;
 
 	return 0;
@@ -375,7 +383,7 @@
 }
 
 void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr)
+		   const unsigned char *addr, unsigned short vlan_id)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;
@@ -389,7 +397,7 @@
 	      source->state == BR_STATE_FORWARDING))
 		return;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, vlan_id);
 	if (likely(fdb)) {
 		/* attempt to update an entry for a local interface */
 		if (unlikely(fdb->is_local)) {
@@ -404,8 +412,8 @@
 		}
 	} else {
 		spin_lock(&br->hash_lock);
-		if (!fdb_find(head, addr))
-			fdb_create(head, source, addr, 0);
+		if (!fdb_find(head, addr, vlan_id))
+			fdb_create(head, source, addr, vlan_id, 0);
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
 		 */
Index: linux-2.6.23/net/bridge/br_input.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_input.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/bridge/br_input.c	2007-12-17 11:55:17.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_vlan.h>
 #include "br_private.h"
 
 /* Bridge group multicast address 802.1d (pg 51). */
@@ -44,13 +45,17 @@
 	struct net_bridge *br;
 	struct net_bridge_fdb_entry *dst;
 	struct sk_buff *skb2;
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_id = 0;
 
 	if (!p || p->state == BR_STATE_DISABLED)
 		goto drop;
 
 	/* insert into forwarding database after filtering to avoid spoofing */
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q))
+		vlan_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
 	br = p->br;
-	br_fdb_update(br, p, eth_hdr(skb)->h_source);
+	br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_id);
 
 	if (p->state == BR_STATE_LEARNING)
 		goto drop;
@@ -66,7 +71,7 @@
 	if (is_multicast_ether_addr(dest)) {
 		br->statistics.multicast++;
 		skb2 = skb;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest, vlan_id)) && dst->is_local) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;
@@ -96,9 +101,13 @@
 static int br_handle_local_finish(struct sk_buff *skb)
 {
 	struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_id = 0;
 
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q))
+		vlan_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
 	if (p)
-		br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_id);
 	return 0;	 /* process further */
 }
 
Index: linux-2.6.23/net/atm/lec.c
===================================================================
--- linux-2.6.23.orig/net/atm/lec.c	2007-12-17 11:53:56.000000000 +0100
+++ linux-2.6.23/net/atm/lec.c	2007-12-17 11:55:17.000000000 +0100
@@ -557,7 +557,7 @@
 				break;
 
 			f = br_fdb_get_hook(dev->br_port->br,
-					    mesg->content.proxy.mac_addr);
+					    mesg->content.proxy.mac_addr, 0);
 			if (f != NULL && f->dst->dev != dev
 			    && f->dst->state == BR_STATE_FORWARDING) {
 				/* hit from bridge table, send LE_ARP_RESPONSE */
Index: linux-2.6.23/include/linux/if_bridge.h
===================================================================
--- linux-2.6.23.orig/include/linux/if_bridge.h	2007-12-17 11:59:46.000000000 +0100
+++ linux-2.6.23/include/linux/if_bridge.h	2007-12-17 12:00:28.000000000 +0100
@@ -97,7 +97,8 @@
 	__u8 port_no;
 	__u8 is_local;
 	__u32 ageing_timer_value;
-	__u32 unused;
+	__u16 vlan_id;
+	__u16 unused;
 };
 
 #ifdef __KERNEL__

-- 

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Stephen Hemminger]

On Mon, 17 Dec 2007 17:13:09 +0100
Jaime Medrano <jaime.medrano@gmail.com> wrote:

> Hi!
> 
> This makes forwarding table aware of 802.1Q vlan ids and stores
> id with MACs in the table.
> 
> It solves problems when having same MAC on diffent pairs
> (vlan, port). Current code gets confused at that situation.
> 
> Local MACs are managed as present on every vlan.
> 
> Signed-off-by: Jaime Medrano <jaime.medrano@gmail.com>

What about the nested vlan case?

I assume it continues to work without Vlan's.
How does this affect use of the userspace RSTP?


> --- linux-2.6.23.orig/include/linux/if_bridge.h	2007-12-17 11:59:46.000000000 +0100
> +++ linux-2.6.23/include/linux/if_bridge.h	2007-12-17 12:00:28.000000000 +0100
> @@ -97,7 +97,8 @@
>  	__u8 port_no;
>  	__u8 is_local;
>  	__u32 ageing_timer_value;
> -	__u32 unused;
> +	__u16 vlan_id;
> +	__u16 unused;
>  };

This is a user/kernel ABI change. Does it break old tools?


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

[-- Attachment #1: vlan.diff --]
[-- Type: text/plain, Size: 12800 bytes --]

Stephen Hemminger wrote:
> 
> What about the nested vlan case?
> 

Below is a new patch that handles the double-tagging case. I'm not sure 
if it is worth a more generic case. ¿Are triple-tagging and so really used?

> This is a user/kernel ABI change. Does it break old tools?

New patch gets rid of the unused field but it still doesn't break old tools.

Anyway, the user part is not really needed. I just think it could be useful.

Regards,
Jaime.

---
Subject: [PATCH] Add vlan id to bridge forward database

This makes forwarding table aware of 802.1Q vlan ids and stores
id with MACs in the table. Up to two vlan tags are handled.

It solves problems when having same MAC on diffent pairs
(vlan, port). Current code gets confused at that situation.

Local MACs are managed as present on every vlan.

Signed-off-by: Jaime Medrano <jaime.medrano@gmail.com>
---
 include/linux/if_bridge.h |    3 ++-
 net/atm/lec.c             |    2 +-
 net/bridge/br_device.c    |   23 +++++++++++++++++++----
 net/bridge/br_fdb.c       |   42 ++++++++++++++++++++++++++++++------------
 net/bridge/br_input.c     |   31 +++++++++++++++++++++++++++----
 net/bridge/br_private.h   |   18 ++++++++++++++----
 6 files changed, 93 insertions(+), 26 deletions(-)

Index: linux-2.6.23/net/bridge/br_private.h
===================================================================
--- linux-2.6.23.orig/net/bridge/br_private.h	2007-12-18 09:42:45.000000000 +0100
+++ linux-2.6.23/net/bridge/br_private.h	2007-12-18 09:43:37.000000000 +0100
@@ -55,6 +55,8 @@
 	atomic_t			use_count;
 	unsigned long			ageing_timer;
 	mac_addr			addr;
+	unsigned short			vlan_first_id;
+	unsigned short			vlan_second_id;
 	unsigned char			is_local;
 	unsigned char			is_static;
 };
@@ -150,9 +152,13 @@
 extern void br_fdb_delete_by_port(struct net_bridge *br,
 				  const struct net_bridge_port *p, int do_all);
 extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-						 const unsigned char *addr);
+						 const unsigned char *addr,
+					         unsigned short vlan_first_id,
+					         unsigned short vlan_second_id);
 extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					       unsigned char *addr);
+					       unsigned char *addr,
+					       unsigned short vlan_first_id,
+					       unsigned short vlan_second_id);
 extern void br_fdb_put(struct net_bridge_fdb_entry *ent);
 extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
 			  unsigned long count, unsigned long off);
@@ -161,7 +167,9 @@
 			 const unsigned char *addr);
 extern void br_fdb_update(struct net_bridge *br,
 			  struct net_bridge_port *source,
-			  const unsigned char *addr);
+			  const unsigned char *addr,
+			  unsigned short vlan_first_id,
+			  unsigned short vlan_second_id);
 
 /* br_forward.c */
 extern void br_deliver(const struct net_bridge_port *to,
@@ -237,7 +245,9 @@
 
 /* br.c */
 extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
-						       unsigned char *addr);
+						       unsigned char *addr,
+						       unsigned short vlan_first_id,
+						       unsigned short vlan_second_id);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
 
 
Index: linux-2.6.23/net/bridge/br_device.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_device.c	2007-12-18 09:42:45.000000000 +0100
+++ linux-2.6.23/net/bridge/br_device.c	2007-12-18 09:43:37.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -42,10 +43,24 @@
 
 	if (dest[0] & 1)
 		br_flood_deliver(br, skb);
-	else if ((dst = __br_fdb_get(br, dest)) != NULL)
-		br_deliver(dst->dst, skb);
-	else
-		br_flood_deliver(br, skb);
+	else {
+		struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+		unsigned short vlan_first_id = 0;
+		unsigned short vlan_second_id = 0;
+		if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+			vlan_first_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
+			if (veth->h_vlan_encapsulated_proto ==
+			    __constant_htons(ETH_P_8021Q)) {
+				struct vlan_hdr *veth2 = (struct vlan_hdr *) (veth+1);
+				vlan_second_id = ntohs(veth2->h_vlan_TCI) & VLAN_VID_MASK;
+			}
+		}
+		if ((dst = __br_fdb_get(br, dest, vlan_first_id,
+				vlan_second_id)) != NULL)
+			br_deliver(dst->dst, skb);
+		else
+			br_flood_deliver(br, skb);
+	}
 
 	return 0;
 }
Index: linux-2.6.23/net/bridge/br_fdb.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_fdb.c	2007-12-18 09:42:45.000000000 +0100
+++ linux-2.6.23/net/bridge/br_fdb.c	2007-12-18 09:43:37.000000000 +0100
@@ -211,13 +211,17 @@
 
 /* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
 struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-					  const unsigned char *addr)
+					  const unsigned char *addr,
+					  unsigned short vlan_first_id,
+					  unsigned short vlan_second_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr)) {
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || (fdb->vlan_first_id == vlan_first_id &&
+			fdb->vlan_second_id == vlan_second_id))) {
 			if (unlikely(has_expired(br, fdb)))
 				break;
 			return fdb;
@@ -229,12 +233,14 @@
 
 /* Interface used by ATM hook that keeps a ref count */
 struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					unsigned char *addr)
+					unsigned char *addr,
+					unsigned short vlan_first_id,
+					unsigned short vlan_second_id)
 {
 	struct net_bridge_fdb_entry *fdb;
 
 	rcu_read_lock();
-	fdb = __br_fdb_get(br, addr);
+	fdb = __br_fdb_get(br, addr, vlan_first_id, vlan_second_id);
 	if (fdb && !atomic_inc_not_zero(&fdb->use_count))
 		fdb = NULL;
 	rcu_read_unlock();
@@ -289,6 +295,8 @@
 			fe->is_local = f->is_local;
 			if (!f->is_static)
 				fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->ageing_timer);
+			fe->vlan_first_id = f->vlan_first_id;
+			fe->vlan_second_id = f->vlan_second_id;
 			++fe;
 			++num;
 		}
@@ -301,13 +309,17 @@
 }
 
 static inline struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
-						    const unsigned char *addr)
+						    const unsigned char *addr,
+						    unsigned short vlan_first_id,
+						    unsigned short vlan_second_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, head, hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr))
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || (fdb->vlan_first_id == vlan_first_id
+			&& fdb->vlan_second_id == vlan_second_id)))
 			return fdb;
 	}
 	return NULL;
@@ -316,6 +328,8 @@
 static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
 					       struct net_bridge_port *source,
 					       const unsigned char *addr,
+					       unsigned short vlan_first_id,
+					       unsigned short vlan_second_id,
 					       int is_local)
 {
 	struct net_bridge_fdb_entry *fdb;
@@ -323,6 +337,8 @@
 	fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
 	if (fdb) {
 		memcpy(fdb->addr.addr, addr, ETH_ALEN);
+		fdb->vlan_first_id = vlan_first_id;
+		fdb->vlan_second_id = vlan_second_id;
 		atomic_set(&fdb->use_count, 1);
 		hlist_add_head_rcu(&fdb->hlist, head);
 
@@ -343,7 +359,7 @@
 	if (!is_valid_ether_addr(addr))
 		return -EINVAL;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, 0, 0);
 	if (fdb) {
 		/* it is okay to have multiple ports with same
 		 * address, just use the first one.
@@ -357,7 +373,7 @@
 		fdb_delete(fdb);
 	}
 
-	if (!fdb_create(head, source, addr, 1))
+	if (!fdb_create(head, source, addr, 0, 0, 1))
 		return -ENOMEM;
 
 	return 0;
@@ -375,7 +391,8 @@
 }
 
 void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr)
+		   const unsigned char *addr, unsigned short vlan_first_id,
+		   unsigned short vlan_second_id)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;
@@ -389,7 +406,7 @@
 	      source->state == BR_STATE_FORWARDING))
 		return;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, vlan_first_id, vlan_second_id);
 	if (likely(fdb)) {
 		/* attempt to update an entry for a local interface */
 		if (unlikely(fdb->is_local)) {
@@ -404,8 +421,9 @@
 		}
 	} else {
 		spin_lock(&br->hash_lock);
-		if (!fdb_find(head, addr))
-			fdb_create(head, source, addr, 0);
+		if (!fdb_find(head, addr, vlan_first_id, vlan_second_id))
+			fdb_create(head, source, addr, vlan_first_id,
+				   vlan_second_id, 0);
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
 		 */
Index: linux-2.6.23/net/bridge/br_input.c
===================================================================
--- linux-2.6.23.orig/net/bridge/br_input.c	2007-12-18 09:42:45.000000000 +0100
+++ linux-2.6.23/net/bridge/br_input.c	2007-12-18 09:43:37.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_vlan.h>
 #include "br_private.h"
 
 /* Bridge group multicast address 802.1d (pg 51). */
@@ -44,13 +45,24 @@
 	struct net_bridge *br;
 	struct net_bridge_fdb_entry *dst;
 	struct sk_buff *skb2;
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_first_id = 0;
+	unsigned short vlan_second_id = 0;
 
 	if (!p || p->state == BR_STATE_DISABLED)
 		goto drop;
 
 	/* insert into forwarding database after filtering to avoid spoofing */
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+		vlan_first_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
+		if (veth->h_vlan_encapsulated_proto ==
+				__constant_htons(ETH_P_8021Q)) {
+			struct vlan_hdr *veth2 = (struct vlan_hdr *) (veth+1);
+			vlan_second_id = ntohs(veth2->h_vlan_TCI) & VLAN_VID_MASK;
+		}
+	}
 	br = p->br;
-	br_fdb_update(br, p, eth_hdr(skb)->h_source);
+	br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_first_id, vlan_second_id);
 
 	if (p->state == BR_STATE_LEARNING)
 		goto drop;
@@ -66,7 +78,7 @@
 	if (is_multicast_ether_addr(dest)) {
 		br->statistics.multicast++;
 		skb2 = skb;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest, vlan_first_id, vlan_second_id)) && dst->is_local) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;
@@ -96,9 +108,20 @@
 static int br_handle_local_finish(struct sk_buff *skb)
 {
 	struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
-
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_first_id = 0;
+	unsigned short vlan_second_id = 0;
+
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+		vlan_first_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
+		if (veth->h_vlan_encapsulated_proto ==
+				__constant_htons(ETH_P_8021Q)) {
+			struct vlan_hdr *veth2 = (struct vlan_hdr *) (veth+1);
+			vlan_second_id = ntohs(veth2->h_vlan_TCI) & VLAN_VID_MASK;
+		}
+	}
 	if (p)
-		br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_first_id, vlan_second_id);
 	return 0;	 /* process further */
 }
 
Index: linux-2.6.23/net/atm/lec.c
===================================================================
--- linux-2.6.23.orig/net/atm/lec.c	2007-12-18 09:42:45.000000000 +0100
+++ linux-2.6.23/net/atm/lec.c	2007-12-18 09:43:37.000000000 +0100
@@ -557,7 +557,7 @@
 				break;
 
 			f = br_fdb_get_hook(dev->br_port->br,
-					    mesg->content.proxy.mac_addr);
+					    mesg->content.proxy.mac_addr, 0, 0);
 			if (f != NULL && f->dst->dev != dev
 			    && f->dst->state == BR_STATE_FORWARDING) {
 				/* hit from bridge table, send LE_ARP_RESPONSE */
Index: linux-2.6.23/include/linux/if_bridge.h
===================================================================
--- linux-2.6.23.orig/include/linux/if_bridge.h	2007-12-18 09:42:45.000000000 +0100
+++ linux-2.6.23/include/linux/if_bridge.h	2007-12-18 09:43:37.000000000 +0100
@@ -97,7 +97,8 @@
 	__u8 port_no;
 	__u8 is_local;
 	__u32 ageing_timer_value;
-	__u32 unused;
+	__u16 vlan_first_id;
+	__u16 vlan_second_id;
 };
 
 #ifdef __KERNEL__

-- 

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

I'm resending this mail since I got no answer.

Is there any major problem in this?

Stephen Hemminger wrote:
>
> What about the nested vlan case?
>

Below is a new patch that handles the double-tagging case. I'm not sure
if it is worth a more generic case. ¿Are triple-tagging and so really used?

> This is a user/kernel ABI change. Does it break old tools?

New patch gets rid of the unused field but it still doesn't break old tools.

Anyway, the user part is not really needed. I just think it could be useful.

Regards,
Jaime.

---
Subject: [PATCH] Add vlan id to bridge forward database

This makes forwarding table aware of 802.1Q vlan ids and stores
id with MACs in the table. Up to two vlan tags are handled.

It solves problems when having same MAC on diffent pairs
(vlan, port). Current code gets confused at that situation.

Local MACs are managed as present on every vlan.

Signed-off-by: Jaime Medrano <jaime.medrano@gmail.com> 
---
 include/linux/if_bridge.h |    3 ++-
 net/atm/lec.c             |    2 +-
 net/bridge/br_device.c    |   25 +++++++++++++++++++++----
 net/bridge/br_fdb.c       |   42 ++++++++++++++++++++++++++++++------------
 net/bridge/br_input.c     |   36 ++++++++++++++++++++++++++++++++----
 net/bridge/br_private.h   |   18 ++++++++++++++----
 6 files changed, 100 insertions(+), 26 deletions(-)

Index: linux-2.6.23.1/net/bridge/br_private.h
===================================================================
--- linux-2.6.23.1.orig/net/bridge/br_private.h	2007-12-04 22:16:53.000000000 +0100
+++ linux-2.6.23.1/net/bridge/br_private.h	2008-01-24 20:29:23.000000000 +0100
@@ -55,6 +55,8 @@
 	atomic_t			use_count;
 	unsigned long			ageing_timer;
 	mac_addr			addr;
+	unsigned short			vlan_first_id;
+	unsigned short			vlan_second_id;
 	unsigned char			is_local;
 	unsigned char			is_static;
 };
@@ -150,9 +152,13 @@
 extern void br_fdb_delete_by_port(struct net_bridge *br,
 				  const struct net_bridge_port *p, int do_all);
 extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-						 const unsigned char *addr);
+						 const unsigned char *addr,
+						 unsigned short vlan_first_id,
+						 unsigned short vlan_second_id);
 extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					       unsigned char *addr);
+					       unsigned char *addr,
+					       unsigned short vlan_first_id,
+					       unsigned short vlan_second_id);
 extern void br_fdb_put(struct net_bridge_fdb_entry *ent);
 extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
 			  unsigned long count, unsigned long off);
@@ -161,7 +167,9 @@
 			 const unsigned char *addr);
 extern void br_fdb_update(struct net_bridge *br,
 			  struct net_bridge_port *source,
-			  const unsigned char *addr);
+			  const unsigned char *addr,
+			  unsigned short vlan_first_id,
+			  unsigned short vlan_second_id);
 
 /* br_forward.c */
 extern void br_deliver(const struct net_bridge_port *to,
@@ -237,7 +245,9 @@
 
 /* br.c */
 extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
-						       unsigned char *addr);
+						       unsigned char *addr,
+						       unsigned short vlan_first_id,
+						       unsigned short vlan_second_id);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
 
 
Index: linux-2.6.23.1/net/bridge/br_device.c
===================================================================
--- linux-2.6.23.1.orig/net/bridge/br_device.c	2007-12-04 22:16:53.000000000 +0100
+++ linux-2.6.23.1/net/bridge/br_device.c	2008-01-24 20:27:41.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -42,10 +43,26 @@
 
 	if (dest[0] & 1)
 		br_flood_deliver(br, skb);
-	else if ((dst = __br_fdb_get(br, dest)) != NULL)
-		br_deliver(dst->dst, skb);
-	else
-		br_flood_deliver(br, skb);
+	else {
+		struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+		unsigned short vlan_first_id = 0;
+		unsigned short vlan_second_id = 0;
+		if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+			vlan_first_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
+			if (veth->h_vlan_encapsulated_proto ==
+			    __constant_htons(ETH_P_8021Q)) {
+				struct vlan_hdr *veth2 = (struct vlan_hdr *)
+							 veth + 1;
+				vlan_second_id = ntohs(veth2->h_vlan_TCI) &
+						 VLAN_VID_MASK;
+			}
+		}
+		if ((dst = __br_fdb_get(br, dest, vlan_first_id,
+				vlan_second_id)) != NULL)
+			br_deliver(dst->dst, skb);
+		else
+			br_flood_deliver(br, skb);
+	}
 
 	return 0;
 }
Index: linux-2.6.23.1/net/bridge/br_fdb.c
===================================================================
--- linux-2.6.23.1.orig/net/bridge/br_fdb.c	2007-12-04 22:16:53.000000000 +0100
+++ linux-2.6.23.1/net/bridge/br_fdb.c	2007-12-17 23:15:37.000000000 +0100
@@ -211,13 +211,17 @@
 
 /* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
 struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-					  const unsigned char *addr)
+					  const unsigned char *addr,
+					  unsigned short vlan_first_id,
+					  unsigned short vlan_second_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr)) {
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || (fdb->vlan_first_id == vlan_first_id &&
+			fdb->vlan_second_id == vlan_second_id))) {
 			if (unlikely(has_expired(br, fdb)))
 				break;
 			return fdb;
@@ -229,12 +233,14 @@
 
 /* Interface used by ATM hook that keeps a ref count */
 struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
-					unsigned char *addr)
+					unsigned char *addr,
+					unsigned short vlan_first_id,
+					unsigned short vlan_second_id)
 {
 	struct net_bridge_fdb_entry *fdb;
 
 	rcu_read_lock();
-	fdb = __br_fdb_get(br, addr);
+	fdb = __br_fdb_get(br, addr, vlan_first_id, vlan_second_id);
 	if (fdb && !atomic_inc_not_zero(&fdb->use_count))
 		fdb = NULL;
 	rcu_read_unlock();
@@ -289,6 +295,8 @@
 			fe->is_local = f->is_local;
 			if (!f->is_static)
 				fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->ageing_timer);
+			fe->vlan_first_id = f->vlan_first_id;
+			fe->vlan_second_id = f->vlan_second_id;
 			++fe;
 			++num;
 		}
@@ -301,13 +309,17 @@
 }
 
 static inline struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
-						    const unsigned char *addr)
+						    const unsigned char *addr,
+						    unsigned short vlan_first_id,
+						    unsigned short vlan_second_id)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, head, hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr))
+		if (!compare_ether_addr(fdb->addr.addr, addr)
+		    && (fdb->is_local || (fdb->vlan_first_id == vlan_first_id
+			&& fdb->vlan_second_id == vlan_second_id)))
 			return fdb;
 	}
 	return NULL;
@@ -316,6 +328,8 @@
 static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
 					       struct net_bridge_port *source,
 					       const unsigned char *addr,
+					       unsigned short vlan_first_id,
+					       unsigned short vlan_second_id,
 					       int is_local)
 {
 	struct net_bridge_fdb_entry *fdb;
@@ -323,6 +337,8 @@
 	fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
 	if (fdb) {
 		memcpy(fdb->addr.addr, addr, ETH_ALEN);
+		fdb->vlan_first_id = vlan_first_id;
+		fdb->vlan_second_id = vlan_second_id;
 		atomic_set(&fdb->use_count, 1);
 		hlist_add_head_rcu(&fdb->hlist, head);
 
@@ -343,7 +359,7 @@
 	if (!is_valid_ether_addr(addr))
 		return -EINVAL;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, 0, 0);
 	if (fdb) {
 		/* it is okay to have multiple ports with same
 		 * address, just use the first one.
@@ -357,7 +373,7 @@
 		fdb_delete(fdb);
 	}
 
-	if (!fdb_create(head, source, addr, 1))
+	if (!fdb_create(head, source, addr, 0, 0, 1))
 		return -ENOMEM;
 
 	return 0;
@@ -375,7 +391,8 @@
 }
 
 void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr)
+		   const unsigned char *addr, unsigned short vlan_first_id,
+		   unsigned short vlan_second_id)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;
@@ -389,7 +406,7 @@
 	      source->state == BR_STATE_FORWARDING))
 		return;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, vlan_first_id, vlan_second_id);
 	if (likely(fdb)) {
 		/* attempt to update an entry for a local interface */
 		if (unlikely(fdb->is_local)) {
@@ -404,8 +421,9 @@
 		}
 	} else {
 		spin_lock(&br->hash_lock);
-		if (!fdb_find(head, addr))
-			fdb_create(head, source, addr, 0);
+		if (!fdb_find(head, addr, vlan_first_id, vlan_second_id))
+			fdb_create(head, source, addr, vlan_first_id,
+				   vlan_second_id, 0);
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
 		 */
Index: linux-2.6.23.1/net/bridge/br_input.c
===================================================================
--- linux-2.6.23.1.orig/net/bridge/br_input.c	2007-12-04 22:16:53.000000000 +0100
+++ linux-2.6.23.1/net/bridge/br_input.c	2008-01-24 20:24:43.000000000 +0100
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#include <linux/if_vlan.h>
 #include "br_private.h"
 
 /* Bridge group multicast address 802.1d (pg 51). */
@@ -44,13 +45,26 @@
 	struct net_bridge *br;
 	struct net_bridge_fdb_entry *dst;
 	struct sk_buff *skb2;
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_first_id = 0;
+	unsigned short vlan_second_id = 0;
 
 	if (!p || p->state == BR_STATE_DISABLED)
 		goto drop;
 
 	/* insert into forwarding database after filtering to avoid spoofing */
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+		vlan_first_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
+		if (veth->h_vlan_encapsulated_proto ==
+				__constant_htons(ETH_P_8021Q)) {
+			struct vlan_hdr *veth2 = (struct vlan_hdr *) veth+1;
+			vlan_second_id = ntohs(veth2->h_vlan_TCI) &
+					 VLAN_VID_MASK;
+		}
+	}
 	br = p->br;
-	br_fdb_update(br, p, eth_hdr(skb)->h_source);
+	br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_first_id,
+		      vlan_second_id);
 
 	if (p->state == BR_STATE_LEARNING)
 		goto drop;
@@ -66,7 +80,8 @@
 	if (is_multicast_ether_addr(dest)) {
 		br->statistics.multicast++;
 		skb2 = skb;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest, vlan_first_id,
+		    vlan_second_id)) && dst->is_local) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;
@@ -96,9 +111,22 @@
 static int br_handle_local_finish(struct sk_buff *skb)
 {
 	struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
-
+	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
+	unsigned short vlan_first_id = 0;
+	unsigned short vlan_second_id = 0;
+
+	if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+		vlan_first_id = ntohs(veth->h_vlan_TCI) & VLAN_VID_MASK;
+		if (veth->h_vlan_encapsulated_proto ==
+				__constant_htons(ETH_P_8021Q)) {
+			struct vlan_hdr *veth2 = (struct vlan_hdr *) veth+1;
+			vlan_second_id = ntohs(veth2->h_vlan_TCI) &
+					 VLAN_VID_MASK;
+		}
+	}
 	if (p)
-		br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_first_id,
+			      vlan_second_id);
 	return 0;	 /* process further */
 }
 
Index: linux-2.6.23.1/net/atm/lec.c
===================================================================
--- linux-2.6.23.1.orig/net/atm/lec.c	2007-12-04 22:16:53.000000000 +0100
+++ linux-2.6.23.1/net/atm/lec.c	2007-12-17 23:05:20.000000000 +0100
@@ -557,7 +557,7 @@
 				break;
 
 			f = br_fdb_get_hook(dev->br_port->br,
-					    mesg->content.proxy.mac_addr);
+					    mesg->content.proxy.mac_addr, 0, 0);
 			if (f != NULL && f->dst->dev != dev
 			    && f->dst->state == BR_STATE_FORWARDING) {
 				/* hit from bridge table, send LE_ARP_RESPONSE */
Index: linux-2.6.23.1/include/linux/if_bridge.h
===================================================================
--- linux-2.6.23.1.orig/include/linux/if_bridge.h	2007-12-17 23:15:01.000000000 +0100
+++ linux-2.6.23.1/include/linux/if_bridge.h	2007-12-17 23:15:37.000000000 +0100
@@ -97,7 +97,8 @@
 	__u8 port_no;
 	__u8 is_local;
 	__u32 ageing_timer_value;
-	__u32 unused;
+	__u16 vlan_first_id;
+	__u16 vlan_second_id;
 };
 
 #ifdef __KERNEL__

[Bridge] Re: [PATCH] Add vlan id to bridge forward database

[Benny Amorsen]

Jaime Medrano <jaime.medrano@gmail.com> writes:

> Below is a new patch that handles the double-tagging case. I'm not sure
> if it is worth a more generic case. ¿Are triple-tagging and so really used?

Yes. The company I work for doesn't do it with Linux right now,
because double tagging is problematic enough already, but it would be
great if we could.

We're getting lots of layer-2 connections to many locations through
one provider, and each connection is double-tagged by the provider
when delivered to the central "office" (in order to make it possible
to have more than 4096 locations). We'd like to be able to deliver
more than one VLAN to each location, and that requires triple-tagging.
Do the same with two providers -- welcome to quadruple-tagging.


/Benny

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Stephen Hemminger]

On Mon, 28 Jan 2008 16:39:14 +0100
Jaime Medrano <jaime.medrano@gmail.com> wrote:

> I'm resending this mail since I got no answer.
> 
> Is there any major problem in this?
> 
> Stephen Hemminger wrote:
> >
> > What about the nested vlan case?
> >
> 
> Below is a new patch that handles the double-tagging case. I'm not sure
> if it is worth a more generic case. ¿Are triple-tagging and so really used?
> 
> > This is a user/kernel ABI change. Does it break old tools?
> 
> New patch gets rid of the unused field but it still doesn't break old tools.
> 
> Anyway, the user part is not really needed. I just think it could be useful.
> 
> Regards,
> Jaime.
> 


Minor stuff:
1. Please use shorter variable names, rather than:
              unsigned short vlan_first_id;
      I would choose:
              u16 vlan1;

2. You probably can use skb->protocol rather than having to look at the packet
    contents to check for 8021Q.

3. Don't use __constant_htons(), just use htons().
   The macro is smart enough to handle the
   constant case, and it reads better, without the __constant_prefix.


Major stuff:
1. This won't work with hardware accel VLAN receive. The tag is not put in
   the skb?

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

On Mon, Mar 17, 2008 at 11:35:37AM -0700, Stephen Hemminger wrote:
> Minor stuff:
> 1. Please use shorter variable names, rather than:
>               unsigned short vlan_first_id;
>       I would choose:
>               u16 vlan1;

Done.

> 2. You probably can use skb->protocol rather than having to look at the packet
>     contents to check for 8021Q.

Done for non-nested case. Unavoidable for nested vlan tags.

> 3. Don't use __constant_htons(), just use htons().
>    The macro is smart enough to handle the
>    constant case, and it reads better, without the __constant_prefix.

Done.

> Major stuff:
> 1. This won't work with hardware accel VLAN receive. The tag is not put in
>    the skb?

It will work with drivers with hardware accel VLAN receive support if no
vlan device is attached to the network device of one of the ports of the
bridge. It will also work if a vlan device is attached to the bridge
device. In those cases, hw accel is not used.

However, it won't work if a vlan device is attached to the network
device of one of the ports. Hw accel will be used and the vlan tag won't
be available. Anyway, this is a bad idea since normal bridging won't work
either. The vlan won't be regenerated at bridge output since current
bridge code doesn't get that tag.

If the vlan device is attached to the bridge device then bridging has no
problems as hw accel receiving is not used.

The patch has also been generalized to support any number of vlan nested
tags. Vlan tag recording can be disabled at all if BR_MAX_VLAN_TAGS is
set to 0.

---
Subject: [PATCH] Add vlan tags to bridge forward database

This makes forwarding table aware of 802.1Q vlan tags and stores
vlan ids with MACs in the table.

It solves problems when having same MAC on diffent pairs
(vlan, port). Current code gets confused at that situation.

Nested vlan tags are also handled.

Local MACs are managed as present on every vlan.

Signed-off-by: Jaime Medrano <jaime.medrano@gmail.com>
---
 net/bridge/br_device.c  |   11 +++--
 net/bridge/br_fdb.c     |   91 ++++++++++++++++++++++++++++++++++++++++++------
 net/bridge/br_input.c   |   15 ++++---
 net/bridge/br_private.h |    9 +++-
 4 files changed, 103 insertions(+), 23 deletions(-)

Index: linux-2.6.25-rc7/net/bridge/br_private.h
===================================================================
--- linux-2.6.25-rc7.orig/net/bridge/br_private.h	2008-04-01 23:54:49.000000000 +0200
+++ linux-2.6.25-rc7/net/bridge/br_private.h	2008-04-01 23:55:13.000000000 +0200
@@ -26,6 +26,8 @@
 #define BR_PORT_BITS	10
 #define BR_MAX_PORTS	(1<<BR_PORT_BITS)
 
+#define BR_MAX_VLAN_TAGS	2
+
 #define BR_VERSION	"2.3"
 
 /* Path to usermode spanning tree program */
@@ -55,6 +57,7 @@
 	atomic_t			use_count;
 	unsigned long			ageing_timer;
 	mac_addr			addr;
+	__be16				vlan_tags[BR_MAX_VLAN_TAGS];
 	unsigned char			is_local;
 	unsigned char			is_static;
 };
@@ -150,7 +153,8 @@
 extern void br_fdb_delete_by_port(struct net_bridge *br,
 				  const struct net_bridge_port *p, int do_all);
 extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-						 const unsigned char *addr);
+						 const unsigned char *addr,
+						 const struct sk_buff *skb);
 extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
 					       unsigned char *addr);
 extern void br_fdb_put(struct net_bridge_fdb_entry *ent);
@@ -161,7 +165,8 @@
 			 const unsigned char *addr);
 extern void br_fdb_update(struct net_bridge *br,
 			  struct net_bridge_port *source,
-			  const unsigned char *addr);
+			  const unsigned char *addr,
+			  const struct sk_buff *skb);
 
 /* br_forward.c */
 extern void br_deliver(const struct net_bridge_port *to,
Index: linux-2.6.25-rc7/net/bridge/br_device.c
===================================================================
--- linux-2.6.25-rc7.orig/net/bridge/br_device.c	2008-04-01 23:54:49.000000000 +0200
+++ linux-2.6.25-rc7/net/bridge/br_device.c	2008-04-01 23:55:13.000000000 +0200
@@ -42,10 +42,13 @@
 
 	if (dest[0] & 1)
 		br_flood_deliver(br, skb);
-	else if ((dst = __br_fdb_get(br, dest)) != NULL)
-		br_deliver(dst->dst, skb);
-	else
-		br_flood_deliver(br, skb);
+	else {
+		dst = __br_fdb_get(br, dest, skb);
+		if (dst != NULL)
+			br_deliver(dst->dst, skb);
+		else
+			br_flood_deliver(br, skb);
+	}
 
 	return 0;
 }
Index: linux-2.6.25-rc7/net/bridge/br_fdb.c
===================================================================
--- linux-2.6.25-rc7.orig/net/bridge/br_fdb.c	2008-04-01 23:54:49.000000000 +0200
+++ linux-2.6.25-rc7/net/bridge/br_fdb.c	2008-04-01 23:55:13.000000000 +0200
@@ -24,6 +24,7 @@
 #include <asm/atomic.h>
 #include <asm/unaligned.h>
 #include "br_private.h"
+#include <linux/if_vlan.h>
 
 static struct kmem_cache *br_fdb_cache __read_mostly;
 static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
@@ -209,15 +210,79 @@
 	spin_unlock_bh(&br->hash_lock);
 }
 
+#if BR_MAX_VLAN_TAGS > 0
+static void set_vlan_tags(const struct sk_buff *skb, __be16 *tags)
+{
+	__be16 proto;
+	int i, off;
+	struct vlan_hdr _hdr;
+	struct vlan_hdr *hdr;
+
+	if (!skb || skb->protocol != htons(ETH_P_8021Q)) {
+		tags[0] = 0;
+		return;
+	}
+
+	proto = skb->protocol;
+
+	for (i = 0, off = 0 ;
+	     i < BR_MAX_VLAN_TAGS && proto == htons(ETH_P_8021Q) ;
+	     i++, off += VLAN_HLEN) {
+		hdr = skb_header_pointer(skb, off, sizeof(_hdr), &_hdr);
+		tags[i] = hdr->h_vlan_TCI & htons(VLAN_VID_MASK);
+		proto = hdr->h_vlan_encapsulated_proto;
+	}
+
+	if (i < BR_MAX_VLAN_TAGS)
+		tags[i] = 0;
+}
+
+static int compare_vlan_tags(const struct sk_buff *skb, const __be16 *tags)
+{
+	__be16 proto;
+	int i, off;
+	struct vlan_hdr _hdr;
+	struct vlan_hdr *hdr;
+
+	if (!skb || skb->protocol != htons(ETH_P_8021Q))
+		return tags[0] != 0;
+
+	proto = skb->protocol;
+
+	for (i = 0, off = 0 ;
+	     i < BR_MAX_VLAN_TAGS && tags[i] && proto == htons(ETH_P_8021Q) ;
+	     i++, off += VLAN_HLEN) {
+		hdr = skb_header_pointer(skb, off, sizeof(_hdr), &_hdr);
+		if ((hdr->h_vlan_TCI & htons(VLAN_VID_MASK)) != tags[i])
+			return 1;
+		proto = hdr->h_vlan_encapsulated_proto;
+	}
+
+	return i < BR_MAX_VLAN_TAGS && (tags[i] || proto == htons(ETH_P_8021Q));
+}
+
+#else
+static void set_vlan_tags(const struct sk_buff *skb, __be16 *tags)
+{
+}
+
+static int compare_vlan_tags(const struct sk_buff *skb, const __be16 *tags)
+{
+	return 0;
+}
+#endif
+
 /* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
 struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
-					  const unsigned char *addr)
+					  const unsigned char *addr,
+					  const struct sk_buff *skb)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr)) {
+		if (!compare_ether_addr(fdb->addr.addr, addr) && (fdb->is_local
+		    || !compare_vlan_tags(skb, fdb->vlan_tags))) {
 			if (unlikely(has_expired(br, fdb)))
 				break;
 			return fdb;
@@ -234,7 +299,7 @@
 	struct net_bridge_fdb_entry *fdb;
 
 	rcu_read_lock();
-	fdb = __br_fdb_get(br, addr);
+	fdb = __br_fdb_get(br, addr, NULL);
 	if (fdb && !atomic_inc_not_zero(&fdb->use_count))
 		fdb = NULL;
 	rcu_read_unlock();
@@ -301,13 +366,15 @@
 }
 
 static inline struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
-						    const unsigned char *addr)
+						    const unsigned char *addr,
+						    const struct sk_buff *skb)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 
 	hlist_for_each_entry_rcu(fdb, h, head, hlist) {
-		if (!compare_ether_addr(fdb->addr.addr, addr))
+		if (!compare_ether_addr(fdb->addr.addr, addr) &&
+		    (fdb->is_local || !compare_vlan_tags(skb, fdb->vlan_tags)))
 			return fdb;
 	}
 	return NULL;
@@ -316,6 +383,7 @@
 static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
 					       struct net_bridge_port *source,
 					       const unsigned char *addr,
+					       const struct sk_buff *skb,
 					       int is_local)
 {
 	struct net_bridge_fdb_entry *fdb;
@@ -323,6 +391,7 @@
 	fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
 	if (fdb) {
 		memcpy(fdb->addr.addr, addr, ETH_ALEN);
+		set_vlan_tags(skb, fdb->vlan_tags);
 		atomic_set(&fdb->use_count, 1);
 		hlist_add_head_rcu(&fdb->hlist, head);
 
@@ -343,7 +412,7 @@
 	if (!is_valid_ether_addr(addr))
 		return -EINVAL;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, NULL);
 	if (fdb) {
 		/* it is okay to have multiple ports with same
 		 * address, just use the first one.
@@ -357,7 +426,7 @@
 		fdb_delete(fdb);
 	}
 
-	if (!fdb_create(head, source, addr, 1))
+	if (!fdb_create(head, source, addr, NULL, 1))
 		return -ENOMEM;
 
 	return 0;
@@ -375,7 +444,7 @@
 }
 
 void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr)
+		   const unsigned char *addr, const struct sk_buff *skb)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;
@@ -389,7 +458,7 @@
 	      source->state == BR_STATE_FORWARDING))
 		return;
 
-	fdb = fdb_find(head, addr);
+	fdb = fdb_find(head, addr, skb);
 	if (likely(fdb)) {
 		/* attempt to update an entry for a local interface */
 		if (unlikely(fdb->is_local)) {
@@ -404,8 +473,8 @@
 		}
 	} else {
 		spin_lock(&br->hash_lock);
-		if (!fdb_find(head, addr))
-			fdb_create(head, source, addr, 0);
+		if (!fdb_find(head, addr, skb))
+			fdb_create(head, source, addr, skb, 0);
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
 		 */
Index: linux-2.6.25-rc7/net/bridge/br_input.c
===================================================================
--- linux-2.6.25-rc7.orig/net/bridge/br_input.c	2008-04-01 23:54:49.000000000 +0200
+++ linux-2.6.25-rc7/net/bridge/br_input.c	2008-04-01 23:55:13.000000000 +0200
@@ -50,7 +50,7 @@
 
 	/* insert into forwarding database after filtering to avoid spoofing */
 	br = p->br;
-	br_fdb_update(br, p, eth_hdr(skb)->h_source);
+	br_fdb_update(br, p, eth_hdr(skb)->h_source, skb);
 
 	if (p->state == BR_STATE_LEARNING)
 		goto drop;
@@ -66,10 +66,13 @@
 	if (is_multicast_ether_addr(dest)) {
 		br->statistics.multicast++;
 		skb2 = skb;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
-		skb2 = skb;
-		/* Do not forward the packet since it's local. */
-		skb = NULL;
+	} else {
+		dst = __br_fdb_get(br, dest, skb);
+		if (dst && dst->is_local) {
+			skb2 = skb;
+			/* Do not forward the packet since it's local. */
+			skb = NULL;
+		}
 	}
 
 	if (skb2 == skb)
@@ -98,7 +101,7 @@
 	struct net_bridge_port *p = rcu_dereference(skb->dev->br_port);
 
 	if (p)
-		br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, skb);
 	return 0;	 /* process further */
 }
 

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Stephen Hemminger]

On Mon, 28 Jan 2008 16:39:14 +0100
Jaime Medrano <jaime.medrano@gmail.com> wrote:

> I'm resending this mail since I got no answer.
> 
> Is there any major problem in this?
> 
> Stephen Hemminger wrote:
> >
> > What about the nested vlan case?
> >
> 
> Below is a new patch that handles the double-tagging case. I'm not sure
> if it is worth a more generic case. ¿Are triple-tagging and so really used?
> 
> > This is a user/kernel ABI change. Does it break old tools?
> 
> New patch gets rid of the unused field but it still doesn't break old tools.
> 
> Anyway, the user part is not really needed. I just think it could be useful.
> 
> Regards,
> Jaime.
> 
> ---
> Subject: [PATCH] Add vlan id to bridge forward database
> 
> This makes forwarding table aware of 802.1Q vlan ids and stores
> id with MACs in the table. Up to two vlan tags are handled.
> 
> It solves problems when having same MAC on diffent pairs
> (vlan, port). Current code gets confused at that situation.
> 
> Local MACs are managed as present on every vlan.

I looked at this, and thought about cleaning it up and putting in
next release but right now can't see how it could work with
vlan accelerated hardware.

Your patch doesn't work if hardware does vlan acceleration. Vlan acceleration
strips the tag off so skb->data points to actual contents and tag is passed
up to vlan_receive_skb out of band. By the time the bridge sees the packet the
actual tag is gone.

You might be able to get tag by using some API to reference the VLAN device,
but it all gets complex and racy. Plus not sure if it is really even technically
correct.

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

On Mon, Apr 28, 2008 at 01:57:54PM -0700, Stephen Hemminger wrote:
> I looked at this, and thought about cleaning it up and putting in
> next release but right now can't see how it could work with
> vlan accelerated hardware.

¿Did you see my last mail? I made the cleaning up you suggested and
generalized the code to support any level of nested tags instead of just
two.

> Your patch doesn't work if hardware does vlan acceleration. Vlan acceleration
> strips the tag off so skb->data points to actual contents and tag is passed
> up to vlan_receive_skb out of band. By the time the bridge sees the packet the
> actual tag is gone.

Yes, I know. However, AFAIK out of band passing of the vlan tag is only
done when a vlan device is created and attached to the ethernet device
of the port. That's a weird case from my point of view:

	brctl addif br0 eth0
	vconfig add eth0 5

If you want to get vlan packets from a bridge, you'd better attach the
device to the bridge instead of one of the ports:

	brctl addif br0 eth0
	vconfig add br0 5

If you attach it to a port, the out of band tag won't reach current
bridge code, and won't be restored on bridge output in case the packet
has to be forwarded.  Attaching the vlan device to the bridge device
doesn't enable hardware vlan acceleration, so that everything works.

> You might be able to get tag by using some API to reference the VLAN device,
> but it all gets complex and racy. Plus not sure if it is really even technically
> correct.

I guess the complexity overwhelms the benefits. Anyway, if that case is
to be handled I think that restoring the tag on output is more
important.

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Benny Amorsen]

Jaime Medrano <jaime.medrano@gmail.com> writes:

> Yes, I know. However, AFAIK out of band passing of the vlan tag is only
> done when a vlan device is created and attached to the ethernet device
> of the port. 

If the driver supports VLAN acceleration, it is usually on by default.
At least I can't tcpdump VLAN packets with intact VLAN tags on my
notebook's tg3 card -- even if I haven't loaded the 8021q module at
all. It's rather annoying, really.

Anyway, you need to explicitly turn off VLAN acceleration on the
devices added to such a bridge.


/Benny

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Jaime Medrano]

On Tue, Apr 29, 2008 at 02:43:42PM +0200, Benny Amorsen wrote:
> Jaime Medrano <jaime.medrano@gmail.com> writes:
> > Yes, I know. However, AFAIK out of band passing of the vlan tag is only
> > done when a vlan device is created and attached to the ethernet device
> > of the port. 
> 
> If the driver supports VLAN acceleration, it is usually on by default.
> At least I can't tcpdump VLAN packets with intact VLAN tags on my
> notebook's tg3 card -- even if I haven't loaded the 8021q module at
> all. It's rather annoying, really.

That's weird.

I've got a tg3 card and I see the tags in tcpdump unless I register a vlan
device to it.

I've taken a look at the source code of the tg3 driver and
vlan_hwaccel_receive_skb() only gets called if a prior
tg3_vlan_rx_register() has been made.

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[richardvoigt]

>  > Your patch doesn't work if hardware does vlan acceleration. Vlan acceleration
>  > strips the tag off so skb->data points to actual contents and tag is passed
>  > up to vlan_receive_skb out of band. By the time the bridge sees the packet the
>  > actual tag is gone.
>
>  Yes, I know. However, AFAIK out of band passing of the vlan tag is only
>  done when a vlan device is created and attached to the ethernet device
>  of the port. That's a weird case from my point of view:
>
>         brctl addif br0 eth0
>         vconfig add eth0 5
>
>  If you want to get vlan packets from a bridge, you'd better attach the
>  device to the bridge instead of one of the ports:
>
>         brctl addif br0 eth0
>         vconfig add br0 5

That's for bridging a trunk, but what about bridging between vlans:

vconfig add eth0 5
vconfig add eth0 6
brctl add br0 eth0.5
brctl add br0 eth0.6

While you could combine the vlans, doing this allows you to force all
traffic through packet filtering/traffic shaping/IDS.

Usually you wouldn't then see the same MAC on two different VLAN but
you might on spanning tree packets.

Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Jonathan Thibault]

I've been trying to get something like that to work for over a year now. 
  It mostly works, only the bridge will stop forwarding some arp replies 
coming in from a non-vlan interface to the trunk.  Bridge sees the 
replies but they never make it onto the wire.  If this was an STP (it's 
not enabled) issue, I assume the port would simply stop forwarding 
alltogether, not just ARP *replies*.

(yeah, I know I sound like a broken record to those who've been 
following my posts ;)

If there's any tests I can do to help on that front, just ask!  I would 
be happy just knowing *why* it doesn't work so far but I don't know the 
code well enough to figure out where the packets might get discarded.

Jonathan

richardvoigt@gmail.com wrote:

> 
> That's for bridging a trunk, but what about bridging between vlans:
> 
> vconfig add eth0 5
> vconfig add eth0 6
> brctl add br0 eth0.5
> brctl add br0 eth0.6
> 
> While you could combine the vlans, doing this allows you to force all
> traffic through packet filtering/traffic shaping/IDS.
> 
> Usually you wouldn't then see the same MAC on two different VLAN but
> you might on spanning tree packets.