All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: bpf@vger.kernel.org, netdev@vger.kernel.org
Cc: "Martin KaFai Lau" <kafai@fb.com>,
	"Hangbin Liu" <liuhangbin@gmail.com>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"Magnus Karlsson" <magnus.karlsson@gmail.com>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>
Subject: [PATCH bpf-next v5 00/19] Clean up and document RCU-based object protection for XDP and TC BPF
Date: Thu, 24 Jun 2021 18:05:50 +0200	[thread overview]
Message-ID: <20210624160609.292325-1-toke@redhat.com> (raw)

During the discussion[0] of Hangbin's multicast patch series, Martin pointed out
that the lifetime of the RCU-protected  map entries used by XDP_REDIRECT is by
no means obvious. I promised to look into cleaning this up, and Paul helpfully
provided some hints and a new unrcu_pointer() helper to aid in this.

It seems[1] that back in the early days of XDP, local_bh_disable() did not
provide RCU protection, which is why the rcu_read_lock() calls were added
to drivers in the first place. But according to Paul[2], in recent kernels
a local_bh_disable()/local_bh_enable() pair functions as one big RCU
read-side section, so no further protection is needed. This even applies to
-rt kernels, which has an explicit rcu_read_lock() in place as part of the
local_bh_disable()[3].

This patch series is mostly a documentation exercise, cleaning up the
description of the lifetime expectations and adding __rcu annotations so
sparse and lockdep can help verify it.

Patches 1-4 are preparatory: Patch 1 adds Paul's unrcu_pointer()
helper (which has already been added to his tree), which we need for some
of the operations in devmap, patches 2 and 3 update the RCU documentation
and patch 4 adds bh context as a valid condition for map lookups. Patch 5
is the main bit that adds the __rcu annotations and updates documentation
comments. Finally, patch 6 removes unneeded rcu_read_lock()s from TC BPF,
and the rest are patches updating the drivers, with one patch per distinct
maintainer.

Unfortunately I don't have any hardware to test any of the driver patches;
Jesper helpfully verified that it doesn't break anything on i40e, but the rest
of the driver patches are only compile-tested.

[0] https://lore.kernel.org/bpf/20210415173551.7ma4slcbqeyiba2r@kafai-mbp.dhcp.thefacebook.com/
[1] https://lore.kernel.org/bpf/c5192ab3-1c05-8679-79f2-59d98299095b@iogearbox.net/
[2] https://lore.kernel.org/bpf/20210417002301.GO4212@paulmck-ThinkPad-P17-Gen-1/
[3] https://lore.kernel.org/bpf/20210419165837.GA975577@paulmck-ThinkPad-P17-Gen-1/

Changelog:
v5:
  - Rebase to bpf-next and fix build error
v4:
  - Move comment about RCU protection into core instead of leaving it in
    drivers
  - Also remove rcu_read_lock() around TC BPF program execution
  - Fold in a couple of patches from Paul updating the RCU documentation
v3:
  - Remove one other unnecessary change to hlist_for_each_entry_rcu()
  - Carry forward another ACK
v2:
  - Add a comment about RCU protection to the drivers where rcu_read_lock()
    is removed
  - Drop unnecessary patch 3 which changed dev_get_by_index_rcu()
  - Add some more text with the history to cover letter
  - Fix a few places where the wrong RCU checks were used in cpumap and
    xskmap code
  - Carry forward ACKs

Paul E. McKenney (2):
  rcu: Create an unrcu_pointer() to remove __rcu from a pointer
  doc: Clarify and expand RCU updaters and corresponding readers

Toke Høiland-Jørgensen (17):
  doc: Give XDP as example of non-obvious RCU reader/updater pairing
  bpf: allow RCU-protected lookups to happen from bh context
  xdp: add proper __rcu annotations to redirect map entries
  sched: remove unneeded rcu_read_lock() around BPF program invocation
  ena: remove rcu_read_lock() around XDP program invocation
  bnxt: remove rcu_read_lock() around XDP program invocation
  thunderx: remove rcu_read_lock() around XDP program invocation
  freescale: remove rcu_read_lock() around XDP program invocation
  net: intel: remove rcu_read_lock() around XDP program invocation
  marvell: remove rcu_read_lock() around XDP program invocation
  mlx4: remove rcu_read_lock() around XDP program invocation
  nfp: remove rcu_read_lock() around XDP program invocation
  qede: remove rcu_read_lock() around XDP program invocation
  sfc: remove rcu_read_lock() around XDP program invocation
  netsec: remove rcu_read_lock() around XDP program invocation
  stmmac: remove rcu_read_lock() around XDP program invocation
  net: ti: remove rcu_read_lock() around XDP program invocation

 Documentation/RCU/checklist.rst               | 55 ++++++++++++-------
 drivers/net/ethernet/amazon/ena/ena_netdev.c  |  3 -
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c |  2 -
 .../net/ethernet/cavium/thunder/nicvf_main.c  |  2 -
 .../net/ethernet/freescale/dpaa/dpaa_eth.c    |  8 +--
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |  3 -
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  2 -
 drivers/net/ethernet/intel/i40e/i40e_xsk.c    |  3 -
 drivers/net/ethernet/intel/ice/ice_txrx.c     |  6 +-
 drivers/net/ethernet/intel/ice/ice_xsk.c      |  3 -
 drivers/net/ethernet/intel/igb/igb_main.c     |  2 -
 drivers/net/ethernet/intel/igc/igc_main.c     |  7 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  2 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  |  3 -
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |  2 -
 drivers/net/ethernet/marvell/mvneta.c         |  2 -
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |  4 --
 drivers/net/ethernet/mellanox/mlx4/en_rx.c    |  8 +--
 .../ethernet/netronome/nfp/nfp_net_common.c   |  2 -
 drivers/net/ethernet/qlogic/qede/qede_fp.c    |  6 --
 drivers/net/ethernet/sfc/rx.c                 |  9 +--
 drivers/net/ethernet/socionext/netsec.c       |  3 -
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +---
 drivers/net/ethernet/ti/cpsw_priv.c           | 10 +---
 include/linux/filter.h                        |  8 +--
 include/linux/rcupdate.h                      | 14 +++++
 include/net/xdp_sock.h                        |  2 +-
 kernel/bpf/cpumap.c                           | 13 +++--
 kernel/bpf/devmap.c                           | 49 +++++++----------
 kernel/bpf/hashtab.c                          | 21 ++++---
 kernel/bpf/helpers.c                          |  6 +-
 kernel/bpf/lpm_trie.c                         |  6 +-
 net/core/filter.c                             | 28 ++++++++++
 net/sched/act_bpf.c                           |  2 -
 net/sched/cls_bpf.c                           |  3 -
 net/xdp/xsk.c                                 |  4 +-
 net/xdp/xsk.h                                 |  4 +-
 net/xdp/xskmap.c                              | 29 ++++++----
 38 files changed, 164 insertions(+), 182 deletions(-)

-- 
2.32.0


             reply	other threads:[~2021-06-24 16:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24 16:05 Toke Høiland-Jørgensen [this message]
2021-06-24 16:05 ` [PATCH bpf-next v5 01/19] rcu: Create an unrcu_pointer() to remove __rcu from a pointer Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 02/19] doc: Clarify and expand RCU updaters and corresponding readers Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 03/19] doc: Give XDP as example of non-obvious RCU reader/updater pairing Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 04/19] bpf: allow RCU-protected lookups to happen from bh context Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 05/19] xdp: add proper __rcu annotations to redirect map entries Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 06/19] sched: remove unneeded rcu_read_lock() around BPF program invocation Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 07/19] ena: remove rcu_read_lock() around XDP " Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 08/19] bnxt: " Toke Høiland-Jørgensen
2021-06-24 16:05 ` [PATCH bpf-next v5 09/19] thunderx: " Toke Høiland-Jørgensen
2021-06-24 16:05   ` Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 10/19] freescale: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 11/19] net: intel: " Toke Høiland-Jørgensen
2021-06-24 16:06   ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-06-24 16:06 ` [PATCH bpf-next v5 12/19] marvell: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 13/19] mlx4: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 14/19] nfp: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 15/19] qede: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 16/19] sfc: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 17/19] netsec: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 18/19] stmmac: " Toke Høiland-Jørgensen
2021-06-24 16:06 ` [PATCH bpf-next v5 19/19] net: ti: " Toke Høiland-Jørgensen
2021-06-24 18:00 ` [PATCH bpf-next v5 00/19] Clean up and document RCU-based object protection for XDP and TC BPF 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=20210624160609.292325-1-toke@redhat.com \
    --to=toke@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=kafai@fb.com \
    --cc=kuba@kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=magnus.karlsson@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=paulmck@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.