From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexei Starovoitov <ast@fb.com>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>, bpf <bpf@vger.kernel.org>,
Networking <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next v2] libbpf: Add getter for pointer to data area for internal maps
Date: Sat, 28 Mar 2020 01:08:28 +0100 [thread overview]
Message-ID: <87r1xdl4sj.fsf@toke.dk> (raw)
In-Reply-To: <dc1ad5b0-d575-034a-ffa5-710bcf94d8f0@fb.com>
Alexei Starovoitov <ast@fb.com> writes:
> On 3/27/20 3:26 PM, Toke Høiland-Jørgensen wrote:
>> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
>>
>>> On Fri, Mar 27, 2020 at 5:58 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>>>
>>>> For internal maps (most notably the maps backing global variables), libbpf
>>>> uses an internal mmaped area to store the data after opening the object.
>>>> This data is subsequently copied into the kernel map when the object is
>>>> loaded.
>>>>
>>>> This adds a getter for the pointer to that internal data store. This can be
>>>> used to modify the data before it is loaded into the kernel, which is
>>>> especially relevant for RODATA, which is frozen on load. This same pointer
>>>> is already exposed to the auto-generated skeletons, so access to it is
>>>> already API; this just adds a way to get at it without pulling in the full
>>>> skeleton infrastructure.
>>>>
>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>>> ---
>>>> v2:
>>>> - Add per-map getter for data area instead of a global rodata getter for bpf_obj
>>>>
>>>> tools/lib/bpf/libbpf.c | 9 +++++++++
>>>> tools/lib/bpf/libbpf.h | 1 +
>>>> tools/lib/bpf/libbpf.map | 1 +
>>>> 3 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>> index 085e41f9b68e..a0055f8908fd 100644
>>>> --- a/tools/lib/bpf/libbpf.c
>>>> +++ b/tools/lib/bpf/libbpf.c
>>>> @@ -6756,6 +6756,15 @@ void *bpf_map__priv(const struct bpf_map *map)
>>>> return map ? map->priv : ERR_PTR(-EINVAL);
>>>> }
>>>>
>>>> +void *bpf_map__data_area(const struct bpf_map *map, size_t *size)
>>>
>>> I'm not entirely thrilled about "data_area" name. This is entirely for
>>> providing initial value for maps, so maybe something like
>>> bpf_map__init_value() or something along those lines?
>>>
>>> Actually, how about a different API altogether:
>>>
>>> bpf_map__set_init_value(struct bpf_map *map, void *data, size_t size)?
>>>
>>> Application will have to prepare data of correct size, which will be
>>> copied to libbpf's internal storage. It also doesn't expose any of
>>> internal pointer. I don't think extra memcopy is a big deal here.
>>> Thoughts?
>>
>> Huh, yeah, that's way better. Why didn't I think of that? Think maybe I
>> was too focused on doing this the same way the skeleton code is. I'll
>> send a v3 :)
>
> Could you please add a selftest as well?
> I'm not excited about new features without tests.
Sure, will do.
-Toke
next prev parent reply other threads:[~2020-03-28 0:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-26 15:17 [PATCH bpf-next] libbpf: Add bpf_object__rodata getter function Toke Høiland-Jørgensen
2020-03-26 19:20 ` Andrii Nakryiko
2020-03-27 12:24 ` Toke Høiland-Jørgensen
2020-03-27 17:49 ` Andrii Nakryiko
2020-03-27 18:39 ` Toke Høiland-Jørgensen
2020-03-27 12:58 ` [PATCH bpf-next v2] libbpf: Add getter for pointer to data area for internal maps Toke Høiland-Jørgensen
2020-03-27 19:17 ` Andrii Nakryiko
2020-03-27 22:26 ` Toke Høiland-Jørgensen
2020-03-27 22:28 ` Alexei Starovoitov
2020-03-28 0:08 ` Toke Høiland-Jørgensen [this message]
2020-03-28 18:28 ` [PATCH v3 1/2] libbpf: Add setter for initial value " Toke Høiland-Jørgensen
2020-03-28 20:01 ` Andrii Nakryiko
2020-03-29 12:17 ` Toke Høiland-Jørgensen
2020-03-29 13:22 ` [PATCH v4 " Toke Høiland-Jørgensen
2020-03-29 20:06 ` Andrii Nakryiko
2020-03-29 23:46 ` Daniel Borkmann
2020-03-29 13:22 ` [PATCH v4 2/2] selftests: Add test for overriding global data value before load Toke Høiland-Jørgensen
2020-03-29 20:13 ` Andrii Nakryiko
2020-03-28 18:28 ` [PATCH v3 " Toke Høiland-Jørgensen
2020-03-28 20:06 ` Andrii Nakryiko
2020-03-29 13:10 ` Toke Høiland-Jørgensen
2020-03-29 20:08 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r1xdl4sj.fsf@toke.dk \
--to=toke@redhat.com \
--cc=andrii.nakryiko@gmail.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).