bpf, cgroup: fix cgroup struct_ops query for a second attach type
PATCH 1/1 bpf, cgroup: fix cgroup struct_ops query for a second attach type
Two things in __cgroup_bpf_query() work only because CGROUP_TCP_SOCK_OPS is the only one struct_ops attach type.
It calls cgroup_bpf_enabled(atype) with an atype that find_atype_by_struct_ops_id() works out at runtime. That macro is an asm goto and needs a constant. Today the compiler can see there is only one value; add a second type and the build breaks with "impossible constraint in 'asm'". Add cgroup_bpf_enabled_runtime(), which reads the key instead, and use it here. This is a syscall path, so the cost does not matter.
And find_atype_by_struct_ops_id() matches on type_id alone. An attach type whose subsystem is not built keeps type_id 0, so a query for type 0 finds it and returns success with nothing instead of -ENOENT. Skip such slots.
--- include/linux/bpf-cgroup.h | 9 +++++++++ kernel/bpf/cgroup.c | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-)
include/linux/bpf-cgroup.h +9 -0
@@ -76,6 +76,14 @@ to_cgroup_bpf_attach_type(enum bpf_attach_type attach_type)extern struct static_key_false cgroup_bpf_enabled_key[MAX_CGROUP_BPF_ATTACH_TYPE ];#define cgroup_bpf_enabled (atype) static_branch_unlikely(&cgroup_bpf_enabled_key[atype]) +/* +* Same test when @atype is not a constant. cgroup_bpf_enabled() uses +* static_branch_unlikely which creates jump-label site and requires statically +* selected key. Since key is selected dynamically, use static_key_enabled here +* and keep it off fast paths. +*/ +#define cgroup_bpf_enabled_runtime (atype) \ + static_key_enabled(&cgroup_bpf_enabled_key[atype])struct bpf_cgroup_storage_map ;
@@ -508,6 +516,7 @@ static inline int cgroup_bpf_struct_ops_attach(struct bpf_map *map, }#define cgroup_bpf_enabled (atype) (0) +#define cgroup_bpf_enabled_runtime (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; })#define BPF_CGROUP_PRE_CONNECT_ENABLED (sk) (0)
kernel/bpf/cgroup.c +2 -1
@@ -45,6 +45,7 @@ static enum cgroup_bpf_attach_type find_atype_by_struct_ops_id(u32 type_id)for (atype =0 ; atype <MAX_CGROUP_BPF_ATTACH_TYPE ; atype++) {if (cgroup_bpf_is_struct_ops_atype (atype) && + cgroup_struct_ops[atype].type_id && cgroup_struct_ops[atype].type_id == type_id)return atype; }
@@ -1448,7 +1449,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,return -ENOENT ; from_atype = to_atype = atype; flags =0 ; -if (!cgroup_bpf_enabled (atype)) +if (!cgroup_bpf_enabled_runtime (atype)) goto skip_count; }else if (type ==BPF_LSM_CGROUP ) { if (!effective_query && attr->query.prog_cnt &&
Signature · 1 line
2.53.0-Meta
Quoted 11 lines · Shakeel Butt
Two things in
__cgroup_bpf_query()work only becauseCGROUP_TCP_SOCK_OPSis the only onestruct_opsattach type.It calls
cgroup_bpf_enabled(atype)with an atype thatfind_atype_by_struct_ops_id()works out at runtime. That macro is an asm goto and needs a constant. Today the compiler can see there is only one value; add a second type and the build breaks with "impossible constraint in 'asm'". Addcgroup_bpf_enabled_runtime(), which reads the key instead, and use it here. This is a syscall path, so the cost does not matter.
Quoted 7 lines
And
find_atype_by_struct_ops_id()matches ontype_idalone. An attach type whose subsystem is not built keepstype_id0, so a query for type 0 finds it and returns success with nothing instead of-ENOENT. Skip such slots.Fixes 369d9dcd8fb8 ("bpf: Add infrastructure to support attaching struct_ops to cgroups")Signed-off-by Shakeel Butt <[email protected]>Acked-by Yafang Shao <[email protected]>
Quoted 17 lines · Amery Hung
Quoted 15 lines · Shakeel Butt
Two things in
__cgroup_bpf_query()work only becauseCGROUP_TCP_SOCK_OPSis the only onestruct_opsattach type.It calls
cgroup_bpf_enabled(atype)with an atype thatfind_atype_by_struct_ops_id()works out at runtime. That macro is an asm goto and needs a constant. Today the compiler can see there is only one value; add a second type and the build breaks with "impossible constraint in 'asm'". Addcgroup_bpf_enabled_runtime(), which reads the key instead, and use it here. This is a syscall path, so the cost does not matter.And
find_atype_by_struct_ops_id()matches ontype_idalone. An attach type whose subsystem is not built keepstype_id0, so a query for type 0 finds it and returns success with nothing instead of-ENOENT. Skip such slots.
Quoted 5 lines
Quoted 3 lines
Fixes 369d9dcd8fb8 ("bpf: Add infrastructure to support attaching struct_ops to cgroups")Signed-off-by Shakeel Butt <[email protected]>Acked-by Yafang Shao <[email protected]>Reviewed-by Amery Hung <[email protected]>
This patch might need a "bpf-next" tag in the subject so CI tests it.
Quoted 10 lines · Shakeel Butt
Two things in
__cgroup_bpf_query()work only becauseCGROUP_TCP_SOCK_OPSis the only onestruct_opsattach type.It calls
cgroup_bpf_enabled(atype)with an atype thatfind_atype_by_struct_ops_id()works out at runtime. That macro is an asm goto and needs a constant. Today the compiler can see there is only one value; add a second type and the build breaks with "impossible constraint in 'asm'". Addcgroup_bpf_enabled_runtime(), which reads the key instead, and use it here. This is a syscall path, so the cost does not matter.
Quoted 7 lines
And
find_atype_by_struct_ops_id()matches ontype_idalone. An attach type whose subsystem is not built keepstype_id0, so a query for type 0 finds it and returns success with nothing instead of-ENOENT. Skip such slots.Fixes 369d9dcd8fb8 ("bpf: Add infrastructure to support attaching struct_ops to cgroups")Signed-off-by Shakeel Butt <[email protected]>Acked-by Yafang Shao <[email protected]>
Thanks.
Signature · 1 line
tejun
Quoted 2 lines · Shakeel Butt
include/linux/bpf-cgroup.h +2
-0+#define cgroup_bpf_enabled_runtime (atype) \ + static_key_enabled(&cgroup_bpf_enabled_key[atype])
[...]
kernel/bpf/cgroup.c static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr, · line 1453
Quoted 7 lines
kernel/bpf/cgroup.c +1
-1@@ -1448,7 +1449,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,return -ENOENT ; from_atype = to_atype = atype; flags =0 ; -if (!cgroup_bpf_enabled (atype)) +if (!cgroup_bpf_enabled_runtime (atype)) goto skip_count;
No need for a new macro.
When the key is off nothing is attached, so progs[atype] and effective[atype] are empty and the loop below leaves total_cnt at 0. All other attach types go through that loop without such check. Just drop the check and the skip_count label.
pw-bot: cr