[patch nf-next] IPVS: ICMPv6 checksum calculation

[patch nf-next] IPVS: ICMPv6 checksum calculation

[Simon Horman]

From: Xiaoyu Du <tingsrain@gmail.com>

Correct the ICMPv6 checksum calculation when IPv6 is is used

Signed-off-by: Xiaoyu Du <tingsrain@gmail.com>
Signed-off-by: Simon Horman <horms@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)
--

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[Julian Anastasov]

	Hello,

On Tue, 17 Aug 2010, Simon Horman wrote:

> From: Xiaoyu Du <tingsrain@gmail.com>
> 
> Correct the ICMPv6 checksum calculation when IPv6 is is used
> 
> Signed-off-by: Xiaoyu Du <tingsrain@gmail.com>
> Signed-off-by: Simon Horman <horms@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 <ja@ssi.bg>

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[Simon Horman]

[ CCing netdev for comment ]

On Tue, Aug 17, 2010 at 12:25:56PM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Tue, 17 Aug 2010, Simon Horman wrote:
> 
> > From: Xiaoyu Du <tingsrain@gmail.com>
> > 
> > Correct the ICMPv6 checksum calculation when IPv6 is is used
> > 
> > Signed-off-by: Xiaoyu Du <tingsrain@gmail.com>
> > Signed-off-by: Simon Horman <horms@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 <ja@ssi.bg>

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[David Miller]

From: Simon Horman <horms@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.

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[xiaoyu Du]

Thanks, I learned this. You gave me a big cake.


2010/8/18 David Miller <davem@davemloft.net>:
> From: Simon Horman <horms@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.
>

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[Simon Horman]

On Wed, Aug 18, 2010 at 12:20:14PM +0800, xiaoyu Du wrote:
> 2010/8/18 David Miller <davem@davemloft.net>:
> > From: Simon Horman <horms@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?

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[xiaoyu Du]

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 <horms@verge.net.au>:
> On Wed, Aug 18, 2010 at 12:20:14PM +0800, xiaoyu Du wrote:
>> 2010/8/18 David Miller <davem@davemloft.net>:
>> > From: Simon Horman <horms@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?
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[Julian Anastasov]

	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 <ja@ssi.bg>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch nf-next] IPVS: ICMPv6 checksum calculation

[xiaoyu Du]

Yes, I had added it and test it, it works.

2010/8/20 Julian Anastasov <ja@ssi.bg>:
>
>        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 <ja@ssi.bg>
>