veth: Free queues on link delete
Subsystems (2)
PATCH v2 1/1 veth: Free queues on link delete
From: David Ahern <[email protected]>
kmemleak reported new suspected memory leaks.
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff8800354d5c00 (size 1024):
comm "ip", pid 836, jiffies 4294722952 (age 25.904s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<(____ptrval____)>] kmemleak_alloc+0x70/0x94
[<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
[<(____ptrval____)>] __kmalloc+0x101/0x142
[<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
[<(____ptrval____)>] veth_newlink+0x147/0x3ac [veth]
...
unreferenced object 0xffff88002e009c00 (size 1024):
comm "ip", pid 836, jiffies 4294722958 (age 25.898s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<(____ptrval____)>] kmemleak_alloc+0x70/0x94
[<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
[<(____ptrval____)>] __kmalloc+0x101/0x142
[<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
[<(____ptrval____)>] veth_newlink+0x219/0x3ac [veth]The allocations in question are veth_alloc_queues for the dev and its peer.
Free the queues on a delete.
---
v2
- free peer dev queues as well
drivers/net/veth.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/net/veth.c +2 -0
@@ -1205,6 +1205,7 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)struct veth_priv *priv;struct net_device *peer; +veth_free_queues (dev); priv =netdev_priv (dev); peer =rtnl_dereference (priv->peer);
@@ -1216,6 +1217,7 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)unregister_netdevice_queue (dev, head);if (peer) { +veth_free_queues (peer); priv =netdev_priv (peer);RCU_INIT_POINTER (priv->peer,NULL );unregister_netdevice_queue (peer, head);
Signature · 1 line
2.11.0
Quoted 54 lines · On 2018/08/15 10:04, [email protected] wrote:
From: David Ahern <[email protected]>
kmemleak reported new suspected memory leaks.
$ cat /sys/kernel/debug/kmemleak unreferenced object 0xffff8800354d5c00 (size 1024): comm "ip", pid 836, jiffies 4294722952 (age 25.904s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<(____ptrval____)>] kmemleak_alloc+0x70/0x94 [<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52 [<(____ptrval____)>] __kmalloc+0x101/0x142 [<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth] [<(____ptrval____)>] veth_newlink+0x147/0x3ac [veth] ... unreferenced object 0xffff88002e009c00 (size 1024): comm "ip", pid 836, jiffies 4294722958 (age 25.898s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<(____ptrval____)>] kmemleak_alloc+0x70/0x94 [<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52 [<(____ptrval____)>] __kmalloc+0x101/0x142 [<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth] [<(____ptrval____)>] veth_newlink+0x219/0x3ac [veth]The allocations in question are
veth_alloc_queuesfor the dev and its peer.Free the queues on a delete.
Fixes 638264dc90227 ("veth: Support per queue XDP ring")Signed-off-by David Ahern <[email protected]>---
v2
- free peer dev queues as welldrivers/net/veth.c | 2 ++ 1 file changed, 2 insertions(+)drivers/net/veth.c +1
-0@@ -1205,6 +1205,7 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)struct veth_priv *priv;struct net_device *peer; +veth_free_queues (dev); priv =netdev_priv (dev); peer =rtnl_dereference (priv->peer);
drivers/net/veth.c static void veth_dellink(struct net_device *dev, struct list_head *head) · line 1223
Quoted 7 lines
drivers/net/veth.c +1
-0@@ -1216,6 +1217,7 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)unregister_netdevice_queue (dev, head);if (peer) { +veth_free_queues (peer); priv =netdev_priv (peer);RCU_INIT_POINTER (priv->peer,NULL );unregister_netdevice_queue (peer, head);
Hmm, on second thought this queues need to be freed after veth_close() to make sure no packet will reference them. That means we need to free them in .ndo_uninit() or destructor.
(rtnl_delete_link() calls dellink() before unregister_netdevice_many() which calls dev_close_many() through rollback_registered_many())
Currently veth has destructor veth_dev_free() for vstats, so we can free queues in the function.
To be in line with vstats, allocation also should be moved to veth_dev_init().
Signature · 1 line
Toshiaki Makita
Quoted 10 lines · Toshiaki Makita
Hmm, on second thought this queues need to be freed after
veth_close()to make sure no packet will reference them. That means we need to free them in .ndo_uninit() or destructor.
(rtnl_delete_link()callsdellink()beforeunregister_netdevice_many()which callsdev_close_many()throughrollback_registered_many())Currently veth has destructor
veth_dev_free()for vstats, so we can free queues in the function.
To be in line with vstats, allocation also should be moved toveth_dev_init().
given that, can you take care of the free in the proper location?
Quoted 7 lines · David Ahern
Quoted 6 lines · Toshiaki Makita
Hmm, on second thought this queues need to be freed after
veth_close()to make sure no packet will reference them. That means we need to free them in .ndo_uninit() or destructor.
(rtnl_delete_link()callsdellink()beforeunregister_netdevice_many()which callsdev_close_many()throughrollback_registered_many())
Quoted 6 lines
Quoted 4 lines
Currently veth has destructor
veth_dev_free()for vstats, so we can free queues in the function.
To be in line with vstats, allocation also should be moved toveth_dev_init().given that, can you take care of the free in the proper location?
Sure, will cook a patch.
Thanks!
Signature · 1 line
Toshiaki Makita