diff mbox series

[net] net: ipv6: fix TCP GSO segmentation with NAT

Message ID 20250224112046.52304-1-nbd@nbd.name (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series [net] net: ipv6: fix TCP GSO segmentation with NAT | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 40 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2025-02-24--12-00 (tests: 895)

Commit Message

Felix Fietkau Feb. 24, 2025, 11:20 a.m. UTC 
When updating the source/destination address, the TCP/UDP checksum needs to
be updated as well.

Fixes: bee88cd5bd83 ("net: add support for segmenting TCP fraglist GSO packets")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/ipv6/tcpv6_offload.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Eric Dumazet Feb. 24, 2025, 1 p.m. UTC | #1 
On Mon, Feb 24, 2025 at 12:21 PM Felix Fietkau <nbd@nbd.name> wrote:
>
> When updating the source/destination address, the TCP/UDP checksum needs to
> be updated as well.
>
> Fixes: bee88cd5bd83 ("net: add support for segmenting TCP fraglist GSO packets")
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>  net/ipv6/tcpv6_offload.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index a45bf17cb2a1..5d0fcdbf57a1 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -113,24 +113,36 @@ static struct sk_buff *__tcpv6_gso_segment_list_csum(struct sk_buff *segs)
>         struct sk_buff *seg;
>         struct tcphdr *th2;
>         struct ipv6hdr *iph2;
> +       bool addr_equal;
>
>         seg = segs;
>         th = tcp_hdr(seg);
>         iph = ipv6_hdr(seg);
>         th2 = tcp_hdr(seg->next);
>         iph2 = ipv6_hdr(seg->next);
> +       addr_equal = ipv6_addr_equal(&iph->saddr, &iph2->saddr) &&
> +                    ipv6_addr_equal(&iph->daddr, &iph2->daddr);
>
>         if (!(*(const u32 *)&th->source ^ *(const u32 *)&th2->source) &&
> -           ipv6_addr_equal(&iph->saddr, &iph2->saddr) &&
> -           ipv6_addr_equal(&iph->daddr, &iph2->daddr))
> +           addr_equal)
>                 return segs;
>
>         while ((seg = seg->next)) {
>                 th2 = tcp_hdr(seg);
>                 iph2 = ipv6_hdr(seg);
>
> -               iph2->saddr = iph->saddr;
> -               iph2->daddr = iph->daddr;
> +               if (!addr_equal) {
> +                       inet_proto_csum_replace16(&th2->check, seg,
> +                                                 iph2->saddr.s6_addr32,
> +                                                 iph->saddr.s6_addr32,
> +                                                 true);
> +                       inet_proto_csum_replace16(&th2->check, seg,
> +                                                 iph2->daddr.s6_addr32,
> +                                                 iph->daddr.s6_addr32,
> +                                                 true);
> +                       iph2->saddr = iph->saddr;
> +                       iph2->daddr = iph->daddr;
> +               }
>                 __tcpv6_gso_segment_csum(seg, &th2->source, th->source);
>                 __tcpv6_gso_segment_csum(seg, &th2->dest, th->dest);

Good catch !

I note that __tcpv4_gso_segment_csum() does the needed csum changes
for both ports and address components.

Have you considered using the same logic for IPv6, to keep
__tcpv6_gso_segment_list_csum()
and __tcpv4_gso_segment_list_csum() similar ?
diff mbox series

Patch

diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index a45bf17cb2a1..5d0fcdbf57a1 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -113,24 +113,36 @@  static struct sk_buff *__tcpv6_gso_segment_list_csum(struct sk_buff *segs)
 	struct sk_buff *seg;
 	struct tcphdr *th2;
 	struct ipv6hdr *iph2;
+	bool addr_equal;
 
 	seg = segs;
 	th = tcp_hdr(seg);
 	iph = ipv6_hdr(seg);
 	th2 = tcp_hdr(seg->next);
 	iph2 = ipv6_hdr(seg->next);
+	addr_equal = ipv6_addr_equal(&iph->saddr, &iph2->saddr) &&
+		     ipv6_addr_equal(&iph->daddr, &iph2->daddr);
 
 	if (!(*(const u32 *)&th->source ^ *(const u32 *)&th2->source) &&
-	    ipv6_addr_equal(&iph->saddr, &iph2->saddr) &&
-	    ipv6_addr_equal(&iph->daddr, &iph2->daddr))
+	    addr_equal)
 		return segs;
 
 	while ((seg = seg->next)) {
 		th2 = tcp_hdr(seg);
 		iph2 = ipv6_hdr(seg);
 
-		iph2->saddr = iph->saddr;
-		iph2->daddr = iph->daddr;
+		if (!addr_equal) {
+			inet_proto_csum_replace16(&th2->check, seg,
+						  iph2->saddr.s6_addr32,
+						  iph->saddr.s6_addr32,
+						  true);
+			inet_proto_csum_replace16(&th2->check, seg,
+						  iph2->daddr.s6_addr32,
+						  iph->daddr.s6_addr32,
+						  true);
+			iph2->saddr = iph->saddr;
+			iph2->daddr = iph->daddr;
+		}
 		__tcpv6_gso_segment_csum(seg, &th2->source, th->source);
 		__tcpv6_gso_segment_csum(seg, &th2->dest, th->dest);
 	}