Message ID | 20240914154040.276933-1-chen.dylane@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | [bpf-next,v3] libbpf: Fix expected_attach_type set when kernel not support | expand |
在 2024/9/14 23:40, Tao Chen 写道: > The commit "5902da6d8a52" set expected_attach_type again with > field of bpf_program after libpf_prepare_prog_load, which makes > expected_attach_type = 0 no sense when kenrel not support the > attach_type feature, so fix it. > > Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to bpf_program__attach_usdt") > Suggested-by: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Tao Chen <chen.dylane@gmail.com> > --- > tools/lib/bpf/libbpf.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > Change list: > - v2 -> v3: > - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by > Andrri > - v1 -> v2: > - restore the original initialization way suggested by Jiri > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 219facd0e66e..a78e24ff354b 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog, > opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; > > /* special check for usdt to use uprobe_multi link */ > - if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) > + if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) { > + /* for BPF_TRACE_KPROBE_MULTI, user might want to query exected_attach_type > + * in prog, and expected_attach_type we set in kenrel is from opts, so we > + * update both. > + */ > prog->expected_attach_type = BPF_TRACE_UPROBE_MULTI; > + opts->expected_attach_type = BPF_TRACE_UPROBE_MULTI; > + } > > if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) { > int btf_obj_fd = 0, btf_type_id = 0, err; > @@ -7443,6 +7449,7 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog > load_attr.attach_btf_id = prog->attach_btf_id; > load_attr.kern_version = kern_version; > load_attr.prog_ifindex = prog->prog_ifindex; > + load_attr.expected_attach_type = prog->expected_attach_type; > > /* specify func_info/line_info only if kernel supports them */ > if (obj->btf && btf__fd(obj->btf) >= 0 && kernel_supports(obj, FEAT_BTF_FUNC)) { > @@ -7474,9 +7481,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog > insns_cnt = prog->insns_cnt; > } > > - /* allow prog_prepare_load_fn to change expected_attach_type */ > - load_attr.expected_attach_type = prog->expected_attach_type; > - > if (obj->gen_loader) { > bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name, > license, insns, insns_cnt, &load_attr, Hi, guys, please review this patch again, the previous versions: v1: https://lore.kernel.org/bpf/20240913121627.153898-1-chen.dylane@gmail.com/ v2: https://lore.kernel.org/bpf/20240913164355.176021-1-chen.dylane@gmail.com/
On Thu, Sep 19, 2024 at 4:05 AM Tao Chen <chen.dylane@gmail.com> wrote: > > 在 2024/9/14 23:40, Tao Chen 写道: > > The commit "5902da6d8a52" set expected_attach_type again with > > field of bpf_program after libpf_prepare_prog_load, which makes > > expected_attach_type = 0 no sense when kenrel not support the > > attach_type feature, so fix it. > > > > Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to bpf_program__attach_usdt") > > Suggested-by: Jiri Olsa <jolsa@kernel.org> > > Signed-off-by: Tao Chen <chen.dylane@gmail.com> > > --- > > tools/lib/bpf/libbpf.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > Change list: > > - v2 -> v3: > > - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by > > Andrri > > - v1 -> v2: > > - restore the original initialization way suggested by Jiri > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 219facd0e66e..a78e24ff354b 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog, > > opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; > > > > /* special check for usdt to use uprobe_multi link */ > > - if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) > > + if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) { > > + /* for BPF_TRACE_KPROBE_MULTI, user might want to query exected_attach_type > > + * in prog, and expected_attach_type we set in kenrel is from opts, so we > > + * update both. > > + */ > > prog->expected_attach_type = BPF_TRACE_UPROBE_MULTI; > > + opts->expected_attach_type = BPF_TRACE_UPROBE_MULTI; > > + } > > > > if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) { > > int btf_obj_fd = 0, btf_type_id = 0, err; > > @@ -7443,6 +7449,7 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog > > load_attr.attach_btf_id = prog->attach_btf_id; > > load_attr.kern_version = kern_version; > > load_attr.prog_ifindex = prog->prog_ifindex; > > + load_attr.expected_attach_type = prog->expected_attach_type; > > > > /* specify func_info/line_info only if kernel supports them */ > > if (obj->btf && btf__fd(obj->btf) >= 0 && kernel_supports(obj, FEAT_BTF_FUNC)) { > > @@ -7474,9 +7481,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog > > insns_cnt = prog->insns_cnt; > > } > > > > - /* allow prog_prepare_load_fn to change expected_attach_type */ > > - load_attr.expected_attach_type = prog->expected_attach_type; > > - > > if (obj->gen_loader) { > > bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name, > > license, insns, insns_cnt, &load_attr, > > Hi, guys, please review this patch again, the previous versions: It looks good, but bpf-next is closed right now due to merge window. I'll apply when the tree is open again. > v1: > https://lore.kernel.org/bpf/20240913121627.153898-1-chen.dylane@gmail.com/ > v2: > https://lore.kernel.org/bpf/20240913164355.176021-1-chen.dylane@gmail.com/ > > -- > Best Regards > Dylane Chen
在 2024/9/19 12:20, Andrii Nakryiko 写道: > On Thu, Sep 19, 2024 at 4:05 AM Tao Chen <chen.dylane@gmail.com> wrote: >> >> 在 2024/9/14 23:40, Tao Chen 写道: >>> The commit "5902da6d8a52" set expected_attach_type again with >>> field of bpf_program after libpf_prepare_prog_load, which makes >>> expected_attach_type = 0 no sense when kenrel not support the >>> attach_type feature, so fix it. >>> >>> Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to bpf_program__attach_usdt") >>> Suggested-by: Jiri Olsa <jolsa@kernel.org> >>> Signed-off-by: Tao Chen <chen.dylane@gmail.com> >>> --- >>> tools/lib/bpf/libbpf.c | 12 ++++++++---- >>> 1 file changed, 8 insertions(+), 4 deletions(-) >>> >>> Change list: >>> - v2 -> v3: >>> - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by >>> Andrri >>> - v1 -> v2: >>> - restore the original initialization way suggested by Jiri >>> >>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >>> index 219facd0e66e..a78e24ff354b 100644 >>> --- a/tools/lib/bpf/libbpf.c >>> +++ b/tools/lib/bpf/libbpf.c >>> @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog, >>> opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; >>> >>> /* special check for usdt to use uprobe_multi link */ >>> - if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) >>> + if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) { >>> + /* for BPF_TRACE_KPROBE_MULTI, user might want to query exected_attach_type >>> + * in prog, and expected_attach_type we set in kenrel is from opts, so we >>> + * update both. >>> + */ >>> prog->expected_attach_type = BPF_TRACE_UPROBE_MULTI; >>> + opts->expected_attach_type = BPF_TRACE_UPROBE_MULTI; >>> + } >>> >>> if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) { >>> int btf_obj_fd = 0, btf_type_id = 0, err; >>> @@ -7443,6 +7449,7 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog >>> load_attr.attach_btf_id = prog->attach_btf_id; >>> load_attr.kern_version = kern_version; >>> load_attr.prog_ifindex = prog->prog_ifindex; >>> + load_attr.expected_attach_type = prog->expected_attach_type; >>> >>> /* specify func_info/line_info only if kernel supports them */ >>> if (obj->btf && btf__fd(obj->btf) >= 0 && kernel_supports(obj, FEAT_BTF_FUNC)) { >>> @@ -7474,9 +7481,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog >>> insns_cnt = prog->insns_cnt; >>> } >>> >>> - /* allow prog_prepare_load_fn to change expected_attach_type */ >>> - load_attr.expected_attach_type = prog->expected_attach_type; >>> - >>> if (obj->gen_loader) { >>> bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name, >>> license, insns, insns_cnt, &load_attr, >> >> Hi, guys, please review this patch again, the previous versions: > > It looks good, but bpf-next is closed right now due to merge window. > I'll apply when the tree is open again. Hi,Andrii, thank you for your response! I’ll wait for the next window. > >> v1: >> https://lore.kernel.org/bpf/20240913121627.153898-1-chen.dylane@gmail.com/ >> v2: >> https://lore.kernel.org/bpf/20240913164355.176021-1-chen.dylane@gmail.com/ >> >> -- >> Best Regards >> Dylane Chen
On Sat, Sep 14, 2024 at 11:40:40PM +0800, Tao Chen wrote: > The commit "5902da6d8a52" set expected_attach_type again with > field of bpf_program after libpf_prepare_prog_load, which makes > expected_attach_type = 0 no sense when kenrel not support the > attach_type feature, so fix it. > > Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to bpf_program__attach_usdt") > Suggested-by: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Tao Chen <chen.dylane@gmail.com> > --- > tools/lib/bpf/libbpf.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > Change list: > - v2 -> v3: > - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by > Andrri > - v1 -> v2: > - restore the original initialization way suggested by Jiri > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 219facd0e66e..a78e24ff354b 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog, > opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; > > /* special check for usdt to use uprobe_multi link */ > - if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) > + if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) { > + /* for BPF_TRACE_KPROBE_MULTI, user might want to query exected_attach_type > + * in prog, and expected_attach_type we set in kenrel is from opts, so we > + * update both. > + */ s/K/U/ in BPF_TRACE_KPROBE_MULTI in above comment and 'kenrel' typo Acked-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > prog->expected_attach_type = BPF_TRACE_UPROBE_MULTI; > + opts->expected_attach_type = BPF_TRACE_UPROBE_MULTI; > + } > > if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) { > int btf_obj_fd = 0, btf_type_id = 0, err; > @@ -7443,6 +7449,7 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog > load_attr.attach_btf_id = prog->attach_btf_id; > load_attr.kern_version = kern_version; > load_attr.prog_ifindex = prog->prog_ifindex; > + load_attr.expected_attach_type = prog->expected_attach_type; > > /* specify func_info/line_info only if kernel supports them */ > if (obj->btf && btf__fd(obj->btf) >= 0 && kernel_supports(obj, FEAT_BTF_FUNC)) { > @@ -7474,9 +7481,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog > insns_cnt = prog->insns_cnt; > } > > - /* allow prog_prepare_load_fn to change expected_attach_type */ > - load_attr.expected_attach_type = prog->expected_attach_type; > - > if (obj->gen_loader) { > bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name, > license, insns, insns_cnt, &load_attr, > -- > 2.25.1 >
On 9/25/24 12:28 PM, Jiri Olsa wrote: > On Sat, Sep 14, 2024 at 11:40:40PM +0800, Tao Chen wrote: >> The commit "5902da6d8a52" set expected_attach_type again with >> field of bpf_program after libpf_prepare_prog_load, which makes >> expected_attach_type = 0 no sense when kenrel not support the >> attach_type feature, so fix it. >> >> Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to bpf_program__attach_usdt") >> Suggested-by: Jiri Olsa <jolsa@kernel.org> >> Signed-off-by: Tao Chen <chen.dylane@gmail.com> >> --- >> tools/lib/bpf/libbpf.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> Change list: >> - v2 -> v3: >> - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by >> Andrri >> - v1 -> v2: >> - restore the original initialization way suggested by Jiri >> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >> index 219facd0e66e..a78e24ff354b 100644 >> --- a/tools/lib/bpf/libbpf.c >> +++ b/tools/lib/bpf/libbpf.c >> @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog, >> opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; >> >> /* special check for usdt to use uprobe_multi link */ >> - if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) >> + if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) { >> + /* for BPF_TRACE_KPROBE_MULTI, user might want to query exected_attach_type >> + * in prog, and expected_attach_type we set in kenrel is from opts, so we >> + * update both. >> + */ > s/K/U/ in BPF_TRACE_KPROBE_MULTI in above comment and 'kenrel' typo > > Acked-by: Jiri Olsa <jolsa@kernel.org> Same typo is also in commit desc, would be good to improve the commit desc a bit if you spin v4 anyway. Thanks!
在 2024/9/25 18:36, Daniel Borkmann 写道: > On 9/25/24 12:28 PM, Jiri Olsa wrote: >> On Sat, Sep 14, 2024 at 11:40:40PM +0800, Tao Chen wrote: >>> The commit "5902da6d8a52" set expected_attach_type again with >>> field of bpf_program after libpf_prepare_prog_load, which makes >>> expected_attach_type = 0 no sense when kenrel not support the >>> attach_type feature, so fix it. >>> >>> Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to >>> bpf_program__attach_usdt") >>> Suggested-by: Jiri Olsa <jolsa@kernel.org> >>> Signed-off-by: Tao Chen <chen.dylane@gmail.com> >>> --- >>> tools/lib/bpf/libbpf.c | 12 ++++++++---- >>> 1 file changed, 8 insertions(+), 4 deletions(-) >>> >>> Change list: >>> - v2 -> v3: >>> - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by >>> Andrri >>> - v1 -> v2: >>> - restore the original initialization way suggested by Jiri >>> >>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >>> index 219facd0e66e..a78e24ff354b 100644 >>> --- a/tools/lib/bpf/libbpf.c >>> +++ b/tools/lib/bpf/libbpf.c >>> @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct >>> bpf_program *prog, >>> opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; >>> /* special check for usdt to use uprobe_multi link */ >>> - if ((def & SEC_USDT) && kernel_supports(prog->obj, >>> FEAT_UPROBE_MULTI_LINK)) >>> + if ((def & SEC_USDT) && kernel_supports(prog->obj, >>> FEAT_UPROBE_MULTI_LINK)) { >>> + /* for BPF_TRACE_KPROBE_MULTI, user might want to query >>> exected_attach_type >>> + * in prog, and expected_attach_type we set in kenrel is >>> from opts, so we >>> + * update both. >>> + */ >> s/K/U/ in BPF_TRACE_KPROBE_MULTI in above comment and 'kenrel' typo >> >> Acked-by: Jiri Olsa <jolsa@kernel.org> > Same typo is also in commit desc, would be good to improve the commit > desc a bit if you spin v4 anyway. Thanks! Hi,Dnaiel,my bad,i will fix it in v4
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 219facd0e66e..a78e24ff354b 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -7352,8 +7352,14 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog, opts->prog_flags |= BPF_F_XDP_HAS_FRAGS; /* special check for usdt to use uprobe_multi link */ - if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) + if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) { + /* for BPF_TRACE_KPROBE_MULTI, user might want to query exected_attach_type + * in prog, and expected_attach_type we set in kenrel is from opts, so we + * update both. + */ prog->expected_attach_type = BPF_TRACE_UPROBE_MULTI; + opts->expected_attach_type = BPF_TRACE_UPROBE_MULTI; + } if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) { int btf_obj_fd = 0, btf_type_id = 0, err; @@ -7443,6 +7449,7 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog load_attr.attach_btf_id = prog->attach_btf_id; load_attr.kern_version = kern_version; load_attr.prog_ifindex = prog->prog_ifindex; + load_attr.expected_attach_type = prog->expected_attach_type; /* specify func_info/line_info only if kernel supports them */ if (obj->btf && btf__fd(obj->btf) >= 0 && kernel_supports(obj, FEAT_BTF_FUNC)) { @@ -7474,9 +7481,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog insns_cnt = prog->insns_cnt; } - /* allow prog_prepare_load_fn to change expected_attach_type */ - load_attr.expected_attach_type = prog->expected_attach_type; - if (obj->gen_loader) { bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name, license, insns, insns_cnt, &load_attr,
The commit "5902da6d8a52" set expected_attach_type again with field of bpf_program after libpf_prepare_prog_load, which makes expected_attach_type = 0 no sense when kenrel not support the attach_type feature, so fix it. Fixes: 5902da6d8a52 ("libbpf: Add uprobe multi link support to bpf_program__attach_usdt") Suggested-by: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Tao Chen <chen.dylane@gmail.com> --- tools/lib/bpf/libbpf.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) Change list: - v2 -> v3: - update BPF_TRACE_UPROBE_MULTI both in prog and opts suggedted by Andrri - v1 -> v2: - restore the original initialization way suggested by Jiri