Two fix for implementation of MLDv2 .

Hi.
I find two bug yesterday when I was thinking whether ip6_mc_del_src(...) should be called in __ipv6_dev_mc_dec(...).

The first change in function is_in(...) is to make process Multicast Address and Source Specific Query same as Multicast Address Specific Query. Although it still doesn't satisfy the requirement in rfc3810, but not at risk of losing multicast traffic.
For example:
A Node's filter for Multicast address M is EXCLUDE (X,Y) and receice a Multicast Address and Source Specific Query Q(M,Z). Without the change No report will send to router, so the node may get multicast traffic from address Z lost.

The second change in function add_grec(...) is to make sure Mode Change Report send properly. When A filter's source address list is not empty, but none of them can be included in report.

Regards

Index:net/ipv6/mcast.c
============================================================

@@ -1256,10 +1256,13 @@ static int is_in(struct ifmcaddr6 *pmc, {

	switch (type) {
	case MLD2_MODE_IS_INCLUDE:
-	case MLD2_MODE_IS_EXCLUDE:
		if (gdeleted || sdeleted)
			return 0;
		return !((pmc->mca_flags & MAF_GSQUERY) && !psf->sf_gsresp);
+	case MLD2_MODE_IS_EXCLUDE:
+		if (gdeleted || sdeleted)
+			return 0;
+		return 1;
	case MLD2_CHANGE_TO_INCLUDE:
		if (gdeleted || sdeleted)
			return 0;
@@ -1428,13 +1431,15 @@ static struct sk_buff *add_grec(struct s
	struct mld2_report *pmr;
	struct mld2_grec *pgr = NULL;
	struct ip6_sf_list *psf, *psf_next, *psf_prev, **psf_list;

- int scount, first, isquery, truncate;
+ int scount, first, isquery, ischange, truncate;

	if (pmc->mca_flags & MAF_NOREPORT)
		return skb;

	isquery = type == MLD2_MODE_IS_INCLUDE ||
		  type == MLD2_MODE_IS_EXCLUDE;
+	ischange = type == MLD2_CHANGE_TO_INCLUDE ||
+		   type == MLD2_CHANGE_TO_EXCLUDE; 
	truncate = type == MLD2_MODE_IS_EXCLUDE ||
		    type == MLD2_CHANGE_TO_EXCLUDE;

@@ -1444,7 +1449,7 @@ static struct sk_buff *add_grec(struct s

		if (type == MLD2_ALLOW_NEW_SOURCES ||
		    type == MLD2_BLOCK_OLD_SOURCES)
			return skb;
-		if (pmc->mca_crcount || isquery) {
+		if (ischange || isquery) {
			/* make sure we have room for group header and at
			 * least one source.
			 */
@@ -1460,9 +1465,12 @@ static struct sk_buff *add_grec(struct s
	pmr = skb ? (struct mld2_report *)skb->h.raw : NULL;

	/* EX and TO_EX get a fresh packet, if needed */

- if (truncate) {
- if (pmr && pmr->ngrec &&
- AVAILABLE(skb) < grec_size(pmc, type, gdeleted, sdeleted)) {
+ if (truncate || ischange) {
+ int min_len;
+ min_len = truncate ? grec_size(pmc, type, gdeleted, sdeleted) : + (sizeof(struct mld2_grec) + sizeof(struct in6_addr));
+ if (((pmr && pmr->ngrec) || ischange) &&

+		    AVAILABLE(skb) < min_len) {
			if (skb)
				mld_sendpack(skb);
			skb = mld_newpack(dev, dev->mtu);
@@ -1471,6 +1479,10 @@ static struct sk_buff *add_grec(struct s
	first = 1;
	scount = 0;
	psf_prev = NULL;

+ if (ischange) {
+ skb = add_grhead(skb, pmc, type, &pgr);
+ first = 0;
+ }

	for (psf=*psf_list; psf; psf=psf_next) {
		struct in6_addr *psrc;

Hi.

Here is my another patch. It makes a node send state change report properly when a multicast address's soure filter mode change is cause by forwarding state change and etc.

Regards.

Index:net/ipv6/mcast.c
===========================================================

@@ -128,7 +128,8 @@ static DEFINE_RWLOCK(ipv6_sk_mc_lock);
 

static struct socket *igmp6_socket;

-int __ipv6_dev_mc_dec(struct inet6_dev *idev, struct in6_addr *addr);
+static int ipv6_dev_mc_dec_intern(struct inet6_dev *idev, + struct in6_addr *addr, int delta);

static void igmp6_join_group(struct ifmcaddr6 *ma); static void igmp6_leave_group(struct ifmcaddr6 *ma); @@ -270,7 +271,7 @@ int ipv6_sock_mc_drop(struct sock *sk, i

				if (idev) {
					(void) ip6_mc_leave_src(sk,mc_lst,idev);
-					__ipv6_dev_mc_dec(idev, &mc_lst->addr);
+					ipv6_dev_mc_dec_intern(idev,&mc_lst->addr,1);
					in6_dev_put(idev);
				}
				dev_put(dev);

@@ -336,7 +337,7 @@ void ipv6_sock_mc_close(struct sock *sk)

			if (idev) {
				(void) ip6_mc_leave_src(sk, mc_lst, idev);
-				__ipv6_dev_mc_dec(idev, &mc_lst->addr);
+				ipv6_dev_mc_dec_intern(idev, &mc_lst->addr, 1);
				in6_dev_put(idev);
			}
			dev_put(dev);
@@ -907,10 +908,9 @@ int ipv6_dev_mc_inc(struct net_device *d
	return 0;
}

-/*
- * device multicast group del
- */
-int __ipv6_dev_mc_dec(struct inet6_dev *idev, struct in6_addr *addr) +
+static int ipv6_dev_mc_dec_intern(struct inet6_dev *idev, + struct in6_addr *addr, int delta)

{
	struct ifmcaddr6 *ma, **map;

@@ -920,20 +920,27 @@ int __ipv6_dev_mc_dec(struct inet6_dev *

			if (--ma->mca_users == 0) {
				*map = ma->next;
				write_unlock_bh(&idev->lock);
-
				igmp6_group_dropped(ma);
-
				ma_put(ma);
				return 0;
			}
			write_unlock_bh(&idev->lock);
+			if (!delta && ip6_mc_del_src(idev, addr, +					MCAST_EXCLUDE, 0, NULL, 0))
+				return -EINVAL; // bug 	
			return 0;
		}
	}
	write_unlock_bh(&idev->lock);
-
	return -ENOENT;
}

+/*
+ * device multicast group del
+ */
+int __ipv6_dev_mc_dec(struct inet6_dev *idev, struct in6_addr *addr) +{
+ return ipv6_dev_mc_dec_intern(idev, addr, 0);
+}

int ipv6_dev_mc_dec(struct net_device *dev, struct in6_addr *addr) {

I find something interesting . Maybe more changes for is_in(...) are needed.

static int is_in(struct ifmcaddr6 *pmc, struct ip6_sf_list *psf, int type,
int gdeleted, int sdeleted)

{
	switch (type) {
	case MLD2_MODE_IS_INCLUDE:
	case MLD2_CHANGE_TO_INCLUDE:
		if (gdeleted || sdeleted)
			return 0;
		if (psf->sf_count[MCAST_INCLUDE] == 0)
			return 0;    // maybe never happen
		if (type == MLD2_CHANGE_TO_INCLUDE)
			return 1;
		return !((pmc->mca_flags & MAF_GSQUERY) && !psf->sf_gsresp);
	case MLD2_MODE_IS_EXCLUDE:
	case MLD2_CHANGE_TO_EXCLUDE:
		if (gdeleted || sdeleted)
			return 0;
		if (pmc->mca_sfcount[MCAST_EXCLUDE] == 0 ||
		    psf->sf_count[MCAST_INCLUDE])
			return 0;
		if (type == MLD2_MODE_IS_EXCLUDE)
			return 1;
		return pmc->mca_sfcount[MCAST_EXCLUDE] ==
			psf->sf_count[MCAST_EXCLUDE];
	case MLD2_ALLOW_NEW_SOURCES:
		if (gdeleted || !psf->sf_crcount)
			return 0;
		return (pmc->mca_sfmode == MCAST_INCLUDE) ^ sdeleted;
	case MLD2_BLOCK_OLD_SOURCES:
		if (pmc->mca_sfmode == MCAST_INCLUDE)
			return gdeleted || (psf->sf_crcount && sdeleted);
		return psf->sf_crcount && !gdeleted && !sdeleted;
	}
	return 0;
}

You can test change in add_grec(...) by follow codes. without the change no change to exclude report will be sent.

please adjust IFINDEX and make sure initial state for FF02::2 is idle
===================================================
#include <stdio.h>
#include <unistd.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <sys/types.h>
#define IFINDEX 6

int main(int argc, char argv[])

{
	int sockfds[2];
	struct ipv6_mreq req;
	struct group_filter filter;
	struct sockaddr_in6 *psin6;
	req.ipv6mr_interface = IFINDEX;
	inet_pton(PF_INET6, "FF02::2", &req.ipv6mr_multiaddr);

	sockfds[0] = socket(PF_INET6, SOCK_DGRAM, 0);
	sockfds[1] = socket(PF_INET6, SOCK_DGRAM, 0);

	filter.gf_interface = IFINDEX;
	filter.gf_fmode = MCAST_INCLUDE;
	filter.gf_numsrc = 1;
	psin6 = (struct sockaddr_in6 *)&filter.gf_group;
	psin6->sin6_family = AF_INET6;
	inet_pton(PF_INET6, "FF02::2", &psin6->sin6_addr);
	psin6 = (struct sockaddr_in6 *)&filter.gf_slist[0];
	psin6->sin6_family = AF_INET6;
	inet_pton(PF_INET6, "2002:de12:1780::1", &psin6->sin6_addr);

	setsockopt(sockfds[0], SOL_IPV6, IPV6_ADD_MEMBERSHIP, &req, sizeof(req));
	setsockopt(sockfds[0], SOL_IPV6, MCAST_MSFILTER, &filter, sizeof(filter));

	sleep(5);
	setsockopt(sockfds[1], SOL_IPV6, IPV6_ADD_MEMBERSHIP, &req, sizeof(req));

	pause();
	return 0;
}

Yan,
I'm looking at this one. My original interpretation was that a
group and source specific query should only be answered if the source were explicitly listed (which is what the code does), but I see your point.
A general or group-specific query should still be answered with the existing code, and the unsolicited reports will also include the EXCLUDE records for that group, so I'm not aware of any actual circumstances where a multicast router wouldn't forward. But maybe you have such a case. An administrator-initiated query, however, would certainly be misleading to the administrator in the example you provided, and so far I tend to agree that there should be a record in the report.
However, I'm not sure your solution is appropriate since it
appears to include EXCLUDE records in cases where they aren't needed.
So, I'll look at this more carefully and see if I still agree
it needs a fix and whether or not your patch, or some alternative method might be more appropriate. But it'll probably be sometime next week before I'll be done reviewing/considering alternatives on this one.

                                        +-DLS

I am sorry, I can't understand your opinion completely. Please forgive my poor english :-)

Here is my opinion:

I think Multicast Address and Source Specific Query is sent only when router want to block traffic from some source. So when the filter mode is exclude, node should send a report which includes a MODE_IS_INCLUDE Record with sources in the query but NOT in the filter's source list. This is the required behaviour in rfc3810, but it need addiction item in struct ifmcaddr6 to record sources in the query. So I think make process Multicast Address Specific Query and Multicast Address and Source Specific Query no difference is a temporary fix.

the secoend change in is_in(...) is because I think include/exclude counts also should be checked when type is MLD2_MODE_IS_INCLUDE or MLD2_MODE_IS_EXCLUDE.

Regards

Here is my modify version is_in(...)
-----------------------------------------------------------------------------------------------
static int is_in(struct ifmcaddr6 *pmc, struct ip6_sf_list *psf, int type,
int gdeleted, int sdeleted)

{
	switch (type) {
	case MLD2_MODE_IS_INCLUDE:
	case MLD2_CHANGE_TO_INCLUDE:
		if (gdeleted || sdeleted)
			return 0;
		if (psf->sf_count[MCAST_INCLUDE] == 0)
			return 0;    // maybe never happen
		if (type == MLD2_CHANGE_TO_INCLUDE)
			return 1;
		return !((pmc->mca_flags & MAF_GSQUERY) && !psf->sf_gsresp);
	case MLD2_MODE_IS_EXCLUDE:
	case MLD2_CHANGE_TO_EXCLUDE:
		if (gdeleted || sdeleted)
			return 0;
		if (pmc->mca_sfcount[MCAST_EXCLUDE] == 0 ||
		    psf->sf_count[MCAST_INCLUDE])
			return 0;
		return pmc->mca_sfcount[MCAST_EXCLUDE] ==
			psf->sf_count[MCAST_EXCLUDE];
	case MLD2_ALLOW_NEW_SOURCES:
		if (gdeleted || !psf->sf_crcount)
			return 0;
		return (pmc->mca_sfmode == MCAST_INCLUDE) ^ sdeleted;
	case MLD2_BLOCK_OLD_SOURCES:
		if (pmc->mca_sfmode == MCAST_INCLUDE)
			return gdeleted || (psf->sf_crcount && sdeleted);
		return psf->sf_crcount && !gdeleted && !sdeleted;
	}
	return 0;
}