Messages in this thread | | | Date | Wed, 22 Feb 2023 15:47:46 +0100 | From | Richard Gobert <> | Subject | Re: [PATCH 2/2] gro: optimise redundant parsing of packets |
| |
> > Currently, the IPv6 extension headers are parsed twice: first in > > ipv6_gro_receive, and then again in ipv6_gro_complete. > > > > The field NAPI_GRO_CB(skb)->proto is used by GRO to hold the layer 4 > > protocol type that comes after the IPv6 layer. I noticed that it is set > > in ipv6_gro_receive, but isn't used anywhere. By using this field, and > > also storing the size of the network header, we can avoid parsing > > extension headers a second time in ipv6_gro_complete. > > > > The implementation had to handle both inner and outer layers in case of > > encapsulation (as they can't use the same field). > > > > I've applied this optimisation to all base protocols (IPv6, IPv4, > > Ethernet). Then, I benchmarked this patch on my machine, using ftrace to > > measure ipv6_gro_complete's performance, and there was an improvement. > > Would be nice to see some perf numbers. "there was an improvement" > doesn't say a lot TBH... >
I just posted raw performance numbers as a reply to Eric's message. Take a look there.
> > @@ -456,12 +459,16 @@ EXPORT_SYMBOL(eth_gro_receive); > > int eth_gro_complete(struct sk_buff *skb, int nhoff) > > { > > struct ethhdr *eh = (struct ethhdr *)(skb->data + nhoff); > > - __be16 type = eh->h_proto; > > + __be16 type; > > Please don't break RCT style when shortening/expanding variable > declaration lines.
Will be fixed in v2.
> > @@ -358,7 +361,13 @@ INDIRECT_CALLABLE_SCOPE int ipv6_gro_complete(struct sk_buff *skb, int nhoff) > > iph->payload_len = htons(payload_len); > > } > > > > - nhoff += sizeof(*iph) + ipv6_exthdrs_len(iph, &ops); > > + if (!skb->encapsulation) { > > + ops = rcu_dereference(inet6_offloads[NAPI_GRO_CB(skb)->transport_proto]); > > + nhoff += NAPI_GRO_CB(skb)->network_len; > > Why not use the same skb_network_header_len() here? Both > skb->network_header and skb->transport_header must be set and correct at > this point (if not, you can always fix that). >
When processing packets with encapsulation the network_header field is overwritten when processing the inner IP header, so skb_network_header_len won't return the correct value.
| |