* IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
@ 2011-08-23 8:41 Ang Way Chuang
2011-08-23 9:57 ` Eric Dumazet
0 siblings, 1 reply; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-23 8:41 UTC (permalink / raw)
To: netdev
Hi all,
Sorry for cross-posting. I have a testbed setup on my VM environment that needs to use MLDv2 report message. My PIM router which is connected to a port on the bridge relies upon MLDv2 message to work. However, the setup failed to work after 2.6.39-rc2 or later. MLDv2 won't be forwarded to any slave ports bound to the bridge interface. Specifically, I track the cause down to this commit id:
ff9a57a62afbbe2d0f3a09af321f1fd7645f38a5
However, as soon as I disable multicast snooping on sysfs, MLDv2 message can be forwarded again.
I am not knowledgeable on the subject of multicast snooping, but is this right behaviour for multicast snooping? IPv4 doesn't seem to suffer the same fate though, IGMP can be forwarded even if multicast_snooping is enabled. Thank you in advance.
Regards,
Ang Way Chuang
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-23 8:41 IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Ang Way Chuang
@ 2011-08-23 9:57 ` Eric Dumazet
2011-08-23 12:31 ` Ang Way Chuang
0 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2011-08-23 9:57 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev, Linus Lüssing, Herbert Xu
Le mardi 23 août 2011 à 17:41 +0900, Ang Way Chuang a écrit :
> Hi all,
>
> Sorry for cross-posting. I have a testbed setup on my VM
> environment that needs to use MLDv2 report message. My PIM router
> which is connected to a port on the bridge relies upon MLDv2 message
> to work. However, the setup failed to work after 2.6.39-rc2 or later.
> MLDv2 won't be forwarded to any slave ports bound to the bridge
> interface. Specifically, I track the cause down to this commit id:
>
> ff9a57a62afbbe2d0f3a09af321f1fd7645f38a5
>
> However, as soon as I disable multicast snooping on sysfs, MLDv2
> message can be forwarded again.
>
> I am not knowledgeable on the subject of multicast snooping, but
> is this right behaviour for multicast snooping? IPv4 doesn't seem to
> suffer the same fate though, IGMP can be forwarded even if
> multicast_snooping is enabled. Thank you in advance.
>
Thanks very much for the report and your tracking.
Added some CC
I wonder if the ff9a57a62afb commit doesnt uncover a prior bug, could
you try the following patch ?
Thanks !
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2d85ca7..51e3b2d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1520,6 +1520,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
err = pskb_trim_rcsum(skb2, len);
if (err)
goto out;
+ icmp6h = icmp6_hdr(skb2);
}
switch (skb2->ip_summed) {
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-23 9:57 ` Eric Dumazet
@ 2011-08-23 12:31 ` Ang Way Chuang
2011-08-23 13:10 ` Eric Dumazet
0 siblings, 1 reply; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-23 12:31 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: Linus Lüssing, Herbert Xu
Sorry for the blurb. I hope I've configured thunderbird properly to send in plain text. Hope you received my reply. I tried your patch on 3.0.3 and it didn't fix the problem.
Regards,
Ang
On 23/08/11 18:57, Eric Dumazet wrote:
> Le mardi 23 août 2011 à 17:41 +0900, Ang Way Chuang a écrit :
>> Hi all,
>>
>> Sorry for cross-posting. I have a testbed setup on my VM
>> environment that needs to use MLDv2 report message. My PIM router
>> which is connected to a port on the bridge relies upon MLDv2 message
>> to work. However, the setup failed to work after 2.6.39-rc2 or later.
>> MLDv2 won't be forwarded to any slave ports bound to the bridge
>> interface. Specifically, I track the cause down to this commit id:
>>
>> ff9a57a62afbbe2d0f3a09af321f1fd7645f38a5
>>
>> However, as soon as I disable multicast snooping on sysfs, MLDv2
>> message can be forwarded again.
>>
>> I am not knowledgeable on the subject of multicast snooping, but
>> is this right behaviour for multicast snooping? IPv4 doesn't seem to
>> suffer the same fate though, IGMP can be forwarded even if
>> multicast_snooping is enabled. Thank you in advance.
>>
> Thanks very much for the report and your tracking.
>
> Added some CC
>
> I wonder if the ff9a57a62afb commit doesnt uncover a prior bug, could
> you try the following patch ?
>
> Thanks !
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2d85ca7..51e3b2d 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1520,6 +1520,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = pskb_trim_rcsum(skb2, len);
> if (err)
> goto out;
> + icmp6h = icmp6_hdr(skb2);
> }
>
> switch (skb2->ip_summed) {
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-23 12:31 ` Ang Way Chuang
@ 2011-08-23 13:10 ` Eric Dumazet
2011-08-23 14:04 ` Ang Way Chuang
0 siblings, 1 reply; 13+ messages in thread
From: Eric Dumazet @ 2011-08-23 13:10 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev, Linus Lüssing, Herbert Xu
Le mardi 23 août 2011 à 21:31 +0900, Ang Way Chuang a écrit :
> Sorry for the blurb. I hope I've configured thunderbird properly to
> send in plain text. Hope you received my reply. I tried your patch on
> 3.0.3 and it didn't fix the problem.
Thanks for trying !
Ok could you please try following patch then ?
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2d85ca7..e9b32a3 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1497,7 +1497,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
if (!pskb_may_pull(skb2, offset + sizeof(struct icmp6hdr)))
goto out;
- len -= offset - skb_network_offset(skb2);
+ len -= offset;
__skb_pull(skb2, offset);
skb_reset_transport_header(skb2);
@@ -1520,6 +1520,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
err = pskb_trim_rcsum(skb2, len);
if (err)
goto out;
+ icmp6h = icmp6_hdr(skb2);
}
switch (skb2->ip_summed) {
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-23 13:10 ` Eric Dumazet
@ 2011-08-23 14:04 ` Ang Way Chuang
2011-08-23 17:24 ` Ang Way Chuang
0 siblings, 1 reply; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-23 14:04 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Linus Lüssing, Herbert Xu
Tough luck :( Got no love from bridge. It still doesn't work with your latest patch. Please keep sending those patches.
On 23/08/11 22:10, Eric Dumazet wrote:
> Le mardi 23 août 2011 à 21:31 +0900, Ang Way Chuang a écrit :
>> Sorry for the blurb. I hope I've configured thunderbird properly to
>> send in plain text. Hope you received my reply. I tried your patch on
>> 3.0.3 and it didn't fix the problem.
>
> Thanks for trying !
>
> Ok could you please try following patch then ?
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2d85ca7..e9b32a3 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1497,7 +1497,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> if (!pskb_may_pull(skb2, offset + sizeof(struct icmp6hdr)))
> goto out;
>
> - len -= offset - skb_network_offset(skb2);
> + len -= offset;
>
> __skb_pull(skb2, offset);
> skb_reset_transport_header(skb2);
> @@ -1520,6 +1520,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = pskb_trim_rcsum(skb2, len);
> if (err)
> goto out;
> + icmp6h = icmp6_hdr(skb2);
> }
>
> switch (skb2->ip_summed) {
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-23 14:04 ` Ang Way Chuang
@ 2011-08-23 17:24 ` Ang Way Chuang
2011-08-24 4:56 ` [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD Ang Way Chuang
2011-08-24 7:04 ` IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Yan, Zheng
0 siblings, 2 replies; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-23 17:24 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Linus Lüssing, Herbert Xu
This is what I found so far from debugging.
The packet is not forwarded due to the failed checksum at br_multicast.c:1533
case CHECKSUM_NONE:
skb2->csum = 0;
if (skb_checksum_complete(skb2))
goto out;
}
Contrary to description of commit ff9a57a6, when the patch of commit ff9a57a6 is applied,
pskb_trim_rcsum is never called at all on my testbed. When commit ff9a57a6 is reverted,
pskb_trim_rcsum will be called. The difference is:
with commit ff9a57a6,
pskb_trim_rcsum is never called, br_multicast_ipv6_rcv returns -EINVAL which causes
br_handle_frame_finish to drop the packet
without commit ff9a57a6,
pskb_trim_rcsum is called overwriting err with 0. br_multicast_ipv6_rcv still fails on the
same line (skb_checksum_complete). But the difference is err is set to 0 this time. Thereby,
allowing the packet to be forwarded.
Anyway, I don't think the behaviour is correct with or without commit ff9a57a6
On 23/08/11 23:04, Ang Way Chuang wrote:
> Tough luck :( Got no love from bridge. It still doesn't work with your latest patch. Please keep sending those patches.
>
> On 23/08/11 22:10, Eric Dumazet wrote:
>> Le mardi 23 août 2011 à 21:31 +0900, Ang Way Chuang a écrit :
>>> Sorry for the blurb. I hope I've configured thunderbird properly to
>>> send in plain text. Hope you received my reply. I tried your patch on
>>> 3.0.3 and it didn't fix the problem.
>>
>> Thanks for trying !
>>
>> Ok could you please try following patch then ?
>>
>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>> index 2d85ca7..e9b32a3 100644
>> --- a/net/bridge/br_multicast.c
>> +++ b/net/bridge/br_multicast.c
>> @@ -1497,7 +1497,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>> if (!pskb_may_pull(skb2, offset + sizeof(struct icmp6hdr)))
>> goto out;
>>
>> - len -= offset - skb_network_offset(skb2);
>> + len -= offset;
>>
>> __skb_pull(skb2, offset);
>> skb_reset_transport_header(skb2);
>> @@ -1520,6 +1520,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>> err = pskb_trim_rcsum(skb2, len);
>> if (err)
>> goto out;
>> + icmp6h = icmp6_hdr(skb2);
>> }
>>
>> switch (skb2->ip_summed) {
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD
2011-08-23 17:24 ` Ang Way Chuang
@ 2011-08-24 4:56 ` Ang Way Chuang
2011-08-24 5:06 ` David Miller
2011-08-24 7:04 ` IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Yan, Zheng
1 sibling, 1 reply; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-24 4:56 UTC (permalink / raw)
To: netdev
Cc: Eric Dumazet, Linus Lüssing, Herbert Xu, yoshfuji,
Stephen Hemminger
Checksum of ICMPv6 is not properly computed because the pseudo header is not used.
Thus, the MLD packet gets dropped by the bridge. This patch fixes the problem for my
testbed. This bug was made visible by commit ff9a57a62afbbe2d0f3a09af321f1fd7645f38a.
This patch has been tested on 3.0.3. Due to lack of understanding on the checksum
optimization and multicast snooping of the kernel stack, can someone please verify the
correctness of this patch?
Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
---
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2d85ca7..bbf361b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1528,9 +1528,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
break;
/*FALLTHROUGH*/
case CHECKSUM_NONE:
- skb2->csum = 0;
- if (skb_checksum_complete(skb2))
+ if (!skb_csum_unnecessary(skb2) && csum_ipv6_magic(&ip6h->saddr,
+ &ip6h->daddr, skb2->len, IPPROTO_ICMPV6,
+ skb_checksum(skb2, 0, skb2->len, 0))) {
+ err = -EINVAL;
goto out;
+ }
}
err = 0;
---
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD
2011-08-24 4:56 ` [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD Ang Way Chuang
@ 2011-08-24 5:06 ` David Miller
0 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2011-08-24 5:06 UTC (permalink / raw)
To: wcang; +Cc: netdev, eric.dumazet, linus.luessing, herbert, yoshfuji, shemminger
From: Ang Way Chuang <wcang@sfc.wide.ad.jp>
Date: Wed, 24 Aug 2011 13:56:13 +0900
> @@ -1528,9 +1528,12 @@ static int br_multicast_ipv6_rcv(struct
> net_bridge *br,
> break;
> /*FALLTHROUGH*/
> case CHECKSUM_NONE:
> - skb2->csum = 0;
> - if (skb_checksum_complete(skb2))
> + if (!skb_csum_unnecessary(skb2) && csum_ipv6_magic(&ip6h->saddr,
> + &ip6h->daddr, skb2->len, IPPROTO_ICMPV6,
Patch has been significantly corrupted by your email client, making
it unusable for us.
Please read linux/Documentation/email-clients.txt, email a test patch
to yourself, and only resend your patch to this list once you can
successfully apply such a patch yourself.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-23 17:24 ` Ang Way Chuang
2011-08-24 4:56 ` [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD Ang Way Chuang
@ 2011-08-24 7:04 ` Yan, Zheng
2011-08-24 7:22 ` Eric Dumazet
2011-08-24 7:38 ` Ang Way Chuang
1 sibling, 2 replies; 13+ messages in thread
From: Yan, Zheng @ 2011-08-24 7:04 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: Eric Dumazet, netdev, Linus Lüssing, Herbert Xu
On Wed, Aug 24, 2011 at 1:24 AM, Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> This is what I found so far from debugging.
>
> The packet is not forwarded due to the failed checksum at
> br_multicast.c:1533
>
> case CHECKSUM_NONE:
> skb2->csum = 0;
> if (skb_checksum_complete(skb2))
> goto out;
> }
>
> Contrary to description of commit ff9a57a6, when the patch of commit
> ff9a57a6 is applied,
> pskb_trim_rcsum is never called at all on my testbed. When commit ff9a57a6
> is reverted,
> pskb_trim_rcsum will be called. The difference is:
>
> with commit ff9a57a6,
> pskb_trim_rcsum is never called, br_multicast_ipv6_rcv returns -EINVAL
> which causes
> br_handle_frame_finish to drop the packet
>
> without commit ff9a57a6,
> pskb_trim_rcsum is called overwriting err with 0. br_multicast_ipv6_rcv
> still fails on the
> same line (skb_checksum_complete). But the difference is err is set to 0
> this time. Thereby,
> allowing the packet to be forwarded.
>
> Anyway, I don't think the behaviour is correct with or without commit
> ff9a57a6
>
>
Looks like a checksum calculation bug. Please try below patch, Thanks.
---
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2d85ca7..22d2d1a 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1520,16 +1520,23 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
err = pskb_trim_rcsum(skb2, len);
if (err)
goto out;
+ err = -EINVAL;
}
+ ip6h = ipv6_hdr(skb2);
+
switch (skb2->ip_summed) {
case CHECKSUM_COMPLETE:
- if (!csum_fold(skb2->csum))
+ if (!csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr, skb2->len,
+ IPPROTO_ICMPV6, skb2->csum))
break;
/*FALLTHROUGH*/
case CHECKSUM_NONE:
- skb2->csum = 0;
- if (skb_checksum_complete(skb2))
+ skb2->csum = ~csum_unfold(csum_ipv6_magic(&ip6h->saddr,
+ &ip6h->daddr,
+ skb2->len,
+ IPPROTO_ICMPV6, 0));
+ if (__skb_checksum_complete(skb2))
goto out;
}
---
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-24 7:04 ` IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Yan, Zheng
@ 2011-08-24 7:22 ` Eric Dumazet
2011-08-24 7:38 ` Ang Way Chuang
1 sibling, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2011-08-24 7:22 UTC (permalink / raw)
To: Yan, Zheng; +Cc: Ang Way Chuang, netdev, Linus Lüssing, Herbert Xu
Le mercredi 24 août 2011 à 15:04 +0800, Yan, Zheng a écrit :
> On Wed, Aug 24, 2011 at 1:24 AM, Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> > This is what I found so far from debugging.
> >
> > The packet is not forwarded due to the failed checksum at
> > br_multicast.c:1533
> >
> > case CHECKSUM_NONE:
> > skb2->csum = 0;
> > if (skb_checksum_complete(skb2))
> > goto out;
> > }
> >
> > Contrary to description of commit ff9a57a6, when the patch of commit
> > ff9a57a6 is applied,
> > pskb_trim_rcsum is never called at all on my testbed. When commit ff9a57a6
> > is reverted,
> > pskb_trim_rcsum will be called. The difference is:
> >
> > with commit ff9a57a6,
> > pskb_trim_rcsum is never called, br_multicast_ipv6_rcv returns -EINVAL
> > which causes
> > br_handle_frame_finish to drop the packet
> >
> > without commit ff9a57a6,
> > pskb_trim_rcsum is called overwriting err with 0. br_multicast_ipv6_rcv
> > still fails on the
> > same line (skb_checksum_complete). But the difference is err is set to 0
> > this time. Thereby,
> > allowing the packet to be forwarded.
> >
> > Anyway, I don't think the behaviour is correct with or without commit
> > ff9a57a6
> >
> >
>
> Looks like a checksum calculation bug. Please try below patch, Thanks.
>
> ---
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2d85ca7..22d2d1a 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1520,16 +1520,23 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = pskb_trim_rcsum(skb2, len);
> if (err)
> goto out;
> + err = -EINVAL;
> }
>
> + ip6h = ipv6_hdr(skb2);
> +
> switch (skb2->ip_summed) {
> case CHECKSUM_COMPLETE:
> - if (!csum_fold(skb2->csum))
> + if (!csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr, skb2->len,
> + IPPROTO_ICMPV6, skb2->csum))
> break;
> /*FALLTHROUGH*/
> case CHECKSUM_NONE:
> - skb2->csum = 0;
> - if (skb_checksum_complete(skb2))
> + skb2->csum = ~csum_unfold(csum_ipv6_magic(&ip6h->saddr,
> + &ip6h->daddr,
> + skb2->len,
> + IPPROTO_ICMPV6, 0));
> + if (__skb_checksum_complete(skb2))
> goto out;
> }
>
> ---
We also could reuse/factorize code, say the one from icmpv6_rcv()
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: IPv6 multicast snooping behaviour on 2.6.39-rc2 and later
2011-08-24 7:04 ` IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Yan, Zheng
2011-08-24 7:22 ` Eric Dumazet
@ 2011-08-24 7:38 ` Ang Way Chuang
1 sibling, 0 replies; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-24 7:38 UTC (permalink / raw)
To: Yan, Zheng ; +Cc: Eric Dumazet, netdev, Linus Lüssing, Herbert Xu
Thanks Zheng Yan. Your patch works as well. I trust others' code more than mine,
so please ignore the patch that I sent earlier.
On 24/08/11 16:04, Yan, Zheng wrote:
> On Wed, Aug 24, 2011 at 1:24 AM, Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
>> This is what I found so far from debugging.
>>
>> The packet is not forwarded due to the failed checksum at
>> br_multicast.c:1533
>>
>> case CHECKSUM_NONE:
>> skb2->csum = 0;
>> if (skb_checksum_complete(skb2))
>> goto out;
>> }
>>
>> Contrary to description of commit ff9a57a6, when the patch of commit
>> ff9a57a6 is applied,
>> pskb_trim_rcsum is never called at all on my testbed. When commit ff9a57a6
>> is reverted,
>> pskb_trim_rcsum will be called. The difference is:
>>
>> with commit ff9a57a6,
>> pskb_trim_rcsum is never called, br_multicast_ipv6_rcv returns -EINVAL
>> which causes
>> br_handle_frame_finish to drop the packet
>>
>> without commit ff9a57a6,
>> pskb_trim_rcsum is called overwriting err with 0. br_multicast_ipv6_rcv
>> still fails on the
>> same line (skb_checksum_complete). But the difference is err is set to 0
>> this time. Thereby,
>> allowing the packet to be forwarded.
>>
>> Anyway, I don't think the behaviour is correct with or without commit
>> ff9a57a6
>>
>>
> Looks like a checksum calculation bug. Please try below patch, Thanks.
>
> ---
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2d85ca7..22d2d1a 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1520,16 +1520,23 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = pskb_trim_rcsum(skb2, len);
> if (err)
> goto out;
> + err = -EINVAL;
> }
>
> + ip6h = ipv6_hdr(skb2);
> +
> switch (skb2->ip_summed) {
> case CHECKSUM_COMPLETE:
> - if (!csum_fold(skb2->csum))
> + if (!csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr, skb2->len,
> + IPPROTO_ICMPV6, skb2->csum))
> break;
> /*FALLTHROUGH*/
> case CHECKSUM_NONE:
> - skb2->csum = 0;
> - if (skb_checksum_complete(skb2))
> + skb2->csum = ~csum_unfold(csum_ipv6_magic(&ip6h->saddr,
> + &ip6h->daddr,
> + skb2->len,
> + IPPROTO_ICMPV6, 0));
> + if (__skb_checksum_complete(skb2))
> goto out;
> }
>
> ---
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD
2011-08-24 6:10 [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD Ang Way Chuang
@ 2011-08-24 7:18 ` Eric Dumazet
0 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2011-08-24 7:18 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev
Le mercredi 24 août 2011 à 15:10 +0900, Ang Way Chuang a écrit :
> Just saw Eric's patch for icmp6_type. Suspect this patch may have issue for reference
> of ip6h after pskb_trim_rcsum()?
>
I dont think so, because ip6h is against skb header, not skb2 one.
> Checksum of ICMPv6 is not properly computed because the pseudo header is not used.
> Thus, the MLD packet gets dropped by the bridge. This patch fixes the problem for my
> testbed. This bug was made visible by commit ff9a57a62afbbe2d0f3a09af321f1fd7645f38a.
>
> This patch has been tested on 3.0.3. Due to lack of understanding on the checksum
> optimization and multicast snooping of the kernel stack, can someone please verify the
> correctness of this patch?
>
> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
> ---
> index 2d85ca7..bbf361b 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1528,9 +1528,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> break;
> /*FALLTHROUGH*/
> case CHECKSUM_NONE:
> - skb2->csum = 0;
> - if (skb_checksum_complete(skb2))
> + if (!skb_csum_unnecessary(skb2) && csum_ipv6_magic(&ip6h->saddr,
> + &ip6h->daddr, skb2->len, IPPROTO_ICMPV6,
> + skb_checksum(skb2, 0, skb2->len, 0))) {
> + err = -EINVAL;
> goto out;
> + }
> }
>
> err = 0;
> ---
Sorry, I cannot comment on this, checksum games are hard :(
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD
@ 2011-08-24 6:10 Ang Way Chuang
2011-08-24 7:18 ` Eric Dumazet
0 siblings, 1 reply; 13+ messages in thread
From: Ang Way Chuang @ 2011-08-24 6:10 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet
Just saw Eric's patch for icmp6_type. Suspect this patch may have issue for reference
of ip6h after pskb_trim_rcsum()?
Checksum of ICMPv6 is not properly computed because the pseudo header is not used.
Thus, the MLD packet gets dropped by the bridge. This patch fixes the problem for my
testbed. This bug was made visible by commit ff9a57a62afbbe2d0f3a09af321f1fd7645f38a.
This patch has been tested on 3.0.3. Due to lack of understanding on the checksum
optimization and multicast snooping of the kernel stack, can someone please verify the
correctness of this patch?
Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
---
index 2d85ca7..bbf361b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1528,9 +1528,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
break;
/*FALLTHROUGH*/
case CHECKSUM_NONE:
- skb2->csum = 0;
- if (skb_checksum_complete(skb2))
+ if (!skb_csum_unnecessary(skb2) && csum_ipv6_magic(&ip6h->saddr,
+ &ip6h->daddr, skb2->len, IPPROTO_ICMPV6,
+ skb_checksum(skb2, 0, skb2->len, 0))) {
+ err = -EINVAL;
goto out;
+ }
}
err = 0;
---
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-08-24 7:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-23 8:41 IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Ang Way Chuang
2011-08-23 9:57 ` Eric Dumazet
2011-08-23 12:31 ` Ang Way Chuang
2011-08-23 13:10 ` Eric Dumazet
2011-08-23 14:04 ` Ang Way Chuang
2011-08-23 17:24 ` Ang Way Chuang
2011-08-24 4:56 ` [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD Ang Way Chuang
2011-08-24 5:06 ` David Miller
2011-08-24 7:04 ` IPv6 multicast snooping behaviour on 2.6.39-rc2 and later Yan, Zheng
2011-08-24 7:22 ` Eric Dumazet
2011-08-24 7:38 ` Ang Way Chuang
2011-08-24 6:10 [PATCH] bridge: Pseudo-header required for the checksum of ICMP6 header of MLD Ang Way Chuang
2011-08-24 7:18 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.