Re: [IPoIB] Identify Multicast packets and fix IGMP breakage

[Roland Dreier <rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>]

 > +	/* According to Jason Gunthorpe byte 24 in the GRH has the MGID */

I think we could put a better explanation here than "Jason said so."  In
fact since the GRH format matches the IPv6 header format, maybe we do
something like "((struct ipv6hdr *) skb->data)->daddr" to get at the
DGID being used.

 > +	if (IN6_IS_ADDR_MULTICAST(skb->data + 24))
 > +

trivial but this blank line isn't needed here.

 > +		skb->pkt_type = PACKET_MULTICAST;
 > +

or here

 > +	else
 > +		skb->pkt_type = PACKET_HOST;

also it's not clear to me why it's OK to do this test of the DGID if the
packet didn't have a GRH -- presumably we are just looking at random
uninitialized memory so we might incorrectly say some packets are
multicast if that byte happens to be 0xff.  (or does that not matter?
if so why can't we just always make everything PACKET_MULTICAST?)

It seems the check should be something like

	if ((wc->wc_flags & IB_WC_GRH) &&
	    IN6_IS_ADDR_MULTICAST(((struct ipv6hdr *) skb->data)->daddr))

 > --- linux-2.6.orig/include/linux/in6.h	2010-08-26 14:11:39.000000000 -0500
 > +++ linux-2.6/include/linux/in6.h	2010-08-26 14:11:52.000000000 -0500
 > @@ -53,6 +53,9 @@
 >  extern const struct in6_addr in6addr_linklocal_allrouters;
 >  #define IN6ADDR_LINKLOCAL_ALLROUTERS_INIT \
 >  		{ { { 0xff,2,0,0,0,0,0,0,0,0,0,0,0,0,0,2 } } }
 > +
 > +#define IN6_IS_ADDR_MULTICAST(a) (((const __u8 *) (a))[0] == 0xff)

We probably want to run this addition past netdev.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html