Source Specific Query of MLDv2

Source Specific Query of MLDv2

[Takashi Hibi]

Hi all,

I found another problem of MLDv2.
The host doesn't respond to Source Specific Query.
It happens in the following scenario.

At first, host join multicast address with source address.
Then MLDv2 router issues Source Specific Query.
The source address of the query packet is link-local address of the 
router's interface, and the destination address is the multicast address 
to be queried. (See section 5.1.4 in draft-vida-mld-v2-08.txt) 

But it seems that the query packet is filtered by ipv6_chk_mcast_addr(), 
since the source address is not included in the source list.
As a result, no response to the query is produced .

Regards,
Takashi Hibi

 

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1.1: Type: text/plain, Size: 2961 bytes --]





Takashi,
      Below is a patch that allows MLD ICMP types without
applying the interface filters to them.

                              +-DLS
[in-line & attached]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F1/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h    2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F1/include/net/addrconf.h  2004-02-06 16:03:34.000000000 -0800
@@ -98,6 +98,7 @@

 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
            struct in6_addr *src_addr);
+extern int is_mld(struct sk_buff *skb);

 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);

diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F1/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c      2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/ip6_input.c    2004-02-06 16:04:00.000000000 -0800
@@ -168,11 +168,19 @@

            smp_read_barrier_depends();
            if (ipprot->flags & INET6_PROTO_FINAL) {
+                 struct ipv6hdr *hdr;
+
                  if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
                        skb->csum = csum_sub(skb->csum,
                                         csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
                        cksum_sub++;
                  }
+                 hdr = skb->nh.ipv6h;
+                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
+                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+                     &hdr->saddr) &&
+                     !is_mld(skb))
+                       goto discard;
            }
            if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
                !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
@@ -211,16 +219,11 @@

 int ip6_mc_input(struct sk_buff *skb)
 {
-     struct ipv6hdr *hdr;
-     int deliver = 0;
+     int deliver = 1;
      int discard = 1;

      IP6_INC_STATS_BH(Ip6InMcastPkts);

-     hdr = skb->nh.ipv6h;
-     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-           deliver = 1;
-
      /*
       *    IPv6 multicast router mode isnt currently supported.
       */
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F1/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c    2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/mcast.c  2004-02-06 16:04:25.000000000 -0800
@@ -901,6 +901,25 @@
 }

 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int is_mld(struct sk_buff *skb)
+{
+     struct icmp6hdr *pic = (struct icmp6hdr *)skb->h.raw;
+
+     switch (pic->icmp6_type) {
+     case ICMPV6_MGM_QUERY:
+     case ICMPV6_MGM_REPORT:
+     case ICMPV6_MGM_REDUCTION:
+     case ICMPV6_MLD2_REPORT:
+           return 1;
+     default:
+           break;
+     }
+     return 0;
+}
+
+/*
  *   check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,

(See attached file: mldx.patch)

[-- Attachment #1.2: Type: text/html, Size: 3085 bytes --]

[-- Attachment #2: mldx.patch --]
[-- Type: application/octet-stream, Size: 2396 bytes --]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F1/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h	2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F1/include/net/addrconf.h	2004-02-06 16:03:34.000000000 -0800
@@ -98,6 +98,7 @@
 
 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
 		struct in6_addr *src_addr);
+extern int is_mld(struct sk_buff *skb);
 
 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);
 
diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F1/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c	2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/ip6_input.c	2004-02-06 16:04:00.000000000 -0800
@@ -168,11 +168,19 @@
 		
 		smp_read_barrier_depends();
 		if (ipprot->flags & INET6_PROTO_FINAL) {
+			struct ipv6hdr *hdr;	
+
 			if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
 				skb->csum = csum_sub(skb->csum,
 						     csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
 				cksum_sub++;
 			}
+			hdr = skb->nh.ipv6h;
+			if (ipv6_addr_is_multicast(&hdr->daddr) &&
+			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+			    &hdr->saddr) &&
+			    !is_mld(skb))
+				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
 		    !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) 
@@ -211,16 +219,11 @@
 
 int ip6_mc_input(struct sk_buff *skb)
 {
-	struct ipv6hdr *hdr;	
-	int deliver = 0;
+	int deliver = 1;
 	int discard = 1;
 
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
-	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-		deliver = 1;
-
 	/*
 	 *	IPv6 multicast router mode isnt currently supported.
 	 */
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F1/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c	2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/mcast.c	2004-02-06 16:04:25.000000000 -0800
@@ -901,6 +901,25 @@
 }
 
 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int is_mld(struct sk_buff *skb)
+{
+	struct icmp6hdr *pic = (struct icmp6hdr *)skb->h.raw;
+
+	switch (pic->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return 1;
+	default:
+		break;
+	}
+	return 0;
+}
+
+/*
  *	check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,

Re: Source Specific Query of MLDv2 [PATCH]

[David S. Miller]

On Fri, 6 Feb 2004 18:01:37 -0700
David Stevens <dlstevens@us.ibm.com> wrote:

> +extern int is_mld(struct sk_buff *skb);

David, please don't pollute global name space so badly :-)
ipv6_is_mld() would be fine.

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1.1: Type: text/plain, Size: 2964 bytes --]






> David, please don't pollute global name space so badly :-)
> ipv6_is_mld() would be fine.

Of course, sorry about that. :-)

                  +-DLS

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F1/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h    2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F1/include/net/addrconf.h  2004-02-06 16:03:34.000000000 -0800
@@ -98,6 +98,7 @@

 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
            struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);

 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);

diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F1/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c      2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/ip6_input.c    2004-02-06 16:04:00.000000000 -0800
@@ -168,11 +168,19 @@

            smp_read_barrier_depends();
            if (ipprot->flags & INET6_PROTO_FINAL) {
+                 struct ipv6hdr *hdr;
+
                  if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
                        skb->csum = csum_sub(skb->csum,
                                         csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
                        cksum_sub++;
                  }
+                 hdr = skb->nh.ipv6h;
+                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
+                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+                     &hdr->saddr) &&
+                     !ipv6_is_mld(skb))
+                       goto discard;
            }
            if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
                !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
@@ -211,16 +219,11 @@

 int ip6_mc_input(struct sk_buff *skb)
 {
-     struct ipv6hdr *hdr;
-     int deliver = 0;
+     int deliver = 1;
      int discard = 1;

      IP6_INC_STATS_BH(Ip6InMcastPkts);

-     hdr = skb->nh.ipv6h;
-     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-           deliver = 1;
-
      /*
       *    IPv6 multicast router mode isnt currently supported.
       */
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F1/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c    2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/mcast.c  2004-02-06 16:04:25.000000000 -0800
@@ -901,6 +901,25 @@
 }

 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+     struct icmp6hdr *pic = (struct icmp6hdr *)skb->h.raw;
+
+     switch (pic->icmp6_type) {
+     case ICMPV6_MGM_QUERY:
+     case ICMPV6_MGM_REPORT:
+     case ICMPV6_MGM_REDUCTION:
+     case ICMPV6_MLD2_REPORT:
+           return 1;
+     default:
+           break;
+     }
+     return 0;
+}
+
+/*
  *   check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,

(See attached file: mldx2.patch)

[-- Attachment #1.2: Type: text/html, Size: 3217 bytes --]

[-- Attachment #2: mldx2.patch --]
[-- Type: application/octet-stream, Size: 2411 bytes --]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F1/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h	2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F1/include/net/addrconf.h	2004-02-06 16:03:34.000000000 -0800
@@ -98,6 +98,7 @@
 
 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
 		struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);
 
 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);
 
diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F1/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c	2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/ip6_input.c	2004-02-06 16:04:00.000000000 -0800
@@ -168,11 +168,19 @@
 		
 		smp_read_barrier_depends();
 		if (ipprot->flags & INET6_PROTO_FINAL) {
+			struct ipv6hdr *hdr;	
+
 			if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
 				skb->csum = csum_sub(skb->csum,
 						     csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
 				cksum_sub++;
 			}
+			hdr = skb->nh.ipv6h;
+			if (ipv6_addr_is_multicast(&hdr->daddr) &&
+			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+			    &hdr->saddr) &&
+			    !ipv6_is_mld(skb))
+				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
 		    !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) 
@@ -211,16 +219,11 @@
 
 int ip6_mc_input(struct sk_buff *skb)
 {
-	struct ipv6hdr *hdr;	
-	int deliver = 0;
+	int deliver = 1;
 	int discard = 1;
 
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
-	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-		deliver = 1;
-
 	/*
 	 *	IPv6 multicast router mode isnt currently supported.
 	 */
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F1/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c	2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F1/net/ipv6/mcast.c	2004-02-06 16:04:25.000000000 -0800
@@ -901,6 +901,25 @@
 }
 
 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+	struct icmp6hdr *pic = (struct icmp6hdr *)skb->h.raw;
+
+	switch (pic->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return 1;
+	default:
+		break;
+	}
+	return 0;
+}
+
+/*
  *	check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OF293C5854.C480643B-ON88256E33.0016BE45-88256E33.0016EBE0@us.ibm.com> (at Fri, 6 Feb 2004 21:12:59 -0700), David Stevens <dlstevens@us.ibm.com> says:

> > David, please don't pollute global name space so badly :-)
> > ipv6_is_mld() would be fine.
> 
> Of course, sorry about that. :-)

Why not in icmpv6_rcv()?
we need to do pskb_pull() before touching type/code.

>  int ip6_mc_input(struct sk_buff *skb)
>  {
> -     struct ipv6hdr *hdr;
> -     int deliver = 0;
> +     int deliver = 1;
>       int discard = 1;
> 
>       IP6_INC_STATS_BH(Ip6InMcastPkts);
> 
> -     hdr = skb->nh.ipv6h;
> -     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
> -           deliver = 1;
> -

Please do not remove this.
We need to check destination (but not source)
because the driver may be in "promisc." mode.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David S. Miller]

On Fri, 6 Feb 2004 21:12:59 -0700
David Stevens <dlstevens@us.ibm.com> wrote:

> @@ -211,16 +219,11 @@
> 
>  int ip6_mc_input(struct sk_buff *skb)
>  {
> -     struct ipv6hdr *hdr;
> -     int deliver = 0;
> +     int deliver = 1;
>       int discard = 1;

'deliver' is always true now, please kill it and all the code
trying to test it :)

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040206202438.13163677.davem@redhat.com> (at Fri, 6 Feb 2004 20:24:38 -0800), "David S. Miller" <davem@redhat.com> says:

> On Fri, 6 Feb 2004 21:12:59 -0700
> David Stevens <dlstevens@us.ibm.com> wrote:
> 
> > @@ -211,16 +219,11 @@
> > 
> >  int ip6_mc_input(struct sk_buff *skb)
> >  {
> > -     struct ipv6hdr *hdr;
> > -     int deliver = 0;
> > +     int deliver = 1;
> >       int discard = 1;
> 
> 'deliver' is always true now, please kill it and all the code
> trying to test it :)

We should test the destination address here.

--yoshfuji

Re: Source Specific Query of MLDv2 [PATCH]

[David S. Miller]

On Sat, 07 Feb 2004 13:29:37 +0900 (JST)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:

> In article <20040206202438.13163677.davem@redhat.com> (at Fri, 6 Feb 2004 20:24:38 -0800), "David S. Miller" <davem@redhat.com> says:
> 
> > On Fri, 6 Feb 2004 21:12:59 -0700
> > David Stevens <dlstevens@us.ibm.com> wrote:
> > 
> > > @@ -211,16 +219,11 @@
> > > 
> > >  int ip6_mc_input(struct sk_buff *skb)
> 
> We should test the destination address here.

Ok, you and David work things out and let me know when the final
patch is ready.

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]







"Hideaki YOSHIFUJI" <yoshfuji@cerberus.hongo.wide.ad.jp> wrote on
02/06/2004 08:22:33 PM:

> Why not in icmpv6_rcv()?

      I don't understand this question. It's needed every place
*except* icmpv6_rcv().

> we need to do pskb_pull() before touching type/code.

      Yes, you're right; I'll add that.

> >
> > -     hdr = skb->nh.ipv6h;
> > -     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
> > -           deliver = 1;
> > -

> Please do not remove this.
> We need to check destination (but not source)
> because the driver may be in "promisc." mode.

      I'm not removing it, I'm moving it. The original code
is:

      ip6_mc_input()
            <filter check> (not much else, since no mc router code)
      ip6_input
            <parse extension headers>
            <deliver to ULP>

new is:
      ip6_mc_input()
            <inc stats, "#if 0" for mc router code>
      ip6_input
            <parse extension headers>
            <filter check>
            <deliver to ULP>

In promiscuous mode, it will parse the extension headers
for non-local multicasts before dropping them. But what
you suggested will do two multicast address look-ups for
every multicast packet (including the valid ones). One
for the destination and another for the destination/source
filter.
      The point where it's filtered is just after the
checksum is validated, and no ICMP errors can be reported
for multicast destinations, so the trade-off is 1) a
performance hit for every multicast packet or 2) parsing
potential extension headers (expected case won't have any
and it doesn't cost anything) for multicast packets that
we may drop.
      This patch implements option #2.

                        +-DLS

[-- Attachment #2: Type: text/html, Size: 2337 bytes --]

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OF9F84892D.B2226F9C-ON88256E33.00185F62-88256E33.00196063@us.ibm.com> (at Fri, 6 Feb 2004 20:39:47 -0800), David Stevens <dlstevens@us.ibm.com> says:

> > Why not in icmpv6_rcv()?
> 
>       I don't understand this question. It's needed every place
> *except* icmpv6_rcv().

Sorry for any confusion.


> > > -     hdr = skb->nh.ipv6h;
> > > -     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
> > > -           deliver = 1;
> > > -
> 
> > Please do not remove this.
> > We need to check destination (but not source)
> > because the driver may be in "promisc." mode.
> 
>       I'm not removing it, I'm moving it. The original code
> is:
:
> 
> In promiscuous mode, it will parse the extension headers
> for non-local multicasts before dropping them. But what
> you suggested will do two multicast address look-ups for
> every multicast packet (including the valid ones). One
> for the destination and another for the destination/source
> filter.

Yes, but it changes the original and traditional behavior.

Well...

It is okay not to check destination here with good hardware / driver.
We however should check the destination before the process of 
extension headers for bad hardware / driver.
ipv6_chk_mcast_addr() in ip6_mc_input() has been helper for them
We should keep it in ip6_mc_input().

So... hmm... Let's check dev->flags ^ (IFF_PROMISC|IFF_ALLMULTI) like this.
I don't think it is so costy for usual operation.
If you think it is, please introduce hash table for mc_list in idev;
it should have been "heavy." :-)
(You can do it later, of course.)

===== net/ipv6/ip6_input.c 1.14 vs edited =====
--- 1.14/net/ipv6/ip6_input.c	Tue Jun 17 00:11:36 2003
+++ edited/net/ipv6/ip6_input.c	Sat Feb  7 14:14:30 2004
@@ -218,7 +218,8 @@
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
 	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
+	if ((skb->dev->flags ^ (IFF_PROMISC|IFF_ALLMULTI)) == 0 ||
+	    ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL))
 		deliver = 1;
 
 	/*
===== net/ipv6/mcast.c 1.50 vs edited =====
--- 1.50/net/ipv6/mcast.c	Thu Jan 29 09:06:25 2004
+++ edited/net/ipv6/mcast.c	Sat Feb  7 13:58:58 2004
@@ -918,7 +918,7 @@
 				break;
 		}
 		if (mc) {
-			if (!ipv6_addr_any(src_addr)) {
+			if (src_addr && !ipv6_addr_any(src_addr)) {
 				struct ip6_sf_list *psf;
 
 				spin_lock_bh(&mc->mca_lock);

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]





Good idea; I'll incorporate all the comments, retest and repost.

> So... hmm... Let's check dev->flags ^ (IFF_PROMISC|IFF_ALLMULTI) like
this.

      I think you mean (dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) here,
right?

> I don't think it is so costy for usual operation.
> If you think it is, please introduce hash table for mc_list in idev;
> it should have been "heavy." :-)
> (You can do it later, of course.)

This certainly could be done and I had this in mind for the future when
I designed the interface filter data structures (made sure everything
for the intersection and union is directly available given only the
ifmcaddr). I should do some testing to see how many MCA's or source
filters are needed to make a hash worthwhile, and maybe how many before
it falls over. :-)
                        thanks!
                              +-DLS

[-- Attachment #2: Type: text/html, Size: 1032 bytes --]

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OF92ABDC1A.4056C324-ON88256E33.002267FA-88256E33.00247258@us.ibm.com> (at Fri, 6 Feb 2004 22:40:42 -0800), David Stevens <dlstevens@us.ibm.com> says:

> Good idea; I'll incorporate all the comments, retest and repost.
> 
> > So... hmm... Let's check dev->flags ^ (IFF_PROMISC|IFF_ALLMULTI) like
> this.
> 
>       I think you mean (dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) here,
> right?

Oops! Sorry, you're right...
And, it'd be good to use unlikely() here.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1.1: Type: text/plain, Size: 4206 bytes --]





Here's a revised version with all the comments incorporated.

                  +-DLS


diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2
F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h    2004-02-03 19:44:17.000000000
-0800
+++ linux-2.6.2F2/include/net/addrconf.h  2004-02-06 16:03:34.000000000
-0800
@@ -98,6 +98,7 @@

 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr
*group,
            struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);

 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);

diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2
F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c      2004-02-03 19:44:14.000000000
-0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c    2004-02-09 14:12:34.000000000
-0800
@@ -168,11 +168,19 @@

            smp_read_barrier_depends();
            if (ipprot->flags & INET6_PROTO_FINAL) {
+                 struct ipv6hdr *hdr;
+
                  if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
                        skb->csum = csum_sub(skb->csum,
                                         csum_partial(skb->nh.raw,
skb->h.raw-skb->nh.raw, 0));
                        cksum_sub++;
                  }
+                 hdr = skb->nh.ipv6h;
+                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
+                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+                     &hdr->saddr) &&
+                     !ipv6_is_mld(skb))
+                       goto discard;
            }
            if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
                !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
@@ -211,50 +219,21 @@

 int ip6_mc_input(struct sk_buff *skb)
 {
-     struct ipv6hdr *hdr;
-     int deliver = 0;
-     int discard = 1;
+     struct ipv6hdr *hdr;
+     int deliver;

      IP6_INC_STATS_BH(Ip6InMcastPkts);

      hdr = skb->nh.ipv6h;
-     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-           deliver = 1;
-
-     /*
-      *    IPv6 multicast router mode isnt currently supported.
-      */
-#if 0
-     if (ipv6_config.multicast_route) {
-           int addr_type;
-
-           addr_type = ipv6_addr_type(&hdr->daddr);
-
-           if (!(addr_type & (IPV6_ADDR_LOOPBACK | IPV6_ADDR_LINKLOCAL))) {
-                 struct sk_buff *skb2;
-                 struct dst_entry *dst;
-
-                 dst = skb->dst;
-
-                 if (deliver) {
-                       skb2 = skb_clone(skb, GFP_ATOMIC);
-                 } else {
-                       discard = 0;
-                       skb2 = skb;
-                 }
-
-                 dst_output(skb2);
-           }
-     }
-#endif
+     deliver = !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) ||
+         ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);

-     if (deliver) {
-           discard = 0;
+     if (likely(deliver)) {
            ip6_input(skb);
+           return 0;
      }
-
-     if (discard)
-           kfree_skb(skb);
+     /* discard */
+     kfree_skb(skb);

      return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c    2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c  2004-02-09 13:12:07.000000000 -0800
@@ -901,6 +901,25 @@
 }

 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+     struct icmp6hdr *pic = (struct icmp6hdr *)skb->h.raw;
+
+     switch (pic->icmp6_type) {
+     case ICMPV6_MGM_QUERY:
+     case ICMPV6_MGM_REPORT:
+     case ICMPV6_MGM_REDUCTION:
+     case ICMPV6_MLD2_REPORT:
+           return 1;
+     default:
+           break;
+     }
+     return 0;
+}
+
+/*
  *   check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +937,7 @@
                        break;
            }
            if (mc) {
-                 if (!ipv6_addr_any(src_addr)) {
+                 if (src_addr && !ipv6_addr_any(src_addr)) {
                        struct ip6_sf_list *psf;

                        spin_lock_bh(&mc->mca_lock);

(See attached file: mldx3.patch)

[-- Attachment #1.2: Type: text/html, Size: 4207 bytes --]

[-- Attachment #2: mldx3.patch --]
[-- Type: application/octet-stream, Size: 3344 bytes --]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h	2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h	2004-02-06 16:03:34.000000000 -0800
@@ -98,6 +98,7 @@
 
 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
 		struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);
 
 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);
 
diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c	2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c	2004-02-09 14:12:34.000000000 -0800
@@ -168,11 +168,19 @@
 		
 		smp_read_barrier_depends();
 		if (ipprot->flags & INET6_PROTO_FINAL) {
+			struct ipv6hdr *hdr;	
+
 			if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
 				skb->csum = csum_sub(skb->csum,
 						     csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
 				cksum_sub++;
 			}
+			hdr = skb->nh.ipv6h;
+			if (ipv6_addr_is_multicast(&hdr->daddr) &&
+			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+			    &hdr->saddr) &&
+			    !ipv6_is_mld(skb))
+				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
 		    !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) 
@@ -211,50 +219,21 @@
 
 int ip6_mc_input(struct sk_buff *skb)
 {
-	struct ipv6hdr *hdr;	
-	int deliver = 0;
-	int discard = 1;
+	struct ipv6hdr *hdr;
+	int deliver;
 
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
 	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-		deliver = 1;
-
-	/*
-	 *	IPv6 multicast router mode isnt currently supported.
-	 */
-#if 0
-	if (ipv6_config.multicast_route) {
-		int addr_type;
-
-		addr_type = ipv6_addr_type(&hdr->daddr);
-
-		if (!(addr_type & (IPV6_ADDR_LOOPBACK | IPV6_ADDR_LINKLOCAL))) {
-			struct sk_buff *skb2;
-			struct dst_entry *dst;
-
-			dst = skb->dst;
-			
-			if (deliver) {
-				skb2 = skb_clone(skb, GFP_ATOMIC);
-			} else {
-				discard = 0;
-				skb2 = skb;
-			}
-
-			dst_output(skb2);
-		}
-	}
-#endif
+	deliver = !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) ||
+	    ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);
 
-	if (deliver) {
-		discard = 0;
+	if (likely(deliver)) {
 		ip6_input(skb);
+		return 0;
 	}
-
-	if (discard)
-		kfree_skb(skb);
+	/* discard */
+	kfree_skb(skb);
 
 	return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c	2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c	2004-02-09 13:12:07.000000000 -0800
@@ -901,6 +901,25 @@
 }
 
 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+	struct icmp6hdr *pic = (struct icmp6hdr *)skb->h.raw;
+
+	switch (pic->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return 1;
+	default:
+		break;
+	}
+	return 0;
+}
+
+/*
  *	check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +937,7 @@
 				break;
 		}
 		if (mc) {
-			if (!ipv6_addr_any(src_addr)) {
+			if (src_addr && !ipv6_addr_any(src_addr)) {
 				struct ip6_sf_list *psf;
 
 				spin_lock_bh(&mc->mca_lock);

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OF40DCBAE1.2110C762-ON88256E35.007F7C77-88256E35.0080020D@us.ibm.com> (at Mon, 9 Feb 2004 16:20:52 -0700), David Stevens <dlstevens@us.ibm.com> says:

> Here's a revised version with all the comments incorporated.

> @@ -211,50 +219,21 @@
> 
>  int ip6_mc_input(struct sk_buff *skb)
>  {
> -     struct ipv6hdr *hdr;
> -     int deliver = 0;
> -     int discard = 1;
> +     struct ipv6hdr *hdr;
> +     int deliver;
> 
>       IP6_INC_STATS_BH(Ip6InMcastPkts);
> 
>       hdr = skb->nh.ipv6h;
> -     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
> -           deliver = 1;
> -
> -     /*
> -      *    IPv6 multicast router mode isnt currently supported.
> -      */
> -#if 0
> -     if (ipv6_config.multicast_route) {
> -           int addr_type;
> -
> -           addr_type = ipv6_addr_type(&hdr->daddr);
> -
> -           if (!(addr_type & (IPV6_ADDR_LOOPBACK | IPV6_ADDR_LINKLOCAL))) {
> -                 struct sk_buff *skb2;
> -                 struct dst_entry *dst;
> -
> -                 dst = skb->dst;
> -
> -                 if (deliver) {
> -                       skb2 = skb_clone(skb, GFP_ATOMIC);
> -                 } else {
> -                       discard = 0;
> -                       skb2 = skb;
> -                 }
> -
> -                 dst_output(skb2);
> -           }
> -     }
> -#endif
> +     deliver = !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) ||
> +         ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);
> 
> -     if (deliver) {
> -           discard = 0;
> +     if (likely(deliver)) {
>             ip6_input(skb);
> +           return 0;
>       }
> -
> -     if (discard)
> -           kfree_skb(skb);
> +     /* discard */
> +     kfree_skb(skb);
> 
>       return 0;

likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)))?

Where is pskb_pull()?
I'd suggest to check the protocol number != icmp there.
After doing pskb_pull() in icmpv6_rcv(), we see type field.

BTW, I want to keep #if 0 ... #endif portion.
We're working on this... :-)

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1.1: Type: text/plain, Size: 5095 bytes --]







"Hideaki YOSHIFUJI" <yoshfuji@cerberus.hongo.wide.ad.jp> wrote on
02/09/2004 07:51:24 PM:

> likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)))?

      If your question is "why use the deliver variable?", I left that
because it'd
need to be re-added when multicast routing is supported. This patch puts it
back,
and eliminates "discard" (including within the "#if 0" code).

> Where is pskb_pull()?
> I'd suggest to check the protocol number != icmp there.
> After doing pskb_pull() in icmpv6_rcv(), we see type field.

      Oops! I missed that one. This patch adds it. I did not
split the protocol check from the type check because they'd be
in two different places (less clear what they're for). By keeping
it in one place, it's clear exactly which packets are excepted,
and the only cost is an extra do-nothing pskb_pull() for ICMP
multicasts coming from a host that is excluded by the source
filters.

> BTW, I want to keep #if 0 ... #endif portion.
> We're working on this... :-)

I put it back in this patch, but edited it to remove "discard".

                        +-DLS

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h    2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h  2004-02-10 15:17:00.000000000 -0800
@@ -98,6 +98,7 @@

 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
            struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);

 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);

diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c      2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c    2004-02-10 16:23:51.808947560 -0800
@@ -168,11 +168,19 @@

            smp_read_barrier_depends();
            if (ipprot->flags & INET6_PROTO_FINAL) {
+                 struct ipv6hdr *hdr;
+
                  if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
                        skb->csum = csum_sub(skb->csum,
                                         csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
                        cksum_sub++;
                  }
+                 hdr = skb->nh.ipv6h;
+                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
+                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+                     &hdr->saddr) &&
+                     !ipv6_is_mld(skb))
+                       goto discard;
            }
            if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
                !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
@@ -211,15 +219,14 @@

 int ip6_mc_input(struct sk_buff *skb)
 {
-     struct ipv6hdr *hdr;
-     int deliver = 0;
-     int discard = 1;
+     struct ipv6hdr *hdr;
+     int deliver;

      IP6_INC_STATS_BH(Ip6InMcastPkts);

      hdr = skb->nh.ipv6h;
-     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-           deliver = 1;
+     deliver = !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) ||
+         ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);

      /*
       *    IPv6 multicast router mode isnt currently supported.
@@ -238,23 +245,21 @@

                  if (deliver) {
                        skb2 = skb_clone(skb, GFP_ATOMIC);
+                       dst_output(skb2);
                  } else {
-                       discard = 0;
-                       skb2 = skb;
+                       dst_output(skb);
+                       return 0;
                  }
-
-                 dst_output(skb2);
            }
      }
 #endif

-     if (deliver) {
-           discard = 0;
+     if (likely(deliver)) {
            ip6_input(skb);
+           return 0;
      }
-
-     if (discard)
-           kfree_skb(skb);
+     /* discard */
+     kfree_skb(skb);

      return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c    2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c  2004-02-10 15:15:52.000000000 -0800
@@ -901,6 +901,30 @@
 }

 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+     struct icmp6hdr *pic;
+
+     if (!pskb_pull(skb, sizeof(struct icmp6hdr)))
+           return 0;
+
+     pic = (struct icmp6hdr *)skb->h.raw;
+
+     switch (pic->icmp6_type) {
+     case ICMPV6_MGM_QUERY:
+     case ICMPV6_MGM_REPORT:
+     case ICMPV6_MGM_REDUCTION:
+     case ICMPV6_MLD2_REPORT:
+           return 1;
+     default:
+           break;
+     }
+     return 0;
+}
+
+/*
  *   check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +942,7 @@
                        break;
            }
            if (mc) {
-                 if (!ipv6_addr_any(src_addr)) {
+                 if (src_addr && !ipv6_addr_any(src_addr)) {
                        struct ip6_sf_list *psf;

                        spin_lock_bh(&mc->mca_lock);
(See attached file: mldx4.patch)

[-- Attachment #1.2: Type: text/html, Size: 5478 bytes --]

[-- Attachment #2: mldx4.patch --]
[-- Type: application/octet-stream, Size: 3239 bytes --]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h	2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h	2004-02-10 15:17:00.000000000 -0800
@@ -98,6 +98,7 @@
 
 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
 		struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);
 
 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);
 
diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c	2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c	2004-02-10 16:23:51.808947560 -0800
@@ -168,11 +168,19 @@
 		
 		smp_read_barrier_depends();
 		if (ipprot->flags & INET6_PROTO_FINAL) {
+			struct ipv6hdr *hdr;	
+
 			if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
 				skb->csum = csum_sub(skb->csum,
 						     csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
 				cksum_sub++;
 			}
+			hdr = skb->nh.ipv6h;
+			if (ipv6_addr_is_multicast(&hdr->daddr) &&
+			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+			    &hdr->saddr) &&
+			    !ipv6_is_mld(skb))
+				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
 		    !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) 
@@ -211,15 +219,14 @@
 
 int ip6_mc_input(struct sk_buff *skb)
 {
-	struct ipv6hdr *hdr;	
-	int deliver = 0;
-	int discard = 1;
+	struct ipv6hdr *hdr;
+	int deliver;
 
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
 	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-		deliver = 1;
+	deliver = !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) ||
+	    ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);
 
 	/*
 	 *	IPv6 multicast router mode isnt currently supported.
@@ -238,23 +245,21 @@
 			
 			if (deliver) {
 				skb2 = skb_clone(skb, GFP_ATOMIC);
+				dst_output(skb2);
 			} else {
-				discard = 0;
-				skb2 = skb;
+				dst_output(skb);
+				return 0;
 			}
-
-			dst_output(skb2);
 		}
 	}
 #endif
 
-	if (deliver) {
-		discard = 0;
+	if (likely(deliver)) {
 		ip6_input(skb);
+		return 0;
 	}
-
-	if (discard)
-		kfree_skb(skb);
+	/* discard */
+	kfree_skb(skb);
 
 	return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c	2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c	2004-02-10 15:15:52.000000000 -0800
@@ -901,6 +901,30 @@
 }
 
 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+	struct icmp6hdr *pic;
+
+	if (!pskb_pull(skb, sizeof(struct icmp6hdr)))
+		return 0;
+
+	pic = (struct icmp6hdr *)skb->h.raw;
+
+	switch (pic->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return 1;
+	default:
+		break;
+	}
+	return 0;
+}
+
+/*
  *	check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +942,7 @@
 				break;
 		}
 		if (mc) {
-			if (!ipv6_addr_any(src_addr)) {
+			if (src_addr && !ipv6_addr_any(src_addr)) {
 				struct ip6_sf_list *psf;
 
 				spin_lock_bh(&mc->mca_lock);

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OFE4F39A6C.5E40D57D-ON88256E37.00008C47-88256E37.0003F08A@us.ibm.com> (at Tue, 10 Feb 2004 17:45:43 -0700), David Stevens <dlstevens@us.ibm.com> says:

> > likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)))?
> 
>       If your question is "why use the deliver variable?", I left that
> because it'd
> need to be re-added when multicast routing is supported. This patch puts it
> back,
> and eliminates "discard" (including within the "#if 0" code).

No. 

My point is, !(skb->dev->flags & IFF_PROMISC|IFF_ALLMULTI) is likely true.

And, let's concentrate on fixing the original bug:

> @@ -238,23 +245,21 @@
> 
>                   if (deliver) {
>                         skb2 = skb_clone(skb, GFP_ATOMIC);
> +                       dst_output(skb2);
>                   } else {
> -                       discard = 0;
> -                       skb2 = skb;
> +                       dst_output(skb);
> +                       return 0;
>                   }
> -
> -                 dst_output(skb2);
>             }
>       }

and

>  #endif
> 
> -     if (deliver) {
> -           discard = 0;
> +     if (likely(deliver)) {
>             ip6_input(skb);
> +           return 0;
>       }
> -
> -     if (discard)
> -           kfree_skb(skb);
> +     /* discard */
> +     kfree_skb(skb);
> 

seems needless to fix the bug.

(Or you may want to submit another patch for them.)

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]







"Hideaki YOSHIFUJI" <yoshfuji@cerberus.hongo.wide.ad.jp> wrote on
02/10/2004 07:02:34 PM:

> My point is, !(skb->dev->flags & IFF_PROMISC|IFF_ALLMULTI) is likely
true.

Yes, "deliver" is set to the expression that includes the check
"!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))" and it is likely
true, which is why the patch I sent has "if (likely(deliver)) {"
in it.

I don't understand what you're suggesting. If you aren't suggesting
that I remove the "deliver" variable (which is needed again when you
enable the "#if 0" part), then I think your suggestion is already
in the patch. Can you explain?

> And, let's concentrate on fixing the original bug:

      I put these in to clean up the code, because Dave Miller
asked me to in the first patch. He was talking about removing
"deliver," which is needed in this revision because of your comments,
but "discard" is not. The stuff within the "#if 0" may need to be
rewritten (certainly needs to be tested), but I didn't want to
leave "discard" references, or code that is definitely incorrect.
So I modified the code within the "#if 0" to also remove the
unneeded "discard".
      The host-only path doesn't need "discard" because it is
always the same as "!deliver", but the declaration should go
away or it'll result in a warning. It could be "#if 0" too, but
I didn't want to leave it as:
      int discard = 1;
      ...
      if(deliver) {
            discard=0;
      ...

because "discard" is completely pointless without any multicast
routing code, and it's unnecessary when the "#if 0" is changed
the way I did.

In other words, removing "discard" is code-cleanup, and editing
within the "#if 0" is just so it won't leave an obvious bug when
it becomes not "#if 0".

                              +-DLS

[-- Attachment #2: Type: text/html, Size: 2449 bytes --]

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OFA69E1A86.43DC582D-ON88256E37.0016A3A7-88256E37.00195210@us.ibm.com> (at Tue, 10 Feb 2004 21:39:15 -0700), David Stevens <dlstevens@us.ibm.com> says:

> "Hideaki YOSHIFUJI" <yoshfuji@cerberus.hongo.wide.ad.jp> wrote on
> 02/10/2004 07:02:34 PM:
> 
> > My point is, !(skb->dev->flags & IFF_PROMISC|IFF_ALLMULTI) is likely
> true.
> 
> Yes, "deliver" is set to the expression that includes the check
> "!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))" and it is likely
> true, which is why the patch I sent has "if (likely(deliver)) {"
> in it.
> 
> I don't understand what you're suggesting. If you aren't suggesting
> that I remove the "deliver" variable (which is needed again when you
> enable the "#if 0" part), then I think your suggestion is already
> in the patch. Can you explain?

David, they produce different assembly code.

If we say
        deliver = likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))) ||
                  ipv6_chk_mcast_addr(skb->dev, &skb->nh.ipv6h->daddr, NULL);
then we can run the next code without breaking pipeline in the CPU
if !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) is true.
This condition is true with good device and driver.

If you say
        deliver = !(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) ||
                  ipv6_chk_mcast_addr(skb->dev, &skb->nh.ipv6h->daddr, NULL);
then you brean the pipeline.

objdump -d will help you. :-)

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1.1: Type: text/plain, Size: 4146 bytes --]





Ok, here's what I hope is the final version of the patch, so we can
finally put this to bed. :-)

                        +-DLS

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h    2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h  2004-02-10 15:17:00.000000000 -0800
@@ -98,6 +98,7 @@

 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
            struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);

 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);

diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c      2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c    2004-02-11 12:34:27.000000000 -0800
@@ -168,11 +168,19 @@

            smp_read_barrier_depends();
            if (ipprot->flags & INET6_PROTO_FINAL) {
+                 struct ipv6hdr *hdr;
+
                  if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
                        skb->csum = csum_sub(skb->csum,
                                         csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
                        cksum_sub++;
                  }
+                 hdr = skb->nh.ipv6h;
+                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
+                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+                     &hdr->saddr) &&
+                     !ipv6_is_mld(skb))
+                       goto discard;
            }
            if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
                !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
@@ -211,15 +219,14 @@

 int ip6_mc_input(struct sk_buff *skb)
 {
-     struct ipv6hdr *hdr;
-     int deliver = 0;
-     int discard = 1;
+     struct ipv6hdr *hdr;
+     int deliver;

      IP6_INC_STATS_BH(Ip6InMcastPkts);

      hdr = skb->nh.ipv6h;
-     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-           deliver = 1;
+     deliver = likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))) ||
+         ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);

      /*
       *    IPv6 multicast router mode isnt currently supported.
@@ -238,23 +245,21 @@

                  if (deliver) {
                        skb2 = skb_clone(skb, GFP_ATOMIC);
+                       dst_output(skb2);
                  } else {
-                       discard = 0;
-                       skb2 = skb;
+                       dst_output(skb);
+                       return 0;
                  }
-
-                 dst_output(skb2);
            }
      }
 #endif

-     if (deliver) {
-           discard = 0;
+     if (likely(deliver)) {
            ip6_input(skb);
+           return 0;
      }
-
-     if (discard)
-           kfree_skb(skb);
+     /* discard */
+     kfree_skb(skb);

      return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c    2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c  2004-02-11 12:34:34.000000000 -0800
@@ -901,6 +901,30 @@
 }

 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+     struct icmp6hdr *pic;
+
+     if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
+           return 0;
+
+     pic = (struct icmp6hdr *)skb->h.raw;
+
+     switch (pic->icmp6_type) {
+     case ICMPV6_MGM_QUERY:
+     case ICMPV6_MGM_REPORT:
+     case ICMPV6_MGM_REDUCTION:
+     case ICMPV6_MLD2_REPORT:
+           return 1;
+     default:
+           break;
+     }
+     return 0;
+}
+
+/*
  *   check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +942,7 @@
                        break;
            }
            if (mc) {
-                 if (!ipv6_addr_any(src_addr)) {
+                 if (src_addr && !ipv6_addr_any(src_addr)) {
                        struct ip6_sf_list *psf;

                        spin_lock_bh(&mc->mca_lock);
(See attached file: mldx5.patch)

[-- Attachment #1.2: Type: text/html, Size: 4119 bytes --]

[-- Attachment #2: mldx5.patch --]
[-- Type: application/octet-stream, Size: 3251 bytes --]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h	2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h	2004-02-10 15:17:00.000000000 -0800
@@ -98,6 +98,7 @@
 
 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
 		struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb);
 
 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);
 
diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c	2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c	2004-02-11 12:34:27.000000000 -0800
@@ -168,11 +168,19 @@
 		
 		smp_read_barrier_depends();
 		if (ipprot->flags & INET6_PROTO_FINAL) {
+			struct ipv6hdr *hdr;	
+
 			if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
 				skb->csum = csum_sub(skb->csum,
 						     csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
 				cksum_sub++;
 			}
+			hdr = skb->nh.ipv6h;
+			if (ipv6_addr_is_multicast(&hdr->daddr) &&
+			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+			    &hdr->saddr) &&
+			    !ipv6_is_mld(skb))
+				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
 		    !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) 
@@ -211,15 +219,14 @@
 
 int ip6_mc_input(struct sk_buff *skb)
 {
-	struct ipv6hdr *hdr;	
-	int deliver = 0;
-	int discard = 1;
+	struct ipv6hdr *hdr;
+	int deliver;
 
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
 	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-		deliver = 1;
+	deliver = likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))) ||
+	    ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);
 
 	/*
 	 *	IPv6 multicast router mode isnt currently supported.
@@ -238,23 +245,21 @@
 			
 			if (deliver) {
 				skb2 = skb_clone(skb, GFP_ATOMIC);
+				dst_output(skb2);
 			} else {
-				discard = 0;
-				skb2 = skb;
+				dst_output(skb);
+				return 0;
 			}
-
-			dst_output(skb2);
 		}
 	}
 #endif
 
-	if (deliver) {
-		discard = 0;
+	if (likely(deliver)) {
 		ip6_input(skb);
+		return 0;
 	}
-
-	if (discard)
-		kfree_skb(skb);
+	/* discard */
+	kfree_skb(skb);
 
 	return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c	2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c	2004-02-11 12:34:34.000000000 -0800
@@ -901,6 +901,30 @@
 }
 
 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb)
+{
+	struct icmp6hdr *pic;
+
+	if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
+		return 0;
+
+	pic = (struct icmp6hdr *)skb->h.raw;
+
+	switch (pic->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return 1;
+	default:
+		break;
+	}
+	return 0;
+}
+
+/*
  *	check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +942,7 @@
 				break;
 		}
 		if (mc) {
-			if (!ipv6_addr_any(src_addr)) {
+			if (src_addr && !ipv6_addr_any(src_addr)) {
 				struct ip6_sf_list *psf;
 
 				spin_lock_bh(&mc->mca_lock);

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OFA4C00EE0.FAE8C2B9-ON88256E37.0072FEDF-88256E37.00732B00@us.ibm.com> (at Wed, 11 Feb 2004 14:00:39 -0700), David Stevens <dlstevens@us.ibm.com> says:

> Ok, here's what I hope is the final version of the patch, so we can
> finally put this to bed. :-)

Well...

> +                 hdr = skb->nh.ipv6h;
> +                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
> +                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
> +                     &hdr->saddr) &&
> +                     !ipv6_is_mld(skb))
> +                       goto discard;
:

You forgot to check if proto is IPPROTO_ICMPV6.
(This is one reason why I want to check the icmpv6 code 
 in icmpv6_rcv().)

  if (ipv6_addr_is_multicast(&hdr->daddr) &&
      !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr) &&
      (proto != IPPROTO_ICMPV6 || !ipv6_is_mld(skb)))
          goto discard;

And, the name ipv6_is_mld should be icmpv6_is_mld or 
something like this now.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1.1: Type: text/plain, Size: 4934 bytes --]







"Hideaki YOSHIFUJI" <yoshfuji@cerberus.hongo.wide.ad.jp> wrote on
02/11/2004 06:03:28 PM:

> You forgot to check if proto is IPPROTO_ICMPV6.
> (This is one reason why I want to check the icmpv6 code
> in icmpv6_rcv().)

      Arg! You're right! I had no idea what you were talking
about with that comment about icmpv6_rcv() (the test can't be
done there, because it'll be discarded without it; I thought
you were looking for an optimization of some kind). Of course,
my tests are using ICMP (MLD and ECHO) and UDP packets that
didn't have MLD types where the ICMPv6 type code would be--
so the filter and exceptions all worked. :-(

      What I intended is for ipv6_is_mld() to check this (not
assume that "skb" is ICMPv6 only). Fixed patch below [blush].
      Thanks for the reviews,

                        +-DLS

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h    2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h  2004-02-11 21:03:10.000000000 -0800
@@ -98,6 +98,7 @@

 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
            struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb, int nexthdr);

 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);

diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c      2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c    2004-02-11 21:00:03.000000000 -0800
@@ -168,11 +168,19 @@

            smp_read_barrier_depends();
            if (ipprot->flags & INET6_PROTO_FINAL) {
+                 struct ipv6hdr *hdr;
+
                  if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
                        skb->csum = csum_sub(skb->csum,
                                         csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
                        cksum_sub++;
                  }
+                 hdr = skb->nh.ipv6h;
+                 if (ipv6_addr_is_multicast(&hdr->daddr) &&
+                     !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+                     &hdr->saddr) &&
+                     !ipv6_is_mld(skb, nexthdr))
+                       goto discard;
            }
            if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
                !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
@@ -211,15 +219,14 @@

 int ip6_mc_input(struct sk_buff *skb)
 {
-     struct ipv6hdr *hdr;
-     int deliver = 0;
-     int discard = 1;
+     struct ipv6hdr *hdr;
+     int deliver;

      IP6_INC_STATS_BH(Ip6InMcastPkts);

      hdr = skb->nh.ipv6h;
-     if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-           deliver = 1;
+     deliver = likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))) ||
+         ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);

      /*
       *    IPv6 multicast router mode isnt currently supported.
@@ -238,23 +245,21 @@

                  if (deliver) {
                        skb2 = skb_clone(skb, GFP_ATOMIC);
+                       dst_output(skb2);
                  } else {
-                       discard = 0;
-                       skb2 = skb;
+                       dst_output(skb);
+                       return 0;
                  }
-
-                 dst_output(skb2);
            }
      }
 #endif

-     if (deliver) {
-           discard = 0;
+     if (likely(deliver)) {
            ip6_input(skb);
+           return 0;
      }
-
-     if (discard)
-           kfree_skb(skb);
+     /* discard */
+     kfree_skb(skb);

      return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c    2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c  2004-02-11 21:01:52.000000000 -0800
@@ -901,6 +901,33 @@
 }

 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb, int nexthdr)
+{
+     struct icmp6hdr *pic;
+
+     if (nexthdr != IPPROTO_ICMPV6)
+           return 0;
+
+     if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
+           return 0;
+
+     pic = (struct icmp6hdr *)skb->h.raw;
+
+     switch (pic->icmp6_type) {
+     case ICMPV6_MGM_QUERY:
+     case ICMPV6_MGM_REPORT:
+     case ICMPV6_MGM_REDUCTION:
+     case ICMPV6_MLD2_REPORT:
+           return 1;
+     default:
+           break;
+     }
+     return 0;
+}
+
+/*
  *   check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +945,7 @@
                        break;
            }
            if (mc) {
-                 if (!ipv6_addr_any(src_addr)) {
+                 if (src_addr && !ipv6_addr_any(src_addr)) {
                        struct ip6_sf_list *psf;

                        spin_lock_bh(&mc->mca_lock);
(See attached file: mldx6.patch)

[-- Attachment #1.2: Type: text/html, Size: 5211 bytes --]

[-- Attachment #2: mldx6.patch --]
[-- Type: application/octet-stream, Size: 3334 bytes --]

diff -ruN linux-2.6.2/include/net/addrconf.h linux-2.6.2F2/include/net/addrconf.h
--- linux-2.6.2/include/net/addrconf.h	2004-02-03 19:44:17.000000000 -0800
+++ linux-2.6.2F2/include/net/addrconf.h	2004-02-11 21:03:10.000000000 -0800
@@ -98,6 +98,7 @@
 
 extern int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
 		struct in6_addr *src_addr);
+extern int ipv6_is_mld(struct sk_buff *skb, int nexthdr);
 
 extern void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len);
 
diff -ruN linux-2.6.2/net/ipv6/ip6_input.c linux-2.6.2F2/net/ipv6/ip6_input.c
--- linux-2.6.2/net/ipv6/ip6_input.c	2004-02-03 19:44:14.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/ip6_input.c	2004-02-11 21:00:03.000000000 -0800
@@ -168,11 +168,19 @@
 		
 		smp_read_barrier_depends();
 		if (ipprot->flags & INET6_PROTO_FINAL) {
+			struct ipv6hdr *hdr;	
+
 			if (!cksum_sub && skb->ip_summed == CHECKSUM_HW) {
 				skb->csum = csum_sub(skb->csum,
 						     csum_partial(skb->nh.raw, skb->h.raw-skb->nh.raw, 0));
 				cksum_sub++;
 			}
+			hdr = skb->nh.ipv6h;
+			if (ipv6_addr_is_multicast(&hdr->daddr) &&
+			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
+			    &hdr->saddr) &&
+			    !ipv6_is_mld(skb, nexthdr))
+				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
 		    !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) 
@@ -211,15 +219,14 @@
 
 int ip6_mc_input(struct sk_buff *skb)
 {
-	struct ipv6hdr *hdr;	
-	int deliver = 0;
-	int discard = 1;
+	struct ipv6hdr *hdr;
+	int deliver;
 
 	IP6_INC_STATS_BH(Ip6InMcastPkts);
 
 	hdr = skb->nh.ipv6h;
-	if (ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, &hdr->saddr))
-		deliver = 1;
+	deliver = likely(!(skb->dev->flags & (IFF_PROMISC|IFF_ALLMULTI))) ||
+	    ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, NULL);
 
 	/*
 	 *	IPv6 multicast router mode isnt currently supported.
@@ -238,23 +245,21 @@
 			
 			if (deliver) {
 				skb2 = skb_clone(skb, GFP_ATOMIC);
+				dst_output(skb2);
 			} else {
-				discard = 0;
-				skb2 = skb;
+				dst_output(skb);
+				return 0;
 			}
-
-			dst_output(skb2);
 		}
 	}
 #endif
 
-	if (deliver) {
-		discard = 0;
+	if (likely(deliver)) {
 		ip6_input(skb);
+		return 0;
 	}
-
-	if (discard)
-		kfree_skb(skb);
+	/* discard */
+	kfree_skb(skb);
 
 	return 0;
 }
diff -ruN linux-2.6.2/net/ipv6/mcast.c linux-2.6.2F2/net/ipv6/mcast.c
--- linux-2.6.2/net/ipv6/mcast.c	2004-02-03 19:44:28.000000000 -0800
+++ linux-2.6.2F2/net/ipv6/mcast.c	2004-02-11 21:01:52.000000000 -0800
@@ -901,6 +901,33 @@
 }
 
 /*
+ * identify MLD packets for MLD filter exceptions
+ */
+int ipv6_is_mld(struct sk_buff *skb, int nexthdr)
+{
+	struct icmp6hdr *pic;
+
+	if (nexthdr != IPPROTO_ICMPV6)
+		return 0;
+
+	if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
+		return 0;
+
+	pic = (struct icmp6hdr *)skb->h.raw;
+
+	switch (pic->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return 1;
+	default:
+		break;
+	}
+	return 0;
+}
+
+/*
  *	check if the interface/address pair is valid
  */
 int ipv6_chk_mcast_addr(struct net_device *dev, struct in6_addr *group,
@@ -918,7 +945,7 @@
 				break;
 		}
 		if (mc) {
-			if (!ipv6_addr_any(src_addr)) {
+			if (src_addr && !ipv6_addr_any(src_addr)) {
 				struct ip6_sf_list *psf;
 
 				spin_lock_bh(&mc->mca_lock);

Re: Source Specific Query of MLDv2 [PATCH]

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OF3D503D9D.B62072AD-ON88256E38.001BCCBD-88256E38.0023E0FA@us.ibm.com> (at Wed, 11 Feb 2004 23:34:37 -0700), David Stevens <dlstevens@us.ibm.com> says:

>       What I intended is for ipv6_is_mld() to check this (not
> assume that "skb" is ICMPv6 only). Fixed patch below [blush].
>       Thanks for the reviews,
:

I'll eat this patch.  Comments, if any?

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Source Specific Query of MLDv2 [PATCH]

[David S. Miller]

On Thu, 12 Feb 2004 15:39:11 +0900 (JST)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:

> In article <OF3D503D9D.B62072AD-ON88256E38.001BCCBD-88256E38.0023E0FA@us.ibm.com> (at Wed, 11 Feb 2004 23:34:37 -0700), David Stevens <dlstevens@us.ibm.com> says:
> 
> >       What I intended is for ipv6_is_mld() to check this (not
> > assume that "skb" is ICMPv6 only). Fixed patch below [blush].
> >       Thanks for the reviews,
> :
> 
> I'll eat this patch.  Comments, if any?

If you both agree I guess it's time for me to apply it :-)

Someone give me a condensed one-line description for the patch, thanks.

Note the stuff I'm applying these days is destined for 2.6.4-pre1, I'll only
send simple one-liner serious bug fixes for 2.6.3-rcX

Re: Source Specific Query of MLDv2 [PATCH]

[David Stevens]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]







"David S. Miller" <davem@redhat.com> wrote on 02/11/2004 10:41:13 PM:

> Someone give me a condensed one-line description for the patch, thanks.

"This patch excepts MLD packets from source filtering." :-)

            +-DLS

[-- Attachment #2: Type: text/html, Size: 338 bytes --]