Add vlan id to bridge forward database
Subsystems (4)
PATCH 1/1 Add vlan id to bridge forward database
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.
--- 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(-)
net/bridge/br_private.h +9 -4
@@ -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);
net/bridge/br_device.c +9 -4
@@ -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 ; }
net/bridge/br_fdb.c +20 -12
@@ -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 */
net/bridge/br_input.c +12 -3
@@ -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 */ }
net/atm/lec.c +1 -1
@@ -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 */
include/linux/if_bridge.h +2 -1
@@ -97,7 +97,8 @@__u8 port_no;__u8 is_local;__u32 ageing_timer_value; -__u32 unused; +__u16 vlan_id; +__u16 unused; };#ifdef __KERNEL__
Quoted 11 lines · On Mon, 17 Dec 2007 17:13:09 +0100 Jaime Medrano <[email protected]>…
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 <[email protected]>
What about the nested vlan case?
I assume it continues to work without Vlan's.
How does this affect use of the userspace RSTP?
include/linux/if_bridge.h line 102
Quoted 10 lines
include/linux/if_bridge.h +2
-1@@ -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?
Signature · 1 line
Stephen Hemminger <[email protected]>
Quoted 3 lines · Stephen Hemminger
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?
Quoted 1 line
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.
---
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(-)
net/bridge/br_private.h +0 -0
@@ -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);
net/bridge/br_device.c +0 -0
@@ -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; }
net/bridge/br_fdb.c +0 -0
@@ -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 */
net/bridge/br_input.c +0 -0
@@ -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 */
}
net/atm/lec.c +0 -0
@@ -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 */include/linux/if_bridge.h +0 -0
@@ -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__
I'm resending this mail since I got no answer.
Is there any major problem in this?
Quoted 3 lines · Stephen Hemminger
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?
Quoted 1 line
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.
--- 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(-)
net/bridge/br_private.h +14 -4
@@ -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);
net/bridge/br_device.c +21 -4
@@ -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 ; }
net/bridge/br_fdb.c +30 -12
122 lines not on this page · raw message
net/bridge/br_input.c +32 -4
@@ -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 */ }
net/atm/lec.c +1 -1
@@ -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 */
include/linux/if_bridge.h +2 -1
@@ -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__
Quoted 2 lines · Jaime Medrano
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
Quoted 12 lines · On Mon, 28 Jan 2008 16:39:14 +0100 Jaime Medrano <[email protected]>…
I'm resending this mail since I got no answer.
Is there any major problem in this?
Quoted 3 lines · Stephen Hemminger
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?
Quoted 9 lines
Quoted 1 line
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?
Quoted 5 lines · Stephen Hemminger
Minor stuff:
1. Please use shorter variable names, rather than:unsigned short vlan_first_id; I would choose: u16 vlan1;
Done.
Quoted 2 lines
2. You probably can use
skb->protocolrather than having to look at the packet contents to check for 8021Q.
Done for non-nested case. Unavoidable for nested vlan tags.
Quoted 3 lines
3. Don't use
__constant_htons(), just usehtons().
The macro is smart enough to handle the
constant case, and it reads better, without the__constant_prefix.
Done.
Quoted 3 lines
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.
--- 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(-)
net/bridge/br_private.h +7 -2
@@ -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,
net/bridge/br_device.c +7 -4
@@ -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 ; }
net/bridge/br_fdb.c +80 -11
169 lines not on this page · raw message
net/bridge/br_input.c +9 -6
@@ -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 */ }
Quoted 24 lines · On Mon, 28 Jan 2008 16:39:14 +0100 Jaime Medrano <[email protected]>…
I'm resending this mail since I got no answer.
Is there any major problem in this?
Quoted 3 lines · Stephen Hemminger
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?
Quoted 1 line
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
Quoted 7 lines
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.
Quoted 3 lines · Stephen Hemminger
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.
Quoted 4 lines
Your patch doesn't work if hardware does vlan acceleration. Vlan acceleration strips the tag off so
skb->datapoints to actual contents and tag is passed up tovlan_receive_skbout 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.
Quoted 3 lines
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.
Quoted 3 lines · Jaime Medrano
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
Quoted 9 lines · Benny Amorsen
Quoted 3 lines · Jaime Medrano
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.
Quoted 9 lines
Quoted 4 lines
Your patch doesn't work if hardware does vlan acceleration. Vlan acceleration strips the tag off so
skb->datapoints to actual contents and tag is passed up tovlan_receive_skbout 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:
Quoted 8 lines
brctl addif br0 eth0
vconfig add eth0 5If 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.
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
Quoted 13 lines · [email protected] 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.6While 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.