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

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

Posted by Felix Fietkau 9 months, 1 week ago

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>
---
v2: move code to make it similar to __tcpv4_gso_segment_list_csum

 net/ipv6/tcpv6_offload.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index a45bf17cb2a1..34dd0cee3ba6 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -94,10 +94,20 @@ INDIRECT_CALLABLE_SCOPE int tcp6_gro_complete(struct sk_buff *skb, int thoff)
 }
 
 static void __tcpv6_gso_segment_csum(struct sk_buff *seg,
+				     struct in6_addr *oldip,
+				     const struct in6_addr *newip,
 				     __be16 *oldport, __be16 newport)
 {
 	struct tcphdr *th;
 
+	if (!ipv6_addr_equal(oldip, newip)) {
+		inet_proto_csum_replace16(&th->check, seg,
+					  oldip->s6_addr32,
+					  newip->s6_addr32,
+					  true);
+		*oldip = *newip;
+	}
+
 	if (*oldport == newport)
 		return;
 
@@ -129,10 +139,10 @@ static struct sk_buff *__tcpv6_gso_segment_list_csum(struct sk_buff *segs)
 		th2 = tcp_hdr(seg);
 		iph2 = ipv6_hdr(seg);
 
-		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);
+		__tcpv6_gso_segment_csum(seg, &iph2->saddr, &iph->saddr,
+					 &th2->source, th->source);
+		__tcpv6_gso_segment_csum(seg, &iph2->daddr, &iph->daddr,
+					 &th2->dest, th->dest);
 	}
 
 	return segs;
-- 
2.47.1

Re: [PATCH net v2] net: ipv6: fix TCP GSO segmentation with NAT

Posted by kernel test robot 9 months, 1 week ago

Hi Felix,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Felix-Fietkau/net-ipv6-fix-TCP-GSO-segmentation-with-NAT/20250310-192256
base:   net/main
patch link:    https://lore.kernel.org/r/20250310112121.73654-1-nbd%40nbd.name
patch subject: [PATCH net v2] net: ipv6: fix TCP GSO segmentation with NAT
config: s390-randconfig-001-20250312 (https://download.01.org/0day-ci/archive/20250312/202503120224.LbO66Ztw-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250312/202503120224.LbO66Ztw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503120224.LbO66Ztw-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/ipv6/tcpv6_offload.c:104:30: warning: variable 'th' is uninitialized when used here [-Wuninitialized]
                   inet_proto_csum_replace16(&th->check, seg,
                                              ^~
   net/ipv6/tcpv6_offload.c:101:19: note: initialize the variable 'th' to silence this warning
           struct tcphdr *th;
                            ^
                             = NULL
   1 warning generated.


vim +/th +104 net/ipv6/tcpv6_offload.c

    95	
    96	static void __tcpv6_gso_segment_csum(struct sk_buff *seg,
    97					     struct in6_addr *oldip,
    98					     const struct in6_addr *newip,
    99					     __be16 *oldport, __be16 newport)
   100	{
   101		struct tcphdr *th;
   102	
   103		if (!ipv6_addr_equal(oldip, newip)) {
 > 104			inet_proto_csum_replace16(&th->check, seg,
   105						  oldip->s6_addr32,
   106						  newip->s6_addr32,
   107						  true);
   108			*oldip = *newip;
   109		}
   110	
   111		if (*oldport == newport)
   112			return;
   113	
   114		th = tcp_hdr(seg);
   115		inet_proto_csum_replace2(&th->check, seg, *oldport, newport, false);
   116		*oldport = newport;
   117	}
   118	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net v2] net: ipv6: fix TCP GSO segmentation with NAT

Posted by Eric Dumazet 9 months, 1 week ago

On Mon, Mar 10, 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>
> ---
> v2: move code to make it similar to __tcpv4_gso_segment_list_csum
>
>  net/ipv6/tcpv6_offload.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index a45bf17cb2a1..34dd0cee3ba6 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -94,10 +94,20 @@ INDIRECT_CALLABLE_SCOPE int tcp6_gro_complete(struct sk_buff *skb, int thoff)
>  }
>
>  static void __tcpv6_gso_segment_csum(struct sk_buff *seg,
> +                                    struct in6_addr *oldip,
> +                                    const struct in6_addr *newip,
>                                      __be16 *oldport, __be16 newport)
>  {
>         struct tcphdr *th;
>
> +       if (!ipv6_addr_equal(oldip, newip)) {
> +               inet_proto_csum_replace16(&th->check, seg,

th is not initialized yet.

> +                                         oldip->s6_addr32,
> +                                         newip->s6_addr32,
> +                                         true);
> +               *oldip = *newip;
> +       }
> +
>         if (*oldport == newport)
>                 return;
>
> @@ -129,10 +139,10 @@ static struct sk_buff *__tcpv6_gso_segment_list_csum(struct sk_buff *segs)
>                 th2 = tcp_hdr(seg);
>                 iph2 = ipv6_hdr(seg);
>
> -               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);
> +               __tcpv6_gso_segment_csum(seg, &iph2->saddr, &iph->saddr,
> +                                        &th2->source, th->source);
> +               __tcpv6_gso_segment_csum(seg, &iph2->daddr, &iph->daddr,
> +                                        &th2->dest, th->dest);
>         }
>
>         return segs;
> --
> 2.47.1
>

Re: [PATCH net v2] net: ipv6: fix TCP GSO segmentation with NAT

Posted by Felix Fietkau 9 months, 1 week ago

On 10.03.25 14:50, Eric Dumazet wrote:
> On Mon, Mar 10, 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>
>> ---
>> v2: move code to make it similar to __tcpv4_gso_segment_list_csum
>>
>>  net/ipv6/tcpv6_offload.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
>> index a45bf17cb2a1..34dd0cee3ba6 100644
>> --- a/net/ipv6/tcpv6_offload.c
>> +++ b/net/ipv6/tcpv6_offload.c
>> @@ -94,10 +94,20 @@ INDIRECT_CALLABLE_SCOPE int tcp6_gro_complete(struct sk_buff *skb, int thoff)
>>  }
>>
>>  static void __tcpv6_gso_segment_csum(struct sk_buff *seg,
>> +                                    struct in6_addr *oldip,
>> +                                    const struct in6_addr *newip,
>>                                      __be16 *oldport, __be16 newport)
>>  {
>>         struct tcphdr *th;
>>
>> +       if (!ipv6_addr_equal(oldip, newip)) {
>> +               inet_proto_csum_replace16(&th->check, seg,
> 
> th is not initialized yet.

Sorry, I had this fixed locally, but forgot to add it before sending. 
Will send v3 later.

- Felix