Re: [PATCH net] netfilter: nat: never update the UDP checksum when it's 0

[Florian Westphal <fw@strlen.de>]

Guillaume Nault <gnault@redhat.com> 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.

Indeed, we're mangling udphdr->csum unconditionally for CSUM_PARTIAL
case. Doesn't make sense to me, so:

Reviewed-by: Florian Westphal <fw@strlen.de>