Messages in this thread | | | Date | Tue, 14 Mar 2023 16:55:47 +0100 | From | Richard Gobert <> | Subject | Re: [PATCH v3 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. > > > > By using the new ->transport_proto field, and also storing the size of the > > network header, we can avoid parsing extension headers a second time in > > ipv6_gro_complete (which saves multiple memory dereferences and conditional > > checks inside ipv6_exthdrs_len for a varying amount of extension headers in > > IPv6 packets). > > > > 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 a similar > > optimisation to Ethernet. > > > > Performance tests for TCP stream over IPv6 with a varying amount of > > extension headers demonstrate throughput improvement of ~0.7%. > > > > In addition, I fixed a potential future problem: > > I would remove all this block. > > We fix current problems, not future hypothetical ones. >
I agree, I did it primarily to avoid an additional branch (the logic remains exactly the same). I'll remove this part from the commit message.
> > - The call to skb_set_inner_network_header at the beginning of > > ipv6_gro_complete calculates inner_network_header based on skb->data by > > calling skb_set_inner_network_header, and setting it to point to the > > beginning of the ip header. > > - If a packet is going to be handled by BIG TCP, the following code block > > is going to shift the packet header, and skb->data is going to be > > changed as well. > > > > When the two flows are combined, inner_network_header will point to the > > wrong place - which might happen if encapsulation of BIG TCP will be > > supported in the future. > > > > The fix is to place the whole encapsulation branch after the BIG TCP code > > block. This way, if encapsulation of BIG TCP will be supported, > > inner_network_header will still be calculated with the correct value of > > skb->data. > > We do not support encapsulated BIG TCP yet. > We will do this later, and whoever does it will make sure to also support GRO. > > > Also, by arranging the code that way, the optimisation does not > > add an additional branch. > > > > Signed-off-by: Richard Gobert <richardbgobert@gmail.com> > > --- > > > > Can you give us a good explanation of why extension headers are used exactly ? > > I am not sure we want to add code to GRO for something that 99.99% of > us do not use.
IMO, some common use cases that will benefit from this patch are: - Parsing of BIG TCP packets which include a hbh ext hdr. - dstopts and routing ext hdrs that are used for Mobile IPv6 features.
Generally, when a packet includes ext hdrs we will avoid the recalculation of the ext hdrs len. When there are no ext hdrs, we will not call the ipv6_exthdrs_len function so the performance isn't negatively impacted (potentially even saving some opcodes in ipv6_exthdrs_len).
| |