All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Guillaume Nault <gnault@redhat.com>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>,
	Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net] netfilter: nat: never update the UDP checksum when it's 0
Date: Sun, 26 Apr 2020 23:57:34 +0200	[thread overview]
Message-ID: <20200426215734.GA7662@salvia> (raw)
In-Reply-To: <335a95d93767f2b58ad89975e4a0b342ee00db91.1587429321.git.gnault@redhat.com>

On Tue, Apr 21, 2020 at 02:42:19AM +0200, Guillaume Nault wrote:
> If the UDP header of a local VXLAN endpoint is NAT-ed, and the VXLAN
> device has disabled UDP checksums and enabled Tx checksum offloading,
> then the skb passed to udp_manip_pkt() has hdr->check == 0 (outer
> checksum disabled) and skb->ip_summed == CHECKSUM_PARTIAL (inner packet
> checksum offloaded).
> 
> Because of the ->ip_summed value, udp_manip_pkt() tries to update the
> outer checksum with the new address and port, leading to an invalid
> checksum sent on the wire, as the original null checksum obviously
> didn't take the old address and port into account.
> 
> So, we can't take ->ip_summed into account in udp_manip_pkt(), as it
> might not refer to the checksum we're acting on. Instead, we can base
> the decision to update the UDP checksum entirely on the value of
> hdr->check, because it's null if and only if checksum is disabled:
> 
>   * A fully computed checksum can't be 0, since a 0 checksum is
>     represented by the CSUM_MANGLED_0 value instead.
> 
>   * A partial checksum can't be 0, since the pseudo-header always adds
>     at least one non-zero value (the UDP protocol type 0x11) and adding
>     more values to the sum can't make it wrap to 0 as the carry is then
>     added to the wrapped number.
> 
>   * A disabled checksum uses the special value 0.
> 
> The problem seems to be there from day one, although it was probably
> not visible before UDP tunnels were implemented.

Applied, thanks.

      parent reply	other threads:[~2020-04-26 21:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-21  0:42 [PATCH net] netfilter: nat: never update the UDP checksum when it's 0 Guillaume Nault
2020-04-23 20:43 ` Florian Westphal
2020-04-26 21:57 ` Pablo Neira Ayuso [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=20200426215734.GA7662@salvia \
    --to=pablo@netfilter.org \
    --cc=fw@strlen.de \
    --cc=gnault@redhat.com \
    --cc=kadlec@netfilter.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.