[PATCH net] net/multicast: clean change record if add new INCLUDE group

[PATCH net] net/multicast: clean change record if add new INCLUDE group

[Hangbin Liu]

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@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.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

Re: [PATCH net] net/multicast: clean change record if add new INCLUDE group

[Hangbin Liu]

Self NAK this patch, there is some issue I need to fix.

On Wed, Jun 13, 2018 at 02:32:31PM +0800, Hangbin Liu wrote:
> 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@redhat.com>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.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
> 

[PATCHv2 net 0/2] multicast: init as INCLUDE when join SSM INCLUDE group

[Hangbin Liu]

Based on RFC3376 5.1 and RFC3810 6.1, we should init as INCLUDE when join SSM
INCLUDE group. In my first version I only clear the group change record. But
this is not enough as when a new group join, it will init as EXCLUDE and
trigger an filter mode change in ip/ip6_mc_add_src(), which will clear all
source addresses' sf_crcount. This will prevent early joined address sending
state change records if multi source addresses joined at the same time.

In this v2 patchset, I fixed it by directly initializing the mode to INCLUDE
for SSM JOIN_SOURCE_GROUP. I also split the original patch into two separated
patches for IPv4 and IPv6.

Test: test by myself and customer.

Hangbin Liu (2):
  ipv4/igmp: init group mode as INCLUDE when join source group
  ipv6/mcast: init as INCLUDE when join SSM INCLUDE group

 include/linux/igmp.h     |  2 ++
 include/net/ipv6.h       |  2 ++
 net/ipv4/igmp.c          | 58 +++++++++++++++++++++++++++++++------------
 net/ipv4/ip_sockglue.c   |  4 +--
 net/ipv6/ipv6_sockglue.c |  5 ++--
 net/ipv6/mcast.c         | 64 ++++++++++++++++++++++++++++++++++--------------
 6 files changed, 96 insertions(+), 39 deletions(-)

-- 
2.5.5

[PATCHv2 net 1/2] ipv4/igmp: init group mode as INCLUDE when join source group

[Hangbin Liu]

Based on RFC3376 5.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().

Function ip_mc_join_group() works correctly for IGMP ASM(Any-Source Multicast)
mode. It adds a group with state EX() and inits crcount to mc_qrv,
so the kernel will send a TO_EX() report message after adding group.

But for IGMPv3 SSM(Source-specific multicast) JOIN_SOURCE_GROUP mode, we
split the group joining into two steps. First we join the group like ASM,
i.e. via ip_mc_join_group(). 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 change,
we used to send both ALLOW(A) and TO_IN(A). After this change we only send
TO_IN(A).

Fix it by adding a new parameter to init group mode. Also add new wrapper
functions so we don't need to change too much code.

v1 -> v2:
In my first version I only cleared the group change record. But this is not
enough. Because when a new group join, it will init as EXCLUDE and trigger
an filter mode change in ip/ip6_mc_add_src(), which will clear all source
addresses' sf_crcount. This will prevent early joined address sending state
change records if multi source addressed joined at the same time.

In v2 patch, I fixed it by directly initializing the mode to INCLUDE for SSM
JOIN_SOURCE_GROUP. I also split the original patch into two separated patches
for IPv4 and IPv6.

Fixes: a052517a8ff65 ("net/multicast: should not send source list records when have filter mode change")
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/linux/igmp.h   |  2 ++
 net/ipv4/igmp.c        | 58 ++++++++++++++++++++++++++++++++++++--------------
 net/ipv4/ip_sockglue.c |  4 ++--
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index f823185..119f539 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -109,6 +109,8 @@ struct ip_mc_list {
 extern int ip_check_mc_rcu(struct in_device *dev, __be32 mc_addr, __be32 src_addr, u8 proto);
 extern int igmp_rcv(struct sk_buff *);
 extern int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr);
+extern int ip_mc_join_group_ssm(struct sock *sk, struct ip_mreqn *imr,
+				unsigned int mode);
 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,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index b26a81a..ef72499 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1200,13 +1200,14 @@ static void igmpv3_del_delrec(struct in_device *in_dev, struct ip_mc_list *im)
 	spin_lock_bh(&im->lock);
 	if (pmc) {
 		im->interface = pmc->interface;
-		im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
 		im->sfmode = pmc->sfmode;
 		if (pmc->sfmode == MCAST_INCLUDE) {
 			im->tomb = pmc->tomb;
 			im->sources = pmc->sources;
 			for (psf = im->sources; psf; psf = psf->sf_next)
-				psf->sf_crcount = im->crcount;
+				psf->sf_crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+		} else {
+			im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
 		}
 		in_dev_put(pmc->interface);
 		kfree(pmc);
@@ -1288,7 +1289,7 @@ static void igmp_group_dropped(struct ip_mc_list *im)
 #endif
 }
 
-static void igmp_group_added(struct ip_mc_list *im)
+static void igmp_group_added(struct ip_mc_list *im, unsigned int mode)
 {
 	struct in_device *in_dev = im->interface;
 #ifdef CONFIG_IP_MULTICAST
@@ -1316,7 +1317,13 @@ static void igmp_group_added(struct ip_mc_list *im)
 	}
 	/* else, v3 */
 
-	im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+	/* 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 (mode == MCAST_EXCLUDE)
+		im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+
 	igmp_ifc_event(in_dev);
 #endif
 }
@@ -1381,8 +1388,7 @@ static void ip_mc_hash_remove(struct in_device *in_dev,
 /*
  *	A socket has joined a multicast group on device dev.
  */
-
-void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
+void __ip_mc_inc_group(struct in_device *in_dev, __be32 addr, unsigned int mode)
 {
 	struct ip_mc_list *im;
 #ifdef CONFIG_IP_MULTICAST
@@ -1394,7 +1400,7 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
 	for_each_pmc_rtnl(in_dev, im) {
 		if (im->multiaddr == addr) {
 			im->users++;
-			ip_mc_add_src(in_dev, &addr, MCAST_EXCLUDE, 0, NULL, 0);
+			ip_mc_add_src(in_dev, &addr, mode, 0, NULL, 0);
 			goto out;
 		}
 	}
@@ -1408,8 +1414,8 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
 	in_dev_hold(in_dev);
 	im->multiaddr = addr;
 	/* initial mode is (EX, empty) */
-	im->sfmode = MCAST_EXCLUDE;
-	im->sfcount[MCAST_EXCLUDE] = 1;
+	im->sfmode = mode;
+	im->sfcount[mode] = 1;
 	refcount_set(&im->refcnt, 1);
 	spin_lock_init(&im->lock);
 #ifdef CONFIG_IP_MULTICAST
@@ -1426,12 +1432,17 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
 #ifdef CONFIG_IP_MULTICAST
 	igmpv3_del_delrec(in_dev, im);
 #endif
-	igmp_group_added(im);
+	igmp_group_added(im, mode);
 	if (!in_dev->dead)
 		ip_rt_multicast_event(in_dev);
 out:
 	return;
 }
+
+void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
+{
+	__ip_mc_inc_group(in_dev, addr, MCAST_EXCLUDE);
+}
 EXPORT_SYMBOL(ip_mc_inc_group);
 
 static int ip_mc_check_iphdr(struct sk_buff *skb)
@@ -1688,7 +1699,7 @@ void ip_mc_remap(struct in_device *in_dev)
 #ifdef CONFIG_IP_MULTICAST
 		igmpv3_del_delrec(in_dev, pmc);
 #endif
-		igmp_group_added(pmc);
+		igmp_group_added(pmc, pmc->sfmode);
 	}
 }
 
@@ -1751,7 +1762,7 @@ void ip_mc_up(struct in_device *in_dev)
 #ifdef CONFIG_IP_MULTICAST
 		igmpv3_del_delrec(in_dev, pmc);
 #endif
-		igmp_group_added(pmc);
+		igmp_group_added(pmc, pmc->sfmode);
 	}
 }
 
@@ -2130,8 +2141,8 @@ static void ip_mc_clear_src(struct ip_mc_list *pmc)
 
 /* Join a multicast group
  */
-
-int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
+static int __ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr,
+			      unsigned int mode)
 {
 	__be32 addr = imr->imr_multiaddr.s_addr;
 	struct ip_mc_socklist *iml, *i;
@@ -2172,15 +2183,30 @@ int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
 	memcpy(&iml->multi, imr, sizeof(*imr));
 	iml->next_rcu = inet->mc_list;
 	iml->sflist = NULL;
-	iml->sfmode = MCAST_EXCLUDE;
+	iml->sfmode = mode;
 	rcu_assign_pointer(inet->mc_list, iml);
-	ip_mc_inc_group(in_dev, addr);
+	__ip_mc_inc_group(in_dev, addr, mode);
 	err = 0;
 done:
 	return err;
 }
+
+/* Join ASM (Any-Source Multicast) group
+ */
+int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
+{
+	return __ip_mc_join_group(sk, imr, MCAST_EXCLUDE);
+}
 EXPORT_SYMBOL(ip_mc_join_group);
 
+/* Join SSM (Source-Specific Multicast) group
+ */
+int ip_mc_join_group_ssm(struct sock *sk, struct ip_mreqn *imr,
+			 unsigned int mode)
+{
+	return __ip_mc_join_group(sk, imr, mode);
+}
+
 static int ip_mc_leave_src(struct sock *sk, struct ip_mc_socklist *iml,
 			   struct in_device *in_dev)
 {
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 57bbb06..5f28607 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -982,7 +982,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 			mreq.imr_multiaddr.s_addr = mreqs.imr_multiaddr;
 			mreq.imr_address.s_addr = mreqs.imr_interface;
 			mreq.imr_ifindex = 0;
-			err = ip_mc_join_group(sk, &mreq);
+			err = ip_mc_join_group_ssm(sk, &mreq, MCAST_INCLUDE);
 			if (err && err != -EADDRINUSE)
 				break;
 			omode = MCAST_INCLUDE;
@@ -1059,7 +1059,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 			mreq.imr_multiaddr = psin->sin_addr;
 			mreq.imr_address.s_addr = 0;
 			mreq.imr_ifindex = greqs.gsr_interface;
-			err = ip_mc_join_group(sk, &mreq);
+			err = ip_mc_join_group_ssm(sk, &mreq, MCAST_INCLUDE);
 			if (err && err != -EADDRINUSE)
 				break;
 			greqs.gsr_interface = mreq.imr_ifindex;
-- 
2.5.5

[PATCHv2 net 2/2] ipv6/mcast: init as INCLUDE when join SSM INCLUDE group

[Hangbin Liu]

This an IPv6 version patch of "ipv4/igmp: init group mode as INCLUDE when
join source group". From RFC3810, part 6.1:

   If no per-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 an
   INCLUDE filter mode and an empty source list.

Which means a new multicast group should start with state IN(). Currently,
for MLDv2 SSM JOIN_SOURCE_GROUP mode, we first call ipv6_sock_mc_join(),
then ip6_mc_source(), which will trigger a TO_IN() message instead of
ALLOW().

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

Fix it by adding a new parameter to init group mode. Also add some wrapper
functions to avoid changing too much code.

v1 -> v2:
In the first version I only cleared the group change record. But this is not
enough. Because when a new group join, it will init as EXCLUDE and trigger
a filter mode change in ip/ip6_mc_add_src(), which will clear all source
addresses sf_crcount. This will prevent early joined address sending state
change records if multi source addressed joined at the same time.

In v2 patch, I fixed it by directly initializing the mode to INCLUDE for SSM
JOIN_SOURCE_GROUP. I also split the original patch into two separated patches
for IPv4 and IPv6.

There is also a difference between v4 and v6 version. For IPv6, when the
interface goes down and up, we will send correct state change record with
unspecified IPv6 address (::) with function ipv6_mc_up(). But after DAD is
completed, we resend the change record TO_IN() in mld_send_initial_cr().
Fix it by sending ALLOW() for INCLUDE mode in mld_send_initial_cr().

Fixes: a052517a8ff65 ("net/multicast: should not send source list records when have filter mode change")
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/net/ipv6.h       |  2 ++
 net/ipv6/ipv6_sockglue.c |  5 ++--
 net/ipv6/mcast.c         | 64 ++++++++++++++++++++++++++++++++++--------------
 3 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 836f31a..c2283ca 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1104,6 +1104,8 @@ void ipv6_sysctl_unregister(void);
 
 int ipv6_sock_mc_join(struct sock *sk, int ifindex,
 		      const struct in6_addr *addr);
+int ipv6_sock_mc_join_ssm(struct sock *sk, int ifindex,
+			  const struct in6_addr *addr, unsigned int mode);
 int ipv6_sock_mc_drop(struct sock *sk, int ifindex,
 		      const struct in6_addr *addr);
 #endif /* _NET_IPV6_H */
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 4d780c7..343c561 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -718,8 +718,9 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 			struct sockaddr_in6 *psin6;
 
 			psin6 = (struct sockaddr_in6 *)&greqs.gsr_group;
-			retv = ipv6_sock_mc_join(sk, greqs.gsr_interface,
-						 &psin6->sin6_addr);
+			retv = ipv6_sock_mc_join_ssm(sk, greqs.gsr_interface,
+						     &psin6->sin6_addr,
+						     MCAST_INCLUDE);
 			/* prior join w/ different source is ok */
 			if (retv && retv != -EADDRINUSE)
 				break;
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 793159d..cba3519 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -95,6 +95,8 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *pmca,
 			  int delta);
 static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 			    struct inet6_dev *idev);
+static int __ipv6_dev_mc_inc(struct net_device *dev,
+			     const struct in6_addr *addr, unsigned int mode);
 
 #define MLD_QRV_DEFAULT		2
 /* RFC3810, 9.2. Query Interval */
@@ -132,7 +134,8 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
 	return iv > 0 ? iv : 1;
 }
 
-int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
+static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
+			       const struct in6_addr *addr, unsigned int mode)
 {
 	struct net_device *dev = NULL;
 	struct ipv6_mc_socklist *mc_lst;
@@ -179,7 +182,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	}
 
 	mc_lst->ifindex = dev->ifindex;
-	mc_lst->sfmode = MCAST_EXCLUDE;
+	mc_lst->sfmode = mode;
 	rwlock_init(&mc_lst->sflock);
 	mc_lst->sflist = NULL;
 
@@ -187,7 +190,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	 *	now add/increase the group membership on the device
 	 */
 
-	err = ipv6_dev_mc_inc(dev, addr);
+	err = __ipv6_dev_mc_inc(dev, addr, mode);
 
 	if (err) {
 		sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
@@ -199,8 +202,19 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 
 	return 0;
 }
+
+int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
+{
+	return __ipv6_sock_mc_join(sk, ifindex, addr, MCAST_EXCLUDE);
+}
 EXPORT_SYMBOL(ipv6_sock_mc_join);
 
+int ipv6_sock_mc_join_ssm(struct sock *sk, int ifindex,
+			  const struct in6_addr *addr, unsigned int mode)
+{
+	return __ipv6_sock_mc_join(sk, ifindex, addr, mode);
+}
+
 /*
  *	socket leave on multicast group
  */
@@ -646,7 +660,7 @@ bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
 	return rv;
 }
 
-static void igmp6_group_added(struct ifmcaddr6 *mc)
+static void igmp6_group_added(struct ifmcaddr6 *mc, unsigned int mode)
 {
 	struct net_device *dev = mc->idev->dev;
 	char buf[MAX_ADDR_LEN];
@@ -672,7 +686,13 @@ static void igmp6_group_added(struct ifmcaddr6 *mc)
 	}
 	/* else v2 */
 
-	mc->mca_crcount = mc->idev->mc_qrv;
+	/* 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 (mode == MCAST_EXCLUDE)
+		mc->mca_crcount = mc->idev->mc_qrv;
+
 	mld_ifc_event(mc->idev);
 }
 
@@ -770,13 +790,14 @@ static void mld_del_delrec(struct inet6_dev *idev, struct ifmcaddr6 *im)
 	spin_lock_bh(&im->mca_lock);
 	if (pmc) {
 		im->idev = pmc->idev;
-		im->mca_crcount = idev->mc_qrv;
 		im->mca_sfmode = pmc->mca_sfmode;
 		if (pmc->mca_sfmode == MCAST_INCLUDE) {
 			im->mca_tomb = pmc->mca_tomb;
 			im->mca_sources = pmc->mca_sources;
 			for (psf = im->mca_sources; psf; psf = psf->sf_next)
-				psf->sf_crcount = im->mca_crcount;
+				psf->sf_crcount = idev->mc_qrv;
+		} else {
+			im->mca_crcount = idev->mc_qrv;
 		}
 		in6_dev_put(pmc->idev);
 		kfree(pmc);
@@ -831,7 +852,8 @@ static void ma_put(struct ifmcaddr6 *mc)
 }
 
 static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
-				   const struct in6_addr *addr)
+				   const struct in6_addr *addr,
+				   unsigned int mode)
 {
 	struct ifmcaddr6 *mc;
 
@@ -849,9 +871,8 @@ static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
 	refcount_set(&mc->mca_refcnt, 1);
 	spin_lock_init(&mc->mca_lock);
 
-	/* initial mode is (EX, empty) */
-	mc->mca_sfmode = MCAST_EXCLUDE;
-	mc->mca_sfcount[MCAST_EXCLUDE] = 1;
+	mc->mca_sfmode = mode;
+	mc->mca_sfcount[mode] = 1;
 
 	if (ipv6_addr_is_ll_all_nodes(&mc->mca_addr) ||
 	    IPV6_ADDR_MC_SCOPE(&mc->mca_addr) < IPV6_ADDR_SCOPE_LINKLOCAL)
@@ -863,7 +884,8 @@ static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
 /*
  *	device multicast group inc (add if not found)
  */
-int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
+static int __ipv6_dev_mc_inc(struct net_device *dev,
+			     const struct in6_addr *addr, unsigned int mode)
 {
 	struct ifmcaddr6 *mc;
 	struct inet6_dev *idev;
@@ -887,14 +909,13 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
 		if (ipv6_addr_equal(&mc->mca_addr, addr)) {
 			mc->mca_users++;
 			write_unlock_bh(&idev->lock);
-			ip6_mc_add_src(idev, &mc->mca_addr, MCAST_EXCLUDE, 0,
-				NULL, 0);
+			ip6_mc_add_src(idev, &mc->mca_addr, mode, 0, NULL, 0);
 			in6_dev_put(idev);
 			return 0;
 		}
 	}
 
-	mc = mca_alloc(idev, addr);
+	mc = mca_alloc(idev, addr, mode);
 	if (!mc) {
 		write_unlock_bh(&idev->lock);
 		in6_dev_put(idev);
@@ -911,11 +932,16 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
 	write_unlock_bh(&idev->lock);
 
 	mld_del_delrec(idev, mc);
-	igmp6_group_added(mc);
+	igmp6_group_added(mc, mode);
 	ma_put(mc);
 	return 0;
 }
 
+int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
+{
+	return __ipv6_dev_mc_inc(dev, addr, MCAST_EXCLUDE);
+}
+
 /*
  *	device multicast group del
  */
@@ -1751,7 +1777,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
 
 		psf_next = psf->sf_next;
 
-		if (!is_in(pmc, psf, type, gdeleted, sdeleted)) {
+		if (!is_in(pmc, psf, type, gdeleted, sdeleted) && !crsend) {
 			psf_prev = psf;
 			continue;
 		}
@@ -2066,7 +2092,7 @@ static void mld_send_initial_cr(struct inet6_dev *idev)
 		if (pmc->mca_sfcount[MCAST_EXCLUDE])
 			type = MLD2_CHANGE_TO_EXCLUDE;
 		else
-			type = MLD2_CHANGE_TO_INCLUDE;
+			type = MLD2_ALLOW_NEW_SOURCES;
 		skb = add_grec(skb, pmc, type, 0, 0, 1);
 		spin_unlock_bh(&pmc->mca_lock);
 	}
@@ -2543,7 +2569,7 @@ void ipv6_mc_up(struct inet6_dev *idev)
 	ipv6_mc_reset(idev);
 	for (i = idev->mc_list; i; i = i->next) {
 		mld_del_delrec(idev, i);
-		igmp6_group_added(i);
+		igmp6_group_added(i, i->mca_sfmode);
 	}
 	read_unlock_bh(&idev->lock);
 }
-- 
2.5.5

Re: [PATCHv2 net 0/2] multicast: init as INCLUDE when join SSM INCLUDE group

[David Miller]

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Tue, 10 Jul 2018 22:41:25 +0800

> Based on RFC3376 5.1 and RFC3810 6.1, we should init as INCLUDE when join SSM
> INCLUDE group. In my first version I only clear the group change record. But
> this is not enough as when a new group join, it will init as EXCLUDE and
> trigger an filter mode change in ip/ip6_mc_add_src(), which will clear all
> source addresses' sf_crcount. This will prevent early joined address sending
> state change records if multi source addresses joined at the same time.
> 
> In this v2 patchset, I fixed it by directly initializing the mode to INCLUDE
> for SSM JOIN_SOURCE_GROUP. I also split the original patch into two separated
> patches for IPv4 and IPv6.
> 
> Test: test by myself and customer.

Series applied, thanks!