bridge: flush br's address entry in fdb when remove the bridge dev

PATCH RESEND Superseded Ding Tianhong 3 participants 5 messages Nov 19, 2013 – Nov 20, 2013

Subsystems (3)

Networking Network drivers Ethernet bridge

lore

Share & tools

lore

Ding Tianhong

author

2013-11-19 02:57 UTC #0 lore raw

PATCH RESEND 1/1 bridge: flush br's address entry in fdb when remove the bridge dev

When the following commands are executed:

brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge

The calltrace will occur:

[  563.312114] device eth1 left promiscuous mode
[  563.312188] br0: port 1(eth1) entered disabled state
[  563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[  563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G           O 3.12.0-0.7-default+ #9
[  563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  563.468200]  0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[  563.468204]  ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[  563.468206]  ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[  563.468209] Call Trace:
[  563.468218]  [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[  563.468234]  [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[  563.468242]  [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[  563.468247]  [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[  563.468254]  [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[  563.468259]  [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[  570.377958] Bridge firewalling registered

------------------------------------------------

The reason is that when the bridge dev's address is changed, the br_fdb_change_mac_address() will add new address in fdb, but when the bridge was removed, the address entry in the fdb did not free, the bridge_fdb_cache still has objects when destroy the cache, Fix this by flushing the bridge address entry when removing the bridge.

Signed-off-by Ding Tianhong <[email protected]>

---
 net/bridge/br_if.c | 4 ++++
 1 file changed, 4 insertions(+)

net/bridge/br_if.c +4 -0

index 6e6194f..98084d8 100644
@@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 		del_nbp(p);
 	}
 
+	spin_lock_bh(&br->hash_lock);
+	fdb_delete_by_addr(br, br->dev->dev_addr, 0);
+	spin_unlock_bh(&br->hash_lock);
+
 	br_vlan_flush(br);
 	del_timer_sync(&br->gc_timer);

Signature · 1 line

1.8.0

Toshiaki Makita

2013-11-19 15:46 UTC #1 lore raw

Quoted 43 lines · Ding Tianhong

When the following commands are executed:
brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge
The calltrace will occur:
[  563.312114] device eth1 left promiscuous mode
[  563.312188] br0: port 1(eth1) entered disabled state
[  563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[  563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G           O 3.12.0-0.7-default+ #9
[  563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  563.468200]  0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[  563.468204]  ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[  563.468206]  ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[  563.468209] Call Trace:
[  563.468218]  [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[  563.468234]  [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[  563.468242]  [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[  563.468247]  [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[  563.468254]  [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[  563.468259]  [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[  570.377958] Bridge firewalling registered
------------------------------------------------
The reason is that when the bridge dev's address is changed, the br_fdb_change_mac_address() will add new address in fdb, but when the bridge was removed, the address entry in the fdb did not free, the bridge_fdb_cache still has objects when destroy the cache, Fix this by flushing the bridge address entry when removing the bridge.
Signed-off-by Ding Tianhong <[email protected]>
---
 net/bridge/br_if.c | 4 ++++
 1 file changed, 4 insertions(+)
net/bridge/br_if.c +0 -0
index 6e6194f..98084d8 100644

net/bridge/br_if.c void br_dev_delete(struct net_device *dev, struct list_head *head) · line 177

Quoted 6 lines

net/bridge/br_if.c +3 -0

@@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 		del_nbp(p);
 	}
 
+	spin_lock_bh(&br->hash_lock);
+	fdb_delete_by_addr(br, br->dev->dev_addr, 0);
+	spin_unlock_bh(&br->hash_lock);

You are not deleting entries with vid other than 0.
If we have added some vid, there might be fdb entries with that vid whose dst is NULL, which also should be cleaned up.

How about deleting all fdb entries whose dst is NULL? br_fdb_delete_by_port() looks able to be called with p == NULL as well.

Thanks,
Toshiaki Makita

Quoted 4 lines

net/bridge/br_if.c +1 -0

+
 	br_vlan_flush(br);
 	del_timer_sync(&br->gc_timer);

Ding Tianhong

author

2013-11-20 01:03 UTC #2 lore raw

Quoted 55 lines · Toshiaki Makita

Quoted 43 lines · Ding Tianhong
When the following commands are executed:
brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge
The calltrace will occur:
[  563.312114] device eth1 left promiscuous mode
[  563.312188] br0: port 1(eth1) entered disabled state
[  563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[  563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G           O 3.12.0-0.7-default+ #9
[  563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  563.468200]  0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[  563.468204]  ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[  563.468206]  ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[  563.468209] Call Trace:
[  563.468218]  [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[  563.468234]  [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[  563.468242]  [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[  563.468247]  [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[  563.468254]  [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[  563.468259]  [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[  570.377958] Bridge firewalling registered
------------------------------------------------
The reason is that when the bridge dev's address is changed, the br_fdb_change_mac_address() will add new address in fdb, but when the bridge was removed, the address entry in the fdb did not free, the bridge_fdb_cache still has objects when destroy the cache, Fix this by flushing the bridge address entry when removing the bridge.
Signed-off-by Ding Tianhong <[email protected]>
---
 net/bridge/br_if.c | 4 ++++
 1 file changed, 4 insertions(+)
net/bridge/br_if.c +0 -0
index 6e6194f..98084d8 100644
net/bridge/br_if.c void br_dev_delete(struct net_device *dev, struct list_head *head) · line 177
Quoted 6 lines
net/bridge/br_if.c +3 -0
@@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 		del_nbp(p);
 	}
 
+	spin_lock_bh(&br->hash_lock);
+	fdb_delete_by_addr(br, br->dev->dev_addr, 0);
+	spin_unlock_bh(&br->hash_lock);
You are not deleting entries with vid other than 0.
If we have added some vid, there might be fdb entries with that vid whose dst is NULL, which also should be cleaned up.

Quoted 6 lines

How about deleting all fdb entries whose dst is NULL? br_fdb_delete_by_port() looks able to be called with p == NULL as well.
Thanks,
Toshiaki Makita

agree, I miss the vid problem, I should deleteing all entries about this br.

Regards
Ding

Quoted 13 lines

net/bridge/br_if.c void br_dev_delete(struct net_device *dev, struct list_head *head) · line 181
Quoted 4 lines
net/bridge/br_if.c +1 -0
+
 	br_vlan_flush(br);
 	del_timer_sync(&br->gc_timer);
 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
.

Vlad Yasevich

2013-11-19 15:52 UTC #3 lore raw

In reply to Ding Tianhong

Quoted 44 lines · Ding Tianhong

When the following commands are executed:
brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge
The calltrace will occur:
[  563.312114] device eth1 left promiscuous mode
[  563.312188] br0: port 1(eth1) entered disabled state
[  563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[  563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G           O 3.12.0-0.7-default+ #9
[  563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  563.468200]  0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[  563.468204]  ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[  563.468206]  ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[  563.468209] Call Trace:
[  563.468218]  [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[  563.468234]  [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[  563.468242]  [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[  563.468247]  [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[  563.468254]  [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[  563.468259]  [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[  570.377958] Bridge firewalling registered
------------------------------------------------
The reason is that when the bridge dev's address is changed, the br_fdb_change_mac_address() will add new address in fdb, but when the bridge was removed, the address entry in the fdb did not free, the bridge_fdb_cache still has objects when destroy the cache, Fix this by flushing the bridge address entry when removing the bridge.
Signed-off-by Ding Tianhong <[email protected]>
---
 net/bridge/br_if.c | 4 ++++
 1 file changed, 4 insertions(+)
net/bridge/br_if.c +0 -0
index 6e6194f..98084d8 100644

net/bridge/br_if.c void br_dev_delete(struct net_device *dev, struct list_head *head) · line 181

Quoted 10 lines

net/bridge/br_if.c +4 -0

@@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 		del_nbp(p);
 	}
 
+	spin_lock_bh(&br->hash_lock);
+	fdb_delete_by_addr(br, br->dev->dev_addr, 0);
+	spin_unlock_bh(&br->hash_lock);
+
 	br_vlan_flush(br);
 	del_timer_sync(&br->gc_timer);

I think you need to use fdb_delete_by_port(br, NULL, 1); here.

The reason is that if the bridge had vlan filtering configured, when the bridge mac address was changed, an fdb would have been created for for ever vlan configured on the bridge. You delete only vlan0, so you would still have a leak.

-vlad

Ding Tianhong

author

2013-11-20 01:05 UTC #4 lore raw

Quoted 58 lines · Vlad Yasevich

Quoted 44 lines · Ding Tianhong
When the following commands are executed:
brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge
The calltrace will occur:
[  563.312114] device eth1 left promiscuous mode
[  563.312188] br0: port 1(eth1) entered disabled state
[  563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[  563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G           O 3.12.0-0.7-default+ #9
[  563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  563.468200]  0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[  563.468204]  ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[  563.468206]  ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[  563.468209] Call Trace:
[  563.468218]  [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[  563.468234]  [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[  563.468242]  [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[  563.468247]  [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[  563.468254]  [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[  563.468259]  [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[  570.377958] Bridge firewalling registered
------------------------------------------------
The reason is that when the bridge dev's address is changed, the br_fdb_change_mac_address() will add new address in fdb, but when the bridge was removed, the address entry in the fdb did not free, the bridge_fdb_cache still has objects when destroy the cache, Fix this by flushing the bridge address entry when removing the bridge.
Signed-off-by Ding Tianhong <[email protected]>
---
 net/bridge/br_if.c | 4 ++++
 1 file changed, 4 insertions(+)
net/bridge/br_if.c +0 -0
index 6e6194f..98084d8 100644
net/bridge/br_if.c void br_dev_delete(struct net_device *dev, struct list_head *head) · line 181
Quoted 10 lines
net/bridge/br_if.c +4 -0
@@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 		del_nbp(p);
 	}
 
+	spin_lock_bh(&br->hash_lock);
+	fdb_delete_by_addr(br, br->dev->dev_addr, 0);
+	spin_unlock_bh(&br->hash_lock);
+
 	br_vlan_flush(br);
 	del_timer_sync(&br->gc_timer);
 
I think you need to use fdb_delete_by_port(br, NULL, 1); here.

Quoted 6 lines

The reason is that if the bridge had vlan filtering configured, when the bridge mac address was changed, an fdb would have been created for for ever vlan configured on the bridge. You delete only vlan0, so you would still have a leak.
-vlad

agree, I miss the vid problem, I will fix it.

Regards
Ding

Quoted 7 lines

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
.