netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Magnus Karlsson <magnus.karlsson@gmail.com>
To: magnus.karlsson@intel.com, bjorn@kernel.org, ast@kernel.org,
	daniel@iogearbox.net, netdev@vger.kernel.org,
	jonathan.lemon@gmail.com, maciej.fijalkowski@intel.com,
	kuba@kernel.org, toke@redhat.com, pabeni@redhat.com,
	davem@davemloft.net, aelior@marvell.com, manishc@marvell.com,
	horatiu.vultur@microchip.com, UNGLinuxDriver@microchip.com,
	mst@redhat.com, jasowang@redhat.com, ioana.ciornei@nxp.com,
	madalin.bucur@nxp.com
Cc: Magnus Karlsson <magnus.karlsson@gmail.com>, bpf@vger.kernel.org
Subject: [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done()
Date: Wed, 25 Jan 2023 08:48:56 +0100	[thread overview]
Message-ID: <20230125074901.2737-1-magnus.karlsson@gmail.com> (raw)

Make sure that xdp_do_flush() is always executed before
napi_complete_done(). This is important for two reasons. First, a
redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
napi context X on CPU Y will be followed by a xdp_do_flush() from the
same napi context and CPU. This is not guaranteed if the
napi_complete_done() is executed before xdp_do_flush(), as it tells
the napi logic that it is fine to schedule napi context X on another
CPU. Details from a production system triggering this bug using the
veth driver can be found in [1].

The second reason is that the XDP_REDIRECT logic in itself relies on
being inside a single NAPI instance through to the xdp_do_flush() call
for RCU protection of all in-kernel data structures. Details can be
found in [2].

The drivers have only been compile-tested since I do not own any of
the HW below. So if you are a maintainer, it would be great if you
could take a quick look to make sure I did not mess something up.

Note that these were the drivers I found that violated the ordering by
running a simple script and manually checking the ones that came up as
potential offenders. But the script was not perfect in any way. There
might still be offenders out there, since the script can generate
false negatives.

v1 -> v2:
* Added acks [Toke, Steen]
* Corrected two spelling errors [Toke]

[1] https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
[2] https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/

Thanks: Magnus

Magnus Karlsson (5):
  qede: execute xdp_do_flush() before napi_complete_done()
  lan966x: execute xdp_do_flush() before napi_complete_done()
  virtio-net: execute xdp_do_flush() before napi_complete_done()
  dpaa_eth: execute xdp_do_flush() before napi_complete_done()
  dpaa2-eth: execute xdp_do_flush() before napi_complete_done()

 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c        | 6 +++---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c      | 9 ++++++---
 drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 6 +++---
 drivers/net/ethernet/qlogic/qede/qede_fp.c            | 7 ++++---
 drivers/net/virtio_net.c                              | 6 +++---
 5 files changed, 19 insertions(+), 15 deletions(-)


base-commit: 2a48216cff7a2e3964fbed16f84d33f68b3e5e42
--
2.34.1

             reply	other threads:[~2023-01-25  7:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-25  7:48 Magnus Karlsson [this message]
2023-01-25  7:48 ` [PATCH net v2 1/5] qede: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
2023-01-25  7:48 ` [PATCH net v2 2/5] lan966x: " Magnus Karlsson
2023-01-25  7:48 ` [PATCH net v2 3/5] virtio-net: " Magnus Karlsson
2023-01-27 10:49   ` Michael S. Tsirkin
2023-01-25  7:49 ` [PATCH net v2 4/5] dpaa_eth: " Magnus Karlsson
2023-01-25 14:58   ` Camelia Alexandra Groza
2023-01-25  7:49 ` [PATCH net v2 5/5] dpaa2-eth: " Magnus Karlsson
2023-01-27 10:50 ` [PATCH net v2 0/5] net: xdp: " Michael S. Tsirkin
2023-01-28  6:40 ` patchwork-bot+netdevbpf

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=20230125074901.2737-1-magnus.karlsson@gmail.com \
    --to=magnus.karlsson@gmail.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=aelior@marvell.com \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=horatiu.vultur@microchip.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=jasowang@redhat.com \
    --cc=jonathan.lemon@gmail.com \
    --cc=kuba@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=madalin.bucur@nxp.com \
    --cc=magnus.karlsson@intel.com \
    --cc=manishc@marvell.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=toke@redhat.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).