Message ID | 1301207244-10428-2-git-send-email-linus.luessing@web.de |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Sun, Mar 27, 2011 at 08:27:23AM +0200, Linus Lüssing wrote: > In contrast to IGMP, the MLDv1/2 message checksum needs to include an > IPv6 "pseudo-header" in the calculations (see RFC2710, section 3.3; > RFC3810, section 5.1.2). > > The multicast snooping feature of the bridge code however did not take > this "pseudo-header" into consideration for the checksum validation when > parsing a snooped IPv6 MLDv1/2 message of another host, leading to > possibly ignored, though valid MLDv1/2 messages. This commit shall fix > this issue. > > Signed-off-by: Linus Lüssing <linus.luessing@web.de> > --- > net/bridge/br_multicast.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index f61eb2e..47fae4f 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -1525,7 +1525,10 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, > break; > /*FALLTHROUGH*/ > case CHECKSUM_NONE: > - skb2->csum = 0; > + skb2->csum = ~csum_unfold(csum_ipv6_magic(&ip6h->saddr, > + &ip6h->daddr, > + skb2->len, > + nexthdr, 0)); You also need to include the pseudo-header for the CHECKSUM_COMPLETE case, before we've fallen through (which only happens if the hardware checksum doesn't match). Thanks,
So instead of 'if (!csum_fold(skb2->csum))' it should be this? --- if (!csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr, skb2->len, nexthdr, skb2->csum)) --- (I'm peeking a little bit at http://lxr.linux.no/linux+*/net/ipv6/netfilter.c#L98) Hmm, if so, then I don't know how to test and verify that now though. Cheers, Linus On Sun, Mar 27, 2011 at 02:37:49PM +0800, Herbert Xu wrote: > On Sun, Mar 27, 2011 at 08:27:23AM +0200, Linus Lüssing wrote: > > In contrast to IGMP, the MLDv1/2 message checksum needs to include an > > IPv6 "pseudo-header" in the calculations (see RFC2710, section 3.3; > > RFC3810, section 5.1.2). > > > > The multicast snooping feature of the bridge code however did not take > > this "pseudo-header" into consideration for the checksum validation when > > parsing a snooped IPv6 MLDv1/2 message of another host, leading to > > possibly ignored, though valid MLDv1/2 messages. This commit shall fix > > this issue. > > > > Signed-off-by: Linus Lüssing <linus.luessing@web.de> > > --- > > net/bridge/br_multicast.c | 5 ++++- > > 1 files changed, 4 insertions(+), 1 deletions(-) > > > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > > index f61eb2e..47fae4f 100644 > > --- a/net/bridge/br_multicast.c > > +++ b/net/bridge/br_multicast.c > > @@ -1525,7 +1525,10 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, > > break; > > /*FALLTHROUGH*/ > > case CHECKSUM_NONE: > > - skb2->csum = 0; > > + skb2->csum = ~csum_unfold(csum_ipv6_magic(&ip6h->saddr, > > + &ip6h->daddr, > > + skb2->len, > > + nexthdr, 0)); > > You also need to include the pseudo-header for the CHECKSUM_COMPLETE > case, before we've fallen through (which only happens if the > hardware checksum doesn't match). > > Thanks, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt > -- 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
On Sun, Mar 27, 2011 at 09:06:45AM +0200, Linus Lüssing wrote: > So instead of 'if (!csum_fold(skb2->csum))' it should be this? > --- > if (!csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr, > skb2->len, nexthdr, skb2->csum)) > --- > (I'm peeking a little bit at > http://lxr.linux.no/linux+*/net/ipv6/netfilter.c#L98) Yes that looks about right. > Hmm, if so, then I don't know how to test and verify that now though. You'd need a NIC that supported this to test. Although I wouldn't worry about it too much as the same code patterns appears over and over again in our stack. Perhaps we should put it in a helper. Thanks,
From: Linus Lüssing <linus.luessing@web.de> Date: Sun, 27 Mar 2011 08:27:23 +0200 > In contrast to IGMP, the MLDv1/2 message checksum needs to include an > IPv6 "pseudo-header" in the calculations (see RFC2710, section 3.3; > RFC3810, section 5.1.2). > > The multicast snooping feature of the bridge code however did not take > this "pseudo-header" into consideration for the checksum validation when > parsing a snooped IPv6 MLDv1/2 message of another host, leading to > possibly ignored, though valid MLDv1/2 messages. This commit shall fix > this issue. > > Signed-off-by: Linus Lüssing <linus.luessing@web.de> I'm waiting for am updated version of this patch which addresses Herbert Xu's feedback. -- 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
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index f61eb2e..47fae4f 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1525,7 +1525,10 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, break; /*FALLTHROUGH*/ case CHECKSUM_NONE: - skb2->csum = 0; + skb2->csum = ~csum_unfold(csum_ipv6_magic(&ip6h->saddr, + &ip6h->daddr, + skb2->len, + nexthdr, 0)); if (skb_checksum_complete(skb2)) goto out; }
In contrast to IGMP, the MLDv1/2 message checksum needs to include an IPv6 "pseudo-header" in the calculations (see RFC2710, section 3.3; RFC3810, section 5.1.2). The multicast snooping feature of the bridge code however did not take this "pseudo-header" into consideration for the checksum validation when parsing a snooped IPv6 MLDv1/2 message of another host, leading to possibly ignored, though valid MLDv1/2 messages. This commit shall fix this issue. Signed-off-by: Linus Lüssing <linus.luessing@web.de> --- net/bridge/br_multicast.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)