bpf: Add tracing multi link
Subsystems (11)
Jiri Olsa
reviewercover bpf: Add tracing multi link
hi,
this is another attempt to add batch attachment support for trampolines.
The patchset adds support to create 'multi' trampoline that is attached to set of functions represented by BTF IDs.
Previous post [0] tried to implement multi trampolines overlapping, but it turned out too complex, so I got back to simpler rules:
(we can discuss possible overlapping changes on top this change)
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:
1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'not used' and are now taken by new 'multi' trampoline
- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs
Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via bpf_trampoline_multi_attach
- D,E,F already have standard trampoline attached
- the bpf_trampoline_multi_attach will create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F
- another program can be attached to A,B,C,D,E,F multi trampoline
- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
- D,E,F functions are still attachable by any new trampoline
Also now that we have trampoline helpers for function arguments, we can just simply use function declaration with maximum arguments for any multi trampoline or related single trampoline.
There are couple of things missing in this post (that I know of), which I'll add when we agree that this is the way to go:
- attaching by functions names
- cookies support
- find out better way of locking trampolines in bpf_trampoline_multi_attach and bpf_trampoline_multi_detach
- bpf_tramp_update_set logic of calling multiple times register_ftrace_direct_multi function can be replaced by calling single update ftrace function that I have prototype for, but I will send it out separately to ftrace for review
- arm trampoline code changes (won't compile now)
- tests for error paths
thanks,
jirka
[0] - https://lore.kernel.org/bpf/[email protected]/
---
Jiri Olsa (17):
bpf: Link shimlink directly in trampoline
bpf: Replace bpf_tramp_links with bpf_tramp_progs
bpf: Store trampoline progs in arrays
bpf: Add multi tracing attach types
bpf: Add bpf_tramp_id object
bpf: Pass image struct to reg/unreg/modify fentry functions
bpf: Add support to postpone trampoline update
bpf: Factor bpf_trampoline_lookup function
bpf: Factor bpf_trampoline_put function
bpf: Add support to attach program to multiple trampolines
bpf: Add support to create tracing multi link
libbpf: Add btf__find_by_glob_kind function
libbpf: Add support to create tracing multi link selftests/bpf: Add fentry tracing multi func test selftests/bpf: Add fexit tracing multi func test selftests/bpf: Add fentry/fexit tracing multi func test selftests/bpf: Add mixed tracing multi func test
arch/x86/net/bpf_jit_comp.c | 38 ++--- include/linux/bpf.h | 98 ++++++++---- include/linux/trace_events.h | 5 + include/uapi/linux/bpf.h | 7 + kernel/bpf/bpf_struct_ops.c | 30 ++-- kernel/bpf/syscall.c | 56 +++++-- kernel/bpf/trampoline.c | 723 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------- kernel/bpf/verifier.c | 8 +- kernel/trace/bpf_trace.c | 240 +++++++++++++++++++++++++++++ net/bpf/bpf_dummy_struct_ops.c | 16 +- net/bpf/test_run.c | 2 + tools/include/uapi/linux/bpf.h | 7 + tools/lib/bpf/bpf.c | 7 + tools/lib/bpf/bpf.h | 4 + tools/lib/bpf/btf.c | 41 +++++ tools/lib/bpf/btf.h | 3 + tools/lib/bpf/libbpf.c | 91 ++++++++++- tools/lib/bpf/libbpf.h | 14 ++ tools/lib/bpf/libbpf.map | 1 + tools/lib/bpf/libbpf_internal.h | 1 + tools/testing/selftests/bpf/Makefile | 9 +- tools/testing/selftests/bpf/prog_tests/tracing_multi.c | 158 ++++++++++++++++++++ tools/testing/selftests/bpf/progs/tracing_multi_check.c | 158 ++++++++++++++++++++ tools/testing/selftests/bpf/progs/tracing_multi_fentry.c | 17 +++ tools/testing/selftests/bpf/progs/tracing_multi_fentry_fexit.c | 28 ++++ tools/testing/selftests/bpf/progs/tracing_multi_fexit.c | 20 +++ tools/testing/selftests/bpf/progs/tracing_multi_mixed.c | 43 ++++++ 27 files changed, 1624 insertions(+), 201 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/tracing_multi.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_check.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_fentry.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_fentry_fexit.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_fexit.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_mixed.c
Jiri Olsa
reviewerRFC PATCH 1/17 bpf: Link shimlink directly in trampoline
We are going to get rid of struct bpf_tramp_link in following changes and cgroup_shim_find logic does not fit to that.
We can store the link directly in the trampoline and omit the cgroup_shim_find searching logic.
--- include/linux/bpf.h | 3 +++ kernel/bpf/trampoline.c | 23 +++-------------------- 2 files changed, 6 insertions(+), 20 deletions(-)
include/linux/bpf.h +3 -0
@@ -841,6 +841,8 @@ struct bpf_tramp_image { }; }; +struct bpf_shim_tramp_link ; +struct bpf_trampoline {/* hlist for trampoline_table */ struct hlist_node hlist;
@@ -868,6 +870,7 @@ struct bpf_trampoline {struct bpf_tramp_image *cur_image;u64 selector;struct module *mod; +struct bpf_shim_tramp_link *shim_link; };struct bpf_attach_target_info {
kernel/bpf/trampoline.c +3 -20
@@ -689,24 +689,6 @@ static struct bpf_shim_tramp_link *cgroup_shim_alloc(const struct bpf_prog *progreturn shim_link; } -static struct bpf_shim_tramp_link *cgroup_shim_find (struct bpf_trampoline *tr, -bpf_func_t bpf_func) -{ -struct bpf_tramp_link *link; -int kind; - -for (kind =0 ; kind <BPF_TRAMP_MAX ; kind++) { -hlist_for_each_entry (link, &tr->progs_hlist[kind], tramp_hlist) { -struct bpf_prog *p = link->link.prog; - -if (p->bpf_func == bpf_func) - return container_of(link, struct bpf_shim_tramp_link, link); - } - } - -return NULL ; -} -int bpf_trampoline_link_cgroup_shim (struct bpf_prog *prog,int cgroup_atype) {
@@ -733,7 +715,7 @@ int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,mutex_lock (&tr->mutex); - shim_link =cgroup_shim_find (tr, bpf_func); + shim_link = tr->shim_link;if (shim_link) {/* Reusing existing shim attached by the other program. */ bpf_link_inc (&shim_link->link.link);
@@ -756,6 +738,7 @@ int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog, goto err; shim_link->trampoline = tr; + tr->shim_link = shim_link;/* note, we're still holding tr refcnt from above */ mutex_unlock (&tr->mutex);
@@ -789,7 +772,7 @@ void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)return ;mutex_lock (&tr->mutex); - shim_link =cgroup_shim_find (tr, bpf_func); + shim_link = tr->shim_link;mutex_unlock (&tr->mutex);if (shim_link)
Signature · 1 line
2.37.1
Song Liu
maintainerRe: [RFC PATCH bpf-next 01/17] bpf: Link shimlink directly in trampoline
Quoted 27 lines · Jiri Olsa
We are going to get rid of struct
bpf_tramp_linkin following changes andcgroup_shim_findlogic does not fit to that.We can store the link directly in the trampoline and omit the
cgroup_shim_findsearching logic.Signed-off-by Jiri Olsa <[email protected]>--- include/linux/bpf.h | 3 +++ kernel/bpf/trampoline.c | 23 +++-------------------- 2 files changed, 6 insertions(+), 20 deletions(-)include/linux/bpf.h +2
-0@@ -841,6 +841,8 @@ struct bpf_tramp_image { }; }; +struct bpf_shim_tramp_link ; +struct bpf_trampoline {/* hlist for trampoline_table */ struct hlist_node hlist;
include/linux/bpf.h struct bpf_trampoline { · line 874
Quoted 5 lines
include/linux/bpf.h +1
-0@@ -868,6 +870,7 @@ struct bpf_trampoline {struct bpf_tramp_image *cur_image;u64 selector;struct module *mod; +struct bpf_shim_tramp_link *shim_link; };
Hi Stanislav,
Is it possible to have multiple shim_link per bpf_trampoline? If so, I guess this won't work.
Thanks,
Song
[...]
Re: [RFC PATCH bpf-next 01/17] bpf: Link shimlink directly in trampoline
Quoted 37 lines · Song Liu
Quoted 27 lines · Jiri Olsa
We are going to get rid of struct
bpf_tramp_linkin following changes andcgroup_shim_findlogic does not fit to that.We can store the link directly in the trampoline and omit the
cgroup_shim_findsearching logic.Signed-off-by Jiri Olsa <[email protected]>--- include/linux/bpf.h | 3 +++ kernel/bpf/trampoline.c | 23 +++-------------------- 2 files changed, 6 insertions(+), 20 deletions(-)include/linux/bpf.h +2
-0@@ -841,6 +841,8 @@ struct bpf_tramp_image { }; }; +struct bpf_shim_tramp_link ; +struct bpf_trampoline {/* hlist for trampoline_table */ struct hlist_node hlist;include/linux/bpf.h struct bpf_trampoline { · line 874
Quoted 5 lines
include/linux/bpf.h +1
-0@@ -868,6 +870,7 @@ struct bpf_trampoline {struct bpf_tramp_image *cur_image;u64 selector;struct module *mod; +struct bpf_shim_tramp_link *shim_link; };Hi Stanislav,
Quoted 2 lines
Is it possible to have multiple
shim_linkperbpf_trampoline? If so, I guess this won't work.
There is only one shim_link per bpf_trampoline. But I'm not sure storing the pointer is enough. We have to do 'shim_link=NULL' when the (final) shim is removed. (multiple lsm_cgroup progs can share the same shim)
Re: [RFC PATCH bpf-next 01/17] bpf: Link shimlink directly in trampoline
Quoted 38 lines · Stanislav Fomichev
Quoted 37 lines · Song Liu
Quoted 27 lines · Jiri Olsa
We are going to get rid of struct
bpf_tramp_linkin following changes andcgroup_shim_findlogic does not fit to that.We can store the link directly in the trampoline and omit the
cgroup_shim_findsearching logic.Signed-off-by Jiri Olsa <[email protected]>--- include/linux/bpf.h | 3 +++ kernel/bpf/trampoline.c | 23 +++-------------------- 2 files changed, 6 insertions(+), 20 deletions(-)include/linux/bpf.h +2
-0@@ -841,6 +841,8 @@ struct bpf_tramp_image { }; }; +struct bpf_shim_tramp_link ; +struct bpf_trampoline {/* hlist for trampoline_table */ struct hlist_node hlist;include/linux/bpf.h struct bpf_trampoline { · line 874
Quoted 5 lines
include/linux/bpf.h +1
-0@@ -868,6 +870,7 @@ struct bpf_trampoline {struct bpf_tramp_image *cur_image;u64 selector;struct module *mod; +struct bpf_shim_tramp_link *shim_link; };Hi Stanislav,
Quoted 7 lines
Quoted 2 lines
Is it possible to have multiple
shim_linkperbpf_trampoline? If so, I guess this won't work.There is only one
shim_linkperbpf_trampoline. But I'm not sure storing the pointer is enough. We have to do 'shim_link=NULL' when the (final) shim is removed. (multiplelsm_cgroupprogs can share the same shim)
ok, will check on that
thanks,
jirka
Jiri Olsa
reviewerRFC PATCH 2/17 bpf: Replace bpf_tramp_links with bpf_tramp_progs
In reply to Jiri OlsaThis is first part of replacing link based program storage in trampoline with array of programs.
Changing bpf_trampoline_get_progs to return bpf_tramp_progs instead of bpf_tramp_links and use this object all the way down from bpf_trampoline_update to arch jit generation.
TODO now that we have ARM trampoline support we also need to change that one the same way.
--- arch/x86/net/bpf_jit_comp.c | 38 +++++++++++++++--------------- include/linux/bpf.h | 16 +++++++++---- kernel/bpf/bpf_struct_ops.c | 18 +++++++-------- kernel/bpf/trampoline.c | 42 +++++++++++++++++++--------------- net/bpf/bpf_dummy_struct_ops.c | 10 ++++---- 5 files changed, 67 insertions(+), 57 deletions(-)
arch/x86/net/bpf_jit_comp.c +19 -19
@@ -1782,7 +1782,7 @@ static void restore_regs(const struct btf_func_model *m, u8 **prog, int nr_args, }static int invoke_bpf_prog (const struct btf_func_model *m,u8 **pprog, -struct bpf_tramp_link *l,int stack_size, +struct bpf_tramp_prog *tp,int stack_size,int run_ctx_off,bool save_ret) { void (*exit)(struct bpf_prog *prog, u64 start,
@@ -1792,8 +1792,8 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,u8 *prog = *pprog;u8 *jmp_insn;int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx , bpf_cookie); -struct bpf_prog *p = l->link.prog; -u64 cookie = l->cookie; +struct bpf_prog *p = tp->prog; +u64 cookie = tp->cookie;/* mov rdi, cookie */ emit_mov_imm64 (&prog,BPF_REG_1 , (long ) cookie >>32 , (u32 ) (long ) cookie);
@@ -1899,14 +1899,14 @@ static int emit_cond_near_jump(u8 **pprog, void *func, void *ip, u8 jmp_cond) }static int invoke_bpf (const struct btf_func_model *m,u8 **pprog, -struct bpf_tramp_links *tl,int stack_size, +struct bpf_tramp_progs *tp,int stack_size,int run_ctx_off,bool save_ret) {int i;u8 *prog = *pprog; -for (i =0 ; i < tl->nr_links; i++) { -if (invoke_bpf_prog (m, &prog, tl->links[i], stack_size, +for (i =0 ; i < tp->nr_progs; i++) { +if (invoke_bpf_prog (m, &prog, &tp->progs[i], stack_size, run_ctx_off, save_ret))return -EINVAL ; }
@@ -1915,7 +1915,7 @@ static int invoke_bpf(const struct btf_func_model *m, u8 **pprog, }static int invoke_bpf_mod_ret (const struct btf_func_model *m,u8 **pprog, -struct bpf_tramp_links *tl,int stack_size, +struct bpf_tramp_progs *tp,int stack_size,int run_ctx_off,u8 **branches) {u8 *prog = *pprog;
@@ -1926,8 +1926,8 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,*/ emit_mov_imm32 (&prog, false,BPF_REG_0 ,0 );emit_stx (&prog,BPF_DW ,BPF_REG_FP ,BPF_REG_0 ,-8 ); -for (i =0 ; i < tl->nr_links; i++) { -if (invoke_bpf_prog (m, &prog, tl->links[i], stack_size, run_ctx_off, true)) +for (i =0 ; i < tp->nr_progs; i++) { +if (invoke_bpf_prog (m, &prog, &tp->progs[i], stack_size, run_ctx_off, true))return -EINVAL ;/* mod_ret prog stored return value into [rbp - 8]. Emit:
@@ -2012,14 +2012,14 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,*/ int arch_prepare_bpf_trampoline (struct bpf_tramp_image *im,void *image,void *image_end,const struct btf_func_model *m,u32 flags, -struct bpf_tramp_links *tlinks, +struct bpf_tramp_progs *tprogs,void *orig_call) {int ret, i, nr_args = m->nr_args;int regs_off, ip_off, args_off, stack_size = nr_args *8 , run_ctx_off; -struct bpf_tramp_links *fentry = &tlinks[BPF_TRAMP_FENTRY ]; -struct bpf_tramp_links *fexit = &tlinks[BPF_TRAMP_FEXIT ]; -struct bpf_tramp_links *fmod_ret = &tlinks[BPF_TRAMP_MODIFY_RETURN ]; +struct bpf_tramp_progs *fentry = &tprogs[BPF_TRAMP_FENTRY ]; +struct bpf_tramp_progs *fexit = &tprogs[BPF_TRAMP_FEXIT ]; +struct bpf_tramp_progs *fmod_ret = &tprogs[BPF_TRAMP_MODIFY_RETURN ];u8 **branches =NULL ;u8 *prog;bool save_ret;
@@ -2112,13 +2112,13 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i } } -if (fentry->nr_links) +if (fentry->nr_progs)if (invoke_bpf (m, &prog, fentry, regs_off, run_ctx_off, flags &BPF_TRAMP_F_RET_FENTRY_RET ))return -EINVAL ; -if (fmod_ret->nr_links) { - branches =kcalloc (fmod_ret->nr_links,sizeof (u8 *), +if (fmod_ret->nr_progs) { + branches =kcalloc (fmod_ret->nr_progs,sizeof (u8 *),GFP_KERNEL );if (!branches)return -ENOMEM ;
@@ -2150,7 +2150,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i prog +=X86_PATCH_SIZE ; } -if (fmod_ret->nr_links) { +if (fmod_ret->nr_progs) {/* From Intel 64 and IA-32 Architectures Optimization * Reference Manual, 3.4.1.4 Code Alignment, Assembly/Compiler * Coding Rule 11: All branch targets should be 16-byte
@@ -2160,12 +2160,12 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i/* Update the branches saved in invoke_bpf_mod_ret with the * aligned address of do_fexit. */ -for (i =0 ; i < fmod_ret->nr_links; i++) +for (i =0 ; i < fmod_ret->nr_progs; i++)emit_cond_near_jump (&branches[i], prog, branches[i],X86_JNE ); } -if (fexit->nr_links) +if (fexit->nr_progs)if (invoke_bpf (m, &prog, fexit, regs_off, run_ctx_off, false)) { ret = -EINVAL ; goto cleanup;
include/linux/bpf.h +11 -5
@@ -767,9 +767,14 @@ struct btf_func_model {*/ #define BPF_MAX_TRAMP_LINKS 38 -struct bpf_tramp_links { -struct bpf_tramp_link *links[BPF_MAX_TRAMP_LINKS ]; -int nr_links; +struct bpf_tramp_prog { +struct bpf_prog *prog; +u64 cookie; +}; + +struct bpf_tramp_progs { +struct bpf_tramp_prog progs[BPF_MAX_TRAMP_LINKS ]; +int nr_progs; };struct bpf_tramp_run_ctx ;
@@ -797,7 +802,7 @@ struct bpf_tramp_run_ctx;struct bpf_tramp_image ;int arch_prepare_bpf_trampoline (struct bpf_tramp_image *tr,void *image,void *image_end,const struct btf_func_model *m,u32 flags, -struct bpf_tramp_links *tlinks, +struct bpf_tramp_progs *tprogs,void *orig_call);/* these two functions are called from generated trampoline */ u64 notrace __bpf_prog_enter(struct bpf_prog *prog, struct bpf_tramp_run_ctx *run_ctx);
@@ -907,6 +912,7 @@ static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func( }#ifdef CONFIG_BPF_JIT +struct bpf_tramp_link ;int bpf_trampoline_link_prog (struct bpf_tramp_link *link,struct bpf_trampoline *tr);int bpf_trampoline_unlink_prog (struct bpf_tramp_link *link,struct bpf_trampoline *tr); struct bpf_trampoline *bpf_trampoline_get(u64 key,
@@ -1236,7 +1242,7 @@ bool bpf_struct_ops_get(const void *kdata);void bpf_struct_ops_put (const void *kdata);int bpf_struct_ops_map_sys_lookup_elem (struct bpf_map *map,void *key,void *value); -int bpf_struct_ops_prepare_trampoline (struct bpf_tramp_links *tlinks, +int bpf_struct_ops_prepare_trampoline (struct bpf_tramp_progs *tprogs,struct bpf_tramp_link *link,const struct btf_func_model *model,void *image,void *image_end);
kernel/bpf/bpf_struct_ops.c +9 -9
@@ -332,21 +332,21 @@ const struct bpf_link_ops bpf_struct_ops_link_lops = { .dealloc = bpf_struct_ops_link_dealloc, }; -int bpf_struct_ops_prepare_trampoline (struct bpf_tramp_links *tlinks, +int bpf_struct_ops_prepare_trampoline (struct bpf_tramp_progs *tprogs,struct bpf_tramp_link *link,const struct btf_func_model *model,void *image,void *image_end) {u32 flags; - tlinks[BPF_TRAMP_FENTRY ].links[0 ] = link; - tlinks[BPF_TRAMP_FENTRY ].nr_links =1 ; + tprogs[BPF_TRAMP_FENTRY ].progs[0 ].prog = link->link.prog; + tprogs[BPF_TRAMP_FENTRY ].nr_progs =1 ;/* BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, * and it must be used alone. */ flags = model->ret_size >0 ?BPF_TRAMP_F_RET_FENTRY_RET :0 ;return arch_prepare_bpf_trampoline (NULL , image, image_end, - model, flags, tlinks,NULL ); + model, flags, tprogs,NULL ); } static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
@@ -357,7 +357,7 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,struct bpf_struct_ops_value *uvalue, *kvalue;const struct btf_member *member;const struct btf_type *t = st_ops->type; -struct bpf_tramp_links *tlinks =NULL ; +struct bpf_tramp_progs *tprogs =NULL ;void *udata, *kdata;int prog_fd, err =0 ;void *image, *image_end;
@@ -381,8 +381,8 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,if (uvalue->state ||refcount_read (&uvalue->refcnt))return -EINVAL ; - tlinks =kcalloc (BPF_TRAMP_MAX ,sizeof (*tlinks),GFP_KERNEL ); -if (!tlinks) + tprogs =kcalloc (BPF_TRAMP_MAX ,sizeof (*tprogs),GFP_KERNEL ); +if (!tprogs)return -ENOMEM ; uvalue = (struct bpf_struct_ops_value *)st_map->uvalue;
@@ -478,7 +478,7 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key, &bpf_struct_ops_link_lops, prog); st_map->links[i] = &link->link; - err =bpf_struct_ops_prepare_trampoline (tlinks, link, + err =bpf_struct_ops_prepare_trampoline (tprogs, link, &st_ops->func_models[i], image, image_end);if (err <0 )
@@ -520,7 +520,7 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,memset (uvalue,0 , map->value_size);memset (kvalue,0 , map->value_size); unlock: -kfree (tlinks); +kfree (tprogs);mutex_unlock (&st_map->lock);return err; }
kernel/bpf/trampoline.c +23 -19
96 lines not on this page · raw message
net/bpf/bpf_dummy_struct_ops.c +5 -5
@@ -80,7 +80,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,const struct bpf_struct_ops *st_ops = &bpf_bpf_dummy_ops;const struct btf_type *func_proto;struct bpf_dummy_ops_test_args *args; -struct bpf_tramp_links *tlinks; +struct bpf_tramp_progs *tprogs;struct bpf_tramp_link *link =NULL ;void *image =NULL ;unsigned int op_idx;
@@ -95,8 +95,8 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,if (IS_ERR (args))return PTR_ERR (args); - tlinks =kcalloc (BPF_TRAMP_MAX ,sizeof (*tlinks),GFP_KERNEL ); -if (!tlinks) { + tprogs =kcalloc (BPF_TRAMP_MAX ,sizeof (*tprogs),GFP_KERNEL ); +if (!tprogs) { err = -ENOMEM ; goto out; }
@@ -118,7 +118,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,bpf_link_init (&link->link,BPF_LINK_TYPE_STRUCT_OPS , &bpf_struct_ops_link_lops, prog); op_idx = prog->expected_attach_type; - err =bpf_struct_ops_prepare_trampoline (tlinks, link, + err =bpf_struct_ops_prepare_trampoline (tprogs, link, &st_ops->func_models[op_idx], image, image +PAGE_SIZE );if (err <0 )
@@ -138,7 +138,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,bpf_jit_free_exec (image);if (link)bpf_link_put (&link->link); -kfree (tlinks); +kfree (tprogs);return err; }
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 3/17 bpf: Store trampoline progs in arrays
In reply to Jiri OlsaStoring programs for trampoline in array instead of in link based list. This way we can have same program coming from one link being shared across multiple trampolines.
Replacing list_head links array with bpf_prog_array objects array that now stores all trampoline programs.
We already have bpf_trampoline_get_progs returning bpf_tramp_progs object, so this patch does the rest:
- storing trampoline programs of given type in bpf_prog_array objects
- using bpf_tramp_prog object as program reference in link/unlink functions:
int bpf_trampoline_link_prog(struct bpf_tramp_prog *tp,
struct bpf_trampoline *tr);
int bpf_trampoline_unlink_prog(struct bpf_tramp_prog *tp,
struct bpf_trampoline *tr);
- changing all the callers on above link/unlink to work with new interface
- removing bpf_tramp_link struct, because it's no longer needed
--- include/linux/bpf.h | 25 ++++---- kernel/bpf/bpf_struct_ops.c | 16 +++-- kernel/bpf/syscall.c | 19 +++--- kernel/bpf/trampoline.c | 105 ++++++++++++++++++++------------- net/bpf/bpf_dummy_struct_ops.c | 8 +-- 5 files changed, 96 insertions(+), 77 deletions(-)
include/linux/bpf.h +10 -15
@@ -868,7 +868,7 @@ struct bpf_trampoline {*/ struct bpf_prog *extension_prog;/* list of BPF programs using this trampoline */ -struct hlist_head progs_hlist[BPF_TRAMP_MAX ]; +struct bpf_prog_array *progs_array[BPF_TRAMP_MAX ];/* Number of attached programs. A counter per kind. */ int progs_cnt[BPF_TRAMP_MAX ];/* Executable image of trampoline */
@@ -912,9 +912,8 @@ static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func( }#ifdef CONFIG_BPF_JIT -struct bpf_tramp_link ; -int bpf_trampoline_link_prog (struct bpf_tramp_link *link,struct bpf_trampoline *tr); -int bpf_trampoline_unlink_prog (struct bpf_tramp_link *link,struct bpf_trampoline *tr); +int bpf_trampoline_link_prog (struct bpf_tramp_prog *tp,struct bpf_trampoline *tr); +int bpf_trampoline_unlink_prog (struct bpf_tramp_prog *tp,struct bpf_trampoline *tr);struct bpf_trampoline *bpf_trampoline_get (u64 key,struct bpf_attach_target_info *tgt_info);void bpf_trampoline_put (struct bpf_trampoline *tr);
@@ -963,12 +962,12 @@ int bpf_jit_charge_modmem(u32 size);void bpf_jit_uncharge_modmem (u32 size);bool bpf_prog_has_trampoline (const struct bpf_prog *prog);#else -static inline int bpf_trampoline_link_prog (struct bpf_tramp_link *link, +static inline int bpf_trampoline_link_prog (struct bpf_tramp_prog *tp,struct bpf_trampoline *tr) {return -ENOTSUPP ; } -static inline int bpf_trampoline_unlink_prog (struct bpf_tramp_link *link, +static inline int bpf_trampoline_unlink_prog (struct bpf_tramp_prog *tp,struct bpf_trampoline *tr) {return -ENOTSUPP ;
@@ -1187,19 +1186,15 @@ struct bpf_link_ops { struct bpf_link_info *info); }; -struct bpf_tramp_link { -struct bpf_link link; -struct hlist_node tramp_hlist; -u64 cookie; -}; -struct bpf_shim_tramp_link { -struct bpf_tramp_link link; +struct bpf_link link; +struct bpf_tramp_prog tp;struct bpf_trampoline *trampoline; };struct bpf_tracing_link { -struct bpf_tramp_link link; +struct bpf_link link; +struct bpf_tramp_prog tp;enum bpf_attach_type attach_type;struct bpf_trampoline *trampoline;struct bpf_prog *tgt_prog;
@@ -1243,7 +1238,7 @@ void bpf_struct_ops_put(const void *kdata);int bpf_struct_ops_map_sys_lookup_elem (struct bpf_map *map,void *key,void *value);int bpf_struct_ops_prepare_trampoline (struct bpf_tramp_progs *tprogs, -struct bpf_tramp_link *link, +struct bpf_prog *prog,const struct btf_func_model *model,void *image,void *image_end);static inline bool bpf_try_module_get (const void *data,struct module *owner)
kernel/bpf/bpf_struct_ops.c +7 -9
@@ -322,9 +322,7 @@ static void bpf_struct_ops_link_release(struct bpf_link *link)static void bpf_struct_ops_link_dealloc (struct bpf_link *link) { - struct bpf_tramp_link *tlink = container_of(link, struct bpf_tramp_link, link); - -kfree (tlink); +kfree (link); }const struct bpf_link_ops bpf_struct_ops_link_lops = {
@@ -333,13 +331,13 @@ const struct bpf_link_ops bpf_struct_ops_link_lops = { };int bpf_struct_ops_prepare_trampoline (struct bpf_tramp_progs *tprogs, -struct bpf_tramp_link *link, +struct bpf_prog *prog,const struct btf_func_model *model,void *image,void *image_end) {u32 flags; - tprogs[BPF_TRAMP_FENTRY ].progs[0 ].prog = link->link.prog; + tprogs[BPF_TRAMP_FENTRY ].progs[0 ].prog = prog; tprogs[BPF_TRAMP_FENTRY ].nr_progs =1 ;/* BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, * and it must be used alone.
@@ -405,7 +403,7 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,for_each_member (i, t, member) {const struct btf_type *mtype, *ptype;struct bpf_prog *prog; -struct bpf_tramp_link *link; +struct bpf_link *link;u32 moff; moff =__btf_member_bit_offset (t, member) /8 ;
@@ -474,11 +472,11 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key, err = -ENOMEM ; goto reset_unlock; } -bpf_link_init (&link->link,BPF_LINK_TYPE_STRUCT_OPS , +bpf_link_init (link,BPF_LINK_TYPE_STRUCT_OPS , &bpf_struct_ops_link_lops, prog); - st_map->links[i] = &link->link; + st_map->links[i] = link; - err =bpf_struct_ops_prepare_trampoline (tprogs, link, + err =bpf_struct_ops_prepare_trampoline (tprogs, prog, &st_ops->func_models[i], image, image_end);if (err <0 )
kernel/bpf/syscall.c +10 -9
@@ -2887,9 +2887,9 @@ EXPORT_SYMBOL(bpf_link_get_from_fd);static void bpf_tracing_link_release (struct bpf_link *link) {struct bpf_tracing_link *tr_link = - container_of(link, struct bpf_tracing_link, link.link); + container_of(link, struct bpf_tracing_link, link); -WARN_ON_ONCE (bpf_trampoline_unlink_prog (&tr_link->link, +WARN_ON_ONCE (bpf_trampoline_unlink_prog (&tr_link->tp, tr_link->trampoline));bpf_trampoline_put (tr_link->trampoline);
@@ -2902,7 +2902,7 @@ static void bpf_tracing_link_release(struct bpf_link *link)static void bpf_tracing_link_dealloc (struct bpf_link *link) {struct bpf_tracing_link *tr_link = - container_of(link, struct bpf_tracing_link, link.link); + container_of(link, struct bpf_tracing_link, link);kfree (tr_link); }
@@ -2911,7 +2911,7 @@ static void bpf_tracing_link_show_fdinfo(const struct bpf_link *link, struct seq_file *seq) {struct bpf_tracing_link *tr_link = - container_of(link, struct bpf_tracing_link, link.link); + container_of(link, struct bpf_tracing_link, link); seq_printf(seq, "attach_type:\t%d\n",
@@ -2922,7 +2922,7 @@ static int bpf_tracing_link_fill_link_info(const struct bpf_link *link, struct bpf_link_info *info) {struct bpf_tracing_link *tr_link = - container_of(link, struct bpf_tracing_link, link.link); + container_of(link, struct bpf_tracing_link, link); info->tracing.attach_type = tr_link->attach_type; bpf_trampoline_unpack_key(tr_link->trampoline->key,
@@ -3004,10 +3004,11 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, err = -ENOMEM ; goto out_put_prog; } -bpf_link_init (&link->link.link,BPF_LINK_TYPE_TRACING , +bpf_link_init (&link->link,BPF_LINK_TYPE_TRACING , &bpf_tracing_link_lops, prog); link->attach_type = prog->expected_attach_type; - link->link.cookie = bpf_cookie; + link->tp.cookie = bpf_cookie; + link->tp.prog = prog;mutex_lock (&prog->aux->dst_mutex);
@@ -3075,11 +3076,11 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, tgt_prog = prog->aux->dst_prog; } - err =bpf_link_prime (&link->link.link, &link_primer); + err =bpf_link_prime (&link->link, &link_primer);if (err) goto out_unlock; - err =bpf_trampoline_link_prog (&link->link, tr); + err =bpf_trampoline_link_prog (&link->tp, tr);if (err) {bpf_link_cleanup (&link_primer); link =NULL ;
kernel/bpf/trampoline.c +65 -40
248 lines not on this page · raw message
net/bpf/bpf_dummy_struct_ops.c +4 -4
@@ -81,7 +81,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,const struct btf_type *func_proto;struct bpf_dummy_ops_test_args *args;struct bpf_tramp_progs *tprogs; -struct bpf_tramp_link *link =NULL ; +struct bpf_link *link =NULL ;void *image =NULL ;unsigned int op_idx;int prog_ret;
@@ -115,10 +115,10 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr, }/* prog doesn't take the ownership of the reference from caller */ bpf_prog_inc (prog); -bpf_link_init (&link->link,BPF_LINK_TYPE_STRUCT_OPS , &bpf_struct_ops_link_lops, prog); +bpf_link_init (link,BPF_LINK_TYPE_STRUCT_OPS , &bpf_struct_ops_link_lops, prog); op_idx = prog->expected_attach_type; - err =bpf_struct_ops_prepare_trampoline (tprogs, link, + err =bpf_struct_ops_prepare_trampoline (tprogs, prog, &st_ops->func_models[op_idx], image, image +PAGE_SIZE );if (err <0 )
@@ -137,7 +137,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,kfree (args);bpf_jit_free_exec (image);if (link) -bpf_link_put (&link->link); +bpf_link_put (link);kfree (tprogs);return err; }
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 4/17 bpf: Add multi tracing attach types
In reply to Jiri OlsaAdding new attach type to identify multi tracing attachment:
BPF_TRACE_FENTRY_MULTI
BPF_TRACE_FEXIT_MULTI
Programs with such attach type will use specific link attachment interface coming in following changes.
This was suggested by Andrii some (long) time ago and turned out to be easier than having special program flag for that.
--- include/linux/bpf.h | 5 +++++ include/uapi/linux/bpf.h | 2 ++ kernel/bpf/syscall.c | 35 ++++++++++++++++++++++++++++++---- kernel/bpf/trampoline.c | 3 +++ kernel/bpf/verifier.c | 8 +++++++- net/bpf/test_run.c | 2 ++ tools/include/uapi/linux/bpf.h | 2 ++ 7 files changed, 52 insertions(+), 5 deletions(-)
include/linux/bpf.h +5 -0
@@ -1256,6 +1256,11 @@ static inline void bpf_module_put(const void *data, struct module *owner)module_put (owner); } +static inline bool is_tracing_multi (enum bpf_attach_type type) +{ +return type ==BPF_TRACE_FENTRY_MULTI || type ==BPF_TRACE_FEXIT_MULTI ; +} +#ifdef CONFIG_NET /* Define it here to avoid the use of forward declaration */ struct bpf_dummy_ops_state {
include/uapi/linux/bpf.h +2 -0
@@ -999,6 +999,8 @@ enum bpf_attach_type {BPF_PERF_EVENT ,BPF_TRACE_KPROBE_MULTI ,BPF_LSM_CGROUP , +BPF_TRACE_FENTRY_MULTI , +BPF_TRACE_FEXIT_MULTI , __MAX_BPF_ATTACH_TYPE };
kernel/bpf/syscall.c +31 -4
@@ -35,6 +35,7 @@#include <linux/rcupdate_trace.h> #include <linux/memcontrol.h> #include <linux/trace_events.h> +#include <linux/btf_ids.h> #define IS_FD_ARRAY (map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \ (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
@@ -2304,7 +2305,8 @@ static int bpf_prog_load_check_attach(enum bpf_prog_type prog_type, enum bpf_attach_type expected_attach_type, struct btf *attach_btf, u32 btf_id, - struct bpf_prog *dst_prog) + struct bpf_prog *dst_prog, + bool multi_func) { if (btf_id) { if (btf_id > BTF_MAX_TYPE)
@@ -2324,6 +2326,14 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type, } } +if (multi_func) { +if (prog_type !=BPF_PROG_TYPE_TRACING ) +return -EINVAL ; +if (!attach_btf || btf_id) +return -EINVAL ; +return 0 ; + } +if (attach_btf && (!btf_id || dst_prog))return -EINVAL ;
@@ -2447,6 +2457,16 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type) } } +#define DEFINE_BPF_MULTI_FUNC(args...) \ + extern int bpf_multi_func(args); \ + int __init bpf_multi_func(args) { return 0; } + +DEFINE_BPF_MULTI_FUNC(unsigned long a1, unsigned long a2, + unsigned long a3, unsigned long a4, + unsigned long a5, unsigned long a6) + +BTF_ID_LIST_SINGLE(bpf_multi_func_btf_id, func, bpf_multi_func) + /* last field in 'union bpf_attr' used by this command */ #define BPF_PROG_LOAD_LAST_FIELD core_relo_rec_size
@@ -2457,6 +2477,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)struct btf *attach_btf =NULL ;int err;char license[128 ]; +bool multi_func;bool is_gpl;if (CHECK_ATTR (BPF_PROG_LOAD ))
@@ -2498,6 +2519,8 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)if (is_perfmon_prog_type (type) && !perfmon_capable ())return -EPERM ; + multi_func =is_tracing_multi (attr->expected_attach_type); +/* attach_prog_fd/attach_btf_obj_fd can specify fd of either bpf_prog * or btf, we need to check which one it is */
@@ -2516,7 +2539,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)return -ENOTSUPP ; } } - }else if (attr->attach_btf_id) { + }else if (attr->attach_btf_id || multi_func) {/* fall back to vmlinux BTF, if BTF type ID is specified */ attach_btf =bpf_get_btf_vmlinux ();if (IS_ERR (attach_btf))
@@ -2529,7 +2552,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)bpf_prog_load_fixup_attach_type (attr);if (bpf_prog_load_check_attach (type, attr->expected_attach_type, attach_btf, attr->attach_btf_id, - dst_prog)) { + dst_prog, multi_func)) {if (dst_prog)bpf_prog_put (dst_prog);if (attach_btf)
@@ -2549,7 +2572,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) prog->expected_attach_type = attr->expected_attach_type; prog->aux->attach_btf = attach_btf; - prog->aux->attach_btf_id = attr->attach_btf_id; + prog->aux->attach_btf_id = multi_func ? bpf_multi_func_btf_id[0 ] : attr->attach_btf_id; prog->aux->dst_prog = dst_prog; prog->aux->offload_requested = !!attr->prog_ifindex; prog->aux->sleepable = attr->prog_flags &BPF_F_SLEEPABLE ;
@@ -2955,6 +2978,8 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,case BPF_PROG_TYPE_TRACING :if (prog->expected_attach_type !=BPF_TRACE_FENTRY && prog->expected_attach_type !=BPF_TRACE_FEXIT && + prog->expected_attach_type !=BPF_TRACE_FENTRY_MULTI && + prog->expected_attach_type !=BPF_TRACE_FEXIT_MULTI && prog->expected_attach_type !=BPF_MODIFY_RETURN ) { err = -EINVAL ; goto out_put_prog;
@@ -3429,6 +3454,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)case BPF_TRACE_RAW_TP :case BPF_TRACE_FENTRY :case BPF_TRACE_FEXIT : +case BPF_TRACE_FENTRY_MULTI : +case BPF_TRACE_FEXIT_MULTI :case BPF_MODIFY_RETURN :return BPF_PROG_TYPE_TRACING ;case BPF_LSM_MAC :
kernel/bpf/trampoline.c +3 -0
@@ -112,6 +112,7 @@ bool bpf_prog_has_trampoline(const struct bpf_prog *prog)return (ptype ==BPF_PROG_TYPE_TRACING && (eatype ==BPF_TRACE_FENTRY || eatype ==BPF_TRACE_FEXIT || + eatype ==BPF_TRACE_FENTRY_MULTI || eatype ==BPF_TRACE_FEXIT_MULTI || eatype ==BPF_MODIFY_RETURN )) || (ptype ==BPF_PROG_TYPE_LSM && eatype ==BPF_LSM_MAC ); }
@@ -529,10 +530,12 @@ static enum bpf_tramp_prog_type bpf_attach_type_to_tramp(struct bpf_prog *prog) {switch (prog->expected_attach_type) {case BPF_TRACE_FENTRY : +case BPF_TRACE_FENTRY_MULTI :return BPF_TRAMP_FENTRY ;case BPF_MODIFY_RETURN :return BPF_TRAMP_MODIFY_RETURN ;case BPF_TRACE_FEXIT : +case BPF_TRACE_FEXIT_MULTI :return BPF_TRAMP_FEXIT ;case BPF_LSM_MAC :if (!prog->aux->attach_func_proto->type)
kernel/bpf/verifier.c +7 -1
@@ -10526,6 +10526,8 @@ static int check_return_code(struct bpf_verifier_env *env)switch (env->prog->expected_attach_type) {case BPF_TRACE_FENTRY :case BPF_TRACE_FEXIT : +case BPF_TRACE_FENTRY_MULTI : +case BPF_TRACE_FEXIT_MULTI : range =tnum_const (0 );break ;case BPF_TRACE_RAW_TP :
@@ -14289,6 +14291,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)if (prog_type ==BPF_PROG_TYPE_TRACING && insn->imm == BPF_FUNC_get_func_ret) {if (eatype ==BPF_TRACE_FEXIT || + eatype ==BPF_TRACE_FEXIT_MULTI || eatype ==BPF_MODIFY_RETURN ) {/* Load nr_args from ctx - 8 */ insn_buf[0 ] =BPF_LDX_MEM (BPF_DW ,BPF_REG_0 ,BPF_REG_1 ,-8 );
@@ -14937,6 +14940,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,case BPF_LSM_CGROUP :case BPF_TRACE_FENTRY :case BPF_TRACE_FEXIT : +case BPF_TRACE_FENTRY_MULTI : +case BPF_TRACE_FEXIT_MULTI :if (!btf_type_is_func (t)) {bpf_log (log,"attach_btf_id %u is not a function\n" , btf_id);
@@ -15093,7 +15098,8 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)if (!bpf_iter_prog_supported (prog))return -EINVAL ;return 0 ; - } + }else if (is_tracing_multi (prog->expected_attach_type)) +return prog->type ==BPF_PROG_TYPE_TRACING ?0 : -EINVAL ;if (prog->type ==BPF_PROG_TYPE_LSM ) { ret =bpf_lsm_verify_prog (&env->log, prog);
net/bpf/test_run.c +2 -0
@@ -760,6 +760,8 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,switch (prog->expected_attach_type) {case BPF_TRACE_FENTRY :case BPF_TRACE_FEXIT : +case BPF_TRACE_FENTRY_MULTI : +case BPF_TRACE_FEXIT_MULTI : if (bpf_fentry_test1(1) != 2 || bpf_fentry_test2(2, 3) != 5 || bpf_fentry_test3(4, 5, 6) != 15 ||
tools/include/uapi/linux/bpf.h +2 -0
@@ -999,6 +999,8 @@ enum bpf_attach_type {BPF_PERF_EVENT ,BPF_TRACE_KPROBE_MULTI ,BPF_LSM_CGROUP , +BPF_TRACE_FENTRY_MULTI , +BPF_TRACE_FEXIT_MULTI , __MAX_BPF_ATTACH_TYPE };
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 5/17 bpf: Add bpf_tramp_id object
In reply to Jiri OlsaAdding bpf_tramp_id object that allows to store multiple BTF function ids together with their resolved addresses.
It will be used in following changes to identify and attach multiple functions to trampolines.
The bpf_tramp_id object will be shared between trampoline and link in following changes, so it keeps refcount for that.
--- include/linux/bpf.h | 12 ++++++++++++ kernel/bpf/trampoline.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+)
include/linux/bpf.h +12 -0
@@ -27,6 +27,7 @@#include <linux/bpfptr.h> #include <linux/btf.h> #include <linux/rcupdate_trace.h> +#include <linux/refcount.h> struct bpf_verifier_env ;struct bpf_verifier_log ;
@@ -846,6 +847,15 @@ struct bpf_tramp_image { }; }; +struct bpf_tramp_id { +u32 max; +u32 cnt; +u32 obj_id; +u32 *id; +void **addr; +refcount_t refcnt; +}; +struct bpf_shim_tramp_link ;struct bpf_trampoline {
@@ -917,6 +927,8 @@ int bpf_trampoline_unlink_prog(struct bpf_tramp_prog *tp, struct bpf_trampolinestruct bpf_trampoline *bpf_trampoline_get (u64 key,struct bpf_attach_target_info *tgt_info);void bpf_trampoline_put (struct bpf_trampoline *tr); +struct bpf_tramp_id *bpf_tramp_id_alloc (u32 cnt); +void bpf_tramp_id_put (struct bpf_tramp_id *id);int arch_prepare_bpf_dispatcher (void *image,s64 *funcs,int num_funcs);#define BPF_DISPATCHER_INIT (_name) { \ .mutex = __MUTEX_INITIALIZER(_name.mutex), \
kernel/bpf/trampoline.c +38 -0
@@ -149,6 +149,44 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym) PAGE_SIZE, true, ksym->name); } +struct bpf_tramp_id *bpf_tramp_id_alloc (u32 max) +{ +struct bpf_tramp_id *id; + + id =kzalloc (sizeof (*id),GFP_KERNEL ); +if (id) { + id->id =kcalloc (max,sizeof (u32),GFP_KERNEL ); + id->addr =kcalloc (max,sizeof (*id->addr),GFP_KERNEL ); +if (!id->id || !id->addr) { +kfree (id->id); +kfree (id->addr); +kfree (id); +return NULL ; + } + id->max = max; +refcount_set (&id->refcnt,1 ); + } +return id; +} + +__maybe_unused +static struct bpf_tramp_id *bpf_tramp_id_get (struct bpf_tramp_id *id) +{ +refcount_inc (&id->refcnt); +return id; +} + +void bpf_tramp_id_put (struct bpf_tramp_id *id) +{ +if (!id) +return ; +if (!refcount_dec_and_test (&id->refcnt)) +return ; +kfree (id->addr); +kfree (id->id); +kfree (id); +} +static struct bpf_trampoline *bpf_trampoline_lookup (u64 key) {struct bpf_trampoline *tr;
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 6/17 bpf: Pass image struct to reg/unreg/modify fentry functions
In reply to Jiri OlsaPassing image struct to reg/unreg/modify fentry functions as a preparation for following change, where we need to use the whole image in the update logic.
--- kernel/bpf/trampoline.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
kernel/bpf/trampoline.c +10 -6
@@ -244,8 +244,9 @@ static void bpf_trampoline_module_put(struct bpf_trampoline *tr) tr->mod =NULL ; } -static int unregister_fentry (struct bpf_trampoline *tr,void *old_addr) +static int unregister_fentry (struct bpf_trampoline *tr,struct bpf_tramp_image *im) { +void *old_addr = im->image;void *ip = tr->func.addr;int ret;
@@ -259,9 +260,11 @@ static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)return ret; } -static int modify_fentry (struct bpf_trampoline *tr,void *old_addr,void *new_addr, +static int modify_fentry (struct bpf_trampoline *tr,struct bpf_tramp_image *im,bool lock_direct_mutex) { +void *old_addr = tr->cur_image->image; +void *new_addr = im->image;void *ip = tr->func.addr;int ret;
@@ -277,8 +280,9 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad }/* first time registering */ -static int register_fentry (struct bpf_trampoline *tr,void *new_addr) +static int register_fentry (struct bpf_trampoline *tr,struct bpf_tramp_image *im) { +void *new_addr = im->image;void *ip = tr->func.addr;unsigned long faddr;int ret;
@@ -486,7 +490,7 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mutreturn PTR_ERR (tprogs);if (total ==0 ) { - err =unregister_fentry (tr, tr->cur_image->image); + err =unregister_fentry (tr, tr->cur_image);bpf_tramp_image_put (tr->cur_image); tr->cur_image =NULL ; tr->selector =0 ;
@@ -532,10 +536,10 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mutWARN_ON (!tr->cur_image && tr->selector);if (tr->cur_image)/* progs already running at this address */ - err =modify_fentry (tr, tr->cur_image->image, im->image, lock_direct_mutex); + err =modify_fentry (tr, im, lock_direct_mutex);else /* first time registering */ - err =register_fentry (tr, im->image); + err =register_fentry (tr, im);#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS if (err == -EAGAIN ) {
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 7/17 bpf: Add support to postpone trampoline update
In reply to Jiri OlsaAdding support to postpone the trampoline update and record it to the update list. If the update list is provided, all the reg/unreg/modify functions only add trampoline to the update list and stores the requested update information/data to the trampoline.
It will bed used in following changes where we need to do the actual trampoline update at the end of the attachment.
--- include/linux/bpf.h | 31 ++++++++++++----- kernel/bpf/trampoline.c | 76 +++++++++++++++++++++++++++++------------ 2 files changed, 78 insertions(+), 29 deletions(-)
include/linux/bpf.h +23 -8
@@ -768,6 +768,28 @@ struct btf_func_model {*/ #define BPF_MAX_TRAMP_LINKS 38 +enum bpf_tramp_update_action { +BPF_TRAMP_UPDATE_REG , +BPF_TRAMP_UPDATE_UNREG , +BPF_TRAMP_UPDATE_MODIFY , +}; + +enum bpf_tramp_prog_type { +BPF_TRAMP_FENTRY , +BPF_TRAMP_FEXIT , +BPF_TRAMP_MODIFY_RETURN , +BPF_TRAMP_MAX , +BPF_TRAMP_REPLACE ,/* more than MAX */ +}; + +struct bpf_tramp_update { +enum bpf_tramp_update_action action; +struct bpf_tramp_image *im; +enum bpf_tramp_prog_type kind; +struct bpf_prog_array *old_array; +struct list_head list; +}; +struct bpf_tramp_prog {struct bpf_prog *prog;u64 cookie;
@@ -827,14 +849,6 @@ struct bpf_ksym {bool prog; }; -enum bpf_tramp_prog_type { -BPF_TRAMP_FENTRY , -BPF_TRAMP_FEXIT , -BPF_TRAMP_MODIFY_RETURN , -BPF_TRAMP_MAX , -BPF_TRAMP_REPLACE ,/* more than MAX */ -}; -struct bpf_tramp_image {void *image;struct bpf_ksym ksym;
@@ -886,6 +900,7 @@ struct bpf_trampoline {u64 selector;struct module *mod;struct bpf_shim_tramp_link *shim_link; +struct bpf_tramp_update update; };struct bpf_attach_target_info {
kernel/bpf/trampoline.c +55 -21
202 lines not on this page · raw message
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 8/17 bpf: Factor bpf_trampoline_lookup function
In reply to Jiri OlsaFactoring bpf_trampoline_lookup function and adding to new functions which will be needed in following changes:
struct bpf_trampoline *__bpf_trampoline_lookup(u64 key)
- returns trampoline without locking trampoline_mutex
static struct bpf_trampoline *bpf_trampoline_alloc(void)
- alocates trampoline object
The bpf_trampoline_lookup logic stays the same.
--- kernel/bpf/trampoline.c | 45 ++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 12 deletions(-)
kernel/bpf/trampoline.c +33 -12
@@ -188,38 +188,59 @@ void bpf_tramp_id_put(struct bpf_tramp_id *id)kfree (id); } -static struct bpf_trampoline *bpf_trampoline_lookup (u64 key) +static struct bpf_trampoline *__bpf_trampoline_lookup (u64 key) {struct bpf_trampoline *tr;struct hlist_head *head; -mutex_lock (&trampoline_mutex); head = &trampoline_table[hash_64 (key,TRAMPOLINE_HASH_BITS )];hlist_for_each_entry (tr, head, hlist) { -if (tr->key == key) { -refcount_inc (&tr->refcnt); - goto out; - } +if (tr->key == key) +return tr; } +return NULL ; +} + +static struct bpf_trampoline *bpf_trampoline_alloc (void ) +{ +struct bpf_trampoline *tr; + tr =kzalloc (sizeof (*tr),GFP_KERNEL );if (!tr) - goto out; +return NULL ; + +INIT_HLIST_NODE (&tr->hlist); +refcount_set (&tr->refcnt,1 ); +mutex_init (&tr->mutex);#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS tr->fops =kzalloc (sizeof (struct ftrace_ops ),GFP_KERNEL );if (!tr->fops) {kfree (tr); - tr =NULL ; - goto out; +return NULL ; } tr->fops->private = tr; tr->fops->ops_func = bpf_tramp_ftrace_ops_func;#endif +return tr; +} + +static struct bpf_trampoline *bpf_trampoline_lookup (u64 key) +{ +struct bpf_trampoline *tr; +struct hlist_head *head; +mutex_lock (&trampoline_mutex); + tr =__bpf_trampoline_lookup (key); +if (tr) { +refcount_inc (&tr->refcnt); + goto out; + } + tr =bpf_trampoline_alloc (); +if (!tr) + goto out; tr->key = key; -INIT_HLIST_NODE (&tr->hlist); + head = &trampoline_table[hash_64 (key,TRAMPOLINE_HASH_BITS )];hlist_add_head (&tr->hlist, head); -refcount_set (&tr->refcnt,1 ); -mutex_init (&tr->mutex); out:mutex_unlock (&trampoline_mutex);return tr;
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 9/17 bpf: Factor bpf_trampoline_put function
In reply to Jiri OlsaFactoring bpf_trampoline_put function and adding new __bpf_trampoline_put function without locking trampoline_mutex.
It will be needed in following changes.
--- kernel/bpf/trampoline.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
kernel/bpf/trampoline.c +11 -7
@@ -928,22 +928,19 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,return tr; } -void bpf_trampoline_put (struct bpf_trampoline *tr) +static void __bpf_trampoline_put (struct bpf_trampoline *tr) {int i; -if (!tr) -return ; -mutex_lock (&trampoline_mutex);if (!refcount_dec_and_test (&tr->refcnt)) - goto out; +return ;WARN_ON_ONCE (mutex_is_locked (&tr->mutex));for (i =0 ; i <BPF_TRAMP_MAX ; i++) {if (!tr->progs_array[i])continue ;if (WARN_ON_ONCE (!bpf_prog_array_is_empty (tr->progs_array[i]))) - goto out; +return ; }/* This code will be executed even when the last bpf_tramp_image
@@ -958,7 +955,14 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)kfree (tr->fops); }kfree (tr); -out: +} + +void bpf_trampoline_put (struct bpf_trampoline *tr) +{ +if (!tr) +return ; +mutex_lock (&trampoline_mutex); +__bpf_trampoline_put (tr);mutex_unlock (&trampoline_mutex); }
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 10/17 bpf: Add support to attach program to multiple trampolines
In reply to Jiri OlsaAdding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct bpf_tramp_prog *tp,
struct bpf_tramp_id *id)
int bpf_trampoline_multi_detach(struct bpf_tramp_prog *tp,
struct bpf_tramp_id *id)
The program is passed as bpf_tramp_prog object and trampolines to attach it to are passed as bpf_tramp_id object.
The interface creates new bpf_trampoline object which is initialized as 'multi' trampoline and stored separtely from standard trampolines.
There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:
1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline
- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.
Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via bpf_trampoline_multi_attach
- D,E,F already have standard trampoline attached
- the bpf_trampoline_multi_attach will create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F
- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
- D,E,F functions are still attachable by any new trampoline
If the bpf_tramp_id object contains ID that exists as the standard trampoline the interface will add the program
These limitations are there to avoid extra complexity of splitting existing trampolines, which turned out to be nightmare. However the current code does not prevent any such feature improvement in the future.
--- include/linux/bpf.h | 8 + kernel/bpf/trampoline.c | 393 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 394 insertions(+), 7 deletions(-)
include/linux/bpf.h +8 -0
@@ -901,6 +901,12 @@ struct bpf_trampoline {struct module *mod;struct bpf_shim_tramp_link *shim_link;struct bpf_tramp_update update; +struct { +struct list_head list; +struct bpf_tramp_id *id; +struct bpf_tramp_id *id_multi; +struct bpf_tramp_id *id_singles; + } multi; };struct bpf_attach_target_info {
@@ -945,6 +951,8 @@ void bpf_trampoline_put(struct bpf_trampoline *tr);struct bpf_tramp_id *bpf_tramp_id_alloc (u32 cnt);void bpf_tramp_id_put (struct bpf_tramp_id *id);int arch_prepare_bpf_dispatcher (void *image,s64 *funcs,int num_funcs); +int bpf_trampoline_multi_detach (struct bpf_tramp_prog *tp,struct bpf_tramp_id *id); +int bpf_trampoline_multi_attach (struct bpf_tramp_prog *tp,struct bpf_tramp_id *id);#define BPF_DISPATCHER_INIT (_name) { \ .mutex = __MUTEX_INITIALIZER(_name.mutex), \ .func = &_name##_func, \
kernel/bpf/trampoline.c +386 -7
474 lines not on this page · raw message
Signature · 1 line
2.37.1
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attach
Quoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.
We still need sinle bpf_link_multi to contain btf_ids of all functions, but it can point to many bpf tramps. One for each attach function.
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.arch_prepare_bpf_trampoline is fast.bpf_tramp_image_alloc is fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands of bpf_tramp_image_put,
but it's all async now. We used to have synchronize_rcu() which could be the reason why this approach was slow.
Or is this unregister_fentry that slows it down?
But register_ftrace_direct_multi() interface should have solved it for both register and unregister?
Hopefully the bpf_tramp_id concept won't be needed.
It's bpf_link that will keep a link list or array of btf_ids and tramps. bpf_tracing_link_multi ?
link destroy is simply going over all tramps and removing prog from them.
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.
Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?
I think it's the synchronize_rcu_tasks at the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the end
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call.. I was already checking on that and looks doable
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
jirka
Quoted 5 lines
Hopefully the
bpf_tramp_idconcept won't be needed.
It'sbpf_linkthat will keep a link list or array ofbtf_idsand tramps.bpf_tracing_link_multi?
link destroy is simply going over all tramps and removing prog from them.
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 73 lines · Jiri Olsa
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?
Quoted 3 lines
I think it's the
synchronize_rcu_tasksat the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the end
hmm. Can synchronize_rcu_tasks be made optional?
For ftrace_direct that points to bpf tramps is it really needed?
Quoted 4 lines
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call..
right
Quoted 1 line
I was already checking on that and looks doable
awesome.
Quoted 2 lines
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and brand new mutexes, so no contention.
But thousands of cmpxchg-s will take time. Would be good to measure though. It might not be that bad.
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 93 lines · Alexei Starovoitov
Quoted 73 lines · Jiri Olsa
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?Quoted 3 lines
I think it's the
synchronize_rcu_tasksat the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the endhmm. Can
synchronize_rcu_tasksbe made optional?
Forftrace_directthat points to bpf tramps is it really needed?Quoted 4 lines
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call..
right
Quoted 1 line
I was already checking on that and looks doable
awesome.
Quoted 7 lines
Quoted 2 lines
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and brand new mutexes, so no contention.
But thousands of cmpxchg-s will take time. Would be good to measure though. It might not be that bad.
What about the memory overhead of thousands of trampolines and trampoline images? Seems very wasteful to create one per each attach, when each attachment in general will be identical.
If I remember correctly, last time we were also discussing creating a generic BPF trampoline that would save all 6 input registers, regardless of function's BTF signature. Such BPF trampoline should support calling both generic fentry/fexit programs and typed ones, because all the necessary data is stored on the stack correctly.
For the case when typed (non-generic) BPF trampoline is already attached to a function and now we are attaching generic fentry, why can't we "upgrade" existing BPF trampoline to become generic, and then just add generic multi-fentry program to its trampoline image? Once that multi-fentry is detached, we might choose to convert trampoline back to typed BPF trampoline (i.e., save only necessary registers, not all 6 of them), but that's more like an optimization, it doesn't have to happen.
Or is there something that would make such generic trampoline impossible?
If we go with this approach, then each multi-fentry attachment will be creating minimum amount of trampolines, determined by all the combinations of attached programs at that point. If after we attach multi-fentry to some set of functions we need to attach another multi-fentry or typed fentry, we'd potentially need to split trampolines and create a bit more of them. But while that sounds a bit complicated, we do all that under locks so there isn't much problem in doing that, no?
But in general, I agree with Alexei that this restriction on not being able to attach to a function once multi-attach trampoline is attached to it is a really-really bad restriction in production, where we can't control exactly what BPF apps run and in which order.
P.S. I think this generic typeless BPF trampoline is a useful thing in itself and we are half-way there already with bpf_get_func_ip() and bpf_get_func_arg_cnt() helpers and storing such "parameters" on the stack, so tbh, we can probably split the problem into two and try to address a somewhat simpler and more straightforward generic BPF trampoline first. Such generic type-less BPF trampoline will make fentry a better and more generic alternative to kprobe, by being less demanding about specifying BTF ID (even if we don't care about input argument types) yet faster to trigger than kprobe.
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 134 lines · Andrii Nakryiko
Quoted 93 lines · Alexei Starovoitov
Quoted 73 lines · Jiri Olsa
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?Quoted 3 lines
I think it's the
synchronize_rcu_tasksat the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the endhmm. Can
synchronize_rcu_tasksbe made optional?
Forftrace_directthat points to bpf tramps is it really needed?Quoted 4 lines
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call..
right
Quoted 1 line
I was already checking on that and looks doable
awesome.
Quoted 7 lines
Quoted 2 lines
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and brand new mutexes, so no contention.
But thousands of cmpxchg-s will take time. Would be good to measure though. It might not be that bad.What about the memory overhead of thousands of trampolines and trampoline images? Seems very wasteful to create one per each attach, when each attachment in general will be identical.
If I remember correctly, last time we were also discussing creating a generic BPF trampoline that would save all 6 input registers, regardless of function's BTF signature. Such BPF trampoline should support calling both generic fentry/fexit programs and typed ones, because all the necessary data is stored on the stack correctly.
For the case when typed (non-generic) BPF trampoline is already attached to a function and now we are attaching generic fentry, why can't we "upgrade" existing BPF trampoline to become generic, and then just add generic multi-fentry program to its trampoline image? Once that multi-fentry is detached, we might choose to convert trampoline back to typed BPF trampoline (i.e., save only necessary registers, not all 6 of them), but that's more like an optimization, it doesn't have to happen.
Or is there something that would make such generic trampoline impossible?
If we go with this approach, then each multi-fentry attachment will be creating minimum amount of trampolines, determined by all the combinations of attached programs at that point. If after we attach multi-fentry to some set of functions we need to attach another multi-fentry or typed fentry, we'd potentially need to split trampolines and create a bit more of them. But while that sounds a bit complicated, we do all that under locks so there isn't much problem in doing that, no?
Quoted 4 lines
But in general, I agree with Alexei that this restriction on not being able to attach to a function once multi-attach trampoline is attached to it is a really-really bad restriction in production, where we can't control exactly what BPF apps run and in which order.
ah ok.. attaching single trampoline on top of attached multi trampoline should be possible to add.. as long as one side of the problem is single trampoline it should be doable, I'll check
leaving the restriction only to attaching one multi trampoline over another (not equal) attached multi trampoline
would that be acceptable?
Quoted 10 lines
P.S. I think this generic typeless BPF trampoline is a useful thing in itself and we are half-way there already withbpf_get_func_ip()andbpf_get_func_arg_cnt()helpers and storing such "parameters" on the stack, so tbh, we can probably split the problem into two and try to address a somewhat simpler and more straightforward generic BPF trampoline first. Such generic type-less BPF trampoline will make fentry a better and more generic alternative to kprobe, by being less demanding about specifying BTF ID (even if we don't care about input argument types) yet faster to trigger than kprobe.
yes, with the help of those helpers the only 'generic' thing for trampoline is its BTF type
jirka
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 141 lines · Jiri Olsa
Quoted 134 lines · Andrii Nakryiko
Quoted 93 lines · Alexei Starovoitov
Quoted 73 lines · Jiri Olsa
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?Quoted 3 lines
I think it's the
synchronize_rcu_tasksat the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the endhmm. Can
synchronize_rcu_tasksbe made optional?
Forftrace_directthat points to bpf tramps is it really needed?Quoted 4 lines
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call..
right
Quoted 1 line
I was already checking on that and looks doable
awesome.
Quoted 7 lines
Quoted 2 lines
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and brand new mutexes, so no contention.
But thousands of cmpxchg-s will take time. Would be good to measure though. It might not be that bad.What about the memory overhead of thousands of trampolines and trampoline images? Seems very wasteful to create one per each attach, when each attachment in general will be identical.
If I remember correctly, last time we were also discussing creating a generic BPF trampoline that would save all 6 input registers, regardless of function's BTF signature. Such BPF trampoline should support calling both generic fentry/fexit programs and typed ones, because all the necessary data is stored on the stack correctly.
For the case when typed (non-generic) BPF trampoline is already attached to a function and now we are attaching generic fentry, why can't we "upgrade" existing BPF trampoline to become generic, and then just add generic multi-fentry program to its trampoline image? Once that multi-fentry is detached, we might choose to convert trampoline back to typed BPF trampoline (i.e., save only necessary registers, not all 6 of them), but that's more like an optimization, it doesn't have to happen.
Or is there something that would make such generic trampoline impossible?
If we go with this approach, then each multi-fentry attachment will be creating minimum amount of trampolines, determined by all the combinations of attached programs at that point. If after we attach multi-fentry to some set of functions we need to attach another multi-fentry or typed fentry, we'd potentially need to split trampolines and create a bit more of them. But while that sounds a bit complicated, we do all that under locks so there isn't much problem in doing that, no?
Quoted 4 lines
But in general, I agree with Alexei that this restriction on not being able to attach to a function once multi-attach trampoline is attached to it is a really-really bad restriction in production, where we can't control exactly what BPF apps run and in which order.
Quoted 8 lines
ah ok.. attaching single trampoline on top of attached multi trampoline should be possible to add.. as long as one side of the problem is single trampoline it should be doable, I'll check
leaving the restriction only to attaching one multi trampoline over another (not equal) attached multi trampoline
would that be acceptable?
I guess I'm missing what's fundamentally different between multi-trampoline + single trampoline vs multi-tramp + multi-tramp? Multi-tramp is already saving all registers, so can "host" other generic fentry/fexit. So why this multi + multi restriction?
Quoted 16 lines
Quoted 11 lines
P.S. I think this generic typeless BPF trampoline is a useful thing in itself and we are half-way there already withbpf_get_func_ip()andbpf_get_func_arg_cnt()helpers and storing such "parameters" on the stack, so tbh, we can probably split the problem into two and try to address a somewhat simpler and more straightforward generic BPF trampoline first. Such generic type-less BPF trampoline will make fentry a better and more generic alternative to kprobe, by being less demanding about specifying BTF ID (even if we don't care about input argument types) yet faster to trigger than kprobe.yes, with the help of those helpers the only 'generic' thing for trampoline is its BTF type
jirka
Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Quoted 149 lines · Andrii Nakryiko
Quoted 148 lines · Jiri Olsa
Quoted 134 lines · Andrii Nakryiko
Quoted 93 lines · Alexei Starovoitov
Quoted 73 lines · Jiri Olsa
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?Quoted 3 lines
I think it's the
synchronize_rcu_tasksat the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the endhmm. Can
synchronize_rcu_tasksbe made optional?
Forftrace_directthat points to bpf tramps is it really needed?Quoted 4 lines
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call..
right
Quoted 1 line
I was already checking on that and looks doable
awesome.
Quoted 7 lines
Quoted 2 lines
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and brand new mutexes, so no contention.
But thousands of cmpxchg-s will take time. Would be good to measure though. It might not be that bad.What about the memory overhead of thousands of trampolines and trampoline images? Seems very wasteful to create one per each attach, when each attachment in general will be identical.
If I remember correctly, last time we were also discussing creating a generic BPF trampoline that would save all 6 input registers, regardless of function's BTF signature. Such BPF trampoline should support calling both generic fentry/fexit programs and typed ones, because all the necessary data is stored on the stack correctly.
For the case when typed (non-generic) BPF trampoline is already attached to a function and now we are attaching generic fentry, why can't we "upgrade" existing BPF trampoline to become generic, and then just add generic multi-fentry program to its trampoline image? Once that multi-fentry is detached, we might choose to convert trampoline back to typed BPF trampoline (i.e., save only necessary registers, not all 6 of them), but that's more like an optimization, it doesn't have to happen.
Or is there something that would make such generic trampoline impossible?
If we go with this approach, then each multi-fentry attachment will be creating minimum amount of trampolines, determined by all the combinations of attached programs at that point. If after we attach multi-fentry to some set of functions we need to attach another multi-fentry or typed fentry, we'd potentially need to split trampolines and create a bit more of them. But while that sounds a bit complicated, we do all that under locks so there isn't much problem in doing that, no?
Quoted 4 lines
But in general, I agree with Alexei that this restriction on not being able to attach to a function once multi-attach trampoline is attached to it is a really-really bad restriction in production, where we can't control exactly what BPF apps run and in which order.
ah ok.. attaching single trampoline on top of attached multi trampoline should be possible to add.. as long as one side of the problem is single trampoline it should be doable, I'll check
leaving the restriction only to attaching one multi trampoline over another (not equal) attached multi trampoline
Quoted 6 lines
Quoted 1 line
would that be acceptable?
I guess I'm missing what's fundamentally different between multi-trampoline + single trampoline vs multi-tramp + multi-tramp? Multi-tramp is already saving all registers, so can "host" other generic fentry/fexit. So why this multi + multi restriction?
so I did not find good generic solution for multi trampoline being attached on top of already attached multi trampolines
say we have following multi trampolines:
multi_a [1,2,3] P1
multi_b [4,5,6] P2
and want to add another multi trampoline:
multi_c [1,4,7] P3
you end up with 5 new trampolines:
multi_1 [1] P1,P3
multi_2 [2,3] P1
multi_3 [4] P2,P3
multi_4 [5,6] P2
multi_5 [7] P3
pain points were:
- lookup for intersection on top of BTF ids, doable but tricky
https://lore.kernel.org/bpf/[email protected]/
- splitting existing trampolines and rollback in case of error, because the image update and ip are 2 separate things but we do them together
https://lore.kernel.org/bpf/[email protected]/
- trampoline can't be stored and managed in link, because it can be split into multiple new trampolines, so I added new layer to keep them
https://lore.kernel.org/bpf/[email protected]/
- all this must be locked.. all involved trampolines or one big lock
- any new attachment of single trampoline is possibly splitting existing multi trampoline
- when multi_c trampoline is detached we don't roll back to get original multi_a and multi_b - we keep the split trampolines, so each new attachment is making more trampolines and makes the new attachment possibly slower
the RFC for this is here:
https://lore.kernel.org/bpf/[email protected]/
it did not bring too much attention so I simplified it down to the current version ;-)
so far I could not think of better way than start with basic functionality and add/rethink the complex multi/multi stuff later if needed, hopefully with some better idea how to do that
note this was no problem for kprobe_multi which uses fprobe/ftrace_ops code that takes care of this - we just say this function should be called from set of ips and ftrace machinery does all the merging with existing attachments
but bpf trampolines use ftrace direct interface, which only attaches trampoline to given function without any other logic
jirka
Song Liu
maintainerRe: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
In reply to Alexei StarovoitovQuoted 91 lines · Alexei Starovoitov
Quoted 73 lines · Jiri Olsa
Quoted 58 lines · Alexei Starovoitov
Quoted 34 lines · Jiri Olsa
Adding support to attach program to multiple trampolines with new attach/detach interface:
int bpf_trampoline_multi_attach(struct
bpf_tramp_prog*tp,
structbpf_tramp_id*id)
int bpf_trampoline_multi_detach(structbpf_tramp_prog*tp,
structbpf_tramp_id*id)The program is passed as
bpf_tramp_progobject and trampolines to attach it to are passed asbpf_tramp_idobject.The interface creates new
bpf_trampolineobject which is initialized as 'multi' trampoline and stored separtely from standard trampolines.There are following rules how the standard and multi trampolines go along:
- multi trampoline can attach on top of existing single trampolines, which creates 2 types of function IDs:1) single-IDs - functions that are attached within existing single trampolines
2) multi-IDs - functions that were 'free' and are now taken by new 'multi' trampoline- we allow overlapping of 2 'multi' trampolines if they are attached to same IDs
- we do now allow any other overlapping of 2 'multi' trampolines
- any new 'single' trampoline cannot attach to existing multi-IDs IDs.Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attachQuoted 8 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
This restriction is probably too severe.
Song added support for trampoline and KLPs to co-exist on the same function. This multi trampoline restriction will resurface the same issue. afiak this restriction is only because multi trampoline image
is the same for A,B,C. This memory optimization is probably going too far. How about we keep existing logic of one tramp image per function. Pretend that multi-prog P matches BTF of the target function, create normal tramp for it and attach prog P there.
The prototype of P allows six u64. The args are potentially rearding garbage, but there are no safety issues, since multi progs don't know BTF types.We still need sinle
bpf_link_multito containbtf_idsof all functions, but it can point to many bpf tramps. One for each attach function.Quoted 12 lines
iirc we discussed something like this long ago, but I don't remember why we didn't go that route.
arch_prepare_bpf_trampolineis fast.bpf_tramp_image_allocis fast too.
So attaching one multi-prog to thousands of btf_id-s should be fast too. The destroy part is interesting.
There we will be doing thousands ofbpf_tramp_image_put,
but it's all async now. We used to havesynchronize_rcu()which could be the reason why this approach was slow.
Or is thisunregister_fentrythat slows it down?
Butregister_ftrace_direct_multi()interface should have solved it for both register and unregister?Quoted 3 lines
I think it's the
synchronize_rcu_tasksat the end of each ftrace update, that's why we added un/register_ftrace_direct_multi that makes the changes for multiple ips and syncs once at the endhmm. Can
synchronize_rcu_tasksbe made optional?
Forftrace_directthat points to bpf tramps is it really needed?Quoted 4 lines
un/register_ftrace_direct_multi will attach/detach multiple multiple ips to single address (trampoline), so for this approach we would need to add new ftrace direct api that would allow to set multiple ips to multiple trampolines within one call..
right
Quoted 1 line
I was already checking on that and looks doable
Quoted 7 lines
awesome.
Quoted 2 lines
another problem might be that this update function will need to be called with all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and brand new mutexes, so no contention.
I guess we still need to lock existing tr->mutex in some cases? Say, we have 3 functions, A, B, C, and A already have tr_A. If we want to attach tr_multi for all three, we still need to lock tr_A->mutex, no?
Thanks,
Song
Quoted 2 lines
But thousands of cmpxchg-s will take time. Would be good to measure though. It might not be that bad.
Jiri Olsa
reviewerRFC PATCH 11/17 bpf: Add support to create tracing multi link
In reply to Jiri OlsaAdding new link to allow to attach program to multiple function BTF IDs.
New fields are added to bpf_attr::link_create to pass array of BTF IDs:
struct {
__aligned_u64 btf_ids; /* addresses to attach */
__u32 btf_ids_cnt; /* addresses count */
} multi;The new link code will load these IDs into bpf_tramp_id object and resolve their ips.
The resolve itself is done as per Andrii's suggestion:
- lookup all names for given IDs
- store and sort them by name
- go through all kallsyms symbols and use bsearch to find it in provided names
- if name is found, store the address for the name
- resort the names array based on ID
If there are multi symbols of the same name the first one will be used to resolve the address.
The link is using bpf_trampoline_multi_attach interface to attach this IDs to the program.
--- include/linux/trace_events.h | 5 + include/uapi/linux/bpf.h | 5 + kernel/bpf/syscall.c | 2 + kernel/trace/bpf_trace.c | 240 +++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 5 + 5 files changed, 257 insertions(+)
include/linux/trace_events.h +5 -0
@@ -747,6 +747,7 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, u32 *fd_type, const char **buf, u64 *probe_offset, u64 *probe_addr);int bpf_kprobe_multi_link_attach (const union bpf_attr *attr,struct bpf_prog *prog); +int bpf_tracing_multi_attach (struct bpf_prog *prog,const union bpf_attr *attr);#else static inline unsigned int trace_call_bpf (struct trace_event_call *call,void *ctx) {
@@ -793,6 +794,10 @@ bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) {return -EOPNOTSUPP ; } +int bpf_tracing_multi_attach (struct bpf_prog *prog,const union bpf_attr *attr) +{ +return -EOPNOTSUPP ; +}#endif enum {
include/uapi/linux/bpf.h +5 -0
@@ -1017,6 +1017,7 @@ enum bpf_link_type {BPF_LINK_TYPE_PERF_EVENT =7 ,BPF_LINK_TYPE_KPROBE_MULTI =8 ,BPF_LINK_TYPE_STRUCT_OPS =9 , +BPF_LINK_TYPE_TRACING_MULTI =10 , MAX_BPF_LINK_TYPE, };
@@ -1503,6 +1504,10 @@ union bpf_attr {*/ __u64 cookie; } tracing; +struct { +__aligned_u64 btf_ids;/* addresses to attach */ +__u32 btf_ids_cnt;/* addresses count */ + } tracing_multi; }; } link_create;
kernel/bpf/syscall.c +2 -0
@@ -4604,6 +4604,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr) ret =bpf_iter_link_attach (attr, uattr, prog); else if (prog->expected_attach_type == BPF_LSM_CGROUP) ret =cgroup_bpf_link_attach (attr, prog); + else if (is_tracing_multi(prog->expected_attach_type)) + ret =bpf_tracing_multi_attach (prog, attr); else ret = bpf_tracing_prog_attach(prog, attr->link_create.target_fd,
kernel/trace/bpf_trace.c +240 -0
243 lines not on this page · raw message
tools/include/uapi/linux/bpf.h +5 -0
@@ -1017,6 +1017,7 @@ enum bpf_link_type {BPF_LINK_TYPE_PERF_EVENT =7 ,BPF_LINK_TYPE_KPROBE_MULTI =8 ,BPF_LINK_TYPE_STRUCT_OPS =9 , +BPF_LINK_TYPE_TRACING_MULTI =10 , MAX_BPF_LINK_TYPE, };
@@ -1503,6 +1504,10 @@ union bpf_attr {*/ __u64 cookie; } tracing; +struct { +__aligned_u64 btf_ids;/* addresses to attach */ +__u32 btf_ids_cnt;/* addresses count */ + } tracing_multi; }; } link_create;
Signature · 1 line
2.37.1
kernel test robot
build botRe: [RFC PATCH bpf-next 11/17] bpf: Add support to create tracing multi link
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in…
Hi Jiri,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf/master]
[also build test WARNING on next-20220808]
[cannot apply to bpf-next/master rostedt-trace/for-next linus/master v5.19] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jiri-Olsa/bpf-Add-tracing-multi-link/20220808-221246 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master config: hexagon-randconfig-r002-20220808 (https://download.01.org/0day-ci/archive/20220809/[email protected]/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c16f49e680e78a8df8ffc125e001217fb8be8e0b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jiri-Olsa/bpf-Add-tracing-multi-link/20220808-221246
git checkout c16f49e680e78a8df8ffc125e001217fb8be8e0b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/time/
If you fix the issue, kindly add following tag where applicable
All warnings (new ones prefixed by >>):
In file included from kernel/time/time.c:33:
In file included from include/linux/syscalls.h:88:
In file included from include/trace/syscall.h:7:
>> include/linux/trace_events.h:797:5: warning: no previous prototype for function 'bpf_tracing_multi_attach' [-Wmissing-prototypes]
int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
^ include/linux/trace_events.h:797:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
^
static
1 warning generated.
--
In file included from kernel/time/hrtimer.c:30:
In file included from include/linux/syscalls.h:88:
In file included from include/trace/syscall.h:7:
>> include/linux/trace_events.h:797:5: warning: no previous prototype for function 'bpf_tracing_multi_attach' [-Wmissing-prototypes]
int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
^
include/linux/trace_events.h:797:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
^
static
kernel/time/hrtimer.c:120:21: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[CLOCK_REALTIME] = HRTIMER_BASE_REALTIME,
^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:118:27: note: previous initialization is here
[0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:121:22: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC,
^~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:118:27: note: previous initialization is here
[0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:122:21: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME,
^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:118:27: note: previous initialization is here
[0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:123:17: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[CLOCK_TAI] = HRTIMER_BASE_TAI,
^~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:118:27: note: previous initialization is here
[0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:147:20: warning: unused function 'is_migration_base' [-Wunused-function]
static inline bool is_migration_base(struct hrtimer_clock_base *base)
^
kernel/time/hrtimer.c:1873:20: warning: unused function '__hrtimer_peek_ahead_timers' [-Wunused-function]
static inline void __hrtimer_peek_ahead_timers(void)
^7 warnings generated.
vim +/bpf_tracing_multi_attach +797 include/linux/trace_events.h
764
765 static inline int
766 perf_event_query_prog_array(struct perf_event *event, void __user *info)
767 {
768 return -EOPNOTSUPP;
769 }
770 static inline int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *p)
771 {
772 return -EOPNOTSUPP;
773 }
774 static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *p)
775 {
776 return -EOPNOTSUPP;
777 }
778 static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
779 {
780 return NULL;
781 }
782 static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
783 {
784 }
785 static inline int bpf_get_perf_event_info(const struct perf_event *event,
786 u32 *prog_id, u32 *fd_type,
787 const char **buf, u64 *probe_offset,
788 u64 *probe_addr)
789 {
790 return -EOPNOTSUPP;
791 }
792 static inline int
793 bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
794 {
795 return -EOPNOTSUPP;
796 }
> 797 int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr) 798 {
799 return -EOPNOTSUPP;
800 }
801 #endif
802
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
kernel test robot
build botRe: [RFC PATCH bpf-next 11/17] bpf: Add support to create tracing multi link
In reply to Jiri Olsa[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in…
Hi Jiri,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on bpf/master]
[also build test ERROR on next-20220808]
[cannot apply to bpf-next/master rostedt-trace/for-next linus/master v5.19] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jiri-Olsa/bpf-Add-tracing-multi-link/20220808-221246 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master config: x86_64-randconfig-a011-20220808 (https://download.01.org/0day-ci/archive/20220809/[email protected]/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c16f49e680e78a8df8ffc125e001217fb8be8e0b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jiri-Olsa/bpf-Add-tracing-multi-link/20220808-221246
git checkout c16f49e680e78a8df8ffc125e001217fb8be8e0b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/i915/i915_trace_points.c:13:
In file included from include/trace/../../drivers/gpu/drm/i915/i915_trace.h:770:
In file included from include/trace/define_trace.h:102:
In file included from include/trace/trace_events.h:21:
>> include/linux/trace_events.h:797:5: error: no previous prototype for function 'bpf_tracing_multi_attach' [-Werror,-Wmissing-prototypes]
int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
^ include/linux/trace_events.h:797:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
^
static
1 error generated.
vim +/bpf_tracing_multi_attach +797 include/linux/trace_events.h
764
765 static inline int
766 perf_event_query_prog_array(struct perf_event *event, void __user *info)
767 {
768 return -EOPNOTSUPP;
769 }
770 static inline int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *p)
771 {
772 return -EOPNOTSUPP;
773 }
774 static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *p)
775 {
776 return -EOPNOTSUPP;
777 }
778 static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
779 {
780 return NULL;
781 }
782 static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
783 {
784 }
785 static inline int bpf_get_perf_event_info(const struct perf_event *event,
786 u32 *prog_id, u32 *fd_type,
787 const char **buf, u64 *probe_offset,
788 u64 *probe_addr)
789 {
790 return -EOPNOTSUPP;
791 }
792 static inline int
793 bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
794 {
795 return -EOPNOTSUPP;
796 }
> 797 int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr) 798 {
799 return -EOPNOTSUPP;
800 }
801 #endif
802
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
Jiri Olsa
reviewerRFC PATCH 12/17 libbpf: Add btf__find_by_glob_kind function
In reply to Jiri OlsaAdding btf__find_by_glob_kind function that returns array of BTF ids that match given kind and allow/deny patterns.
int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
const char *allow_pattern, const char *deny_pattern, __u32 **__ids);
The __ids array is allocated and needs to be manually freed.
The pattern check is done by glob_match function.
--- tools/lib/bpf/btf.c | 41 +++++++++++++++++++++++++++++++++ tools/lib/bpf/btf.h | 3 +++ tools/lib/bpf/libbpf.c | 2 +- tools/lib/bpf/libbpf_internal.h | 1 + 4 files changed, 46 insertions(+), 1 deletion(-)
tools/lib/bpf/btf.c +41 -0
@@ -770,6 +770,47 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,return btf_find_by_name_kind (btf,1 , type_name, kind); } +int btf__find_by_glob_kind (const struct btf *btf,__u32 kind, +const char *allow_pattern,const char *deny_pattern, +__u32 **__ids) +{ +__u32 i, nr_types =btf__type_cnt (btf); +int cnt =0 , alloc =0 ; +__u32 *ids =NULL ; + +for (i =1 ; i < nr_types; i++) { +const struct btf_type *t =btf__type_by_id (btf, i); +const char *name; +__u32 *p; + +if (btf_kind (t) != kind) +continue ; + name =btf__name_by_offset (btf, t->name_off); +if (!name) +continue ; + +if (deny_pattern &&glob_match (name, deny_pattern)) +continue ; +if (allow_pattern && !glob_match (name, allow_pattern)) +continue ; + +if (cnt == alloc) { + alloc =max (16 , alloc *3 /2 ); + p =libbpf_reallocarray (ids, alloc,sizeof (__u32)); +if (!p) { +free (ids); +return -ENOMEM ; + } + ids = p; + } + ids[cnt] = i; + cnt++; + } + + *__ids = ids; +return cnt; +} +static bool btf_is_modifiable (const struct btf *btf) {return (void *)btf->hdr != btf->raw_data;
tools/lib/bpf/btf.h +3 -0
@@ -546,6 +546,9 @@ static inline struct btf_decl_tag *btf_decl_tag(const struct btf_type *t)return (struct btf_decl_tag *)(t +1 ); } +int btf__find_by_glob_kind (const struct btf *btf,__u32 kind, +const char *allow_pattern,const char *deny_pattern, +__u32 **__ids); #ifdef __cplusplus }/* extern "C" */ #endif
tools/lib/bpf/libbpf.c +1 -1
@@ -10153,7 +10153,7 @@ struct bpf_link *bpf_program__attach_ksyscall(const struct bpf_program *prog, }/* Adapted from perf/util/string.c */ -static bool glob_match (const char *str,const char *pat) +bool glob_match (const char *str,const char *pat) {while (*str && *pat && *pat !='*' ) {if (*pat =='?' ) {/* Matches any single character */
tools/lib/bpf/libbpf_internal.h +1 -0
@@ -573,4 +573,5 @@ static inline bool is_pow_of_2(size_t x) return x && (x & (x - 1)) == 0; } +bool glob_match (const char *str,const char *pat);#endif /* __LIBBPF_LIBBPF_INTERNAL_H */
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 13/17 libbpf: Add support to create tracing multi link
In reply to Jiri OlsaAdding new interface tion to attach programs with
tracing multi link:
bpf_program__attach_tracing_multi(const struct bpf_program *prog,
const char *pattern,
const struct bpf_tracing_multi_opts *opts);
The program is attach to functions specified by patter or by btf IDs specified in bpf_tracing_multi_opts object.
Adding support for new sections to attach programs with above functions:
fentry.multi/pattern
fexit.multi/pattern
--- tools/lib/bpf/bpf.c | 7 ++++ tools/lib/bpf/bpf.h | 4 ++ tools/lib/bpf/libbpf.c | 89 ++++++++++++++++++++++++++++++++++++++++ tools/lib/bpf/libbpf.h | 14 +++++++ tools/lib/bpf/libbpf.map | 1 + 5 files changed, 115 insertions(+)
tools/lib/bpf/bpf.c +7 -0
@@ -713,6 +713,13 @@ int bpf_link_create(int prog_fd, int target_fd,if (!OPTS_ZEROED (opts, kprobe_multi))return libbpf_err (-EINVAL );break ; +case BPF_TRACE_FENTRY_MULTI : +case BPF_TRACE_FEXIT_MULTI : + attr.link_create.tracing_multi.btf_ids = (__u64 )OPTS_GET (opts, tracing_multi.btf_ids,0 ); + attr.link_create.tracing_multi.btf_ids_cnt =OPTS_GET (opts, tracing_multi.btf_ids_cnt,0 ); +if (!OPTS_ZEROED (opts, tracing_multi)) +return libbpf_err (-EINVAL ); +break ;case BPF_TRACE_FENTRY :case BPF_TRACE_FEXIT :case BPF_MODIFY_RETURN :
tools/lib/bpf/bpf.h +4 -0
@@ -321,6 +321,10 @@ struct bpf_link_create_opts {struct {__u64 cookie; } tracing; +struct { +__u32 *btf_ids; +__u32 btf_ids_cnt; + } tracing_multi; }; size_t :0; };
tools/lib/bpf/libbpf.c +89 -0
119 lines not on this page · raw message
tools/lib/bpf/libbpf.h +14 -0
@@ -503,6 +503,20 @@ bpf_program__attach_ksyscall(const struct bpf_program *prog,const char *syscall_name, const struct bpf_ksyscall_opts *opts); +struct bpf_tracing_multi_opts { +/* size of this struct, for forward/backward compatibility */ +size_t sz; +__u32 *btf_ids; +size_t cnt; + size_t :0; +}; + +#define bpf_kprobe_multi_opts__last_field retprobe + +LIBBPF_API struct bpf_link * +bpf_program__attach_tracing_multi (const struct bpf_program *prog,const char *pattern, +const struct bpf_tracing_multi_opts *opts); +struct bpf_uprobe_opts {/* size of this struct, for forward/backward compatiblity */ size_t sz;
tools/lib/bpf/libbpf.map +1 -0
@@ -358,6 +358,7 @@ LIBBPF_1.0.0 { bpf_obj_get_opts; bpf_prog_query_opts; bpf_program__attach_ksyscall; + bpf_program__attach_tracing_multi; btf__add_enum64; btf__add_enum64_value; libbpf_bpf_attach_type_str;
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 14/17 selftests/bpf: Add fentry tracing multi func test
In reply to Jiri OlsaAdding selftest for fentry multi func test that attaches to bpf_fentry_test* functions and checks argument values based on the processed function.
We need to cast to real arguments types in multi_arg_check, because the checked value can be shorter than u64.
--- tools/testing/selftests/bpf/Makefile | 3 +- .../selftests/bpf/prog_tests/tracing_multi.c | 34 +++++ .../selftests/bpf/progs/tracing_multi_check.c | 132 ++++++++++++++++++ .../bpf/progs/tracing_multi_fentry.c | 17 +++ 4 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/tracing_multi.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_check.c create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_fentry.c
tools/testing/selftests/bpf/Makefile +2 -1
@@ -348,7 +348,7 @@ SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \ linked_vars.skel.h linked_maps.skel.h \ test_subskeleton.skel.h test_subskeleton_lib.skel.h \ - test_usdt.skel.h + test_usdt.skel.h tracing_multi_fentry_test.skel.h LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \ test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
@@ -366,6 +366,7 @@ linked_maps.skel.h-deps := linked_maps1.o linked_maps2.o test_subskeleton.skel.h-deps := test_subskeleton_lib2.o test_subskeleton_lib.o test_subskeleton.o test_subskeleton_lib.skel.h-deps := test_subskeleton_lib2.o test_subskeleton_lib.o test_usdt.skel.h-deps := test_usdt.o test_usdt_multispec.o +tracing_multi_fentry_test.skel.h-deps := tracing_multi_fentry.o tracing_multi_check.o LINKED_BPF_SRCS := $(patsubst %.o,%.c,$(foreach skel,$(LINKED_SKELS),$($(skel)-deps)))
tools/testing/selftests/bpf/prog_tests/tracing_multi.c new file +34 -0
@@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> +#include "tracing_multi_fentry_test.skel.h" +#include "trace_helpers.h" + +static void multi_fentry_test (void ) +{ +LIBBPF_OPTS (bpf_test_run_opts, topts); +struct tracing_multi_fentry_test *skel =NULL ; +int err, prog_fd; + + skel =tracing_multi_fentry_test__open_and_load (); +if (!ASSERT_OK_PTR (skel,"fentry_multi_skel_load" )) + goto cleanup; + + err =tracing_multi_fentry_test__attach (skel); +if (!ASSERT_OK (err,"fentry_attach" )) + goto cleanup; + + prog_fd =bpf_program__fd (skel->progs.test); + err =bpf_prog_test_run_opts (prog_fd, &topts); +ASSERT_OK (err,"test_run" ); + +ASSERT_EQ (skel->bss->test_result,8 ,"test_result" ); + +cleanup: +tracing_multi_fentry_test__destroy (skel); +} + +void test_tracing_multi_test (void ) +{ +if (test__start_subtest ("fentry" )) +multi_fentry_test (); +}
tools/testing/selftests/bpf/progs/tracing_multi_check.c new file +132 -0
132 lines not on this page · raw message
tools/testing/selftests/bpf/progs/tracing_multi_fentry.c new file +17 -0
@@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +char _license[] SEC("license") = "GPL"; + +__u64 test_result =0 ; + +__hidden extern void multi_arg_check(__u64 *ctx, __u64 *test_result); + +SEC("fentry.multi/bpf_fentry_test*") +int BPF_PROG (test ,__u64 a,__u64 b,__u64 c,__u64 d,__u64 e,__u64 f) +{ +multi_arg_check (ctx, &test_result); +return 0 ; +}
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 15/17 selftests/bpf: Add fexit tracing multi func test
In reply to Jiri OlsaAdding selftest for fexit multi func test that attaches to bpf_fentry_test* functions and checks argument values based on the processed function.
--- tools/testing/selftests/bpf/Makefile | 4 ++- .../selftests/bpf/prog_tests/tracing_multi.c | 28 +++++++++++++++++++ .../selftests/bpf/progs/tracing_multi_check.c | 26 +++++++++++++++++ .../selftests/bpf/progs/tracing_multi_fexit.c | 20 +++++++++++++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_fexit.c
tools/testing/selftests/bpf/Makefile +3 -1
@@ -348,7 +348,8 @@ SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \ linked_vars.skel.h linked_maps.skel.h \ test_subskeleton.skel.h test_subskeleton_lib.skel.h \ - test_usdt.skel.h tracing_multi_fentry_test.skel.h + test_usdt.skel.h tracing_multi_fentry_test.skel.h \ + tracing_multi_fexit_test.skel.h LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \ test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
@@ -367,6 +368,7 @@ test_subskeleton.skel.h-deps := test_subskeleton_lib2.o test_subskeleton_lib.o t test_subskeleton_lib.skel.h-deps := test_subskeleton_lib2.o test_subskeleton_lib.o test_usdt.skel.h-deps := test_usdt.o test_usdt_multispec.o tracing_multi_fentry_test.skel.h-deps := tracing_multi_fentry.o tracing_multi_check.o +tracing_multi_fexit_test.skel.h-deps := tracing_multi_fexit.o tracing_multi_check.o LINKED_BPF_SRCS := $(patsubst %.o,%.c,$(foreach skel,$(LINKED_SKELS),$($(skel)-deps)))
tools/testing/selftests/bpf/prog_tests/tracing_multi.c +28 -0
@@ -1,6 +1,7 @@// SPDX-License-Identifier: GPL-2.0 #include <test_progs.h> #include "tracing_multi_fentry_test.skel.h" +#include "tracing_multi_fexit_test.skel.h" #include "trace_helpers.h" static void multi_fentry_test (void )
@@ -27,8 +28,35 @@ static void multi_fentry_test(void)tracing_multi_fentry_test__destroy (skel); } +static void multi_fexit_test (void ) +{ +LIBBPF_OPTS (bpf_test_run_opts, topts); +struct tracing_multi_fexit_test *skel =NULL ; +int err, prog_fd; + + skel =tracing_multi_fexit_test__open_and_load (); +if (!ASSERT_OK_PTR (skel,"tracing_multi_fentry_test__open_and_load" )) + goto cleanup; + + err =tracing_multi_fexit_test__attach (skel); +if (!ASSERT_OK (err,"tracing_multi_fentry_test__attach" )) + goto cleanup; + + prog_fd =bpf_program__fd (skel->progs.test); + err =bpf_prog_test_run_opts (prog_fd, &topts); +ASSERT_OK (err,"test_run" ); + +ASSERT_EQ (skel->bss->test_arg_result,8 ,"test_arg_result" ); +ASSERT_EQ (skel->bss->test_ret_result,8 ,"test_ret_result" ); + +cleanup: +tracing_multi_fexit_test__destroy (skel); +} +void test_tracing_multi_test (void ) {if (test__start_subtest ("fentry" ))multi_fentry_test (); +if (test__start_subtest ("fexit" )) +multi_fexit_test (); }
tools/testing/selftests/bpf/progs/tracing_multi_check.c +26 -0
@@ -130,3 +130,29 @@ void multi_arg_check(__u64 *ctx, __u64 *test_result) *test_result +=1 ; } } + +void multi_ret_check (void *ctx,__u64 *test_result) +{ +void *ip = (void *)bpf_get_func_ip (ctx); +__u64 ret =0 ; +int err; + +if (bpf_get_func_ret (ctx, &ret)) +return ; +if (ip == &bpf_fentry_test1) + *test_result += ret ==2 ; +else if (ip == &bpf_fentry_test2) + *test_result += ret ==5 ; +else if (ip == &bpf_fentry_test3) + *test_result += ret ==15 ; +else if (ip == &bpf_fentry_test4) + *test_result += ret ==34 ; +else if (ip == &bpf_fentry_test5) + *test_result += ret ==65 ; +else if (ip == &bpf_fentry_test6) + *test_result += ret ==111 ; +else if (ip == &bpf_fentry_test7) + *test_result += ret ==0 ; +else if (ip == &bpf_fentry_test8) + *test_result += ret ==0 ; +}
tools/testing/selftests/bpf/progs/tracing_multi_fexit.c new file +20 -0
@@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +char _license[] SEC("license") = "GPL"; + +__hidden extern void multi_arg_check(__u64 *ctx, __u64 *test_result); +__hidden extern void multi_ret_check(void *ctx, __u64 *test_result); + +__u64 test_arg_result =0 ; +__u64 test_ret_result =0 ; + +SEC("fexit.multi/bpf_fentry_test*") +int BPF_PROG (test ) +{ +multi_arg_check (ctx, &test_arg_result); +multi_ret_check (ctx, &test_ret_result); +return 0 ; +}
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 16/17 selftests/bpf: Add fentry/fexit tracing multi func test
In reply to Jiri OlsaAdding selftest for fentry/fexit multi func tests that attaches to bpf_fentry_test* functions and checks argument values based on the processed function.
--- tools/testing/selftests/bpf/Makefile | 4 ++- .../selftests/bpf/prog_tests/tracing_multi.c | 29 +++++++++++++++++++ .../bpf/progs/tracing_multi_fentry_fexit.c | 28 ++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_fentry_fexit.c
tools/testing/selftests/bpf/Makefile +3 -1
@@ -349,7 +349,8 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \ linked_vars.skel.h linked_maps.skel.h \ test_subskeleton.skel.h test_subskeleton_lib.skel.h \ test_usdt.skel.h tracing_multi_fentry_test.skel.h \ - tracing_multi_fexit_test.skel.h + tracing_multi_fexit_test.skel.h \ + tracing_multi_fentry_fexit_test.skel.h LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \ test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
@@ -369,6 +370,7 @@ test_subskeleton_lib.skel.h-deps := test_subskeleton_lib2.o test_subskeleton_lib test_usdt.skel.h-deps := test_usdt.o test_usdt_multispec.o tracing_multi_fentry_test.skel.h-deps := tracing_multi_fentry.o tracing_multi_check.o tracing_multi_fexit_test.skel.h-deps := tracing_multi_fexit.o tracing_multi_check.o +tracing_multi_fentry_fexit_test.skel.h-deps := tracing_multi_fentry_fexit.o tracing_multi_check.o LINKED_BPF_SRCS := $(patsubst %.o,%.c,$(foreach skel,$(LINKED_SKELS),$($(skel)-deps)))
tools/testing/selftests/bpf/prog_tests/tracing_multi.c +29 -0
@@ -2,6 +2,7 @@#include <test_progs.h> #include "tracing_multi_fentry_test.skel.h" #include "tracing_multi_fexit_test.skel.h" +#include "tracing_multi_fentry_fexit_test.skel.h" #include "trace_helpers.h" static void multi_fentry_test (void )
@@ -53,10 +54,38 @@ static void multi_fexit_test(void)tracing_multi_fexit_test__destroy (skel); } +static void multi_fentry_fexit_test (void ) +{ +LIBBPF_OPTS (bpf_test_run_opts, topts); +struct tracing_multi_fentry_fexit_test *skel =NULL ; +int err, prog_fd; + + skel =tracing_multi_fentry_fexit_test__open_and_load (); +if (!ASSERT_OK_PTR (skel,"tracing_multi_fentry_fexit_test__open_and_load" )) + goto cleanup; + + err =tracing_multi_fentry_fexit_test__attach (skel); +if (!ASSERT_OK (err,"tracing_multi_fentry_fexit_test__attach" )) + goto cleanup; + + prog_fd =bpf_program__fd (skel->progs.test2); + err =bpf_prog_test_run_opts (prog_fd, &topts); +ASSERT_OK (err,"test_run" ); + +ASSERT_EQ (skel->bss->test1_arg_result,8 ,"test1_arg_result" ); +ASSERT_EQ (skel->bss->test2_arg_result,8 ,"test2_arg_result" ); +ASSERT_EQ (skel->bss->test2_ret_result,8 ,"test2_ret_result" ); + +cleanup: +tracing_multi_fentry_fexit_test__destroy (skel); +} +void test_tracing_multi_test (void ) {if (test__start_subtest ("fentry" ))multi_fentry_test ();if (test__start_subtest ("fexit" ))multi_fexit_test (); +if (test__start_subtest ("fentry_fexit" )) +multi_fentry_fexit_test (); }
tools/testing/selftests/bpf/progs/tracing_multi_fentry_fexit.c new file +28 -0
@@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +char _license[] SEC("license") = "GPL"; + +__u64 test1_arg_result =0 ; +__u64 test2_arg_result =0 ; +__u64 test2_ret_result =0 ; + +__hidden extern void multi_arg_check(__u64 *ctx, __u64 *test_result); +__hidden extern void multi_ret_check(void *ctx, __u64 *test_result); + +SEC("fentry.multi/bpf_fentry_test*") +int BPF_PROG (test1 ,__u64 a,__u64 b,__u64 c,__u64 d,__u64 e,__u64 f) +{ +multi_arg_check (ctx, &test1_arg_result); +return 0 ; +} + +SEC("fexit.multi/bpf_fentry_test*") +int BPF_PROG (test2 ,__u64 a,__u64 b,__u64 c,__u64 d,__u64 e,__u64 f,int ret) +{ +multi_arg_check (ctx, &test2_arg_result); +multi_ret_check (ctx, &test2_ret_result); +return 0 ; +}
Signature · 1 line
2.37.1
Jiri Olsa
reviewerRFC PATCH 17/17 selftests/bpf: Add mixed tracing multi func test
In reply to Jiri OlsaAdding selftest for fentry/fexit multi func tests that attaches to bpf_fentry_test* functions, where some of them have already attached single trampoline.
--- tools/testing/selftests/bpf/Makefile | 4 +- .../selftests/bpf/prog_tests/tracing_multi.c | 67 +++++++++++++++++++ .../selftests/bpf/progs/tracing_multi_mixed.c | 43 ++++++++++++ 3 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/tracing_multi_mixed.c
tools/testing/selftests/bpf/Makefile +3 -1
@@ -350,7 +350,8 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \ test_subskeleton.skel.h test_subskeleton_lib.skel.h \ test_usdt.skel.h tracing_multi_fentry_test.skel.h \ tracing_multi_fexit_test.skel.h \ - tracing_multi_fentry_fexit_test.skel.h + tracing_multi_fentry_fexit_test.skel.h \ + tracing_multi_mixed_test.skel.h LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \ test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
@@ -371,6 +372,7 @@ test_usdt.skel.h-deps := test_usdt.o test_usdt_multispec.o tracing_multi_fentry_test.skel.h-deps := tracing_multi_fentry.o tracing_multi_check.o tracing_multi_fexit_test.skel.h-deps := tracing_multi_fexit.o tracing_multi_check.o tracing_multi_fentry_fexit_test.skel.h-deps := tracing_multi_fentry_fexit.o tracing_multi_check.o +tracing_multi_mixed_test.skel.h-deps := tracing_multi_mixed.o tracing_multi_check.o LINKED_BPF_SRCS := $(patsubst %.o,%.c,$(foreach skel,$(LINKED_SKELS),$($(skel)-deps)))
tools/testing/selftests/bpf/prog_tests/tracing_multi.c +67 -0
83 lines not on this page · raw message
tools/testing/selftests/bpf/progs/tracing_multi_mixed.c new file +43 -0
43 lines not on this page · raw message
Signature · 1 line
2.37.1
Song Liu
maintainerQuoted 1 line · Jiri Olsa
[...]
Quoted 11 lines
Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attach- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,F
IIUC, we have 4 trampolines (1 multi, 3 singles) at this moment?
Quoted 5 lines
- another program can be attached to A,B,C,D,E,F multi trampoline
- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
I guess the limitation here is, multi trampolines cannot be splitted after attached. While multi trampoline is motivated by short term use cases like retsnoop, it is allowed to run them for extended periods of time. Then, this limitation might be a real issue in production.
Thanks,
Song
Quoted 3 lines
- D,E,F functions are still attachable by any new trampoline
Quoted 9 lines · Song Liu
Quoted 1 line · Jiri Olsa
[...]
Quoted 6 lines
Maybe better explained on following example:
- you want to attach program P to functions A,B,C,D,E,F via
bpf_trampoline_multi_attach
Quoted 7 lines
Quoted 5 lines
- D,E,F already have standard trampoline attached
- the
bpf_trampoline_multi_attachwill create new 'multi' trampoline which spans over A,B,C functions and attach program P to single trampolines D,E,FIIUC, we have 4 trampolines (1 multi, 3 singles) at this moment?
yes
Quoted 11 lines
Quoted 6 lines
- another program can be attached to A,B,C,D,E,F multi trampoline
- A,B,C functions are now 'not attachable' by any trampoline until the above 'multi' trampoline is released
I guess the limitation here is, multi trampolines cannot be splitted after attached. While multi trampoline is motivated by short term use cases like retsnoop, it is allowed to run them for extended periods of time. Then, this limitation might be a real issue in production.
I think it'd be possible to allow adding single trampoline on top of multi trampoline by removing that single id from it and creating single trampoline for that
I also thought of possibility to convert intersection IDs of two multi trampolines into single trampolines.. but that might get slow if the common set is too big
the difficulty for the common solution was that if you allow to split trampolines then bpf link can't carry pointers to trampolines because they can be split into multiple new trampolines, so link would need to store just array of IDs and use it to attach a program
then un/linking program and doing intersections with stored trampolines and handling all the split cases turned out to be nightmare
but perhaps there's another solution
jirka