Commit a5642ab4 authored by Toshiaki Makita's avatar Toshiaki Makita Committed by David S. Miller

bridge: Fix the way to find old local fdb entries in br_fdb_changeaddr

br_fdb_changeaddr() assumes that there is at most one local entry per port
per vlan. It used to be true, but since commit 36fd2b63 ("bridge: allow
creating/deleting fdb entries via netlink"), it has not been so.
Therefore, the function might fail to search a correct previous address
to be deleted and delete an arbitrary local entry if user has added local
entries manually.

Example of problematic case:
  ip link set eth0 address ee:ff:12:34:56:78
  brctl addif br0 eth0
  bridge fdb add 12:34:56:78:90:ab dev eth0 master
  ip link set eth0 address aa:bb:cc:dd:ee:ff
Then, the address 12:34:56:78:90:ab might be deleted instead of
ee:ff:12:34:56:78, the original mac address of eth0.

Address this issue by introducing a new flag, added_by_user, to struct
net_bridge_fdb_entry.

Note that br_fdb_delete_by_port() has to set added_by_user to 0 in cases
like:
  ip link set eth0 address 12:34:56:78:90:ab
  ip link set eth1 address aa:bb:cc:dd:ee:ff
  brctl addif br0 eth0
  bridge fdb add aa:bb:cc:dd:ee:ff dev eth0 master
  brctl addif br0 eth1
  brctl delif br0 eth0
In this case, kernel should delete the user-added entry aa:bb:cc:dd:ee:ff,
but it also should have been added by "brctl addif br0 eth1" originally,
so we don't delete it and treat it a new kernel-created entry.
Signed-off-by: default avatarToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent b10bd54c
...@@ -104,7 +104,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr) ...@@ -104,7 +104,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
struct net_bridge_fdb_entry *f; struct net_bridge_fdb_entry *f;
f = hlist_entry(h, struct net_bridge_fdb_entry, hlist); f = hlist_entry(h, struct net_bridge_fdb_entry, hlist);
if (f->dst == p && f->is_local) { if (f->dst == p && f->is_local && !f->added_by_user) {
/* maybe another port has same hw addr? */ /* maybe another port has same hw addr? */
struct net_bridge_port *op; struct net_bridge_port *op;
u16 vid = f->vlan_id; u16 vid = f->vlan_id;
...@@ -247,6 +247,7 @@ void br_fdb_delete_by_port(struct net_bridge *br, ...@@ -247,6 +247,7 @@ void br_fdb_delete_by_port(struct net_bridge *br,
ether_addr_equal(op->dev->dev_addr, ether_addr_equal(op->dev->dev_addr,
f->addr.addr)) { f->addr.addr)) {
f->dst = op; f->dst = op;
f->added_by_user = 0;
goto skip_delete; goto skip_delete;
} }
} }
...@@ -397,6 +398,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head, ...@@ -397,6 +398,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
fdb->vlan_id = vid; fdb->vlan_id = vid;
fdb->is_local = 0; fdb->is_local = 0;
fdb->is_static = 0; fdb->is_static = 0;
fdb->added_by_user = 0;
fdb->updated = fdb->used = jiffies; fdb->updated = fdb->used = jiffies;
hlist_add_head_rcu(&fdb->hlist, head); hlist_add_head_rcu(&fdb->hlist, head);
} }
...@@ -447,7 +449,7 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source, ...@@ -447,7 +449,7 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
} }
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid) const unsigned char *addr, u16 vid, bool added_by_user)
{ {
struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)]; struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb; struct net_bridge_fdb_entry *fdb;
...@@ -473,14 +475,19 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, ...@@ -473,14 +475,19 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
/* fastpath: update of existing entry */ /* fastpath: update of existing entry */
fdb->dst = source; fdb->dst = source;
fdb->updated = jiffies; fdb->updated = jiffies;
if (unlikely(added_by_user))
fdb->added_by_user = 1;
} }
} else { } else {
spin_lock(&br->hash_lock); spin_lock(&br->hash_lock);
if (likely(!fdb_find(head, addr, vid))) { if (likely(!fdb_find(head, addr, vid))) {
fdb = fdb_create(head, source, addr, vid); fdb = fdb_create(head, source, addr, vid);
if (fdb) if (fdb) {
if (unlikely(added_by_user))
fdb->added_by_user = 1;
fdb_notify(br, fdb, RTM_NEWNEIGH); fdb_notify(br, fdb, RTM_NEWNEIGH);
} }
}
/* else we lose race and someone else inserts /* else we lose race and someone else inserts
* it first, don't bother updating * it first, don't bother updating
*/ */
...@@ -647,6 +654,7 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, ...@@ -647,6 +654,7 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
modified = true; modified = true;
} }
fdb->added_by_user = 1;
fdb->used = jiffies; fdb->used = jiffies;
if (modified) { if (modified) {
...@@ -664,7 +672,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p, ...@@ -664,7 +672,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
if (ndm->ndm_flags & NTF_USE) { if (ndm->ndm_flags & NTF_USE) {
rcu_read_lock(); rcu_read_lock();
br_fdb_update(p->br, p, addr, vid); br_fdb_update(p->br, p, addr, vid, true);
rcu_read_unlock(); rcu_read_unlock();
} else { } else {
spin_lock_bh(&p->br->hash_lock); spin_lock_bh(&p->br->hash_lock);
......
...@@ -77,7 +77,7 @@ int br_handle_frame_finish(struct sk_buff *skb) ...@@ -77,7 +77,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
/* insert into forwarding database after filtering to avoid spoofing */ /* insert into forwarding database after filtering to avoid spoofing */
br = p->br; br = p->br;
if (p->flags & BR_LEARNING) if (p->flags & BR_LEARNING)
br_fdb_update(br, p, eth_hdr(skb)->h_source, vid); br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, false);
if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) && if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) &&
br_multicast_rcv(br, p, skb, vid)) br_multicast_rcv(br, p, skb, vid))
...@@ -148,7 +148,7 @@ static int br_handle_local_finish(struct sk_buff *skb) ...@@ -148,7 +148,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
br_vlan_get_tag(skb, &vid); br_vlan_get_tag(skb, &vid);
if (p->flags & BR_LEARNING) if (p->flags & BR_LEARNING)
br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid); br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, false);
return 0; /* process further */ return 0; /* process further */
} }
......
...@@ -104,6 +104,7 @@ struct net_bridge_fdb_entry ...@@ -104,6 +104,7 @@ struct net_bridge_fdb_entry
mac_addr addr; mac_addr addr;
unsigned char is_local; unsigned char is_local;
unsigned char is_static; unsigned char is_static;
unsigned char added_by_user;
__u16 vlan_id; __u16 vlan_id;
}; };
...@@ -383,7 +384,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf, unsigned long count, ...@@ -383,7 +384,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf, unsigned long count,
int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source, int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid); const unsigned char *addr, u16 vid);
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid); const unsigned char *addr, u16 vid, bool added_by_user);
int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr, u16 vid); int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr, u16 vid);
int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment