Struct ops cgroup v3#12712
Open
ameryhung wants to merge 15 commits into
Open
Conversation
st_link->map is always written under update_mutex. The paths that read st_link->map with rcu_read_lock() are not in the fast path, so they can simply take update_mutex instead. Remove the __rcu annotation and replace all RCU accessors with direct pointer reads under update_mutex. Use READ_ONCE() in bpf_struct_ops_map_link_poll() which reads the pointer without holding update_mutex. It is a simplification change. Reviewed-by: Eduard Zingerman <eddyz87@gmail.com> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
bpf_struct_ops_map_free() currently waits for both a regular RCU grace period and a tasks RCU grace period for every struct_ops map through synchronize_rcu_mult(call_rcu, call_rcu_tasks). A regular RCU grace period is still required for all struct_ops maps because the struct_ops trampoline ksyms requires a rcu grace period (take a look at the list_del_rcu in __bpf_ksym_del). Add a map_free_pre_rcu() callback so the struct_ops map can remove ksyms before bpf_map_put() wait for the regular rcu grace period. The tasks RCU grace period is only needed by tcp_congestion_ops. Add free_after_tasks_rcu_gp only to struct bpf_struct_ops instead of the bpf_map. When CONFIG_TASKS_RCU=n, synchronize_rcu_tasks() is the same as synchronize_rcu(). Since all struct_ops maps now complete a regular RCU grace period before bpf_struct_ops_map_free() runs, skip the extra synchronize_rcu_tasks() call in this case. This cleanup prepares for a later patch that needs to support free_after_mult_rcu_gp. Reviewed-by: Eduard Zingerman <eddyz87@gmail.com> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
Add the helper functions bpf_struct_ops_map_kdata() and bpf_struct_ops_map_cfi_stubs() in bpf_struct_ops.c. They will be called from cgroup.c in the upcoming patch to create a struct_ops to cgroup attachment link. bpf_struct_ops_valid_to_reg() is also exposed for the upcoming caller in cgroup.c. The link update validation is also refactored into a new function bpf_struct_ops_link_update_check() such that it can be reused by the caller in cgroup.c in the upcoming patch. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
effective_prog_pos(), called from replace_effective_prog() and purge_effective_progs(), tests "!prog_list_prog(pl)" to skip a 'detaching' pl. When detaching a pl, pl->prog and pl->link are set to NULL in case the update_effective_progs() failed. However, replace_effective_prog() is not detaching a pl, so the case "!prog_list_prog()" will not happen. In purge_effective_prog(), the pl->prog and pl->link are restored before calling purge_effective_progs(), so the case "!prog_list_prog()" will not happen either. This patch removes them as a prep work for the upcoming work in attaching struct_ops to cgroup. When attaching a struct_ops to cgroup, there is a link->map case and the prog_list_prog() will not consider the link->map. The replace_effective_prog() and purge_effective_progs() will then incorrectly skip a pl with struct_ops map attached to it. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
… check prog_list_length() and compute_effective_progs() use !prog_list_prog(pl) to skip a 'detaching' pl. When pl->link is not NULL, prog_list_prog(pl) returns the pl->link->link.prog. This does not work for the upcoming struct_ops patch where pl->link is not NULL but pl->link->link.prog is NULL, because a struct_ops map is attached to the cgroup instead of a BPF prog. To prepare for the upcoming struct_ops patch, this patch replaces the prog_list_prog() test with the "!pl->prog && !pl->link". In __cgroup_bpf_detach(), both pl->prog and pl->link are set to NULL, so testing "!pl->prog && !pl->link" is the same test to tell if a pl is being detached. This change should be a no-op. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
…st_id() Add three helpers to abstract operations on a bpf_prog_list entry. Right now, bpf_prog_array_item is initialized from prog_list_prog(pl), which returns either pl->prog or pl->link->link.prog. This will not work when struct_ops is attached to a cgroup because the attachment is backed by a struct_ops map instead of a BPF prog. The same applies to __cgroup_bpf_query(). Instead of always copying a prog id to userspace, struct_ops cgroup attachment will need to copy the struct_ops map id. Refactor bpf_prog_array_item initialization into prog_list_init_item() and prog_list_replace_item(), and refactor id lookup into prog_list_id(). These helpers will be extended to support pl->link->map in a later patch. This is a no-op change. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
Move the LSM trampoline unlink into bpf_cgroup_link_auto_detach(). The purpose is to consolidate the auto_detach cleanup logic. It prepares for the upcoming struct_ops cgroup attachment patch where bpf_cgroup_link_auto_detach() will need to handle the struct_ops case (link->map != NULL). This is a no-op change. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
In the upcoming patch, the array can store a struct_ops map. The array could have a cfi_stubs acting as a dummy instead of the dummy_bpf_prog. The array logic will need to skip the cfi_stubs also in order to support storing struct_ops map in the array. bpf_cgroup_array_length(), bpf_cgroup_array_copy_to_user(), and bpf_cgroup_array_delete_safe_at() are added as a preparation work to allow skipping the cfi_stubs in the upcoming patch. This patch only skips the dummy_bpf_prog which is the same as the existing behavior. The current bpf_prog_array_*() callers are changed to call the new bpf_cgroup_array_*(). This is a no-op change. Unlike bpf_prog_array_copy_to_user(), bpf_cgroup_array_copy_to_user() does not need a temporary buffer. The cgroup caller already holds cgroup_mutex and dereferences the effective array with rcu_dereference_protected(), so it does not copy to userspace from an RCU read-side critical section. Details in commit 0911287. Another addition is the bpf_cgroup_array_free(). This prepares the array to have a different rcu gp for the struct_ops use case, for example, a struct_ops could have mix of sleepable ops and non-sleepable ops. In this patch, bpf_cgroup_array_free() only goes through the regular rcu gp. This is a no-op change also. bpf_prog_dummy() is also added to return the global dummy_bpf_prog. bpf_cgroup_array_dummy() is added to decide the sentinel based on atype. It now always returns bpf_prog_dummy(). In the upcoming patch, it can return a cfi_stubs if the atype belongs to a struct_ops. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
This patch adds necessary infrastructure to attach a struct_ops map to a cgroup. The initial need was to support migrating the legacy BPF_PROG_TYPE_SOCK_OPS to a struct_ops. Recently, there are other struct_ops use cases that need to attach struct_ops to a cgroup. For example, the recent BPF OOM and memcg discussion in LSFMMBPF 2026. The motivation is to create a consistent expectation for attaching struct_ops to cgroup instead of each subsystem creating its own infrastructure. This logic includes hierarchy expectation, ordering expectation, attachment API, and rcu gp. There is already an existing implementation for attaching multiple bpf progs to a cgroup. There are also tools built around it for querying. Attaching a struct_ops map (which is a group of bpf programs) could also adhere to a similar API and potentially reuse most of the existing implementation. A couple of ideas have been tried. One of them is to use mprog.c. In terms of the amount of changes, I eventually came to the same conclusion as in commit 1209339 ("bpf: Implement mprog API on top of existing cgroup progs"). I then shifted the focus to reusing the current {update,compute,activate,purge}_effective_progs() which has the main logic that implements the mprog API. Since then, I tried to add a 'struct cgroup *cgroup' member to the existing 'struct bpf_struct_ops_link' and link_create will create a 'struct bpf_struct_ops_link' object to be stored in the pl->link. This turns out to have more changes on both cgroup.c and bpf_struct_ops.c than I like. This patch directly reuses the 'struct bpf_cgroup_link' which cgroup.c already understands. Add 'struct bpf_map *map' to 'struct bpf_cgroup_link'. In the future, as more subsystems are extended by struct_ops, we may consider to make 'struct bpf_map *map' as a primary citizen of a link like 'struct bpf_prog *prog' and directly add 'struct bpf_map *map' to the generic 'struct bpf_link'. The pl->link could be the traditional 'prog' link or the new 'map' link. The places that need to handle them differently have already been refactored into the new prog_list_*() added in the earlier patch. In those new prog_list_*(), this patch will check "pl->link && pl->link->map", learn that it is a 'map' link and handle it correctly. The bpf_prog_array also needs to handle that its item can store the traditional 'prog' or it can store a struct_ops map. The places that need to handle them differently have also been refactored into the new bpf_cgroup_array_*() added in the earlier patch. The two differences are: - different sentinel (dummy_bpf_prog in prog vs cfi_stub in struct_ops) - the array for struct_ops may need to go through different rcu gp. The bpf_cgroup_array_*() functions use the cgroup_bpf_attach_type (ie atype) to distinguish the array is storing prog or storing struct_ops map. This patch also implements a separate struct bpf_link_ops "cgroup_struct_ops_link_ops" to have a separate link_ops implementation that only handles the cgroup's struct_ops link. Questions: - Although this patch did not change it, it is not obvious to me how the replace_effective_progs() and purge_effective_progs() handle cases when there are existing BPF_F_PREORDER progs attached in the hlist. Misc notes: - CGROUP_TCP_SOCK_OPS is added to the 'enum cgroup_bpf_attach_type'. The actual implementation of the tcp_bpf_ops (a struct_ops) will be added in the next patch. - free_after_mult_rcu_gp is added to 'struct bpf_struct_ops' such that the bpf_prog_array can have a mix of sleepable and non-sleepable prog in a struct_ops. This can tell how the bpf_prog_array should be freed. - For a struct_ops that supports cgroup attachment, it does not need to implement its own reg/unreg function. reg/unreg to a cgroup is done by the common infrastructure added in this patch. - The cgroup's struct_ops link only supports BPF_F_ALLOW_MULTI. This is enforced internally in cgroup_bpf_struct_ops_attach. This should be consistent with the current prog's link behavior in cgroup_bpf_link_attach. In the future, we may allow each subsystem to choose differently. - A cgroup_atype member is added to 'struct bpf_struct_ops'. When a subsystem struct_ops needs to support cgroup attachment, it needs to add a value to 'enum cgroup_bpf_attach_type' and then assign it to the newly added cgroup_atype member in the bpf_struct_ops. - During LINK_CREATE in syscall, the patch uses the same BPF_STRUCT_OPS (in attr->link_create.attach_type). The bpf_struct_ops_link_create learns the map and from the map it learns the st_ops. If the st_ops->cgroup_atype is not 0, it will create a cgroup's link. - When a subsystem registers a struct_ops that supports cgroup attachment, the struct_ops infrastructure will also ask the cgroup infrastructure to remember a few things. This is done by calling cgroup_bpf_struct_ops_register(). Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
bpf_dynptr_from_skb() was only made available to bpf_qdisc, so far the only struct_ops type that needs to read an skb. The upcoming bpf_tcp_ops header-option hooks (parse_hdr/write_hdr_opt) also want to access the TCP options of an skb through a dynptr. All struct_ops programs share BPF_PROG_TYPE_STRUCT_OPS, so register bpf_kfunc_set_skb (which holds bpf_dynptr_from_skb) for that program type once, instead of per struct_ops. This makes bpf_dynptr_from_skb() available to bpf_tcp_ops and any future struct_ops. With the kfunc now provided to all of struct_ops, the bpf_qdisc-specific registration becomes redundant and is dropped: bpf_qdisc_kfunc_filter() only constrains kfuncs listed in qdisc_kfunc_ids, so removing bpf_dynptr_from_skb from that set (and from qdisc_common_kfunc_set) lets it fall through the filter unchanged, and bpf_qdisc keeps access via the generic struct_ops registration. Widening the registration is safe: a struct_ops that does not receive an skb in its context has nothing to pass to the helper. Signed-off-by: Amery Hung <ameryhung@gmail.com>
In LSFMMBPF 2025, I have talked about moving the BPF_PROG_TYPE_SOCK_OPS
to a struct_ops interface [1].
The BPF_SOCK_OPS_*_CB enum interface has grown over time as new TCP
callback points were added. A BPF_PROG_TYPE_SOCK_OPS program now
commonly needs a large switch on sock_ops->op, and the shared
bpf_sock_ops_kern context has become harder to extend because different
callbacks have different locking, argument, skb, and helper
requirements. The existing 'union { u32 args[4]; u32 replylong[4]; }' is
also not reliable in passing args to bpf prog when there are multiple
progs attached to a cgroup.
The above has already been solved in struct_ops. Add a TCP-specific
struct_ops type, bpf_tcp_ops, and support attaching it to cgroups.
This allows each callback have its own func signature and allows
the verifier to select kfuncs/helpers based on the specific
struct_ops member being implemented.
This patch wires up the following existing sock_ops callbacks:
- BPF_SOCK_OPS_TIMEOUT_INIT
- BPF_SOCK_OPS_RWND_INIT
- BPF_SOCK_OPS_RTT_CB
- BPF_SOCK_OPS_STATE_CB
- BPF_SOCK_OPS_RETRANS_CB
- BPF_SOCK_OPS_TCP_CONNECT_CB
- BPF_SOCK_OPS_TCP_LISTEN_CB
- BPF_SOCK_OPS_RTO_CB
- BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB
- BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB
BASE_RTT is ignored as it is not particularly useful. NEEDS_ECN should
be done in bpf-tcp-cc instead. The tstamp ones should be a separate
struct_ops (e.g. "bpf_sock_ops") that can work in both TCP and UDP.
timeout_init and rwnd_init could have a request_sock pointer. This patch
tries a different API and direclty passes the request_sock pointer as
an arg.
Two other approaches were considered before settling on having
bpf_get_retval() read the dispatcher's run_ctx via saved_run_ctx. The
first was to inherit the retval in the trampoline itself: add a helper
in the four __bpf_prog_enter*() paths that, for struct_ops programs,
copies the chained value from the caller's run_ctx (now saved_run_ctx)
into the program's own run_ctx. It works but puts a per-enter
program-type check on the generic trampoline fast path, taxing all
fentry/fexit/lsm callers for a cgroup-struct_ops-only feature. The
second was to do that same inherit only for the int-returning members
via a gen_prologue that emits a hidden kfunc at the start of
timeout_init/rwnd_init; this keeps the cost off the generic path and
scoped to bpf_tcp_ops, but needs a kfunc + BTF_ID + prologue-emission
machinery. The chosen approach avoids both: it touches neither the
trampoline nor the program, since saved_run_ctx already points at the
dispatcher's run_ctx that carries the value.
[1], page 13: https://drive.google.com/file/d/1wjKZth6T0llLJ_ONPAL_6Q_jbxbAjByp/view?usp=sharing
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Signed-off-by: Amery Hung <ameryhung@gmail.com>
Add the TCP header option callbacks to the bpf_tcp_ops struct_ops type:
parse_hdr - parse the options of an incoming skb on an established
connection
hdr_opt_len - reserve space in the TCP header for bpf options
write_hdr_opt - write the reserved bpf options
These mirror the BPF_SOCK_OPS_PARSE_HDR_OPT_CB, _HDR_OPT_LEN_CB and
_WRITE_HDR_OPT_CB legacy sockops callbacks, but are exposed as struct_ops
members so a program can implement them with normal function signatures
and per-member helper sets.
The reserved header window is shared between the legacy sockops and
bpf_tcp_ops paths. tcp_{syn,synack,established}_options() first run the
legacy BPF_SOCK_OPS_HDR_OPT_LEN_CB and then call hdr_opt_len, so both
sources accumulate into opts->bpf_opt_len; at write time the legacy
options are emitted first and bpf_tcp_ops writes after them.
API design
bpf_tcp_ops overloads the sock_ops header-option helpers rather than
introducing a new API: bpf_reserve_hdr_opt(), bpf_store_hdr_opt() and
bpf_load_hdr_opt() are exposed per-member (reserve for hdr_opt_len,
store/load for write_hdr_opt, load for parse_hdr) and share the existing
kernel option-walking core via _bpf_sock_ops{store,load}hdr_opt(), with
the bpf_tcp_ops wrappers synthesizing a temporary bpf_sock_ops_kern from
the program ctx. This keeps a port from the legacy
BPF_SOCK_OPS*_HDR_OPT_CB callbacks mechanical (same helper calls) and
adds no new UAPI helper/kfunc surface.
An alternative considered was to drop the option helpers entirely: have
hdr_opt_len reserve space purely through its return value, and introduce
a dedicated TCP-header-option dynptr used for both reading and writing.
That is a cleaner, more self-contained interface, but it is a larger
change and does not reuse the legacy helpers, making a port from sockops
less mechanical. It can be pursued as a follow-up; the helper-based
interface here keeps this series focused on moving the hooks to
struct_ops.
The hdr_opt_len fast path in tcp_established_options() is gated by
cgroup_bpf_enabled(CGROUP_TCP_SOCK_OPS). Note this is a global,
per-attach-type static branch: it is enabled whenever any bpf_tcp_ops is
attached, even one that does not implement hdr_opt_len or that is attached
to a different cgroup. In those cases the block still runs but
bpf_tcp_ops_hdr_opt_len() no-ops via the per-member check in the dispatch
macro. A per-member/per-cgroup gate could be added later if the extra
fast-path work proves measurable.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
Add bpf_map__attach_cgroup_opts() to attach a struct_ops map to a cgroup through a BPF link. Also extend struct bpf_prog_query_opts with a type_id field so a BPF_STRUCT_OPS query on a cgroup can select the struct_ops type to enumerate. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com>
Exercise attaching the bpf_tcp_ops struct_ops to cgroups via the generic
cgroup link infrastructure. The struct_ops instances record their
execution order and the previous return value to validate correctness.
Subtests:
- query: BPF_F_QUERY_EFFECTIVE and attached query return the maps
- order: BPF_F_PREORDER vs attach order within a cgroup
- before_after: BPF_F_BEFORE/BPF_F_AFTER relative positioning
- update: bpf_link__update_map swaps a link's map, keeping its slot
- retval: int return value chained across timeout_init progs of
multiple bpf_tcp_ops attached to a cgroup
- hierarchy: parent and child attachments merge in the child's
effective array (descendant before ancestor)
- inherit: a child created after the attach inherits the parent's
prog
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Signed-off-by: Amery Hung <ameryhung@gmail.com>
Add a test exercising the bpf_tcp_ops parse_hdr, hdr_opt_len and write_hdr_opt members together with the header option helpers. The struct_ops program (progs/bpf_tcp_ops_hdr.c) reserves space in hdr_opt_len via bpf_reserve_hdr_opt(), writes an experimental option in write_hdr_opt via bpf_store_hdr_opt(), and recovers it in parse_hdr via bpf_load_hdr_opt() on the incoming skb. Each hook bumps a counter and the parse hook records the option payload, so the three callbacks and all three overloaded helpers are covered. Signed-off-by: Amery Hung <ameryhung@gmail.com>
kernel-patches-daemon-bpf
Bot
force-pushed
the
bpf-next_base
branch
15 times, most recently
from
87b0f0c to
787c964
Compare
kernel-patches-daemon-bpf
Bot
force-pushed
the
bpf-next_base
branch
12 times, most recently
from
386cc3c to
ded8f8a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.