| Message ID | 2ce1600b-e733-448b-91ac-9d0ae2b866a4@gmail.com |
|---|---|
| Headers | show |
| Series | net: gro: encapsulation bug fix and flush checks improvements | expand |
Eric Dumazet wrote: > On Thu, Mar 7, 2024 at 2:28 PM Richard Gobert <richardbgobert@gmail.com> 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).
This series fixes a bug in the complete phase of UDP in GRO, in which socket lookup fails due to using network_header when parsing encapsulated packets. The fix is to pass p_off parameter in *_gro_complete. Next, inner_network_header is always set in the receive phase of GRO, this is then leveraged in the next commit to remove some state from napi_gro_cb, and stateful code in {ipv6,inet}_gro_receive which may be unnecessarily complicated due to encapsulation support in GRO. In addition, udpgro_fwd selftest is adjusted to include the socket lookup case for vxlan. This selftest will test its supposed functionality once local bind support is merged (https://lore.kernel.org/netdev/df300a49-7811-4126-a56a-a77100c8841b@gmail.com/). v1 -> v2: - Pass p_off in *_gro_complete to fix UDP bug - Remove more conditionals and memory fetches from inet_gro_flush - v1: https://lore.kernel.org/netdev/e1d22505-c5f8-4c02-a997-64248480338b@gmail.com/ Richard Gobert (4): net: gro: add p_off param in *_gro_complete selftests/net: add local address bind in vxlan selftest net: gro: set inner_network_header in receive phase net: gro: move L3 flush checks to tcp_gro_receive drivers/net/geneve.c | 7 +- drivers/net/vxlan/vxlan_core.c | 11 ++-- include/linux/etherdevice.h | 2 +- include/linux/netdevice.h | 3 +- include/linux/udp.h | 2 +- include/net/gro.h | 33 ++++++---- include/net/inet_common.h | 2 +- include/net/tcp.h | 6 +- include/net/udp.h | 8 +-- include/net/udp_tunnel.h | 2 +- net/8021q/vlan_core.c | 9 ++- net/core/gro.c | 5 +- net/ethernet/eth.c | 5 +- net/ipv4/af_inet.c | 53 ++------------- net/ipv4/fou_core.c | 9 +-- net/ipv4/gre_offload.c | 6 +- net/ipv4/tcp_offload.c | 80 ++++++++++++++++++----- net/ipv4/udp.c | 3 +- net/ipv4/udp_offload.c | 26 ++++---- net/ipv6/ip6_offload.c | 45 +++++-------- net/ipv6/tcpv6_offload.c | 7 +- net/ipv6/udp.c | 3 +- net/ipv6/udp_offload.c | 13 ++-- tools/testing/selftests/net/udpgro_fwd.sh | 10 ++- 24 files changed, 188 insertions(+), 162 deletions(-)