The struct bpf_struct_ops structure in BPF is a framework that allows
subsystems to extend themselves using BPF. In commit 68b04864ca425
("bpf: Create links for BPF struct_ops maps") and commit aef56f2e918bf
("bpf: Update the struct_ops of a bpf_link"), the structure was updated
to include new ->validate() and ->update() callbacks respectively in
support of allowing struct_ops maps to be created with BPF_F_LINK.
The intention was that struct bpf_struct_ops implementations could
support map updates through the link. Because map validation and
registration would take place in two separate steps for struct_ops
maps managed by the link (the first in map update elem, and the latter
in link create), the ->validate() callback was added, and any struct_ops
implementation that wished to use BPF_F_LINK, even just for lifetime
management, would then be required to define both it and ->update().
Not all struct_ops implementations can or will support update, however.
For example, the sched_ext struct_ops implementation proposed in [0]
will not be able to support atomic map updates because it can race with
sysrq, has to cycle tasks through various states in order to safely
transition, etc. It can, however, benefit from letting the BPF link
automatically evict the struc_ops map when the application exits (e.g.
if it crashes).
This patch set therefore:
1. Updates the struct_ops implementation to support default values for
->validate() and ->update() so that struct_ops implementations can
benefit from BPF_F_LINK management even if they can't support
updates.
2. Documents struct bpf_struct_ops so that the semantics are clear and
well defined.
---
v2: https://lore.kernel.org/bpf/[email protected]/T/
Changes from v2 -> v3:
- Add patch 2/2 that documents the struct bpf_struct_ops structure.
- Add Kui-Feng's Acked-by tag to patch 1/2.
v1: https://lore.kernel.org/lkml/20230811150934.GA542801@maniforge/
Changes from v1 -> v2:
- Move the if (!st_map->st_ops->update) check outside of the critical
section before we acquire the update_mutex.
David Vernet (2):
bpf: Support default .validate() and .update() behavior for struct_ops
links
bpf: Document struct bpf_struct_ops fields
include/linux/bpf.h | 43 +++++++++++++++++++++++++++++++++++++
kernel/bpf/bpf_struct_ops.c | 15 +++++++------
2 files changed, 52 insertions(+), 6 deletions(-)
--
2.41.0
Currently, if a struct_ops map is loaded with BPF_F_LINK, it must also
define the .validate() and .update() callbacks in its corresponding
struct bpf_struct_ops in the kernel. Enabling struct_ops link is useful
in its own right to ensure that the map is unloaded if an application
crashes. For example, with sched_ext, we want to automatically unload
the host-wide scheduler if the application crashes. We would likely
never support updating elements of a sched_ext struct_ops map, so we'd
have to implement these callbacks showing that they _can't_ support
element updates just to benefit from the basic lifetime management of
struct_ops links.
Let's enable struct_ops maps to work with BPF_F_LINK even if they
haven't defined these callbacks, by assuming that a struct_ops map
element cannot be updated by default.
Acked-by: Kui-Feng Lee <[email protected]>
Signed-off-by: David Vernet <[email protected]>
---
kernel/bpf/bpf_struct_ops.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index eaff04eefb31..fdc3e8705a3c 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -509,9 +509,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
}
if (st_map->map.map_flags & BPF_F_LINK) {
- err = st_ops->validate(kdata);
- if (err)
- goto reset_unlock;
+ err = 0;
+ if (st_ops->validate) {
+ err = st_ops->validate(kdata);
+ if (err)
+ goto reset_unlock;
+ }
set_memory_rox((long)st_map->image, 1);
/* Let bpf_link handle registration & unregistration.
*
@@ -663,9 +666,6 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
if (attr->value_size != vt->size)
return ERR_PTR(-EINVAL);
- if (attr->map_flags & BPF_F_LINK && (!st_ops->validate || !st_ops->update))
- return ERR_PTR(-EOPNOTSUPP);
-
t = st_ops->type;
st_map_size = sizeof(*st_map) +
@@ -823,6 +823,9 @@ static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct bpf_map
if (!bpf_struct_ops_valid_to_reg(new_map))
return -EINVAL;
+ if (!st_map->st_ops->update)
+ return -EOPNOTSUPP;
+
mutex_lock(&update_mutex);
old_map = rcu_dereference_protected(st_link->map, lockdep_is_held(&update_mutex));
--
2.41.0
Hello:
This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <[email protected]>:
On Mon, 14 Aug 2023 13:59:06 -0500 you wrote:
> The struct bpf_struct_ops structure in BPF is a framework that allows
> subsystems to extend themselves using BPF. In commit 68b04864ca425
> ("bpf: Create links for BPF struct_ops maps") and commit aef56f2e918bf
> ("bpf: Update the struct_ops of a bpf_link"), the structure was updated
> to include new ->validate() and ->update() callbacks respectively in
> support of allowing struct_ops maps to be created with BPF_F_LINK.
>
> [...]
Here is the summary with links:
- [bpf-next,v3,1/2] bpf: Support default .validate() and .update() behavior for struct_ops links
https://git.kernel.org/bpf/bpf-next/c/8ba651ed7fa1
- [bpf-next,v3,2/2] bpf: Document struct bpf_struct_ops fields
https://git.kernel.org/bpf/bpf-next/c/bb48cf1679d2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html