Messages in this thread | | | Date | Tue, 30 Apr 2024 12:55:29 +0200 | Subject | Re: [PATCH v4 net-next v4 6/6] net: add heuristic for enabling TCP fraglist GRO | From | Felix Fietkau <> |
| |
On 30.04.24 12:31, Paolo Abeni wrote: > On Tue, 2024-04-30 at 12:23 +0200, Felix Fietkau wrote: >> On 30.04.24 12:12, Paolo Abeni wrote: >> > On Sat, 2024-04-27 at 20:23 +0200, Felix Fietkau wrote: >> > > When forwarding TCP after GRO, software segmentation is very expensive, >> > > especially when the checksum needs to be recalculated. >> > > One case where that's currently unavoidable is when routing packets over >> > > PPPoE. Performance improves significantly when using fraglist GRO >> > > implemented in the same way as for UDP. >> > > >> > > When NETIF_F_GRO_FRAGLIST is enabled, perform a lookup for an established >> > > socket in the same netns as the receiving device. While this may not >> > > cover all relevant use cases in multi-netns configurations, it should be >> > > good enough for most configurations that need this. >> > > >> > > Here's a measurement of running 2 TCP streams through a MediaTek MT7622 >> > > device (2-core Cortex-A53), which runs NAT with flow offload enabled from >> > > one ethernet port to PPPoE on another ethernet port + cake qdisc set to >> > > 1Gbps. >> > > >> > > rx-gro-list off: 630 Mbit/s, CPU 35% idle >> > > rx-gro-list on: 770 Mbit/s, CPU 40% idle >> > > >> > > Signe-off-by: Felix Fietkau <nbd@nbd.name> >> > > --- >> > > net/ipv4/tcp_offload.c | 32 ++++++++++++++++++++++++++++++++ >> > > net/ipv6/tcpv6_offload.c | 35 +++++++++++++++++++++++++++++++++++ >> > > 2 files changed, 67 insertions(+) >> > > >> > > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c >> > > index 87ae9808e260..3e9b8c6f9c8c 100644 >> > > --- a/net/ipv4/tcp_offload.c >> > > +++ b/net/ipv4/tcp_offload.c >> > > @@ -407,6 +407,36 @@ void tcp_gro_complete(struct sk_buff *skb) >> > > } >> > > EXPORT_SYMBOL(tcp_gro_complete); >> > > >> > > +static void tcp4_check_fraglist_gro(struct list_head *head, struct sk_buff *skb, >> > > + struct tcphdr *th) >> > > +{ >> > > + const struct iphdr *iph; >> > > + struct sk_buff *p; >> > > + struct sock *sk; >> > > + struct net *net; >> > > + int iif, sdif; >> > > + >> > > + if (!(skb->dev->features & NETIF_F_GRO_FRAGLIST)) >> > >> > Should we add an 'unlikely()' here to pair with unlikely(is_flist) in >> > *gro_receive / *gro_complete? >> Not sure if unlikely() will make any difference here. I think it makes >> more sense in the other places than here. > > Why? AFAICS this will be called for every packet on the wire, exactly > as the code getting this annotation in patch 3/6.
I had compared assembly after adding an annotation and didn't see a difference. However, my annotation was wrong. When I add: if (likely(!(skb->dev->features & NETIF_F_GRO_FRAGLIST))) the generated code is different, and I probably should use that.
- Felix
| |