Re: [PATCH v2] netfilter: nf_nat: Fix stale outer UDP checksum on VXLAN encapsulated packets

[lvjunyu <lvjunyu@cmss.chinamobile.com>]

On the Sashiko AI review comments:

> [Medium] Peer call-site divergence: after this patch nf_nat's
> __udp_manip_pkt() treats a CHECKSUM_PARTIAL skb whose offload target...

The flowtable software path (nf_flow_nat_ip_udp) is not reached by
the packets this patch fixes. VXLAN outer headers are locally
generated via vxlan_xmit() -> udp_set_csum() and traverse OUTPUT ->
POSTROUTING, where they hit the conntrack NAT path
(__udp_manip_pkt). The flowtable hook (nf_flow_offload_ip_hook)
processes forwarded flows with offloaded conntrack entries; the
VXLAN outer flow uses a random source port (MASQUERADE
--random-fully) and represents a new conntrack entry that has not
yet been offloaded.

The architectural inconsistency is acknowledged: if flowtable
offload of VXLAN tunnel flows is ever introduced, the same LCO
detection would be needed in nf_flow_nat_ip_udp(). The
zero-checksum gate difference (do_csum = !!hdr->check vs.
udph->check || ip_summed == CHECKSUM_PARTIAL) is pre-existing and
unrelated to this patch.

> [Medium] Incomplete/asymmetric fix: the same unguarded
> nf_csum_update() pattern exists at three sibling sites...

The LCO state being fixed is specific to the UDP encapsulation
path: udp_set_csum() LCO branch writes a complete checksum into
uh->check while setting csum_start to the inner transport header.
This split state does not occur for the sibling call sites:

- tcp_manip_pkt(): TCP checksum includes the pseudo-header
  (addresses + ports); the offload target is the TCP header itself
  (csum_start points at the TCP header, not an inner header), so
  the offload-target test would not match.

- icmpv6_manip_pkt(): ICMPv6 checksum includes a pseudo-header;
  the offload target is the ICMPv6 header itself.

- Headers embedded in ICMP errors (via
  nf_nat_icmp_reply_translation): these are byte copies in the
  ICMP payload, not live checksum-offloaded data; ip_summed is
  typically NONE or COMPLETE.

- nf_nat_*_csum_recalc(): handles length field changes in the
  pseudo-header, a different scenario.

Putting the offload-target test in a shared helper (e.g.,
wrapping nf_csum_update()) is architecturally sound and could be
done as a follow-up. This patch is intentionally scoped to the
UDP path where the bug is observed and verified.

> [Low] The new comment states that both the address and the port
> delta are "applying the address and port deltas with the wrong
> sign"...

The reviewer is correct. The two cases are distinct:

- Port delta (pseudohdr=false, PARTIAL): no-op -- neither branch
  in inet_proto_csum_replace4() is taken.

- Address delta (pseudohdr=true, PARTIAL): applied to wrong seed
  -- the else-if(pseudohdr) branch operates on an uncomplemented
  value.

The comment will be reworded to distinguish "not applied at all"
(port) from "applied to the wrong seed" (address) in the next
revision.

pw-bot: cr