From: Xiaoyu Du <[email protected]>
Correct the ICMPv6 checksum calculation when IPv6 is is used
Signed-off-by: Xiaoyu Du <[email protected]>
Signed-off-by: Simon Horman <[email protected]>
---
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)
--
Hello,
On Tue, 17 Aug 2010, Simon Horman wrote:
> From: Xiaoyu Du <[email protected]>
>
> Correct the ICMPv6 checksum calculation when IPv6 is is used
>
> Signed-off-by: Xiaoyu Du <[email protected]>
> Signed-off-by: Simon Horman <[email protected]>
>
> ---
> 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 <[email protected]>
[ 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 <[email protected]>
> >
> > Correct the ICMPv6 checksum calculation when IPv6 is is used
> >
> > Signed-off-by: Xiaoyu Du <[email protected]>
> > Signed-off-by: Simon Horman <[email protected]>
> >
> > ---
> > 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 <[email protected]>
From: Simon Horman <[email protected]>
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.
2010/8/18 David Miller <[email protected]>:
> From: Simon Horman <[email protected]>
> 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.
>
On Wed, Aug 18, 2010 at 12:20:14PM +0800, xiaoyu Du wrote:
> 2010/8/18 David Miller <[email protected]>:
> > From: Simon Horman <[email protected]>
> > 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?
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 <[email protected]>:
> On Wed, Aug 18, 2010 at 12:20:14PM +0800, xiaoyu Du wrote:
>> 2010/8/18 David Miller <[email protected]>:
>> > From: Simon Horman <[email protected]>
>> > 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?
>
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 <[email protected]>
Yes, I had added it and test it, it works.
2010/8/20 Julian Anastasov <[email protected]>:
>
> 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 <[email protected]>
>