BPF Archive on lore.kernel.org
 help / color / Atom feed
* Re: Need a way to modify the section name for a read program object
       [not found] ` <874kw664dy.fsf@toke.dk>
@ 2020-02-04 19:00   ` Andrii Nakryiko
  2020-02-04 19:26     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2020-02-04 19:00 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Eelco Chaudron, xdp-newbies, bpf

On 2/4/20 2:19 AM, Toke Høiland-Jørgensen wrote:
> "Eelco Chaudron" <echaudro@redhat.com> writes:
> 
>> Hi All,
>>
>> I'm trying to write an xdpdump like utility and have some missing part
>> in libbpf to change the fentry/FUNCTION section name before loading the
>> trace program.
>>
>> In short, I have an eBPF program that has a section name like
>> "fentry/FUNCTION" where FUNCTION needs to be replaced by the name of the
>> XDP program loaded in the interfaces its start function.
>>
>> The code for loading the ftrace part is something like:
>>
>> 	open_opts.attach_prog_fd = bpf_prog_get_fd_by_id(info.id);
>> 	trace_obj = bpf_object__open_file("xdpdump_bpf.o", &open_opts);
>>
>> 	trace_prog_fentry = bpf_object__find_program_by_title(trace_obj,
>> "fentry/FUNCTION");
>>
>> 	/* Here I need to replace the trace_prog_fentry->section_name =
>> "fentry/<INTERFACE PROG NAME> */
>>
>> 	bpf_object__load(trace_obj);
>> 	trace_link_fentry = bpf_program__attach_trace(trace_prog_fentry);
>>
>>
>> See the above, I would like to change the section_name but there is no
>> API to do this, and of course, the struct bpf_program is
>> implementation-specific.
>>
>> Any idea how I would work around this, or what extension to libbpf can
>> be suggested to support this?
> 
> I think what's missing is a way for the caller to set the attach_btf_id.
> Currently, libbpf always tries to discover this based on the section
> name (see libbpf_find_attach_btf_id()). I think the right way to let the
> caller specify this is not to change the section name, though, but just
> to expose a way to explicitly set the btf_id (which the caller can then
> go find on its own).

Yes, I agree, section name should be treated as an immutable identifier 
and a (overrideable) hint to libbpf.

> 
> Not sure if it would be better with a new open_opt (to mirror
> attach_prog_fd), or just a setter (bpf_program__set_attach_btf_id()?).
> Or maybe both? Andrii, WDYT?

open_opts is definitely wrong way to do this, because open_opts apply to 
all BPF programs, while this should be per-program. I'm also not sure 
having API that allows to specify BTF type ID is the best, probably 
better to let libbpf perform the search by name. So I'd say something 
like this:

int bpf_program__set_attach_target(int attach_prog_fd, const char 
*attach_func_name)

This should handle customizing all the tp_btf/fentry/fexit/freplace BPF 
programs we have. We might add extra attach_target_ops for future 
extensibility, if we anticipate that we'll need more knobs in the 
future, I haven't thought too much about that.

> 
> -Toke
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Need a way to modify the section name for a read program object
  2020-02-04 19:00   ` Need a way to modify the section name for a read program object Andrii Nakryiko
@ 2020-02-04 19:26     ` Toke Høiland-Jørgensen
  2020-02-04 19:32       ` Andrii Nakryiko
  0 siblings, 1 reply; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-02-04 19:26 UTC (permalink / raw)
  To: Andrii Nakryiko, Eelco Chaudron, xdp-newbies, bpf\

Andrii Nakryiko <andriin@fb.com> writes:

> On 2/4/20 2:19 AM, Toke Høiland-Jørgensen wrote:
>> "Eelco Chaudron" <echaudro@redhat.com> writes:
>> 
>>> Hi All,
>>>
>>> I'm trying to write an xdpdump like utility and have some missing part
>>> in libbpf to change the fentry/FUNCTION section name before loading the
>>> trace program.
>>>
>>> In short, I have an eBPF program that has a section name like
>>> "fentry/FUNCTION" where FUNCTION needs to be replaced by the name of the
>>> XDP program loaded in the interfaces its start function.
>>>
>>> The code for loading the ftrace part is something like:
>>>
>>> 	open_opts.attach_prog_fd = bpf_prog_get_fd_by_id(info.id);
>>> 	trace_obj = bpf_object__open_file("xdpdump_bpf.o", &open_opts);
>>>
>>> 	trace_prog_fentry = bpf_object__find_program_by_title(trace_obj,
>>> "fentry/FUNCTION");
>>>
>>> 	/* Here I need to replace the trace_prog_fentry->section_name =
>>> "fentry/<INTERFACE PROG NAME> */
>>>
>>> 	bpf_object__load(trace_obj);
>>> 	trace_link_fentry = bpf_program__attach_trace(trace_prog_fentry);
>>>
>>>
>>> See the above, I would like to change the section_name but there is no
>>> API to do this, and of course, the struct bpf_program is
>>> implementation-specific.
>>>
>>> Any idea how I would work around this, or what extension to libbpf can
>>> be suggested to support this?
>> 
>> I think what's missing is a way for the caller to set the attach_btf_id.
>> Currently, libbpf always tries to discover this based on the section
>> name (see libbpf_find_attach_btf_id()). I think the right way to let the
>> caller specify this is not to change the section name, though, but just
>> to expose a way to explicitly set the btf_id (which the caller can then
>> go find on its own).
>
> Yes, I agree, section name should be treated as an immutable identifier 
> and a (overrideable) hint to libbpf.
>
>> 
>> Not sure if it would be better with a new open_opt (to mirror
>> attach_prog_fd), or just a setter (bpf_program__set_attach_btf_id()?).
>> Or maybe both? Andrii, WDYT?
>
> open_opts is definitely wrong way to do this, because open_opts apply to 
> all BPF programs, while this should be per-program.

Yes, of course; silly me :)

> I'm also not sure having API that allows to specify BTF type ID is the
> best, probably better to let libbpf perform the search by name. So I'd
> say something like this:
>
> int bpf_program__set_attach_target(int attach_prog_fd, const char 
> *attach_func_name)
>
> This should handle customizing all the tp_btf/fentry/fexit/freplace BPF 
> programs we have.

Right, that makes sense; I think that would cover it (apart from your
function signature missing a struct bpf_program argument).

> We might add extra attach_target_ops for future extensibility, if we
> anticipate that we'll need more knobs in the future, I haven't thought
> too much about that.

Good question, me neither. Will see if I can think of anything...

-Toke


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Need a way to modify the section name for a read program object
  2020-02-04 19:26     ` Toke Høiland-Jørgensen
@ 2020-02-04 19:32       ` Andrii Nakryiko
  2020-02-11 14:37         ` Eelco Chaudron
  0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2020-02-04 19:32 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Andrii Nakryiko, Eelco Chaudron, Xdp, bpf

On Tue, Feb 4, 2020 at 11:27 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Andrii Nakryiko <andriin@fb.com> writes:
>
> > On 2/4/20 2:19 AM, Toke Høiland-Jørgensen wrote:
> >> "Eelco Chaudron" <echaudro@redhat.com> writes:
> >>
> >>> Hi All,
> >>>
> >>> I'm trying to write an xdpdump like utility and have some missing part
> >>> in libbpf to change the fentry/FUNCTION section name before loading the
> >>> trace program.
> >>>
> >>> In short, I have an eBPF program that has a section name like
> >>> "fentry/FUNCTION" where FUNCTION needs to be replaced by the name of the
> >>> XDP program loaded in the interfaces its start function.
> >>>
> >>> The code for loading the ftrace part is something like:
> >>>
> >>>     open_opts.attach_prog_fd = bpf_prog_get_fd_by_id(info.id);
> >>>     trace_obj = bpf_object__open_file("xdpdump_bpf.o", &open_opts);
> >>>
> >>>     trace_prog_fentry = bpf_object__find_program_by_title(trace_obj,
> >>> "fentry/FUNCTION");
> >>>
> >>>     /* Here I need to replace the trace_prog_fentry->section_name =
> >>> "fentry/<INTERFACE PROG NAME> */
> >>>
> >>>     bpf_object__load(trace_obj);
> >>>     trace_link_fentry = bpf_program__attach_trace(trace_prog_fentry);
> >>>
> >>>
> >>> See the above, I would like to change the section_name but there is no
> >>> API to do this, and of course, the struct bpf_program is
> >>> implementation-specific.
> >>>
> >>> Any idea how I would work around this, or what extension to libbpf can
> >>> be suggested to support this?
> >>
> >> I think what's missing is a way for the caller to set the attach_btf_id.
> >> Currently, libbpf always tries to discover this based on the section
> >> name (see libbpf_find_attach_btf_id()). I think the right way to let the
> >> caller specify this is not to change the section name, though, but just
> >> to expose a way to explicitly set the btf_id (which the caller can then
> >> go find on its own).
> >
> > Yes, I agree, section name should be treated as an immutable identifier
> > and a (overrideable) hint to libbpf.
> >
> >>
> >> Not sure if it would be better with a new open_opt (to mirror
> >> attach_prog_fd), or just a setter (bpf_program__set_attach_btf_id()?).
> >> Or maybe both? Andrii, WDYT?
> >
> > open_opts is definitely wrong way to do this, because open_opts apply to
> > all BPF programs, while this should be per-program.
>
> Yes, of course; silly me :)
>
> > I'm also not sure having API that allows to specify BTF type ID is the
> > best, probably better to let libbpf perform the search by name. So I'd
> > say something like this:
> >
> > int bpf_program__set_attach_target(int attach_prog_fd, const char
> > *attach_func_name)
> >
> > This should handle customizing all the tp_btf/fentry/fexit/freplace BPF
> > programs we have.
>
> Right, that makes sense; I think that would cover it (apart from your
> function signature missing a struct bpf_program argument).

great! and, ha-ha, too object-oriented thinking ;)

>
> > We might add extra attach_target_ops for future extensibility, if we
> > anticipate that we'll need more knobs in the future, I haven't thought
> > too much about that.
>
> Good question, me neither. Will see if I can think of anything...
>
> -Toke
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Need a way to modify the section name for a read program object
  2020-02-04 19:32       ` Andrii Nakryiko
@ 2020-02-11 14:37         ` Eelco Chaudron
  2020-02-11 14:54           ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 5+ messages in thread
From: Eelco Chaudron @ 2020-02-11 14:37 UTC (permalink / raw)
  To: Andrii Nakryiko, Toke Høiland-Jørgensen
  Cc: Andrii Nakryiko, Xdp, bpf



On 4 Feb 2020, at 20:32, Andrii Nakryiko wrote:

> On Tue, Feb 4, 2020 at 11:27 AM Toke Høiland-Jørgensen 
> <toke@redhat.com> wrote:
>>
>> Andrii Nakryiko <andriin@fb.com> writes:
>>
>>> On 2/4/20 2:19 AM, Toke Høiland-Jørgensen wrote:
>>>> "Eelco Chaudron" <echaudro@redhat.com> writes:
>>>>
>>>>> Hi All,
>>>>>
>>>>> I'm trying to write an xdpdump like utility and have some missing 
>>>>> part
>>>>> in libbpf to change the fentry/FUNCTION section name before 
>>>>> loading the
>>>>> trace program.
>>>>>
>>>>> In short, I have an eBPF program that has a section name like
>>>>> "fentry/FUNCTION" where FUNCTION needs to be replaced by the name 
>>>>> of the
>>>>> XDP program loaded in the interfaces its start function.
>>>>>
>>>>> The code for loading the ftrace part is something like:
>>>>>
>>>>>     open_opts.attach_prog_fd = bpf_prog_get_fd_by_id(info.id);
>>>>>     trace_obj = bpf_object__open_file("xdpdump_bpf.o", 
>>>>> &open_opts);
>>>>>
>>>>>     trace_prog_fentry = 
>>>>> bpf_object__find_program_by_title(trace_obj,
>>>>> "fentry/FUNCTION");
>>>>>
>>>>>     /* Here I need to replace the trace_prog_fentry->section_name 
>>>>> =
>>>>> "fentry/<INTERFACE PROG NAME> */
>>>>>
>>>>>     bpf_object__load(trace_obj);
>>>>>     trace_link_fentry = 
>>>>> bpf_program__attach_trace(trace_prog_fentry);
>>>>>
>>>>>
>>>>> See the above, I would like to change the section_name but there 
>>>>> is no
>>>>> API to do this, and of course, the struct bpf_program is
>>>>> implementation-specific.
>>>>>
>>>>> Any idea how I would work around this, or what extension to libbpf 
>>>>> can
>>>>> be suggested to support this?
>>>>
>>>> I think what's missing is a way for the caller to set the 
>>>> attach_btf_id.
>>>> Currently, libbpf always tries to discover this based on the 
>>>> section
>>>> name (see libbpf_find_attach_btf_id()). I think the right way to 
>>>> let the
>>>> caller specify this is not to change the section name, though, but 
>>>> just
>>>> to expose a way to explicitly set the btf_id (which the caller can 
>>>> then
>>>> go find on its own).
>>>
>>> Yes, I agree, section name should be treated as an immutable 
>>> identifier
>>> and a (overrideable) hint to libbpf.
>>>
>>>>
>>>> Not sure if it would be better with a new open_opt (to mirror
>>>> attach_prog_fd), or just a setter 
>>>> (bpf_program__set_attach_btf_id()?).
>>>> Or maybe both? Andrii, WDYT?
>>>
>>> open_opts is definitely wrong way to do this, because open_opts 
>>> apply to
>>> all BPF programs, while this should be per-program.
>>
>> Yes, of course; silly me :)
>>
>>> I'm also not sure having API that allows to specify BTF type ID is 
>>> the
>>> best, probably better to let libbpf perform the search by name. So 
>>> I'd
>>> say something like this:
>>>
>>> int bpf_program__set_attach_target(int attach_prog_fd, const char
>>> *attach_func_name)
>>>
>>> This should handle customizing all the tp_btf/fentry/fexit/freplace 
>>> BPF
>>> programs we have.
>>
>> Right, that makes sense; I think that would cover it (apart from your
>> function signature missing a struct bpf_program argument).
>
> great! and, ha-ha, too object-oriented thinking ;)

Thanks for your feedback, assuming you are not working on it, I’ll 
implement/test it and sent out a patch.

//Eelco


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Need a way to modify the section name for a read program object
  2020-02-11 14:37         ` Eelco Chaudron
@ 2020-02-11 14:54           ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-02-11 14:54 UTC (permalink / raw)
  To: Eelco Chaudron, Andrii Nakryiko; +Cc: Andrii Nakryiko, Xdp, bpf

"Eelco Chaudron" <echaudro@redhat.com> writes:

> On 4 Feb 2020, at 20:32, Andrii Nakryiko wrote:
>
>> On Tue, Feb 4, 2020 at 11:27 AM Toke Høiland-Jørgensen 
>> <toke@redhat.com> wrote:
>>>
>>> Andrii Nakryiko <andriin@fb.com> writes:
>>>
>>>> On 2/4/20 2:19 AM, Toke Høiland-Jørgensen wrote:
>>>>> "Eelco Chaudron" <echaudro@redhat.com> writes:
>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> I'm trying to write an xdpdump like utility and have some missing 
>>>>>> part
>>>>>> in libbpf to change the fentry/FUNCTION section name before 
>>>>>> loading the
>>>>>> trace program.
>>>>>>
>>>>>> In short, I have an eBPF program that has a section name like
>>>>>> "fentry/FUNCTION" where FUNCTION needs to be replaced by the name 
>>>>>> of the
>>>>>> XDP program loaded in the interfaces its start function.
>>>>>>
>>>>>> The code for loading the ftrace part is something like:
>>>>>>
>>>>>>     open_opts.attach_prog_fd = bpf_prog_get_fd_by_id(info.id);
>>>>>>     trace_obj = bpf_object__open_file("xdpdump_bpf.o", 
>>>>>> &open_opts);
>>>>>>
>>>>>>     trace_prog_fentry = 
>>>>>> bpf_object__find_program_by_title(trace_obj,
>>>>>> "fentry/FUNCTION");
>>>>>>
>>>>>>     /* Here I need to replace the trace_prog_fentry->section_name 
>>>>>> =
>>>>>> "fentry/<INTERFACE PROG NAME> */
>>>>>>
>>>>>>     bpf_object__load(trace_obj);
>>>>>>     trace_link_fentry = 
>>>>>> bpf_program__attach_trace(trace_prog_fentry);
>>>>>>
>>>>>>
>>>>>> See the above, I would like to change the section_name but there 
>>>>>> is no
>>>>>> API to do this, and of course, the struct bpf_program is
>>>>>> implementation-specific.
>>>>>>
>>>>>> Any idea how I would work around this, or what extension to libbpf 
>>>>>> can
>>>>>> be suggested to support this?
>>>>>
>>>>> I think what's missing is a way for the caller to set the 
>>>>> attach_btf_id.
>>>>> Currently, libbpf always tries to discover this based on the 
>>>>> section
>>>>> name (see libbpf_find_attach_btf_id()). I think the right way to 
>>>>> let the
>>>>> caller specify this is not to change the section name, though, but 
>>>>> just
>>>>> to expose a way to explicitly set the btf_id (which the caller can 
>>>>> then
>>>>> go find on its own).
>>>>
>>>> Yes, I agree, section name should be treated as an immutable 
>>>> identifier
>>>> and a (overrideable) hint to libbpf.
>>>>
>>>>>
>>>>> Not sure if it would be better with a new open_opt (to mirror
>>>>> attach_prog_fd), or just a setter 
>>>>> (bpf_program__set_attach_btf_id()?).
>>>>> Or maybe both? Andrii, WDYT?
>>>>
>>>> open_opts is definitely wrong way to do this, because open_opts 
>>>> apply to
>>>> all BPF programs, while this should be per-program.
>>>
>>> Yes, of course; silly me :)
>>>
>>>> I'm also not sure having API that allows to specify BTF type ID is 
>>>> the
>>>> best, probably better to let libbpf perform the search by name. So 
>>>> I'd
>>>> say something like this:
>>>>
>>>> int bpf_program__set_attach_target(int attach_prog_fd, const char
>>>> *attach_func_name)
>>>>
>>>> This should handle customizing all the tp_btf/fentry/fexit/freplace 
>>>> BPF
>>>> programs we have.
>>>
>>> Right, that makes sense; I think that would cover it (apart from your
>>> function signature missing a struct bpf_program argument).
>>
>> great! and, ha-ha, too object-oriented thinking ;)
>
> Thanks for your feedback, assuming you are not working on it, I’ll 
> implement/test it and sent out a patch.

Please do! :)

-Toke


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <D0F8E306-ABEE-480E-BDFD-D43E3A98DC5A@redhat.com>
     [not found] ` <874kw664dy.fsf@toke.dk>
2020-02-04 19:00   ` Need a way to modify the section name for a read program object Andrii Nakryiko
2020-02-04 19:26     ` Toke Høiland-Jørgensen
2020-02-04 19:32       ` Andrii Nakryiko
2020-02-11 14:37         ` Eelco Chaudron
2020-02-11 14:54           ` Toke Høiland-Jørgensen

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git