On 2/15/24 15:50, Andrii Nakryiko wrote:
On Tue, Feb 13, 2024 at 6:08 PM <thinker.li@xxxxxxxxx> wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
This RFC is for gathering feedback/opinions on the design.
Based on the feedback received for v1, I made some modifications.
== Pointers to Shadow Copies ==
With the current implementation, the code generator will create a
pointer to a shadow copy of the struct_ops map for each map. For
instance, if we define a testmod_1 as a struct_ops map, we can access
its corresponding shadow variable "data" using the pointer.
skel->struct_ops.testmod1->data
== Shadow Info ==
The code generator also generates a shadow info to describe the layout
of the data pointed to by all these pointers. For instance, the
following shadow info describes the layout of a struct_ops map called
testmod_1, which has 3 members: test_1, test_2, and data.
static struct bpf_struct_ops_member_info member_info_testmod_1[] = {
{
.name = "test_1",
.offset = .....,
.size = .....,
},
{
.name = "test_2",
.offset = .....,
.size = .....,
},
{
.name = "data",
.offset = .....,
.size = .....,
},
};
static struct bpf_struct_ops_map_info map_info[] = {
{
.name = "testmod_1",
.members = member_info_testmod_1,
.cnt = ARRAY_SIZE(member_info_testmod_1),
.data_size = sizeof(struct_ops->testmod_1),
},
};
static struct bpf_struct_ops_shadow_info shadow_info = {
.maps = map_info,
.cnt = ARRAY_SIZE(map_info),
};
A shadow info describes the layout of the shadow copies of all
struct_ops maps included in a skeleton. (Defined in *__shadow_info())
I must be missing something, but libbpf knows the layout of struct_ops
struct through BTF, why do we need all these descriptors?
I explain it in the response for part 1.
== libbpf Creates Shadow Copies ==
This shadow info should be passed to bpf_object__open_skeleton() as a
part of "opts" so that libbpf can create shadow copies with the layout
described by the shadow info. For now, *__open() in the skeleton will
automatically pass the shadow info to bpf_object__open_skeleton(),
looking like the following example.
static inline struct struct_ops_module *
struct_ops_module__open(void)
{
DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
opts.struct_ops_shadow = struct_ops_module__shadow_info();
return struct_ops_module__open_opts(*** BLURB HERE ***opts);
}
The function bpf_map__initial_value() will return the shadow copy that
is created based on the received shadow info. Therefore, in the
function *__open_opts() in the skeleton, the pointers to shadow copies
will be initialized with the values returned from
bpf_map__initial_value(). For instance,
obj->struct_ops.testmod_1 =
bpf_map__initial_value(obj->maps.testmod_1, NULL);
I also don't get why you need to allocate some extra "shadow memory"
if we already have struct bpf_struct_ops->data pointer malloc()'ed
during bpf_map initialization, and its size matches exactly the
struct_ops's type size.
I assume that the alignments & padding of BPF and the user space
programs are different. (Check the response for part 1 as well)
This line of code will be included in the *__open_opts() function. If
the opts.struct_ops_shadow is not set, bpf_map__initial_value() will
return a NULL.
========================================
DESCRIPTION form v1
========================================
you probably don't need to keep cover letter from previous versions if
they are not relevant anymore
[...]
Sure!
It explains what the feature is and how to use this feature.
So, I keep it here for people just found this discussion.
---
v1: https://lore.kernel.org/all/20240124224130.859921-1-thinker.li@xxxxxxxxx/
Kui-Feng Lee (3):
libbpf: Create a shadow copy for each struct_ops map if necessary.
bpftool: generated shadow variables for struct_ops maps.
selftests/bpf: Test if shadow variables work.
tools/bpf/bpftool/gen.c | 358 +++++++++++++++++-
tools/lib/bpf/libbpf.c | 195 +++++++++-
tools/lib/bpf/libbpf.h | 34 +-
tools/lib/bpf/libbpf.map | 1 +
tools/lib/bpf/libbpf_internal.h | 1 +
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 6 +-
.../selftests/bpf/bpf_testmod/bpf_testmod.h | 1 +
.../bpf/prog_tests/test_struct_ops_module.c | 16 +-
.../selftests/bpf/progs/struct_ops_module.c | 8 +
9 files changed, 596 insertions(+), 24 deletions(-)
--
2.34.1
