Eric Dumazet wrote: > On Thu, Mar 7, 2024 at 2:28 PM Richard Gobert <richardbgobert@xxxxxxxxx> wrote: >> >> This patch sets network_header and inner_network_header to their respective >> values during the receive phase of GRO. This allows us to use >> inner_network_header later on in GRO. network_header is already set in >> dev_gro_receive and under encapsulation inner_network_header is set. >> > >> +static inline int skb_gro_network_offset(const struct sk_buff *skb) >> +{ >> + const u32 mask = NAPI_GRO_CB(skb)->encap_mark - 1; >> + >> + return (skb_network_offset(skb) & mask) | (skb_inner_network_offset(skb) & ~mask); > > Presumably this is not needed. > >> +} >> + >> static inline void *skb_gro_network_header(const struct sk_buff *skb) >> { >> + const int offset = skb_gro_network_offset(skb); >> + >> if (skb_gro_may_pull(skb, skb_gro_offset(skb))) >> - return skb_gro_header_fast(skb, skb_network_offset(skb)); >> + return skb_gro_header_fast(skb, offset); >> >> - return skb_network_header(skb); >> + return skb->data + offset; >> } > > I would instead add a new offset parameter to this function. > > Again, ideally GRO should work without touching any skb->{offset}. > > GRO stack should maintain the offsets it needs in its own storage > (stack parameter, or other storage if needed) > > Upper stack can not trust any of these skb fields, otherwise we would > have some troubles with napi_reuse_skb() Inner and outer network headers are needed for commit #4, I will store them in the CB. Moreover I will work on another patch series, that changes GRO receive phase to use stack params as you suggested. (prev offset, and I will also work on a version with data_offset too).