[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4d0f7392-06bf-414d-ab3e-158dc2c3f296@nbd.name>
Date: Fri, 26 Apr 2024 11:41:55 +0200
From: Felix Fietkau <nbd@....name>
To: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
Eric Dumazet <edumazet@...gle.com>, "David S. Miller" <davem@...emloft.net>,
David Ahern <dsahern@...nel.org>, Jakub Kicinski <kuba@...nel.org>
Cc: willemdebruijn.kernel@...il.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 net-next v3 3/6] net: add code for TCP fraglist GRO
On 26.04.24 10:21, Paolo Abeni wrote:
> On Fri, 2024-04-26 at 08:51 +0200, Felix Fietkau wrote:
>> This implements fraglist GRO similar to how it's handled in UDP, however
>> no functional changes are added yet. The next change adds a heuristic for
>> using fraglist GRO instead of regular GRO.
>>
>> Signed-off-by: Felix Fietkau <nbd@....name>
>> ---
>> net/ipv4/tcp_offload.c | 22 ++++++++++++++++++++++
>> net/ipv6/tcpv6_offload.c | 9 +++++++++
>> 2 files changed, 31 insertions(+)
>>
>> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
>> index c493e95e09a5..ffd6b7a4163a 100644
>> --- a/net/ipv4/tcp_offload.c
>> +++ b/net/ipv4/tcp_offload.c
>> @@ -332,6 +332,19 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
>> flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq);
>> flush |= skb_cmp_decrypted(p, skb);
>>
>> + if (NAPI_GRO_CB(p)->is_flist) {
>> + flush |= (__force int)(flags ^ tcp_flag_word(th2));
>> + flush |= skb->ip_summed != p->ip_summed;
>> + flush |= skb->csum_level != p->csum_level;
>> + flush |= !pskb_may_pull(skb, skb_gro_offset(skb));
>
> I'm sorry, I'm lagging behind. I think the TCP flags handling here is
> correct - preserving the original ones should work.
>
> The question a made WRT 2 above checks being non necessary/redundant:
>
> flush |= (__force int)(flags ^ tcp_flag_word(th2));
This one is not redundant, because the earlier flags check includes this
part: & ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH))
> flush |= !pskb_may_pull(skb, skb_gro_offset(skb));
This one looks like a redundant leftover, I will remove it in the next
version.
Thanks,
- Felix
Powered by blists - more mailing lists