Netdev List
 help / color / mirror / Atom feed
From: "Emil Tsalapatis" <emil@etsalapatis.com>
To: "Amery Hung" <ameryhung@gmail.com>, <bpf@vger.kernel.org>
Cc: <netdev@vger.kernel.org>, <alexei.starovoitov@gmail.com>,
	<andrii@kernel.org>, <daniel@iogearbox.net>, <eddyz87@gmail.com>,
	<memxor@gmail.com>, <martin.lau@kernel.org>,
	<shakeel.butt@linux.dev>, <roman.gushchin@linux.dev>,
	<kuniyu@google.com>, <kerneljasonxing@gmail.com>,
	<kernel-team@meta.com>
Subject: Re: [PATCH bpf-next v3 09/15] bpf: Add infrastructure to support attaching struct_ops to cgroups
Date: Tue, 14 Jul 2026 02:21:31 -0400	[thread overview]
Message-ID: <DJY2SPXTF9HL.10SSKCT9OMMBV@etsalapatis.com> (raw)
In-Reply-To: <20260706171918.317102-10-ameryhung@gmail.com>

On Mon Jul 6, 2026 at 1:19 PM EDT, Amery Hung wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> 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 120933984460 ("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>

Hi Amery,

> ---
>  include/linux/bpf-cgroup-defs.h |   1 +
>  include/linux/bpf-cgroup.h      |  28 +++
>  include/linux/bpf.h             |  19 +-
>  include/uapi/linux/bpf.h        |   4 +-
>  kernel/bpf/bpf_struct_ops.c     |  29 +++
>  kernel/bpf/btf.c                |  23 +-
>  kernel/bpf/cgroup.c             | 375 ++++++++++++++++++++++++++++++--
>  kernel/bpf/syscall.c            |   1 +
>  tools/include/uapi/linux/bpf.h  |   4 +-
>  9 files changed, 463 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/bpf-cgroup-defs.h b/include/linux/bpf-cgroup-defs.h
> index c9e6b26abab6..0147b8bec973 100644
> --- a/include/linux/bpf-cgroup-defs.h
> +++ b/include/linux/bpf-cgroup-defs.h
> @@ -47,6 +47,7 @@ enum cgroup_bpf_attach_type {
>  	CGROUP_INET6_GETSOCKNAME,
>  	CGROUP_UNIX_GETSOCKNAME,
>  	CGROUP_INET_SOCK_RELEASE,
> +	CGROUP_TCP_SOCK_OPS,
>  	CGROUP_LSM_START,
>  	CGROUP_LSM_END = CGROUP_LSM_START + CGROUP_LSM_NUM - 1,
>  	MAX_CGROUP_BPF_ATTACH_TYPE
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 4d0cc65976a1..8a75a6cd7309 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -100,6 +100,8 @@ struct bpf_cgroup_storage {
>  struct bpf_cgroup_link {
>  	struct bpf_link link;
>  	struct cgroup *cgroup;
> +	struct bpf_map *map;
> +	wait_queue_head_t wait_hup;
>  };
>  
>  struct bpf_prog_list {
> @@ -110,6 +112,18 @@ struct bpf_prog_list {
>  	u32 flags;
>  };
>  
> +#define bpf_cgroup_struct_ops_foreach(var, item, cgrp, atype)		\
> +	for (item = rcu_dereference((cgrp)->bpf.effective[atype])->items;\
> +	     ((var) = READ_ONCE(item->kdata));				\
> +	     item++)
> +
> +static inline bool cgroup_bpf_is_struct_ops_atype(enum cgroup_bpf_attach_type atype)
> +{
> +	return atype == CGROUP_TCP_SOCK_OPS;
> +}
> +void cgroup_bpf_struct_ops_register(int atype, u32 type_id, void *cfi_stubs, bool mult_trace);
> +int cgroup_bpf_struct_ops_attach(struct bpf_map *map, const union bpf_attr *attr);
> +
>  void __init cgroup_bpf_lifetime_notifier_init(void);
>  
>  int __cgroup_bpf_run_filter_skb(struct sock *sk,
> @@ -479,6 +493,20 @@ static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
>  	return 0;
>  }
>  
> +static inline bool cgroup_bpf_is_struct_ops_atype(int atype)
> +{
> +	return false;
> +}
> +static inline void cgroup_bpf_struct_ops_register(int atype, u32 type_id, void *cfi_stubs,
> +						  bool mult_trace)
> +{
> +}
> +static inline int cgroup_bpf_struct_ops_attach(struct bpf_map *map,
> +					       const union bpf_attr *attr)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #define cgroup_bpf_enabled(atype) (0)
>  #define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, atype, t_ctx) ({ 0; })
>  #define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, atype) ({ 0; })
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e371a4733135..df95ae690da5 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2100,11 +2100,18 @@ struct btf_member;
>   *	   unloaded while in use.
>   * @name: The name of the struct bpf_struct_ops object.
>   * @func_models: Func models
> + * @cgroup_atype: A value in enum cgroup_bpf_attach_type for cgroup attachment.
> + *		  0 means the struct_ops type does not support cgroup attachment.
> + *		  If cgroup_atype is non-zero, the @reg and @unreg must be NULL
> + *		  because the attachment/detachment will be handled by the bpf core.
>   * @free_after_tasks_rcu_gp: Set to true if it needs the bpf core to wait for
>   *                           a tasks_rcu gp before freeing the struct_ops map
>   *                           and its progs. It is unnecessary if the @unreg
>   *                           has waited for the correct rcu gp or the @unreg
>   *                           has ensured all struct_ops prog has finished running.
> + * @free_after_mult_rcu_gp: Same as @free_after_tasks_rcu_gp but waiting for
> + *                          both tasks_trace_rcu and regular rcu grace period.
> + *                          It is usually needed if the struct_ops has sleepable prog.
>   */
>  struct bpf_struct_ops {
>  	const struct bpf_verifier_ops *verifier_ops;
> @@ -2123,7 +2130,9 @@ struct bpf_struct_ops {
>  	struct module *owner;
>  	const char *name;
>  	struct btf_func_model func_models[BPF_STRUCT_OPS_MAX_NR_MEMBERS];
> +	int cgroup_atype;
>  	bool free_after_tasks_rcu_gp;
> +	bool free_after_mult_rcu_gp;
>  };
>  
>  /* Every member of a struct_ops type has an instance even a member is not
> @@ -2258,6 +2267,7 @@ void *bpf_struct_ops_map_cfi_stubs(struct bpf_map *map);
>  bool bpf_struct_ops_valid_to_reg(struct bpf_map *map);
>  int bpf_struct_ops_link_update_check(struct bpf_map *new_map, struct bpf_map *old_map,
>  				     struct bpf_map *expected_old_map);
> +int bpf_struct_ops_map_cgroup_atype(struct bpf_map *map);
>  
>  #ifdef CONFIG_NET
>  /* Define it here to avoid the use of forward declaration */
> @@ -2330,6 +2340,10 @@ static inline u32 bpf_struct_ops_kdata_map_id(void *kdata)
>  {
>  	return 0;
>  }
> +static inline int bpf_struct_ops_map_cgroup_atype(struct bpf_map *map)
> +{
> +	return 0;
> +}
>  static inline void *bpf_struct_ops_map_cfi_stubs(struct bpf_map *map)
>  {
>  	return NULL;
> @@ -2519,7 +2533,10 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
>   * since other cpus are walking the array of pointers in parallel.
>   */
>  struct bpf_prog_array_item {
> -	struct bpf_prog *prog;
> +	union {
> +		struct bpf_prog *prog;
> +		void *kdata;
> +	};
>  	union {
>  		struct bpf_cgroup_storage *cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE];
>  		u64 bpf_cookie;
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c91b5a4bda03..c982801ec7db 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1756,7 +1756,7 @@ union bpf_attr {
>  			__u32	prog_cnt;
>  			__u32	count;
>  		};
> -		__u32		:32;
> +		__u32		type_id;
>  		/* output: per-program attach_flags.
>  		 * not allowed to be set during effective query.
>  		 */
> @@ -6818,6 +6818,8 @@ struct bpf_link_info {
>  		} xdp;
>  		struct {
>  			__u32 map_id;
> +			__u32 :32;
> +			__u64 cgroup_id;
>  		} struct_ops;
>  		struct {
>  			__u32 pf;
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 3d650a7e9f68..06d72cdca601 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -13,6 +13,7 @@
>  #include <linux/btf_ids.h>
>  #include <linux/rcupdate_wait.h>
>  #include <linux/poll.h>
> +#include <linux/bpf-cgroup.h>
>  
>  struct bpf_struct_ops_value {
>  	struct bpf_struct_ops_common_value common;
> @@ -1076,6 +1077,11 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
>  		goto errout;
>  	}
>  
> +	if (st_ops_desc->st_ops->cgroup_atype && !(attr->map_flags & BPF_F_LINK)) {
> +		ret = -EOPNOTSUPP;
> +		goto errout;
> +	}
> +
>  	vt = st_ops_desc->value_type;
>  	if (attr->value_size != vt->size) {
>  		ret = -EINVAL;
> @@ -1116,6 +1122,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
>  
>  	mutex_init(&st_map->lock);
>  	bpf_map_init_from_attr(map, attr);
> +	map->free_after_mult_rcu_gp = st_ops_desc->st_ops->free_after_mult_rcu_gp;
>  	map->free_after_rcu_gp = true;
>  
>  	return map;
> @@ -1244,6 +1251,14 @@ void *bpf_struct_ops_map_kdata(struct bpf_map *map)
>  	return st_map->kvalue.data;
>  }
>  
> +int bpf_struct_ops_map_cgroup_atype(struct bpf_map *map)
> +{
> +	struct bpf_struct_ops_map *st_map;
> +
> +	st_map = container_of(map, struct bpf_struct_ops_map, map);
> +	return st_map->st_ops_desc->st_ops->cgroup_atype;
> +}
> +
>  void *bpf_struct_ops_map_cfi_stubs(struct bpf_map *map)
>  {
>  	struct bpf_struct_ops_map *st_map;
> @@ -1419,6 +1434,7 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
>  	struct bpf_link_primer link_primer;
>  	struct bpf_struct_ops_map *st_map;
>  	struct bpf_map *map;
> +	int cgroup_atype;
>  	int err;
>  
>  	map = bpf_map_get(attr->link_create.map_fd);
> @@ -1432,6 +1448,19 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
>  		goto err_out;
>  	}
>  
> +	cgroup_atype = st_map->st_ops_desc->st_ops->cgroup_atype;
> +	if (cgroup_atype) {
> +		err = cgroup_bpf_struct_ops_attach(map, attr);
> +		bpf_map_put(map);
> +		return err;
> +	}
> +
> +	if (memchr_inv(&attr->link_create.cgroup, 0, sizeof(attr->link_create.cgroup)) ||
> +	    attr->link_create.target_fd) {
> +		err = -EINVAL;
> +		goto err_out;
> +	}
> +
>  	link = kzalloc_obj(*link, GFP_USER);
>  	if (!link) {
>  		err = -ENOMEM;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 64572f85edc8..d591f306ace5 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -20,6 +20,7 @@
>  #include <linux/btf.h>
>  #include <linux/btf_ids.h>
>  #include <linux/bpf.h>
> +#include <linux/bpf-cgroup.h>
>  #include <linux/bpf_lsm.h>
>  #include <linux/skmsg.h>
>  #include <linux/perf_event.h>
> @@ -9836,6 +9837,7 @@ btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops,
>  		   struct bpf_verifier_log *log)
>  {
>  	struct btf_struct_ops_tab *tab, *new_tab;
> +	int cgroup_atype;
>  	int i, err;
>  
>  	tab = btf->struct_ops_tab;
> @@ -9847,8 +9849,10 @@ btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops,
>  		btf->struct_ops_tab = tab;
>  	}
>  
> +	cgroup_atype = st_ops->cgroup_atype;
>  	for (i = 0; i < tab->cnt; i++)
> -		if (tab->ops[i].st_ops == st_ops)
> +		if (tab->ops[i].st_ops == st_ops ||
> +		    (cgroup_atype && cgroup_atype == tab->ops[i].st_ops->cgroup_atype))
>  			return -EEXIST;
>  
>  	if (tab->cnt == tab->capacity) {
> @@ -9868,6 +9872,23 @@ btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops,
>  	if (err)
>  		return err;
>  
> +	if (cgroup_atype) {
> +		if (!cgroup_bpf_is_struct_ops_atype(cgroup_atype) ||

Can we add a comment here as to why free_after_tasks_rcu_gp is always
invalid here (which IIUC is because it's 
free_after_mult_rcu_gp)?

> +		    st_ops->reg || st_ops->unreg || st_ops->free_after_tasks_rcu_gp) {
> +			bpf_struct_ops_desc_release(&tab->ops[btf->struct_ops_tab->cnt]);
> +			return -EINVAL;
> +		}
> +
> +		/* There is no need to unregister from cgroup when the
> +		 * btf_free(). No struct_ops map and its cgroup link
> +		 * can be created once its btf is gone.
> +		 */

Network-style comment, should be regular kernel-style.

In terms of the content: What happens if the struct_ops are in a module,
then we load/unload/reload? IIUC even if the signatures are identical,
the btf_id could be different so it's not like we can always reuse the
previous cgroup registration.

> +		cgroup_bpf_struct_ops_register(cgroup_atype,
> +					       tab->ops[btf->struct_ops_tab->cnt].type_id,
> +					       st_ops->cfi_stubs,
> +					       st_ops->free_after_mult_rcu_gp);
> +	}
> +
>  	btf->struct_ops_tab->cnt++;
>  
>  	return 0;
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 081d81de1816..745e826c9f61 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -24,6 +24,29 @@
>  DEFINE_STATIC_KEY_ARRAY_FALSE(cgroup_bpf_enabled_key, MAX_CGROUP_BPF_ATTACH_TYPE);
>  EXPORT_SYMBOL(cgroup_bpf_enabled_key);
>  
> +static u32 struct_ops_type_id[MAX_CGROUP_BPF_ATTACH_TYPE];
> +static void *struct_ops_cfi_stubs[MAX_CGROUP_BPF_ATTACH_TYPE];
> +static bool struct_ops_mult_rcu[MAX_CGROUP_BPF_ATTACH_TYPE];


Can we make a struct out of those? Right now we're carrying them loose
into the module even though every index corresponds to a single
registered struct_oips.

> +
> +void cgroup_bpf_struct_ops_register(int atype, u32 type_id, void *cfi_stubs, bool mult_rcu)
> +{
> +	struct_ops_type_id[atype] = type_id;
> +	struct_ops_cfi_stubs[atype] = cfi_stubs;
> +	struct_ops_mult_rcu[atype] = mult_rcu;
> +}
> +
> +static enum cgroup_bpf_attach_type find_atype_by_struct_ops_id(u32 type_id)
> +{
> +	enum cgroup_bpf_attach_type atype;
> +
> +	for (atype = 0; atype < MAX_CGROUP_BPF_ATTACH_TYPE; atype++) {
> +		if (cgroup_bpf_is_struct_ops_atype(atype) &&
> +		    struct_ops_type_id[atype] == type_id)
> +			return atype;
> +	}
> +	return CGROUP_BPF_ATTACH_TYPE_INVALID;
> +}
> +
>  /*
>   * cgroup bpf destruction makes heavy use of work items and there can be a lot
>   * of concurrent destructions.  Use a separate workqueue so that cgroup bpf
> @@ -306,6 +329,19 @@ static void bpf_cgroup_storages_link(struct bpf_cgroup_storage *storages[],
>  		bpf_cgroup_storage_link(storages[stype], cgrp, attach_type);
>  }
>  
> +static void cgroup_struct_ops_link_detach_wake(struct bpf_cgroup_link *link, bool wake_poll)
> +{
> +	cgroup_put(link->cgroup);
> +	link->cgroup = NULL;
> +
> +	bpf_map_put(link->map);
> +	/* READ_ONCE in cgroup_struct_ops_link_poll */
> +	WRITE_ONCE(link->map, NULL);
> +
> +	if (wake_poll)
> +		wake_up_interruptible_poll(&link->wait_hup, EPOLLHUP);
> +}
> +
>  /* Called when bpf_cgroup_link is auto-detached from dying cgroup.
>   * It drops cgroup and bpf_prog refcounts, and marks bpf_link as defunct. It
>   * doesn't free link memory, which will eventually be done by bpf_link's
> @@ -313,21 +349,37 @@ static void bpf_cgroup_storages_link(struct bpf_cgroup_storage *storages[],
>   */
>  static void bpf_cgroup_link_auto_detach(struct bpf_cgroup_link *link)
>  {
> -	if (link->link.prog->expected_attach_type == BPF_LSM_CGROUP)
> -		bpf_trampoline_unlink_cgroup_shim(link->link.prog);
> -	cgroup_put(link->cgroup);
> -	link->cgroup = NULL;
> +	if (link->map) {
> +		cgroup_struct_ops_link_detach_wake(link, true);
> +	} else {
> +		if (link->link.prog->expected_attach_type == BPF_LSM_CGROUP)
> +			bpf_trampoline_unlink_cgroup_shim(link->link.prog);
> +		cgroup_put(link->cgroup);
> +		link->cgroup = NULL;
> +	}

Nit: If we write this as:
	if (link->map) {
		cgroup_struct_ops_link_detach_wake(link, true);
		return
	}

it's slightly less churn.

> +}
> +
> +static void bpf_cgroup_array_free_rcu(struct rcu_head *rcu)
> +{
> +	kfree(container_of(rcu, struct bpf_prog_array, rcu));
>  }
>  
> -static void bpf_cgroup_array_free(struct bpf_prog_array *array)
> +static void bpf_cgroup_array_free(struct bpf_prog_array *array,
> +				  enum cgroup_bpf_attach_type atype)
>  {
>  	if (!array || array == &bpf_empty_prog_array)
>  		return;
> -	kfree_rcu(array, rcu);
> +	if (struct_ops_mult_rcu[atype])
> +		/* RCU tasks trace grace period implies RCU grace period. */
> +		call_rcu_tasks_trace(&array->rcu, bpf_cgroup_array_free_rcu);
> +	else
> +		kfree_rcu(array, rcu);
>  }
>  
>  static void *bpf_cgroup_array_dummy(enum cgroup_bpf_attach_type atype)
>  {
> +	if (cgroup_bpf_is_struct_ops_atype(atype))
> +		return struct_ops_cfi_stubs[atype];
>  	return bpf_prog_dummy();
>  }
>  
> @@ -355,7 +407,12 @@ static int bpf_cgroup_array_copy_to_user(struct bpf_prog_array *array,
>  	for (item = array->items; item->prog && i < cnt; item++) {
>  		if (item->prog == bpf_cgroup_array_dummy(atype))

Retroactive nit for bpf_cgroup_array_dummy: The name is pretty confusing, something
that explain that this is really a tombstone value would be nicer.
Especially since we're repurposing the cfi_stubs for per-attach type
tombstones.

>  			continue;
> -		id = item->prog->aux->id;
> +
> +		if (cgroup_bpf_is_struct_ops_atype(atype))
> +			id = bpf_struct_ops_id(item->kdata);
> +		else
> +			id = item->prog->aux->id;
> +
>  		if (copy_to_user(prog_ids + i, &id, sizeof(id)))
>  			return -EFAULT;
>  		i++;
> @@ -417,7 +474,7 @@ static void cgroup_bpf_release(struct work_struct *work)
>  		old_array = rcu_dereference_protected(
>  				cgrp->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
> -		bpf_cgroup_array_free(old_array);
> +		bpf_cgroup_array_free(old_array, atype);
>  	}
>  
>  	list_for_each_entry_safe(storage, stmp, storages, list_cg) {
> @@ -461,17 +518,26 @@ static struct bpf_prog *prog_list_prog(struct bpf_prog_list *pl)
>  
>  static void prog_list_init_item(struct bpf_prog_list *pl, struct bpf_prog_array_item *item)
>  {
> -	item->prog = prog_list_prog(pl);
> -	bpf_cgroup_storages_assign(item->cgroup_storage, pl->storage);
> +	if (pl->link && pl->link->map) {
> +		item->kdata = bpf_struct_ops_map_kdata(pl->link->map);
> +	} else {
> +		item->prog = prog_list_prog(pl);
> +		bpf_cgroup_storages_assign(item->cgroup_storage, pl->storage);
> +	}

Same nit as above, an if-return avoid reformatting existing code.

>  }
>  
>  static void prog_list_replace_item(struct bpf_prog_list *pl, struct bpf_prog_array_item *item)
>  {
> -	WRITE_ONCE(item->prog, pl->link->link.prog);
> +	if (pl->link && pl->link->map)

Can we somehow encapsulate the pl->link && pl->link->map to give it a
more descriptive name? It's mentioned in previous commit descriptions
what it is for, but as it stands it takes some thought to realize it's
checking if this is a struct_ops item or a regular prog.

> +		WRITE_ONCE(item->kdata, bpf_struct_ops_map_kdata(pl->link->map));
> +	else
> +		WRITE_ONCE(item->prog, pl->link->link.prog);
>  }
>  
>  static u32 prog_list_id(struct bpf_prog_list *pl)
>  {
> +	if (pl->link && pl->link->map)

Ditto.

> +		return pl->link->map->id;
>  	return prog_list_prog(pl)->aux->id;
>  }
>  
> @@ -591,7 +657,7 @@ static void activate_effective_progs(struct cgroup *cgrp,
>  	/* free prog array after grace period, since __cgroup_bpf_run_*()
>  	 * might be still walking the array
>  	 */
> -	bpf_cgroup_array_free(old_array);
> +	bpf_cgroup_array_free(old_array, atype);
>  }
>  
>  /**
> @@ -631,7 +697,7 @@ static int cgroup_bpf_inherit(struct cgroup *cgrp)
>  	return 0;
>  cleanup:
>  	for (i = 0; i < NR; i++)
> -		bpf_cgroup_array_free(arrays[i]);
> +		bpf_cgroup_array_free(arrays[i], i);
>  
>  	for (p = cgroup_parent(cgrp); p; p = cgroup_parent(p))
>  		cgroup_bpf_put(p);
> @@ -686,7 +752,7 @@ static int update_effective_progs(struct cgroup *cgrp,
>  
>  		if (percpu_ref_is_zero(&desc->bpf.refcnt)) {
>  			if (unlikely(desc->bpf.inactive)) {
> -				bpf_cgroup_array_free(desc->bpf.inactive);
> +				bpf_cgroup_array_free(desc->bpf.inactive, atype);
>  				desc->bpf.inactive = NULL;
>  			}
>  			continue;
> @@ -705,7 +771,7 @@ static int update_effective_progs(struct cgroup *cgrp,
>  	css_for_each_descendant_pre(css, &cgrp->self) {
>  		struct cgroup *desc = container_of(css, struct cgroup, self);
>  
> -		bpf_cgroup_array_free(desc->bpf.inactive);
> +		bpf_cgroup_array_free(desc->bpf.inactive, atype);
>  		desc->bpf.inactive = NULL;
>  	}
>  
> @@ -940,7 +1006,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
>  	if (pl) {
>  		old_prog = pl->prog;
>  	} else {
> -		pl = kmalloc_obj(*pl);
> +		pl = kzalloc_obj(*pl);
>  		if (!pl) {
>  			bpf_cgroup_storages_free(new_storage);
>  			return -ENOMEM;
> @@ -1337,7 +1403,17 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
>  	if (effective_query && prog_attach_flags)
>  		return -EINVAL;
>  
> -	if (type == BPF_LSM_CGROUP) {
> +	if (type == BPF_STRUCT_OPS) {
> +		u32 type_id = attr->query.type_id;
> +
> +		atype = find_atype_by_struct_ops_id(type_id);
> +		if (atype == CGROUP_BPF_ATTACH_TYPE_INVALID)
> +			return -ENOENT;
> +		from_atype = to_atype = atype;
> +		flags = 0;
> +		if (!cgroup_bpf_enabled(atype))
> +			goto skip_count;
> +	} else if (type == BPF_LSM_CGROUP) {
>  		if (!effective_query && attr->query.prog_cnt &&
>  		    prog_ids && !prog_attach_flags)
>  			return -EINVAL;
> @@ -1363,6 +1439,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
>  		}
>  	}
>  
> +skip_count:
>  	/* always output uattr->query.attach_flags as 0 during effective query */
>  	flags = effective_query ? 0 : flags;
>  	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)))
> @@ -2820,6 +2897,270 @@ const struct bpf_verifier_ops cg_sockopt_verifier_ops = {
>  const struct bpf_prog_ops cg_sockopt_prog_ops = {
>  };
>  
> +static int __cgroup_struct_ops_link_detach(struct bpf_link *link, bool wake_poll)
> +{
> +	struct bpf_cgroup_link *cg_link = container_of(link, struct bpf_cgroup_link, link);
> +	enum cgroup_bpf_attach_type atype;
> +	struct bpf_prog_list *pl;
> +	struct bpf_map *map;
> +	struct cgroup *cgrp;
> +
> +	cgroup_lock();
> +
> +	cgrp = cg_link->cgroup;
> +	if (!cgrp) {
> +		cgroup_unlock();
> +		return 0;
> +	}
> +
> +	map = cg_link->map;
> +	atype = bpf_struct_ops_map_cgroup_atype(map);
> +
> +	hlist_for_each_entry(pl, &cgrp->bpf.progs[atype], node) {
> +		if (pl->link == cg_link)
> +			break;
> +	}

What happens if the link isn't found here? Update deals this with
-ENOENT. Even though it's only possible if there's a bug I think
we should at least have a warning.

> +
> +	/* mark deleted so compute_effective_progs() skips it */
> +	pl->link = NULL;
> +	if (update_effective_progs(cgrp, atype)) {
> +		pl->link = cg_link;
> +		purge_effective_progs(cgrp, pl, atype);
> +	}
> +
> +	hlist_del(&pl->node);
> +	cgroup_struct_ops_link_detach_wake(cg_link, wake_poll);
> +	cgrp->bpf.revisions[atype]++;
> +
> +	kfree(pl);
> +	static_branch_dec(&cgroup_bpf_enabled_key[atype]);
> +
> +	cgroup_unlock();
> +
> +	return 0;
> +}
> +
> +static int cgroup_struct_ops_link_detach(struct bpf_link *link)
> +{
> +	return __cgroup_struct_ops_link_detach(link, true);
> +}
> +
> +static void cgroup_struct_ops_link_dealloc(struct bpf_link *link)
> +{
> +	struct bpf_cgroup_link *cg_link = container_of(link, struct bpf_cgroup_link, link);
> +
> +	__cgroup_struct_ops_link_detach(link, false);
> +	kfree(cg_link);
> +}
> +
> +static void cgroup_struct_ops_link_show_fdinfo(const struct bpf_link *link, struct seq_file *seq)
> +{
> +	struct bpf_cgroup_link *cg_link =
> +		container_of(link, struct bpf_cgroup_link, link);
> +
> +	cgroup_lock();
> +	if (!cg_link->cgroup) {
> +		cgroup_unlock();
> +		return;
> +	}
> +
> +	seq_printf(seq, "map_id:\t%u\n", cg_link->map->id);
> +	seq_printf(seq, "cgroup_id:\t%llu\n", cgroup_id(cg_link->cgroup));
> +	cgroup_unlock();
> +}
> +
> +static int cgroup_struct_ops_link_fill_link_info(const struct bpf_link *link,
> +						 struct bpf_link_info *info)
> +{
> +	struct bpf_cgroup_link *cg_link = container_of(link, struct bpf_cgroup_link, link);
> +
> +	cgroup_lock();
> +	if (!cg_link->cgroup) {
> +		cgroup_unlock();
> +		return 0;
> +	}
> +
> +	info->struct_ops.map_id = cg_link->map->id;
> +	info->struct_ops.cgroup_id = cgroup_id(cg_link->cgroup);
> +	cgroup_unlock();
> +	return 0;
> +}
> +
> +static int cgroup_struct_ops_link_update(struct bpf_link *link, struct bpf_map *new_map,
> +					 struct bpf_map *expected_old_map)
> +{
> +	struct bpf_cgroup_link *cg_link = container_of(link, struct bpf_cgroup_link, link);
> +	enum cgroup_bpf_attach_type atype;
> +	struct bpf_prog_list *pl;
> +	struct bpf_map *old_map;
> +	struct cgroup *cgrp;
> +	bool found = false;
> +	int err;
> +
> +	if (!bpf_struct_ops_valid_to_reg(new_map))
> +		return -EINVAL;
> +
> +	cgroup_lock();
> +
> +	cgrp = cg_link->cgroup;
> +	if (!cgrp) {
> +		err = -ENOLINK;
> +		goto out;
> +	}
> +
> +	old_map = cg_link->map;
> +	err = bpf_struct_ops_link_update_check(new_map, old_map, expected_old_map);
> +	if (err)
> +		goto out;
> +
> +	atype = bpf_struct_ops_map_cgroup_atype(new_map);
> +
> +	hlist_for_each_entry(pl, &cgrp->bpf.progs[atype], node) {
> +		if (pl->link == cg_link) {
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		err = -ENOENT;
> +		goto out;
> +	}
> +
> +	bpf_map_inc(new_map);
> +	WRITE_ONCE(cg_link->map, new_map);
> +	replace_effective_prog(cgrp, atype, pl);
> +	bpf_map_put(old_map);
> +	cgrp->bpf.revisions[atype]++;
> +
> +out:
> +	cgroup_unlock();
> +	return err;
> +}
> +
> +static __poll_t cgroup_struct_ops_link_poll(struct file *file, struct poll_table_struct *pts)
> +{
> +	struct bpf_cgroup_link *link = file->private_data;
> +
> +	poll_wait(file, &link->wait_hup, pts);
> +
> +	return READ_ONCE(link->map) ? 0 : EPOLLHUP;
> +}
> +
> +static const struct bpf_link_ops cgroup_struct_ops_link_ops = {
> +	.dealloc = cgroup_struct_ops_link_dealloc,
> +	.detach = cgroup_struct_ops_link_detach,
> +	.show_fdinfo = cgroup_struct_ops_link_show_fdinfo,
> +	.fill_link_info = cgroup_struct_ops_link_fill_link_info,
> +	.update_map = cgroup_struct_ops_link_update,
> +	.poll = cgroup_struct_ops_link_poll,
> +};
> +
> +int cgroup_bpf_struct_ops_attach(struct bpf_map *map, const union bpf_attr *attr)
> +{
> +	u32 flags = attr->link_create.flags;
> +	u32 pl_flags = (flags & BPF_F_PREORDER) | BPF_F_ALLOW_MULTI;
> +	enum cgroup_bpf_attach_type atype;
> +	struct bpf_link_primer link_primer;
> +	struct bpf_cgroup_link *link;
> +	struct bpf_prog_list *pl = NULL;
> +	struct hlist_head *progs;
> +	struct cgroup *cgrp;
> +	int err;
> +
> +	if (flags & ~BPF_F_LINK_ATTACH_MASK)
> +		return -EINVAL;
> +
> +	/*
> +	 * Attaching struct_ops to cgroup is through link only. All relative
> +	 * position must be corresponding to a link id or fd.
> +	 */
> +	if (attr->link_create.cgroup.relative_fd && !(flags & BPF_F_LINK))
> +		return -EINVAL;
> +
> +	link = kzalloc_obj(*link, GFP_USER);
> +	if (!link)
> +		return -ENOMEM;
> +
> +	bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS,
> +		      &cgroup_struct_ops_link_ops, NULL,
> +		      attr->link_create.attach_type);
> +
> +	err = bpf_link_prime(&link->link, &link_primer);
> +	if (err) {
> +		kfree(link);
> +		return err;
> +	}
> +
> +	cgrp = cgroup_get_from_fd(attr->link_create.target_fd);
> +	if (IS_ERR(cgrp)) {
> +		err = PTR_ERR(cgrp);
> +		goto cleanup;
> +	}
> +
> +	bpf_map_inc(map);
> +	link->map = map;
> +	link->cgroup = cgrp;
> +	init_waitqueue_head(&link->wait_hup);
> +
> +	atype = bpf_struct_ops_map_cgroup_atype(map);
> +	progs = &cgrp->bpf.progs[atype];
> +
> +	cgroup_lock();
> +
> +	if (attr->link_create.cgroup.expected_revision &&
> +	    attr->link_create.cgroup.expected_revision != cgrp->bpf.revisions[atype]) {
> +		err = -ESTALE;
> +		goto unlock;
> +	}
> +
> +	if (prog_list_length(progs, NULL) >= BPF_CGROUP_MAX_PROGS) {
> +		err = -E2BIG;
> +		goto unlock;
> +	}
> +
> +	pl = kzalloc_obj(*pl);
> +	if (!pl) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	pl->link = link;
> +	pl->flags = pl_flags;
> +	cgrp->bpf.flags[atype] = BPF_F_ALLOW_MULTI;
> +
> +	err = insert_pl_to_hlist(pl, progs, NULL, link,
> +				 flags | BPF_F_ALLOW_MULTI, attr->link_create.cgroup.relative_fd);
> +	if (err)
> +		goto unlock;
> +
> +	err = update_effective_progs(cgrp, atype);
> +	if (err) {
> +		hlist_del(&pl->node);
> +		goto unlock;
> +	}
> +
> +	cgrp->bpf.revisions[atype]++;
> +	static_branch_inc(&cgroup_bpf_enabled_key[atype]);
> +
> +	cgroup_unlock();
> +
> +	return bpf_link_settle(&link_primer);
> +
> +unlock:
> +	cgroup_unlock();
> +
> +cleanup:
> +	kfree(pl);
> +	if (link->cgroup) {
> +		cgroup_put(link->cgroup);
> +		link->cgroup = NULL;
> +		bpf_map_put(link->map);
> +		link->map = NULL;
> +	}
> +	bpf_link_cleanup(&link_primer);
> +	return err;
> +}
> +
>  /* Common helpers for cgroup hooks. */
>  const struct bpf_func_proto *
>  cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index b07acf37ad1d..b439c0b0eadd 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4813,6 +4813,7 @@ static int bpf_prog_query(const union bpf_attr *attr,
>  	case BPF_CGROUP_GETSOCKOPT:
>  	case BPF_CGROUP_SETSOCKOPT:
>  	case BPF_LSM_CGROUP:
> +	case BPF_STRUCT_OPS:
>  		return cgroup_bpf_prog_query(attr, uattr, uattr_size);
>  	case BPF_LIRC_MODE2:
>  		return lirc_prog_query(attr, uattr);
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index c91b5a4bda03..c982801ec7db 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1756,7 +1756,7 @@ union bpf_attr {
>  			__u32	prog_cnt;
>  			__u32	count;
>  		};
> -		__u32		:32;
> +		__u32		type_id;
>  		/* output: per-program attach_flags.
>  		 * not allowed to be set during effective query.
>  		 */
> @@ -6818,6 +6818,8 @@ struct bpf_link_info {
>  		} xdp;
>  		struct {
>  			__u32 map_id;
> +			__u32 :32;
> +			__u64 cgroup_id;
>  		} struct_ops;
>  		struct {
>  			__u32 pf;


  reply	other threads:[~2026-07-14  6:21 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 17:19 [PATCH bpf-next v3 00/15] bpf: A common way to attach struct_ops to a cgroup Amery Hung
2026-07-06 17:19 ` [PATCH bpf-next v3 01/15] bpf: Remove __rcu tagging in st_link->map Amery Hung
2026-07-13 19:02   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 02/15] bpf: Make struct_ops tasks_rcu grace period optional Amery Hung
2026-07-13 19:01   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 03/15] bpf: Add bpf_struct_ops accessor helpers Amery Hung
2026-07-13 20:36   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 04/15] bpf: Remove unnecessary prog_list_prog() check Amery Hung
2026-07-13 20:35   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 05/15] bpf: Replace prog_list_prog() check with direct pl->prog and pl->link check Amery Hung
2026-07-13 21:27   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 06/15] bpf: Add prog_list_init_item(), prog_list_replace_item(), and prog_list_id() Amery Hung
2026-07-13 21:56   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 07/15] bpf: Move LSM trampoline unlink into bpf_cgroup_link_auto_detach() Amery Hung
2026-07-13 21:57   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 08/15] bpf: Add a few bpf_cgroup_array_* helper functions Amery Hung
2026-07-13 21:57   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 09/15] bpf: Add infrastructure to support attaching struct_ops to cgroups Amery Hung
2026-07-14  6:21   ` Emil Tsalapatis [this message]
2026-07-06 17:19 ` [PATCH bpf-next v3 10/15] bpf: Allow all struct_ops to use bpf_dynptr_from_skb() Amery Hung
2026-07-14  6:23   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 11/15] bpf: tcp: Support selected sock_ops callbacks as struct_ops Amery Hung
2026-07-06 18:28   ` bot+bpf-ci
2026-07-06 23:11     ` Amery Hung
2026-07-06 17:19 ` [PATCH bpf-next v3 12/15] bpf: tcp: Support parse/len/write header option hooks in bpf_tcp_ops Amery Hung
2026-07-06 17:19 ` [PATCH bpf-next v3 13/15] libbpf: Support attaching struct_ops to a cgroup Amery Hung
2026-07-14  6:48   ` Emil Tsalapatis
2026-07-06 17:19 ` [PATCH bpf-next v3 14/15] selftests/bpf: Test " Amery Hung
2026-07-06 17:19 ` [PATCH bpf-next v3 15/15] selftests/bpf: Add test for bpf_tcp_ops header option hooks Amery Hung

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=DJY2SPXTF9HL.10SSKCT9OMMBV@etsalapatis.com \
    --to=emil@etsalapatis.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ameryhung@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=kernel-team@meta.com \
    --cc=kerneljasonxing@gmail.com \
    --cc=kuniyu@google.com \
    --cc=martin.lau@kernel.org \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    /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