* [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT
@ 2025-02-24 11:20 Felix Fietkau
2025-02-24 13:00 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Felix Fietkau @ 2025-02-24 11:20 UTC (permalink / raw)
To: netdev, Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima,
David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Simon Horman, Willem de Bruijn
Cc: linux-kernel
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);
}
--
2.47.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT
2025-02-24 11:20 [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT Felix Fietkau
@ 2025-02-24 13:00 ` Eric Dumazet
2025-02-27 10:30 ` Paolo Abeni
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2025-02-24 13:00 UTC (permalink / raw)
To: Felix Fietkau
Cc: netdev, Neal Cardwell, Kuniyuki Iwashima, David S. Miller,
David Ahern, Jakub Kicinski, Paolo Abeni, Simon Horman,
Willem de Bruijn, linux-kernel
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 ?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT
2025-02-24 13:00 ` Eric Dumazet
@ 2025-02-27 10:30 ` Paolo Abeni
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Abeni @ 2025-02-27 10:30 UTC (permalink / raw)
To: Eric Dumazet, Felix Fietkau
Cc: netdev, Neal Cardwell, Kuniyuki Iwashima, David S. Miller,
David Ahern, Jakub Kicinski, Simon Horman, Willem de Bruijn,
linux-kernel
On 2/24/25 2:00 PM, Eric Dumazet wrote:
> 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 ?
I agree with Eric, I think we should try to keep ipv4 and ipv6 code and
behavior similar, where/when it makes sense (like here).
@Felix, could you please update the patch accordingly?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-27 10:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-24 11:20 [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT Felix Fietkau
2025-02-24 13:00 ` Eric Dumazet
2025-02-27 10:30 ` Paolo Abeni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox