diff options
author | Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> | 2014-02-07 02:48:18 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2014-02-10 17:34:33 -0500 |
commit | a5642ab4744bc8c5a8c7ce7c6e30c01bd6bbc691 (patch) | |
tree | 5ede15fbe48d33f5620de09ac5e5514a9ea5bf44 /net | |
parent | b10bd54c05a6c3c96549f4642d7de23c99b9853b (diff) |
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 36fd2b63e3b4 ("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: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r-- | net/bridge/br_fdb.c | 16 | ||||
-rw-r--r-- | net/bridge/br_input.c | 4 | ||||
-rw-r--r-- | net/bridge/br_private.h | 3 |
3 files changed, 16 insertions, 7 deletions
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index c5f5a4a933f4..ce5411995a63 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c | |||
@@ -104,7 +104,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr) | |||
104 | struct net_bridge_fdb_entry *f; | 104 | struct net_bridge_fdb_entry *f; |
105 | 105 | ||
106 | f = hlist_entry(h, struct net_bridge_fdb_entry, hlist); | 106 | f = hlist_entry(h, struct net_bridge_fdb_entry, hlist); |
107 | if (f->dst == p && f->is_local) { | 107 | if (f->dst == p && f->is_local && !f->added_by_user) { |
108 | /* maybe another port has same hw addr? */ | 108 | /* maybe another port has same hw addr? */ |
109 | struct net_bridge_port *op; | 109 | struct net_bridge_port *op; |
110 | u16 vid = f->vlan_id; | 110 | u16 vid = f->vlan_id; |
@@ -247,6 +247,7 @@ void br_fdb_delete_by_port(struct net_bridge *br, | |||
247 | ether_addr_equal(op->dev->dev_addr, | 247 | ether_addr_equal(op->dev->dev_addr, |
248 | f->addr.addr)) { | 248 | f->addr.addr)) { |
249 | f->dst = op; | 249 | f->dst = op; |
250 | f->added_by_user = 0; | ||
250 | goto skip_delete; | 251 | goto skip_delete; |
251 | } | 252 | } |
252 | } | 253 | } |
@@ -397,6 +398,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head, | |||
397 | fdb->vlan_id = vid; | 398 | fdb->vlan_id = vid; |
398 | fdb->is_local = 0; | 399 | fdb->is_local = 0; |
399 | fdb->is_static = 0; | 400 | fdb->is_static = 0; |
401 | fdb->added_by_user = 0; | ||
400 | fdb->updated = fdb->used = jiffies; | 402 | fdb->updated = fdb->used = jiffies; |
401 | hlist_add_head_rcu(&fdb->hlist, head); | 403 | hlist_add_head_rcu(&fdb->hlist, head); |
402 | } | 404 | } |
@@ -447,7 +449,7 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source, | |||
447 | } | 449 | } |
448 | 450 | ||
449 | void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, | 451 | void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, |
450 | const unsigned char *addr, u16 vid) | 452 | const unsigned char *addr, u16 vid, bool added_by_user) |
451 | { | 453 | { |
452 | struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)]; | 454 | struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)]; |
453 | struct net_bridge_fdb_entry *fdb; | 455 | struct net_bridge_fdb_entry *fdb; |
@@ -473,13 +475,18 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, | |||
473 | /* fastpath: update of existing entry */ | 475 | /* fastpath: update of existing entry */ |
474 | fdb->dst = source; | 476 | fdb->dst = source; |
475 | fdb->updated = jiffies; | 477 | fdb->updated = jiffies; |
478 | if (unlikely(added_by_user)) | ||
479 | fdb->added_by_user = 1; | ||
476 | } | 480 | } |
477 | } else { | 481 | } else { |
478 | spin_lock(&br->hash_lock); | 482 | spin_lock(&br->hash_lock); |
479 | if (likely(!fdb_find(head, addr, vid))) { | 483 | if (likely(!fdb_find(head, addr, vid))) { |
480 | fdb = fdb_create(head, source, addr, vid); | 484 | fdb = fdb_create(head, source, addr, vid); |
481 | if (fdb) | 485 | if (fdb) { |
486 | if (unlikely(added_by_user)) | ||
487 | fdb->added_by_user = 1; | ||
482 | fdb_notify(br, fdb, RTM_NEWNEIGH); | 488 | fdb_notify(br, fdb, RTM_NEWNEIGH); |
489 | } | ||
483 | } | 490 | } |
484 | /* else we lose race and someone else inserts | 491 | /* else we lose race and someone else inserts |
485 | * it first, don't bother updating | 492 | * it first, don't bother updating |
@@ -647,6 +654,7 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, | |||
647 | 654 | ||
648 | modified = true; | 655 | modified = true; |
649 | } | 656 | } |
657 | fdb->added_by_user = 1; | ||
650 | 658 | ||
651 | fdb->used = jiffies; | 659 | fdb->used = jiffies; |
652 | if (modified) { | 660 | if (modified) { |
@@ -664,7 +672,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p, | |||
664 | 672 | ||
665 | if (ndm->ndm_flags & NTF_USE) { | 673 | if (ndm->ndm_flags & NTF_USE) { |
666 | rcu_read_lock(); | 674 | rcu_read_lock(); |
667 | br_fdb_update(p->br, p, addr, vid); | 675 | br_fdb_update(p->br, p, addr, vid, true); |
668 | rcu_read_unlock(); | 676 | rcu_read_unlock(); |
669 | } else { | 677 | } else { |
670 | spin_lock_bh(&p->br->hash_lock); | 678 | spin_lock_bh(&p->br->hash_lock); |
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index bf8dc7d308d6..28d544627422 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c | |||
@@ -77,7 +77,7 @@ int br_handle_frame_finish(struct sk_buff *skb) | |||
77 | /* insert into forwarding database after filtering to avoid spoofing */ | 77 | /* insert into forwarding database after filtering to avoid spoofing */ |
78 | br = p->br; | 78 | br = p->br; |
79 | if (p->flags & BR_LEARNING) | 79 | if (p->flags & BR_LEARNING) |
80 | br_fdb_update(br, p, eth_hdr(skb)->h_source, vid); | 80 | br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, false); |
81 | 81 | ||
82 | if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) && | 82 | if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) && |
83 | br_multicast_rcv(br, p, skb, vid)) | 83 | br_multicast_rcv(br, p, skb, vid)) |
@@ -148,7 +148,7 @@ static int br_handle_local_finish(struct sk_buff *skb) | |||
148 | 148 | ||
149 | br_vlan_get_tag(skb, &vid); | 149 | br_vlan_get_tag(skb, &vid); |
150 | if (p->flags & BR_LEARNING) | 150 | if (p->flags & BR_LEARNING) |
151 | br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid); | 151 | br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, false); |
152 | return 0; /* process further */ | 152 | return 0; /* process further */ |
153 | } | 153 | } |
154 | 154 | ||
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index fcd12333c59b..939a59e15036 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h | |||
@@ -104,6 +104,7 @@ struct net_bridge_fdb_entry | |||
104 | mac_addr addr; | 104 | mac_addr addr; |
105 | unsigned char is_local; | 105 | unsigned char is_local; |
106 | unsigned char is_static; | 106 | unsigned char is_static; |
107 | unsigned char added_by_user; | ||
107 | __u16 vlan_id; | 108 | __u16 vlan_id; |
108 | }; | 109 | }; |
109 | 110 | ||
@@ -383,7 +384,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf, unsigned long count, | |||
383 | int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source, | 384 | int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source, |
384 | const unsigned char *addr, u16 vid); | 385 | const unsigned char *addr, u16 vid); |
385 | void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, | 386 | void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, |
386 | const unsigned char *addr, u16 vid); | 387 | const unsigned char *addr, u16 vid, bool added_by_user); |
387 | int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr, u16 vid); | 388 | int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr, u16 vid); |
388 | 389 | ||
389 | int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], | 390 | int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], |