[PATCH net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[PATCH net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[Hangbin Liu]

The ip address autojoin is not working for IPv6 as ipv6_add_addr()
will return -EADDRNOTAVAIL when adding a multicast address.

Reported-by: Jianlin Shi <jishi@redhat.com>
Fixes: 93a714d6b53d ("multicast: Extend ip address command to enable multicast group join/leave on")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv6/addrconf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index dc73888c7859..ced995f3fec4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1045,7 +1045,8 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 	int err = 0;
 
 	if (addr_type == IPV6_ADDR_ANY ||
-	    addr_type & IPV6_ADDR_MULTICAST ||
+	    (addr_type & IPV6_ADDR_MULTICAST &&
+	     !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) ||
 	    (!(idev->dev->flags & IFF_LOOPBACK) &&
 	     !netif_is_l3_master(idev->dev) &&
 	     addr_type & IPV6_ADDR_LOOPBACK))
-- 
2.19.2

Re: [PATCH net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[David Miller]

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Tue, 13 Aug 2019 21:52:32 +0800

> The ip address autojoin is not working for IPv6 as ipv6_add_addr()
> will return -EADDRNOTAVAIL when adding a multicast address.
> 
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Fixes: 93a714d6b53d ("multicast: Extend ip address command to enable multicast group join/leave on")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

I don't understand how all of this works and why ipv6_add_addr(), which
seems designed explicitly to exclude multicast addresses, should accept
them and what all of the possible fallout might be from such a change.

Your commit message is way too terse and makes it impossible to evaluate
your change.  Really, a change of this nature should have a couple paragraphs
of text explaining the existing situation, what is wrong with it, how you
are fixing it, and why you are fixing it that way.

Thanks.

[PATCHv2 net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[Hangbin Liu]

In commit 93a714d6b53d ("multicast: Extend ip address command to enable
multicast group join/leave on") we added a new flag IFA_F_MCAUTOJOIN
to make user able to add multicast address on ethernet interface.

This works for IPv4, but not for IPv6. See the inet6_addr_add code.

static int inet6_addr_add()
{
	...
	if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
		ipv6_mc_config(net->ipv6.mc_autojoin_sk, true...)
	}

	ifp = ipv6_add_addr(idev, cfg, true, extack); <- always fail with maddr
	if (!IS_ERR(ifp)) {
		...
	} else if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
		ipv6_mc_config(net->ipv6.mc_autojoin_sk, false...)
	}
}

But in ipv6_add_addr() it will check the address type and reject multicast
address directly. So this feature is never worked for IPv6.

We should not remove the multicast address check totally in ipv6_add_addr(),
but could accept multicast address only when IFA_F_MCAUTOJOIN flag supplied.

v2: update commit description

Reported-by: Jianlin Shi <jishi@redhat.com>
Fixes: 93a714d6b53d ("multicast: Extend ip address command to enable multicast group join/leave on")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv6/addrconf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index dc73888c7859..ced995f3fec4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1045,7 +1045,8 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 	int err = 0;
 
 	if (addr_type == IPV6_ADDR_ANY ||
-	    addr_type & IPV6_ADDR_MULTICAST ||
+	    (addr_type & IPV6_ADDR_MULTICAST &&
+	     !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) ||
 	    (!(idev->dev->flags & IFF_LOOPBACK) &&
 	     !netif_is_l3_master(idev->dev) &&
 	     addr_type & IPV6_ADDR_LOOPBACK))
-- 
2.19.2

Re: [PATCHv2 net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[David Ahern]

On 8/19/19 10:19 PM, Hangbin Liu wrote:
> But in ipv6_add_addr() it will check the address type and reject multicast
> address directly. So this feature is never worked for IPv6.

If true, that is really disappointing.

We need to get a functional test script started for various address cases.

Re: [PATCHv2 net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[Hangbin Liu]

On Mon, Aug 19, 2019 at 10:33:58PM -0400, David Ahern wrote:
> On 8/19/19 10:19 PM, Hangbin Liu wrote:
> > But in ipv6_add_addr() it will check the address type and reject multicast
> > address directly. So this feature is never worked for IPv6.
> 
> If true, that is really disappointing.
> 
> We need to get a functional test script started for various address cases.

Do you mean an `ip addr add` testing for all kinds of address types?

Thanks
Hangbin

Re: [PATCHv2 net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[David Ahern]

On 8/22/19 4:20 AM, Hangbin Liu wrote:
> On Mon, Aug 19, 2019 at 10:33:58PM -0400, David Ahern wrote:
>> On 8/19/19 10:19 PM, Hangbin Liu wrote:
>>> But in ipv6_add_addr() it will check the address type and reject multicast
>>> address directly. So this feature is never worked for IPv6.
>>
>> If true, that is really disappointing.
>>
>> We need to get a functional test script started for various address cases.
> 
> Do you mean an `ip addr add` testing for all kinds of address types?
> 
yes.

Re: [PATCHv2 net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set

[David Miller]

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Tue, 20 Aug 2019 10:19:47 +0800

> In commit 93a714d6b53d ("multicast: Extend ip address command to enable
> multicast group join/leave on") we added a new flag IFA_F_MCAUTOJOIN
> to make user able to add multicast address on ethernet interface.
> 
> This works for IPv4, but not for IPv6. See the inet6_addr_add code.
> 
> static int inet6_addr_add()
> {
> 	...
> 	if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
> 		ipv6_mc_config(net->ipv6.mc_autojoin_sk, true...)
> 	}
> 
> 	ifp = ipv6_add_addr(idev, cfg, true, extack); <- always fail with maddr
> 	if (!IS_ERR(ifp)) {
> 		...
> 	} else if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
> 		ipv6_mc_config(net->ipv6.mc_autojoin_sk, false...)
> 	}
> }
> 
> But in ipv6_add_addr() it will check the address type and reject multicast
> address directly. So this feature is never worked for IPv6.
> 
> We should not remove the multicast address check totally in ipv6_add_addr(),
> but could accept multicast address only when IFA_F_MCAUTOJOIN flag supplied.
> 
> v2: update commit description
> 
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Fixes: 93a714d6b53d ("multicast: Extend ip address command to enable multicast group join/leave on")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Applied and queued up for -stable.