Date:   Wed, 13 Jun 2018 14:32:31 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        Stefano Brivio <sbrivio@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        WANG Cong <xiyou.wangcong@...il.com>,
        <hideaki.yoshifuji@...aclelinux.com>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: [PATCH net] net/multicast: clean change record if add new INCLUDE group

Based on RFC3376 5.1 and RFC3810 6.1:
   If no interface
   state existed for that multicast address before the change (i.e., the
   change consisted of creating a new per-interface record), or if no
   state exists after the change (i.e., the change consisted of deleting
   a per-interface record), then the "non-existent" state is considered
   to have a filter mode of INCLUDE and an empty source list.

Which means a new multicast group should start with state IN(). That is
exactly what we did with ip_mc_join_group()/ipv6_sock_mc_join(), which
adds a group with state EX() and init crcount to mc_qrv. The kernel will
send a TO_EX() report message after adding group. This is what IGMPv3/MLDv2
ASM(Any-Source Multicast) mode should look like.

But for IGMPv3/MLDv2 SSM JOIN_SOURCE_GROUP mode, we split the group
joining into two steps. First step we join the group like ASM, i.e. via
ip_mc_join_group()/ipv6_sock_mc_join(). So the state changes from IN() to EX().

Then we add the Source-specific address with INCLUDE mode. So the state
changes from EX() to IN(A).

Before the first step sends a group change record, we finished the second step.
So we will only send the second change record. i.e. TO_IN(A)

Regarding the RFC stands, we should actually send an ALLOW(A) message for
SSM JOIN_SOURCE_GROUP as the state should mimic the 'IN() to IN(A)' transition.

The issue was exposed by commit a052517a8ff65 ("net/multicast: should not send
source list records when have filter mode change"). Before this commit we will
send both ALLOW(A) and TO_IN(A). After this commit we only send TO_IN(A).

Fix it by adding a is_new key to clean the crcount when we add a new
INCLUDE SSM group.

Fixes: a052517a8ff65 ("net/multicast: should not send source list records when have filter mode change")
Reviewed-by: Paolo Abeni <pabeni@...hat.com>
Reviewed-by: Stefano Brivio <sbrivio@...hat.com>
Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
---
 include/linux/igmp.h     |  2 +-
 include/net/ipv6.h       |  2 +-
 net/ipv4/igmp.c          | 27 ++++++++++++++++++++++++++-
 net/ipv4/ip_sockglue.c   |  8 ++++++--
 net/ipv6/ipv6_sockglue.c |  4 +++-
 net/ipv6/mcast.c         | 25 ++++++++++++++++++++++++-
 6 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index f823185..32cb02b 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -112,7 +112,7 @@ extern int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr);
 extern int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr);
 extern void ip_mc_drop_socket(struct sock *sk);
 extern int ip_mc_source(int add, int omode, struct sock *sk,
-		struct ip_mreq_source *mreqs, int ifindex);
+		struct ip_mreq_source *mreqs, int ifindex, bool is_new);
 extern int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf,int ifindex);
 extern int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 		struct ip_msfilter __user *optval, int __user *optlen);
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 836f31a..754c5cb 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1065,7 +1065,7 @@ struct group_source_req;
 struct group_filter;
 
 int ip6_mc_source(int add, int omode, struct sock *sk,
-		  struct group_source_req *pgsr);
+		  struct group_source_req *pgsr, bool is_new);
 int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf);
 int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
 		  struct group_filter __user *optval, int __user *optlen);
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index b26a81a..8d6ecc3 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2249,8 +2249,27 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
 }
 EXPORT_SYMBOL(ip_mc_leave_group);
 
+static void ip_mc_clear_cr(struct in_device *in_dev, __be32 pmca)
+{
+#ifdef CONFIG_IP_MULTICAST
+	struct ip_mc_list *pmc;
+
+	rcu_read_lock();
+	for_each_pmc_rcu(in_dev, pmc) {
+		if (pmca == pmc->multiaddr)
+			break;
+	}
+	if (pmc) {
+		spin_lock_bh(&pmc->lock);
+		pmc->crcount = 0;
+		spin_unlock_bh(&pmc->lock);
+	}
+	rcu_read_unlock();
+#endif
+}
+
 int ip_mc_source(int add, int omode, struct sock *sk, struct
-	ip_mreq_source *mreqs, int ifindex)
+	ip_mreq_source *mreqs, int ifindex, bool is_new)
 {
 	int err;
 	struct ip_mreqn imr;
@@ -2301,6 +2320,12 @@ int ip_mc_source(int add, int omode, struct sock *sk, struct
 		ip_mc_del_src(in_dev, &mreqs->imr_multiaddr, pmc->sfmode, 0,
 			NULL, 0);
 		pmc->sfmode = omode;
+		/* Based on RFC3376 5.1, for newly added INCLUDE SSM, we should
+		 * not send filter-mode change record as the mode should be
+		 * from IN() to IN(A).
+		 */
+		if (is_new)
+			ip_mc_clear_cr(in_dev, mreqs->imr_multiaddr);
 	}
 
 	psl = rtnl_dereference(pmc->sflist);
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 57bbb06..8d8c0cd 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -962,6 +962,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 	case IP_DROP_SOURCE_MEMBERSHIP:
 	{
 		struct ip_mreq_source mreqs;
+		bool is_new = false;
 		int omode, add;
 
 		if (optlen != sizeof(struct ip_mreq_source))
@@ -987,11 +988,12 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 				break;
 			omode = MCAST_INCLUDE;
 			add = 1;
+			is_new = true;
 		} else /* IP_DROP_SOURCE_MEMBERSHIP */ {
 			omode = MCAST_INCLUDE;
 			add = 0;
 		}
-		err = ip_mc_source(add, omode, sk, &mreqs, 0);
+		err = ip_mc_source(add, omode, sk, &mreqs, 0, is_new);
 		break;
 	}
 	case MCAST_JOIN_GROUP:
@@ -1027,6 +1029,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 		struct group_source_req greqs;
 		struct ip_mreq_source mreqs;
 		struct sockaddr_in *psin;
+		bool is_new = false;
 		int omode, add;
 
 		if (optlen != sizeof(struct group_source_req))
@@ -1065,12 +1068,13 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 			greqs.gsr_interface = mreq.imr_ifindex;
 			omode = MCAST_INCLUDE;
 			add = 1;
+			is_new = true;
 		} else /* MCAST_LEAVE_SOURCE_GROUP */ {
 			omode = MCAST_INCLUDE;
 			add = 0;
 		}
 		err = ip_mc_source(add, omode, sk, &mreqs,
-				   greqs.gsr_interface);
+				   greqs.gsr_interface, is_new);
 		break;
 	}
 	case MCAST_MSFILTER:
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 4d780c7..36e7c40 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -695,6 +695,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 	case MCAST_UNBLOCK_SOURCE:
 	{
 		struct group_source_req greqs;
+		bool is_new = false;
 		int omode, add;
 
 		if (optlen < sizeof(struct group_source_req))
@@ -725,11 +726,12 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 				break;
 			omode = MCAST_INCLUDE;
 			add = 1;
+			is_new = true;
 		} else /* MCAST_LEAVE_SOURCE_GROUP */ {
 			omode = MCAST_INCLUDE;
 			add = 0;
 		}
-		retv = ip6_mc_source(add, omode, sk, &greqs);
+		retv = ip6_mc_source(add, omode, sk, &greqs, is_new);
 		break;
 	}
 	case MCAST_MSFILTER:
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 793159d..f508a1c 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -315,8 +315,25 @@ void ipv6_sock_mc_close(struct sock *sk)
 	rtnl_unlock();
 }
 
+static void ip6_mc_clear_cr(struct inet6_dev *idev, const struct in6_addr *pmca)
+{
+	struct ifmcaddr6 *pmc;
+
+	read_lock_bh(&idev->lock);
+	for (pmc = idev->mc_list; pmc; pmc = pmc->next) {
+		if (ipv6_addr_equal(pmca, &pmc->mca_addr))
+			break;
+	}
+	if (pmc) {
+		spin_lock_bh(&pmc->mca_lock);
+		pmc->mca_crcount = 0;
+		spin_unlock_bh(&pmc->mca_lock);
+	}
+	read_unlock_bh(&idev->lock);
+}
+
 int ip6_mc_source(int add, int omode, struct sock *sk,
-	struct group_source_req *pgsr)
+	struct group_source_req *pgsr, bool is_new)
 {
 	struct in6_addr *source, *group;
 	struct ipv6_mc_socklist *pmc;
@@ -365,6 +382,12 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
 		ip6_mc_add_src(idev, group, omode, 0, NULL, 0);
 		ip6_mc_del_src(idev, group, pmc->sfmode, 0, NULL, 0);
 		pmc->sfmode = omode;
+		/* Based on RFC3810 6.1, for newly added INCLUDE SSM, we
+		 * should not send filter-mode change record as the mode
+		 * should be from IN() to IN(A).
+		 */
+		if (is_new)
+			ip6_mc_clear_cr(idev, group);
 	}
 
 	write_lock(&pmc->sflock);
-- 
2.5.5

Powered by blists - more mailing lists