Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[patch nf-next] IPVS: ICMPv6 checksum calculation

180 views
Skip to first unread message

Simon Horman

unread,
Aug 17, 2010, 3:10:02 AM8/17/10
to
From: Xiaoyu Du <ting...@gmail.com>

Correct the ICMPv6 checksum calculation when IPv6 is is used

Signed-off-by: Xiaoyu Du <ting...@gmail.com>
Signed-off-by: Simon Horman <ho...@verge.net.au>

---
net/netfilter/ipvs/ip_vs_core.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

I wonder if this is -stable or .35 material.

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 4f8ddba..124a1ae 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -638,8 +638,12 @@ void ip_vs_nat_icmp_v6(struct sk_buff *skb,
struct ip_vs_protocol *pp,

/* And finally the ICMP checksum */
icmph->icmp6_cksum = 0;
- /* TODO IPv6: is this correct for ICMPv6? */
ip_vs_checksum_complete(skb, icmp_offset);
+ icmph->icmp6_cksum = csum_ipv6_magic(&iph->saddr,
+ &iph->daddr,
+ skb->len - icmp_offset, IPPROTO_ICMPV6,
+ skb->csum);
+
skb->ip_summed = CHECKSUM_UNNECESSARY;

if (inout)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Julian Anastasov

unread,
Aug 17, 2010, 5:30:02 AM8/17/10
to

Hello,

On Tue, 17 Aug 2010, Simon Horman wrote:

> From: Xiaoyu Du <ting...@gmail.com>
>
> Correct the ICMPv6 checksum calculation when IPv6 is is used
>
> Signed-off-by: Xiaoyu Du <ting...@gmail.com>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> ---
> net/netfilter/ipvs/ip_vs_core.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> I wonder if this is -stable or .35 material.
>
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 4f8ddba..124a1ae 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -638,8 +638,12 @@ void ip_vs_nat_icmp_v6(struct sk_buff *skb,
> struct ip_vs_protocol *pp,
>
> /* And finally the ICMP checksum */
> icmph->icmp6_cksum = 0;
> - /* TODO IPv6: is this correct for ICMPv6? */

May be ip_vs_checksum_complete() is not needed here?
Also, skb->csum is not valid for every ip_summed value.
May be we need to apply CHECKSUM_PARTIAL kind of setup for the IPv6.
Such example is net/ipv6/udp.c:udp6_ufo_send_check().
Later dev_queue_xmit() and skb_checksum_help() should take care
for the next steps. Something like this can be tested:

icmph->icmp6_cksum = csum_ipv6_magic(&iph->saddr,

&iph->daddr,


skb->len - icmp_offset, IPPROTO_ICMPV6,

0);
skb->csum_start = skb_network_header(skb) - skb->head +
icmp_offset;
skb->csum_offset = offsetof(struct icmp6hdr, icmp6_cksum);
skb->ip_summed = CHECKSUM_PARTIAL;

> ip_vs_checksum_complete(skb, icmp_offset);
> + icmph->icmp6_cksum = csum_ipv6_magic(&iph->saddr,
> + &iph->daddr,
> + skb->len - icmp_offset, IPPROTO_ICMPV6,
> + skb->csum);
> +
> skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> if (inout)

Regards

--
Julian Anastasov <j...@ssi.bg>

Simon Horman

unread,
Aug 17, 2010, 8:30:02 PM8/17/10
to
[ CCing netdev for comment ]

David Miller

unread,
Aug 17, 2010, 8:30:02 PM8/17/10
to
From: Simon Horman <ho...@verge.net.au>
Date: Wed, 18 Aug 2010 09:21:49 +0900

> [ CCing netdev for comment ]
>
> On Tue, Aug 17, 2010 at 12:25:56PM +0300, Julian Anastasov wrote:

...


>> May be ip_vs_checksum_complete() is not needed here?
>> Also, skb->csum is not valid for every ip_summed value.
>> May be we need to apply CHECKSUM_PARTIAL kind of setup for the IPv6.
>> Such example is net/ipv6/udp.c:udp6_ufo_send_check().
>> Later dev_queue_xmit() and skb_checksum_help() should take care
>> for the next steps. Something like this can be tested:
>>
>> icmph->icmp6_cksum = csum_ipv6_magic(&iph->saddr,
>> &iph->daddr,
>> skb->len - icmp_offset, IPPROTO_ICMPV6,
>> 0);
>> skb->csum_start = skb_network_header(skb) - skb->head +
>> icmp_offset;
>> skb->csum_offset = offsetof(struct icmp6hdr, icmp6_cksum);
>> skb->ip_summed = CHECKSUM_PARTIAL;
>>

Yes, using CHECKSUM_PARTIAL unconditionally would work.

xiaoyu Du

unread,
Aug 18, 2010, 12:30:02 AM8/18/10
to
Thanks, I learned this. You gave me a big cake.


2010/8/18 David Miller <da...@davemloft.net>:

Simon Horman

unread,
Aug 18, 2010, 2:00:02 AM8/18/10
to
On Wed, Aug 18, 2010 at 12:20:14PM +0800, xiaoyu Du wrote:
> 2010/8/18 David Miller <da...@davemloft.net>:
> > From: Simon Horman <ho...@verge.net.au>
> > Date: Wed, 18 Aug 2010 09:21:49 +0900
> >
> >> [ CCing netdev for comment ]
> >>
> >> On Tue, Aug 17, 2010 at 12:25:56PM +0300, Julian Anastasov wrote:
> >  ...
> >>>      May be ip_vs_checksum_complete() is not needed here?
> >>> Also, skb->csum is not valid for every ip_summed value.
> >>> May be we need to apply CHECKSUM_PARTIAL kind of setup for the IPv6.
> >>> Such example is net/ipv6/udp.c:udp6_ufo_send_check().
> >>> Later dev_queue_xmit() and skb_checksum_help() should take care
> >>> for the next steps. Something like this can be tested:
> >>>
> >>>      icmph->icmp6_cksum = csum_ipv6_magic(&iph->saddr,
> >>>              &iph->daddr,
> >>>              skb->len - icmp_offset, IPPROTO_ICMPV6,
> >>>              0);
> >>>      skb->csum_start = skb_network_header(skb) - skb->head +
> >>>                        icmp_offset;
> >>>      skb->csum_offset = offsetof(struct icmp6hdr, icmp6_cksum);
> >>>      skb->ip_summed = CHECKSUM_PARTIAL;
> >>>
> >
> > Yes, using CHECKSUM_PARTIAL unconditionally would work.
> >
> Thanks, I learned this. You gave me a big cake.

[ please don't top-post ]

Xiaoyu, are you in a position to test the code proposed by Julian?

xiaoyu Du

unread,
Aug 20, 2010, 7:20:01 AM8/20/10
to
I just replied to thank for his idea then, as he gived me some ideas
on how to manipulate
checksum. I tested the code today, but it doesn't seem to work. (I just wrote a
netfilter module and hooked the code on local_out , then ping6 the
address on another PC,
It failed.). I'll find out why.

2010/8/18 Simon Horman <ho...@verge.net.au>:

Julian Anastasov

unread,
Aug 20, 2010, 9:50:01 AM8/20/10
to

Hello,

On Fri, 20 Aug 2010, xiaoyu Du wrote:

> I just replied to thank for his idea then, as he gived me some ideas
> on how to manipulate
> checksum. I tested the code today, but it doesn't seem to work. (I just wrote a
> netfilter module and hooked the code on local_out , then ping6 the
> address on another PC,
> It failed.). I'll find out why.

May be because I copied csum_ipv6_magic() code from
your patch. Please, try with one '~' before csum_ipv6_magic.

> >> >>>      icmph->icmp6_cksum = csum_ipv6_magic(&iph->saddr,
> >> >>>              &iph->daddr,
> >> >>>              skb->len - icmp_offset, IPPROTO_ICMPV6,
> >> >>>              0);
> >> >>>      skb->csum_start = skb_network_header(skb) - skb->head +
> >> >>>                        icmp_offset;
> >> >>>      skb->csum_offset = offsetof(struct icmp6hdr, icmp6_cksum);
> >> >>>      skb->ip_summed = CHECKSUM_PARTIAL;

Regards

--
Julian Anastasov <j...@ssi.bg>

xiaoyu Du

unread,
Aug 22, 2010, 11:00:02 PM8/22/10
to
Yes, I had added it and test it, it works.

2010/8/20 Julian Anastasov <j...@ssi.bg>:

Simon Horman

unread,
Aug 25, 2010, 4:10:02 AM8/25/10
to
Cc: Xiaoyu Du <ting...@gmail.com>
Cc: Julian Anastasov <j...@ssi.bg>
Signed-off-by: Simon Horman <ho...@verge.net.au>

--

Xiaoyu, is this the change that you tested?
If so could you give me a Tested-by line?

Julian, could I get an ack from you?
Alternatively, if you want to take ownership of the patch,
can I get a signed-off-by from you? In that case I'll change
the From accordingly.

Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c
===================================================================
--- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_core.c 2010-08-25 16:57:37.000000000 +0900
+++ nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c 2010-08-25 17:02:35.000000000 +0900
@@ -637,9 +637,11 @@ void ip_vs_nat_icmp_v6(struct sk_buff *s


}

/* And finally the ICMP checksum */

- icmph->icmp6_cksum = 0;


- /* TODO IPv6: is this correct for ICMPv6? */

- ip_vs_checksum_complete(skb, icmp_offset);
+ icmph->icmp6_cksum = ~csum_ipv6_magic(&iph->saddr, &iph->daddr,


+ skb->len - icmp_offset,

+ IPPROTO_ICMPV6, 0);
+ skb->csum_start = skb_network_header(skb) - skb->head + icmp_offset;
+ skb->csum_offset = offsetof(struct icmp6hdr, icmp6_cksum);
skb->ip_summed = CHECKSUM_UNNECESSARY;

if (inout)

Julian Anastasov

unread,
Aug 25, 2010, 4:00:02 PM8/25/10
to

Hello,

On Wed, 25 Aug 2010, Simon Horman wrote:

> Cc: Xiaoyu Du <ting...@gmail.com>
> Cc: Julian Anastasov <j...@ssi.bg>

Signed-off-by: Julian Anastasov <j...@ssi.bg>

should be enough

> Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> --
>
> Xiaoyu, is this the change that you tested?
> If so could you give me a Tested-by line?
>
> Julian, could I get an ack from you?
> Alternatively, if you want to take ownership of the patch,
> can I get a signed-off-by from you? In that case I'll change
> the From accordingly.
>
> Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c
> ===================================================================
> --- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_core.c 2010-08-25 16:57:37.000000000 +0900
> +++ nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c 2010-08-25 17:02:35.000000000 +0900
> @@ -637,9 +637,11 @@ void ip_vs_nat_icmp_v6(struct sk_buff *s
> }
>
> /* And finally the ICMP checksum */
> - icmph->icmp6_cksum = 0;
> - /* TODO IPv6: is this correct for ICMPv6? */
> - ip_vs_checksum_complete(skb, icmp_offset);
> + icmph->icmp6_cksum = ~csum_ipv6_magic(&iph->saddr, &iph->daddr,
> + skb->len - icmp_offset,
> + IPPROTO_ICMPV6, 0);
> + skb->csum_start = skb_network_header(skb) - skb->head + icmp_offset;
> + skb->csum_offset = offsetof(struct icmp6hdr, icmp6_cksum);
> skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> if (inout)
>

Regards

--
Julian Anastasov <j...@ssi.bg>

0 new messages