Fix for add_grec(...)

Hi

When ifmcaddr6's mca_sources is not NULL, but none of the sources in the list can be included in report, (For example: when filter mode is exclude and the only source in the list has include count greater than zero.) add_grec(...) may returns without add any data to the sk_buff. So MLD2_CHANGE_TO_EXCLUDE or MLD2_MODE_IS_EXCLUDE report may be eliminated.

You can check this bug by test1.c in attachments. You will notice that there is no MLD2_CHANGE_TO_EXCLUDE report.

Regards

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

@@ -1445,18 +1445,21 @@ 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;
+ type == MLD2_CHANGE_TO_EXCLUDE;

	psf_list = sdeleted ? &pmc->mca_tomb : &pmc->mca_sources;

+#if 0

	if (!*psf_list) {
		if (type == MLD2_ALLOW_NEW_SOURCES ||
		    type == MLD2_BLOCK_OLD_SOURCES)
@@ -1474,12 +1477,15 @@ static struct sk_buff *add_grec(struct s
		}
		return skb;
	}
+#endif
	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 && AVAILABLE(skb) < min_len) {
			if (skb)
				mld_sendpack(skb);
			skb = mld_newpack(dev, dev->mtu);
@@ -1488,6 +1494,10 @@ static struct sk_buff *add_grec(struct s
	first = 1;
	scount = 0;
	psf_prev = NULL;

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

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

#include <stdio.h>
#include <unistd.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <sys/types.h>
#define IFINDEX 5 //Please adjust me first.

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::2000", &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::2000", &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(10); //wait state change reports
	printf("change mode to exclude\n");
	setsockopt(sockfds[1], SOL_IPV6, IPV6_ADD_MEMBERSHIP, &req, sizeof(req));

	pause();
	return 0;
}

Yan,
I think your patch has some problems.

Yan Zheng <[email protected]> wrote on 11/09/2005 03:58:20 AM:

This "truncate" code is to handle exclude records that may be
truncated. It gets a new packet when adding this record and the whole thing won't fit in a single packet. This is not appropriate for anything but IS_EX and TO_EX, but "ischange" in your patch will be true for TO_IN. So, I think this will waste space in a report that could hold some of these TO_IN sources.

I haven't run your test code, or tested with your patch yet,
just observing the differences from the original code path and your patch (and they appear to be more than you intended).

                                                        +-DLS

ischange implicits mca_crcount has already been ckecked in mld_send_cr(...)
Actually, check mca_crcount directly will get mode change report sent one times less than you intend, because mca_crcount has decreased by one before call add_grec(...).

Now We only have MLD2_MODE_IS_INCLUDE left. but "mode is include and source list is empty" is an impossible event.

When type is MLD2_MODE_IS_INCLUDE, min_len is equal to "sizeof(struct mld2_grec) + sizeof(struct in6_addr)". it satisfies the comment above. (make sure we have room for group header and at least one source.)