From: Praveen Chaudhary <praveen5582@gmail.com>
To: fw@strlen.de
Cc: astracner@linkedin.com, davem@davemloft.net,
kadlec@netfilter.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, pablo@netfilter.org,
praveen5582@gmail.com, zxu@linkedin.com
Subject: RE: [netfilter]: Fix skb->csum calculation when netfilter
Date: Wed, 23 Oct 2019 22:19:33 -0700 [thread overview]
Message-ID: <1571894373-11188-1-git-send-email-pchaudhary@linkedin.com> (raw)
In-Reply-To: <20191024011218.GT25052@breakpoint.cc>
Hi Florian
Thanks for giving time for review. This fix is pretty important for SONiC OS (https://azure.github.io/SONiC/). So I really appreciate it.
>> inet_proto_csum_replace16 is called from many places, whereas this fix is
>> applicable only for nf_nat_ipv6_csum_update. where we need to update
>> skb->csum for ipv6 src/dst address change.
>
>Under which circumstances does inet_proto_csum_replace16 upate
>skb->csum correctly?
inet_proto_csum_replace16 calculates skb->csum correctly if skb->csum does not include 16-bit sum of IPv6 Header i.e skb->data points to UDP\TCP\ICMPv6 header while calling __skb_checksum_complete() on packet. Function inet_proto_csum_replace16 is called from nf_nat_ipv6_csum_update(), nf_flow_nat_ipv6_tcp(), nf_flow_nat_ipv6_udp() and update_ipv6_checksum(). For all nf_*() functions, inet_proto_csum_replace16() will not update skb->csum correctly\completely.
But I am not sure about update_ipv6_checksum() (in net/openvswitch/actions.c). This is where I seek help from experts. If even for update_ipv6_checksum(), skb->csum includes 16-bit sum of IPv6 Header then inet_proto_csum_replace16() does updates skb->csum correctly. Then our fix will be to remove below line from this function. Because change in UDP\TCP\ICMPv6 header checksum field and change in IPv6 SRC\DST address cancels each other for checksum calculation, i.e. no update to skb->csum is needed.
```
if (skb->ip_summed == CHECKSUM_COMPLETE && pseudohdr)
skb->csum = ~csum_partial(diff, sizeof(diff),
~skb->csum);
```
>
>> So, I added a new function. Basically, I used a safe approach to fix it,
>> without impacting other cases. Let me know other options, I am open to
>> suggestions.
>
>You seem to imply inet_proto_csum_replace16 is fine and only broken for ipv6
>nat.
Yeah, as mentioned above, I took safe approach to fix only nf_nat_ipv6_csum_update() part, where I am sure that skb->csum is broken. But I am not sure if (net/openvswitch/actions.c) needs this fix or not. Consider this my lack of expertise, So kindly suggest whether net/openvswitch/actions.c needs this fix or not. Note: I may not be able to test this part. After your suggestion, I will change my patch. Again Thanks for your time.
prev parent reply other threads:[~2019-10-24 5:20 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-23 19:02 [PATCH] Fix skb->csum calculation when netfilter manipulation for NF_NAT_MANIP_SRC\DST is done on IPV6 packet Praveen Chaudhary
2019-10-23 19:02 ` [PATCH] [netfilter]: " Praveen Chaudhary
2019-10-23 19:33 ` Florian Westphal
2019-10-23 20:56 ` [PATCH] [netfilter]: Fix skb->csum calculation when netfilter Praveen Chaudhary
[not found] ` <CAJ_cd4qHM3kqz24Uywpyyz0mPz7axiNZk0Q385ROd4O8XZ11fA@mail.gmail.com>
2019-10-24 1:12 ` [PATCH] [netfilter]: Fix skb->csum calculation when netfilter manipulation for NF_NAT_MANIP_SRC\DST is done on IPV6 packet Florian Westphal
2019-10-24 5:19 ` Praveen Chaudhary [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1571894373-11188-1-git-send-email-pchaudhary@linkedin.com \
--to=praveen5582@gmail.com \
--cc=astracner@linkedin.com \
--cc=davem@davemloft.net \
--cc=fw@strlen.de \
--cc=kadlec@netfilter.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=zxu@linkedin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®