Re: Regression: net/ipv6/mld running system out of memory (not a leak)
From: Hangbin Liu <hidden>
Date: 2020-03-03 09:00:45
Subsystem:
networking [general], networking [ipv4/ipv6], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Ido Schimmel, Linus Torvalds
On Tue, Mar 03, 2020 at 07:16:44AM +0100, Rafał Miłecki wrote:
It appears that every interface up & down sequence results in adding a new ff02::2 entry to the idev->mc_tomb. Doing that over and over will obviously result in running out of memory at some point. That list isn't cleared until removing an interface.
Thanks Rafał, this info is very useful. When we set interface up, we will call ipv6_add_dev() and add in6addr_linklocal_allrouters to the mcast list. But we only remove it in ipv6_mc_destroy_dev(). This make the link down save the list and link up add a new one. Maybe we should remove the list in ipv6_mc_down(). like:
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index eaa4c2cc2fbb..786352ff7704 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c@@ -2533,6 +2533,18 @@ void ipv6_mc_down(struct inet6_dev *idev) { struct ifmcaddr6 *i; + /* Delete all-nodes address. */ + /* We cannot call ipv6_dev_mc_dec() directly, our caller in + * addrconf.c has NULL'd out dev->ip6_ptr so in6_dev_get() will + * fail. + */ + __ipv6_dev_mc_dec(idev, &in6addr_interfacelocal_allnodes); + __ipv6_dev_mc_dec(idev, &in6addr_linklocal_allnodes); + + if (idev->cnf.forwarding) + __ipv6_dev_mc_dec(idev, &in6addr_linklocal_allrouters); + + /* Withdraw multicast list */ read_lock_bh(&idev->lock);
@@ -2603,16 +2615,6 @@ void ipv6_mc_destroy_dev(struct inet6_dev *idev) ipv6_mc_down(idev); mld_clear_delrec(idev); - /* Delete all-nodes address. */ - /* We cannot call ipv6_dev_mc_dec() directly, our caller in - * addrconf.c has NULL'd out dev->ip6_ptr so in6_dev_get() will - * fail. - */ - __ipv6_dev_mc_dec(idev, &in6addr_linklocal_allnodes); - - if (idev->cnf.forwarding) - __ipv6_dev_mc_dec(idev, &in6addr_linklocal_allrouters); - write_lock_bh(&idev->lock); while ((i = idev->mc_list) != NULL) { idev->mc_list = i->next;
Thanks Hangbin