[PATCH 0/2] IGMP snooping: set mrouters_only flag properly

[PATCH 0/2] IGMP snooping: set mrouters_only flag properly

[Fernando Luis Vazquez Cao]

Hi Herbert,

I am sending you a fix for a IGMP snooping bug that bit us when we tried
to deploy it in production. This bug has been present since day one in
the IGMP code and I think the fix deserves a backport to -stable
(>=2.6.34).

The reason I am sending you separate patches for IPv4 and IPv6 is that I
wanted to acknowledge Hayato Kakuta for his extensive testing of IPv4's
IGMP code.

Thanks,
Fernando

IGMP snooping: set mrouters_only flag for IPv4 traffic properly

[Fernando Luis Vazquez Cao]

Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
mrouters_only flag in a skb that may be a clone of the original skb, which
means that sometimes the bridge loses track of membership report packets (cb
buffers are tied to a specifici skb and not shared) and it ends up forwading
join requests to the bridge interface.

This can cause unexpected membership timeouts and intermitent/permanent loss of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:

    A snooping switch should forward IGMP Membership Reports only to
    those ports where multicast routers are attached.
    [...]
    Sending membership reports to other hosts can result, for IGMPv1
    and IGMPv2, in unintentionally preventing a host from joining a
    specific multicast group.


Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>
---

diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
--- linux-3.0-rc2-orig/net/bridge/br_multicast.c	2011-06-09 13:34:04.164261031 +0900
+++ linux-3.0-rc2/net/bridge/br_multicast.c	2011-06-09 20:04:23.473930447 +0900
@@ -1424,7 +1424,7 @@ static int br_multicast_ipv4_rcv(struct
 	switch (ih->type) {
 	case IGMP_HOST_MEMBERSHIP_REPORT:
 	case IGMPV2_HOST_MEMBERSHIP_REPORT:
-		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
+		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
 		err = br_ip4_multicast_add_group(br, port, ih->group);
 		break;
 	case IGMPV3_HOST_MEMBERSHIP_REPORT:

Re: IGMP snooping: set mrouters_only flag for IPv4 traffic properly

[Stephen Hemminger]

On Mon, 13 Jun 2011 12:02:43 +0900
Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp> wrote:

> Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
> mrouters_only flag in a skb that may be a clone of the original skb, which
> means that sometimes the bridge loses track of membership report packets (cb
> buffers are tied to a specifici skb and not shared) and it ends up forwading
> join requests to the bridge interface.
> 
> This can cause unexpected membership timeouts and intermitent/permanent loss of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:
> 
>     A snooping switch should forward IGMP Membership Reports only to
>     those ports where multicast routers are attached.
>     [...]
>     Sending membership reports to other hosts can result, for IGMPv1
>     and IGMPv2, in unintentionally preventing a host from joining a
>     specific multicast group.
> 
> 
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>
> ---
> 
> diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
> --- linux-3.0-rc2-orig/net/bridge/br_multicast.c	2011-06-09 13:34:04.164261031 +0900
> +++ linux-3.0-rc2/net/bridge/br_multicast.c	2011-06-09 20:04:23.473930447 +0900
> @@ -1424,7 +1424,7 @@ static int br_multicast_ipv4_rcv(struct
>  	switch (ih->type) {
>  	case IGMP_HOST_MEMBERSHIP_REPORT:
>  	case IGMPV2_HOST_MEMBERSHIP_REPORT:
> -		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
> +		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>  		err = br_ip4_multicast_add_group(br, port, ih->group);
>  		break;
>  	case IGMPV3_HOST_MEMBERSHIP_REPORT:

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Re: IGMP snooping: set mrouters_only flag for IPv4 traffic properly

[Fernando Luis Vázquez Cao]

On Tue, 2011-06-14 at 13:22 -0400, Stephen Hemminger wrote:
> On Mon, 13 Jun 2011 12:02:43 +0900
> Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp> wrote:
> 
> > Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
> > mrouters_only flag in a skb that may be a clone of the original skb, which
> > means that sometimes the bridge loses track of membership report packets (cb
> > buffers are tied to a specifici skb and not shared) and it ends up forwading
> > join requests to the bridge interface.
> > 
> > This can cause unexpected membership timeouts and intermitent/permanent loss of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:
> > 
> >     A snooping switch should forward IGMP Membership Reports only to
> >     those ports where multicast routers are attached.
> >     [...]
> >     Sending membership reports to other hosts can result, for IGMPv1
> >     and IGMPv2, in unintentionally preventing a host from joining a
> >     specific multicast group.
> > 
> > 
> > Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> > Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>
> > ---
> > 
> > diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
> > --- linux-3.0-rc2-orig/net/bridge/br_multicast.c	2011-06-09 13:34:04.164261031 +0900
> > +++ linux-3.0-rc2/net/bridge/br_multicast.c	2011-06-09 20:04:23.473930447 +0900
> > @@ -1424,7 +1424,7 @@ static int br_multicast_ipv4_rcv(struct
> >  	switch (ih->type) {
> >  	case IGMP_HOST_MEMBERSHIP_REPORT:
> >  	case IGMPV2_HOST_MEMBERSHIP_REPORT:
> > -		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
> > +		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> >  		err = br_ip4_multicast_add_group(br, port, ih->group);
> >  		break;
> >  	case IGMPV3_HOST_MEMBERSHIP_REPORT:
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Can I take that as an acked-by for the IPv6 patch too?

Thanks,
Fernando

[PATCH 2/2] IGMP snooping: set mrouters_only flag for IPv6 traffic properly

[Fernando Luis Vazquez Cao]

Upon reception of a MGM report packet the kernel sets the mrouters_only flag
in a skb that is a clone of the original skb, which means that the bridge
loses track of MGM packets (cb buffers are tied to a specifici skb and not
shared) and it ends up forwading join requests to the bridge interface.

This can cause unexpected membership timeouts and intermitent/permanent loss
of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:

    A snooping switch should forward IGMP Membership Reports only to
    those ports where multicast routers are attached.
    [...]
    Sending membership reports to other hosts can result, for IGMPv1
    and IGMPv2, in unintentionally preventing a host from joining a
    specific multicast group.


Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
---

diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
--- linux-3.0-rc2-orig/net/bridge/br_multicast.c	2011-06-13 12:10:27.805554187 +0900
+++ linux-3.0-rc2/net/bridge/br_multicast.c	2011-06-13 12:12:27.591626915 +0900
@@ -1543,7 +1543,7 @@ static int br_multicast_ipv6_rcv(struct
 			goto out;
 		}
 		mld = (struct mld_msg *)skb_transport_header(skb2);
-		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
+		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
 		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);
 		break;
 	    }

Re: [PATCH 0/2] IGMP snooping: set mrouters_only flag properly

[Fernando Luis Vázquez Cao]

On Mon, 2011-06-13 at 11:59 +0900, Fernando Luis Vazquez Cao wrote:
> I am sending you a fix for a IGMP snooping bug that bit us when we tried
> to deploy it in production. This bug has been present since day one in
> the IGMP code and I think the fix deserves a backport to -stable
> (>=2.6.34).
> 
> The reason I am sending you separate patches for IPv4 and IPv6 is that I
> wanted to acknowledge Hayato Kakuta for his extensive testing of IPv4's
> IGMP code.

I fat fingered the subject of the first patch and found some typos. I
will be replying to this email with the updated patches. I am sorry for
the noise.

David, would you be picking up these fixes or should they go through
someone else's tree. Hopefully I got the mailing list right this time
around.

Thanks,
Fernando

[PATCH 1/2] IGMP snooping: set mrouters_only flag for IPv4 traffic properly

[Fernando Luis Vázquez Cao]

Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
mrouters_only flag in a skb that may be a clone of the original skb, which
means that sometimes the bridge loses track of membership report packets (cb
buffers are tied to a specific skb and not shared) and it ends up forwading
join requests to the bridge interface.

This can cause unexpected membership timeouts and intermitent/permanent loss
of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:

    A snooping switch should forward IGMP Membership Reports only to
    those ports where multicast routers are attached.
    [...]
    Sending membership reports to other hosts can result, for IGMPv1
    and IGMPv2, in unintentionally preventing a host from joining a
    specific multicast group.


Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>
---

diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
--- linux-3.0-rc2-orig/net/bridge/br_multicast.c	2011-06-09 13:34:04.164261031 +0900
+++ linux-3.0-rc2/net/bridge/br_multicast.c	2011-06-09 20:04:23.473930447 +0900
@@ -1424,7 +1424,7 @@ static int br_multicast_ipv4_rcv(struct
 	switch (ih->type) {
 	case IGMP_HOST_MEMBERSHIP_REPORT:
 	case IGMPV2_HOST_MEMBERSHIP_REPORT:
-		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
+		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
 		err = br_ip4_multicast_add_group(br, port, ih->group);
 		break;
 	case IGMPV3_HOST_MEMBERSHIP_REPORT:

Re: [PATCH 1/2] IGMP snooping: set mrouters_only flag for IPv4 traffic properly

[David Miller]

From: Fernando Luis Vázquez Cao <fernando@oss.ntt.co.jp>
Date: Tue, 14 Jun 2011 10:04:43 +0900

> Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
> mrouters_only flag in a skb that may be a clone of the original skb, which
> means that sometimes the bridge loses track of membership report packets (cb
> buffers are tied to a specific skb and not shared) and it ends up forwading
> join requests to the bridge interface.
> 
> This can cause unexpected membership timeouts and intermitent/permanent loss
> of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:
> 
>     A snooping switch should forward IGMP Membership Reports only to
>     those ports where multicast routers are attached.
>     [...]
>     Sending membership reports to other hosts can result, for IGMPv1
>     and IGMPv2, in unintentionally preventing a host from joining a
>     specific multicast group.
> 
> 
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>

Applied.

Re: [PATCH 1/2] IGMP snooping: set mrouters_only flag for IPv4 traffic properly

[Fernando Luis Vazquez Cao]

David Miller <davem@davemloft.net> wrote:
> From: Fernando Luis Vázquez Cao <fernando@oss.ntt.co.jp>
> Date: Tue, 14 Jun 2011 10:04:43 +0900
> 
> > Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
> > mrouters_only flag in a skb that may be a clone of the original skb, which
> > means that sometimes the bridge loses track of membership report packets (cb
> > buffers are tied to a specific skb and not shared) and it ends up forwading
> > join requests to the bridge interface.
> > 
> > This can cause unexpected membership timeouts and intermitent/permanent loss
> > of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:
> > 
> >     A snooping switch should forward IGMP Membership Reports only to
> >     those ports where multicast routers are attached.
> >     [...]
> >     Sending membership reports to other hosts can result, for IGMPv1
> >     and IGMPv2, in unintentionally preventing a host from joining a
> >     specific multicast group.
> > 
> > 
> > Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> > Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>
> 
> Applied.

Thank you David.

By the way, What kernel version(s) are you targeting for these two
patches. It would be great if we could get them upstream before 3.0
comes out. Without this fix the IGMP snooping code is simply unusable in
certain configurations.

- Fernando

[PATCH 2/2] IGMP snooping: set mrouters_only flag for IPv6 traffic properly

[Fernando Luis Vázquez Cao]

Upon reception of a MGM report packet the kernel sets the mrouters_only flag
in a skb that is a clone of the original skb, which means that the bridge
loses track of MGM packets (cb buffers are tied to a specific skb and not
shared) and it ends up forwading join requests to the bridge interface.

This can cause unexpected membership timeouts and intermitent/permanent loss
of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:

    A snooping switch should forward IGMP Membership Reports only to
    those ports where multicast routers are attached.
    [...]
    Sending membership reports to other hosts can result, for IGMPv1
    and IGMPv2, in unintentionally preventing a host from joining a
    specific multicast group.


Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
---

diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
--- linux-3.0-rc2-orig/net/bridge/br_multicast.c	2011-06-13 12:10:27.805554187 +0900
+++ linux-3.0-rc2/net/bridge/br_multicast.c	2011-06-13 12:12:27.591626915 +0900
@@ -1543,7 +1543,7 @@ static int br_multicast_ipv6_rcv(struct
 			goto out;
 		}
 		mld = (struct mld_msg *)skb_transport_header(skb2);
-		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
+		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
 		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);
 		break;
 	    }

Re: [PATCH 2/2] IGMP snooping: set mrouters_only flag for IPv6 traffic properly

[David Miller]

From: Fernando Luis Vázquez Cao <fernando@oss.ntt.co.jp>
Date: Tue, 14 Jun 2011 10:06:58 +0900

> Upon reception of a MGM report packet the kernel sets the mrouters_only flag
> in a skb that is a clone of the original skb, which means that the bridge
> loses track of MGM packets (cb buffers are tied to a specific skb and not
> shared) and it ends up forwading join requests to the bridge interface.
> 
> This can cause unexpected membership timeouts and intermitent/permanent loss
> of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:
> 
>     A snooping switch should forward IGMP Membership Reports only to
>     those ports where multicast routers are attached.
>     [...]
>     Sending membership reports to other hosts can result, for IGMPv1
>     and IGMPv2, in unintentionally preventing a host from joining a
>     specific multicast group.
> 
> 
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>

Applied.