netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Gobert <richardbgobert@gmail.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, willemdebruijn.kernel@gmail.com,
	dsahern@kernel.org, tom@herbertland.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, gal@nvidia.com
Subject: [PATCH 0/1] net: gro: fix misuse of CB in udp socket lookup
Date: Fri, 7 Jul 2023 14:16:55 +0200	[thread overview]
Message-ID: <20230707121650.GA17677@debian> (raw)

GRO stack uses `udp_lib_lookup_skb` which relies on IP/IPv6 CB's info, and
at the GRO stage, CB holds `napi_gro_cb` info.  Specifically,
`udp_lib_lookup_skb` tries to fetch `iff` and `flags` information from the
CB, to find the relevant udp tunnel socket (GENEVE/VXLAN/..).  Up until a
patch I submitted recently [0], it worked merely by luck, due
to the layouts of `napi_gro_cb` and IP6CB.

AFAIU it worked because: 
`IP6CB(skb)->flags` is at offset 16 inside IP6CB:
 - Before the patch: `flags` was mapped to `flush`.
 - After the patch: `flags` was mapped to `data_offset`.

`IP6CB(skb)->iff` is at offset 0 inside IP6CB:
 - Before the patch: `iif` was mapped to `frag0`.
 - After the patch: `iif` was mapped to a union of `frag0` and `last`.

After my patch, on the receive phase, while `data_offset` is 40 (since IPv6
header is 40 bytes), `inet_iif` calls `ipv6_l3mdev_skb`, which checks
whether `IP6CB(skb)->flags`'s `IP6SKB_L3SLAVE` bit is on or off (in our
case its off). If it is off, `inet_iif` returns `IP6CB(skb)->iif`, which is
mapped to `napi_gro_cb->frag0`, making `inet_iif` return 0 most of the
times. `inet_sdif` returns zero due to a similar reason caused by
`data_offset` being equal to 40 (and less than 64).

On the other hand, the complete phase behaves differently.
`data_offset` is usually greater than 64 and less than 128 so the
`IP6SKB_L3SLAVE` flag is on.  Thus, `inet_sdif` returns `IP6CB(skb)->iif`,
which is mapped to `last` which contains a pointer. This causes
`udp_sk_bound_dev_eq` to fail, which leads to `udp6_lib_lookup2` failing
and not returning a socket. This leads the receive phase of GRO
to find the right socket, and on the complete phase, it fails to find it and
makes the throughput go down to nearly zero.

Before [0] `flags` was mapped to `flush`. `flush`'s possible
values were 1 and 0, making `inet6_iff` always returning `skb->skb_iif` and
`inet6_sdif` returning 0, and leading to `udp_sk_bound_dev_eq` returning
true.

A fix is to not rely on CB, and get `iff` and `sdif` using skb->dev. l3mdev
case requires special attention since it has a master and a slave device.

[0] https://lore.kernel.org/netdev/20230601160924.GA9194@debian/

Richard Gobert (1):
  net: gro: fix misuse of CB in udp socket lookup

 include/net/udp.h      |  2 ++
 net/ipv4/udp.c         | 21 +++++++++++++++++++--
 net/ipv4/udp_offload.c |  7 +++++--
 net/ipv6/udp.c         | 21 +++++++++++++++++++--
 net/ipv6/udp_offload.c |  7 +++++--
 5 files changed, 50 insertions(+), 8 deletions(-)

--
2.36.1


             reply	other threads:[~2023-07-07 12:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-07 12:16 Richard Gobert [this message]
2023-07-07 12:26 ` [PATCH 1/1] net: gro: fix misuse of CB in udp socket lookup Richard Gobert
2023-07-07 13:44   ` Eric Dumazet
2023-07-10 14:54     ` Richard Gobert
2023-07-07 18:42   ` Kuniyuki Iwashima
2023-07-07 21:40   ` David Ahern
2023-07-10 14:58     ` Richard Gobert
2023-07-10 15:50       ` David Ahern
2023-07-17 10:41   ` Gal Pressman
2023-07-19  7:27     ` Richard Gobert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230707121650.GA17677@debian \
    --to=richardbgobert@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=gal@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tom@herbertland.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).