ipv4/igmp: init group mode as INCLUDE when join source group

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>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index f8231854b5d60316310fc5d8e57eea8625fe3078..119f53941c124c22452bf615f9ccca5a9130bb87 100644 (file)
--- 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 85b617b655bc2d602563b1bd174f436554c9d046..b3c899a630a0fe696207f8749f5ceb510daa445e 100644 (file)
--- 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 fc32fdbeefa61c18da5b9330d4da73ca6db992bd..64c76dcf73863a81addc1c7effcd3a687a97483d 100644 (file)
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -984,7 +984,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;
@@ -1061,7 +1061,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;