* [PATCH net] ipv6/addrconf: call ipv6_mc_up() for non-Ethernet interface
@ 2020-03-10 7:20 Hangbin Liu
2020-03-10 7:27 ` [PATCHv2 " Hangbin Liu
0 siblings, 1 reply; 4+ messages in thread
From: Hangbin Liu @ 2020-03-10 7:20 UTC (permalink / raw)
To: netdev
Cc: Rafał Miłecki, Hideaki YOSHIFUJI, David S . Miller,
Eric Dumazet, Alexey Kuznetsov, John Crispin, Herbert Xu,
Hangbin Liu
Rafał found an issue that for non-Ethernet interface, if we down and up
frequently, the memory will be consumed slowly.
The reason is we add allnodes/allrouters addressed in multicast list in
ipv6_add_dev(). When link down, we call ipv6_mc_down(), store all multicast
addresses via mld_add_delrec(). But when link up, we don't call ipv6_mc_up()
for non-Ethernet interface to remove the addresses. This makes idev->mc_tomb
getting bigger and bigger. The call stack looks like:
addrconf_notify(NETDEV_REGISTER)
ipv6_add_dev
ipv6_dev_mc_inc(ff01::1)
ipv6_dev_mc_inc(ff02::1)
ipv6_dev_mc_inc(ff02::2)
addrconf_notify(NETDEV_UP)
addrconf_dev_config
/* Alas, we support only Ethernet autoconfiguration. */
return;
addrconf_notify(NETDEV_DOWN)
addrconf_ifdown
ipv6_mc_down
igmp6_group_dropped(ff02::2)
mld_add_delrec(ff02::2)
igmp6_group_dropped(ff02::1)
igmp6_group_dropped(ff01::1)
After investigating, I can't found a rule to disable multicast on
non-Ethernet interface. In RFC2460, the link could be Ethernet, PPP, ATM,
tunnels, etc. In IPv4, it doesn't check the dev type when calls ip_mc_up()
in inetdev_event(). Even for IPv6, we don't check the dev type and call
ipv6_add_dev(), ipv6_dev_mc_inc() after register device.
So I think it's OK to fix this memory consumer by calling ipv6_mc_up() for
non-Ethernet interface.
Reported-by: Rafał Miłecki <zajec5@gmail•com>
Fixes: 74235a25c673 ("[IPV6] addrconf: Fix IPv6 on tuntap tunnels")
Fixes: 1666d49e1d41 ("mld: do not remove mld souce list info when set link down")
Signed-off-by: Hangbin Liu <liuhangbin@gmail•com>
---
net/ipv6/addrconf.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index e6e1290ea06f..56cdd3fda366 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3347,6 +3347,9 @@ static void addrconf_dev_config(struct net_device *dev)
(dev->type != ARPHRD_NONE) &&
(dev->type != ARPHRD_RAWIP)) {
/* Alas, we support only Ethernet autoconfiguration. */
+ idev = __in6_dev_get(dev);
+ if (!IS_ERR_OR_NULL(idev) && dev->flags & IFF_UP)
+ ipv6_mc_up(idev);
return;
}
--
2.19.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCHv2 net] ipv6/addrconf: call ipv6_mc_up() for non-Ethernet interface
2020-03-10 7:20 [PATCH net] ipv6/addrconf: call ipv6_mc_up() for non-Ethernet interface Hangbin Liu
@ 2020-03-10 7:27 ` Hangbin Liu
2020-03-10 11:17 ` Rafał Miłecki
2020-03-10 22:38 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Hangbin Liu @ 2020-03-10 7:27 UTC (permalink / raw)
To: netdev
Cc: Rafał Miłecki, Hideaki YOSHIFUJI, David S . Miller,
Eric Dumazet, Alexey Kuznetsov, John Crispin, Herbert Xu,
Hangbin Liu
Rafał found an issue that for non-Ethernet interface, if we down and up
frequently, the memory will be consumed slowly.
The reason is we add allnodes/allrouters addressed in multicast list in
ipv6_add_dev(). When link down, we call ipv6_mc_down(), store all multicast
addresses via mld_add_delrec(). But when link up, we don't call ipv6_mc_up()
for non-Ethernet interface to remove the addresses. This makes idev->mc_tomb
getting bigger and bigger. The call stack looks like:
addrconf_notify(NETDEV_REGISTER)
ipv6_add_dev
ipv6_dev_mc_inc(ff01::1)
ipv6_dev_mc_inc(ff02::1)
ipv6_dev_mc_inc(ff02::2)
addrconf_notify(NETDEV_UP)
addrconf_dev_config
/* Alas, we support only Ethernet autoconfiguration. */
return;
addrconf_notify(NETDEV_DOWN)
addrconf_ifdown
ipv6_mc_down
igmp6_group_dropped(ff02::2)
mld_add_delrec(ff02::2)
igmp6_group_dropped(ff02::1)
igmp6_group_dropped(ff01::1)
After investigating, I can't found a rule to disable multicast on
non-Ethernet interface. In RFC2460, the link could be Ethernet, PPP, ATM,
tunnels, etc. In IPv4, it doesn't check the dev type when calls ip_mc_up()
in inetdev_event(). Even for IPv6, we don't check the dev type and call
ipv6_add_dev(), ipv6_dev_mc_inc() after register device.
So I think it's OK to fix this memory consumer by calling ipv6_mc_up() for
non-Ethernet interface.
v2: Also check IFF_MULTICAST flag to make sure the interface supports
multicast
Reported-by: Rafał Miłecki <zajec5@gmail•com>
Fixes: 74235a25c673 ("[IPV6] addrconf: Fix IPv6 on tuntap tunnels")
Fixes: 1666d49e1d41 ("mld: do not remove mld souce list info when set link down")
Signed-off-by: Hangbin Liu <liuhangbin@gmail•com>
---
net/ipv6/addrconf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index e6e1290ea06f..46d614b611db 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3347,6 +3347,10 @@ static void addrconf_dev_config(struct net_device *dev)
(dev->type != ARPHRD_NONE) &&
(dev->type != ARPHRD_RAWIP)) {
/* Alas, we support only Ethernet autoconfiguration. */
+ idev = __in6_dev_get(dev);
+ if (!IS_ERR_OR_NULL(idev) && dev->flags & IFF_UP &&
+ dev->flags & IFF_MULTICAST)
+ ipv6_mc_up(idev);
return;
}
--
2.19.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCHv2 net] ipv6/addrconf: call ipv6_mc_up() for non-Ethernet interface
2020-03-10 7:27 ` [PATCHv2 " Hangbin Liu
@ 2020-03-10 11:17 ` Rafał Miłecki
2020-03-10 22:38 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: Rafał Miłecki @ 2020-03-10 11:17 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Hideaki YOSHIFUJI, David S . Miller, Eric Dumazet,
Alexey Kuznetsov, John Crispin, Herbert Xu
On 10.03.2020 08:27, Hangbin Liu wrote:
> Rafał found an issue that for non-Ethernet interface, if we down and up
> frequently, the memory will be consumed slowly.
>
> The reason is we add allnodes/allrouters addressed in multicast list in
> ipv6_add_dev(). When link down, we call ipv6_mc_down(), store all multicast
> addresses via mld_add_delrec(). But when link up, we don't call ipv6_mc_up()
> for non-Ethernet interface to remove the addresses. This makes idev->mc_tomb
> getting bigger and bigger. The call stack looks like:
>
> addrconf_notify(NETDEV_REGISTER)
> ipv6_add_dev
> ipv6_dev_mc_inc(ff01::1)
> ipv6_dev_mc_inc(ff02::1)
> ipv6_dev_mc_inc(ff02::2)
>
> addrconf_notify(NETDEV_UP)
> addrconf_dev_config
> /* Alas, we support only Ethernet autoconfiguration. */
> return;
>
> addrconf_notify(NETDEV_DOWN)
> addrconf_ifdown
> ipv6_mc_down
> igmp6_group_dropped(ff02::2)
> mld_add_delrec(ff02::2)
> igmp6_group_dropped(ff02::1)
> igmp6_group_dropped(ff01::1)
>
> After investigating, I can't found a rule to disable multicast on
> non-Ethernet interface. In RFC2460, the link could be Ethernet, PPP, ATM,
> tunnels, etc. In IPv4, it doesn't check the dev type when calls ip_mc_up()
> in inetdev_event(). Even for IPv6, we don't check the dev type and call
> ipv6_add_dev(), ipv6_dev_mc_inc() after register device.
>
> So I think it's OK to fix this memory consumer by calling ipv6_mc_up() for
> non-Ethernet interface.
>
> v2: Also check IFF_MULTICAST flag to make sure the interface supports
> multicast
>
> Reported-by: Rafał Miłecki <zajec5@gmail•com>
> Fixes: 74235a25c673 ("[IPV6] addrconf: Fix IPv6 on tuntap tunnels")
> Fixes: 1666d49e1d41 ("mld: do not remove mld souce list info when set link down")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail•com>
I'm really grateful for all your help. I verified this patch to fix
reported issue. Thank you!
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCHv2 net] ipv6/addrconf: call ipv6_mc_up() for non-Ethernet interface
2020-03-10 7:27 ` [PATCHv2 " Hangbin Liu
2020-03-10 11:17 ` Rafał Miłecki
@ 2020-03-10 22:38 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2020-03-10 22:38 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, zajec5, yoshfuji, eric.dumazet, kuznet, john, herbert
From: Hangbin Liu <liuhangbin@gmail•com>
Date: Tue, 10 Mar 2020 15:27:37 +0800
> Rafał found an issue that for non-Ethernet interface, if we down and up
> frequently, the memory will be consumed slowly.
>
> The reason is we add allnodes/allrouters addressed in multicast list in
> ipv6_add_dev(). When link down, we call ipv6_mc_down(), store all multicast
> addresses via mld_add_delrec(). But when link up, we don't call ipv6_mc_up()
> for non-Ethernet interface to remove the addresses. This makes idev->mc_tomb
> getting bigger and bigger. The call stack looks like:
>
> addrconf_notify(NETDEV_REGISTER)
> ipv6_add_dev
> ipv6_dev_mc_inc(ff01::1)
> ipv6_dev_mc_inc(ff02::1)
> ipv6_dev_mc_inc(ff02::2)
>
> addrconf_notify(NETDEV_UP)
> addrconf_dev_config
> /* Alas, we support only Ethernet autoconfiguration. */
> return;
>
> addrconf_notify(NETDEV_DOWN)
> addrconf_ifdown
> ipv6_mc_down
> igmp6_group_dropped(ff02::2)
> mld_add_delrec(ff02::2)
> igmp6_group_dropped(ff02::1)
> igmp6_group_dropped(ff01::1)
>
> After investigating, I can't found a rule to disable multicast on
> non-Ethernet interface. In RFC2460, the link could be Ethernet, PPP, ATM,
> tunnels, etc. In IPv4, it doesn't check the dev type when calls ip_mc_up()
> in inetdev_event(). Even for IPv6, we don't check the dev type and call
> ipv6_add_dev(), ipv6_dev_mc_inc() after register device.
>
> So I think it's OK to fix this memory consumer by calling ipv6_mc_up() for
> non-Ethernet interface.
>
> v2: Also check IFF_MULTICAST flag to make sure the interface supports
> multicast
>
> Reported-by: Rafał Miłecki <zajec5@gmail•com>
> Fixes: 74235a25c673 ("[IPV6] addrconf: Fix IPv6 on tuntap tunnels")
> Fixes: 1666d49e1d41 ("mld: do not remove mld souce list info when set link down")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail•com>
Applied and queued up for -stable, thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-10 22:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-10 7:20 [PATCH net] ipv6/addrconf: call ipv6_mc_up() for non-Ethernet interface Hangbin Liu
2020-03-10 7:27 ` [PATCHv2 " Hangbin Liu
2020-03-10 11:17 ` Rafał Miłecki
2020-03-10 22:38 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox