| Message ID | 20240214020836.1845354-1-thinker.li@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Create shadow variables for struct_ops in skeletons | expand |
On Tue, Feb 13, 2024 at 6:08 PM <thinker.li@gmail.com> wrote: > > From: Kui-Feng Lee <thinker.li@gmail.com> > > 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? > > == 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. > 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 [...] > > --- > > v1: https://lore.kernel.org/all/20240124224130.859921-1-thinker.li@gmail.com/ > > 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 >
On 2/15/24 15:50, Andrii Nakryiko wrote: > On Tue, Feb 13, 2024 at 6:08 PM <thinker.li@gmail.com> wrote: >> >> From: Kui-Feng Lee <thinker.li@gmail.com> >> >> 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@gmail.com/ >> >> 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 >>
On Thu, Feb 15, 2024 at 6:40 PM Kui-Feng Lee <sinquersw@gmail.com> wrote: > > > > On 2/15/24 15:50, Andrii Nakryiko wrote: > > On Tue, Feb 13, 2024 at 6:08 PM <thinker.li@gmail.com> wrote: > >> > >> From: Kui-Feng Lee <thinker.li@gmail.com> > >> > >> 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. yep, replied there. Let's keep it simple and restrict this functionality to 64-bit host architecture > > > > >> > >> == 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. > Just lore link to the previous revision should be enough if anyone is interested in history. Cover letter should represent the current state of things. > > > >> > >> --- > >> > >> v1: https://lore.kernel.org/all/20240124224130.859921-1-thinker.li@gmail.com/ > >> > >> 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 > >>
From: Kui-Feng Lee <thinker.li@gmail.com> 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()) == 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); 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 ======================================== Create shadow variables for the fields of struct_ops maps in a skeleton with the same name as the respective fields. For instance, if struct bpf_testmod_ops has a "data" field as following, you can access or modify its value through a shadow variable also named "data" in the skeleton. SEC(".struct_ops.link") struct bpf_testmod_ops testmod_1 = { .data = 0x1, }; Then, you can change the value in the following manner as shown in the code below. skel->struct_ops.testmod_1->data = 13; It is helpful to configure a struct_ops map during the execution of a program. For instance, you can include a flag in a struct_ops type to modify the way the kernel handles an instance. By implementing this feature, a user space program can alter the flag's value prior to loading an instance into the kernel. The code generator for skeletons will produce code that copies values to shadow variables from the internal data buffer when a skeleton is opened. It will also copy values from shadow variables back to the internal data buffer before a skeleton is loaded into the kernel. The code generator will calculate the offset of a field and generate code that copies the value of the field from or to the shadow variable to or from the struct_ops internal data, with an offset relative to the beginning of the internal data. For instance, if the "data" field in struct bpf_testmod_ops is situated 16 bytes from the beginning of the struct, the address for the "data" field of struct bpf_testmod_ops will be the starting address plus 16. The offset is calculated during code generation, so it is always in line with the internal data in the skeleton. Even if the user space programs and the BPF program were not compiled together, any differences in versions would not impact correctness. This means that even if the BPF program and the user space program reference different versions of the "struct bpf_testmod_ops" and have different offsets for "data", these offsets computed by the code generator would still function correctly. The user space programs can only modify values by using these shadow variables when the skeleton is open, but before being loaded. Once the skeleton is loaded, the value is copied to the kernel, and any future changes only affect the shadow variables in the user space memory and do not update the kernel space. The shadow variables are not initialized before opening a skeleton, so you cannot update values through them before opening. For function pointers (operators), you can change/replace their values with other BPF programs. For example, the test case in test_struct_ops_module.c points .test_2 to test_3() while it was pointed to test_2() by assigning a new value to the shadow variable. skel->st_ops_vars.testmod_1->test_2 = skel->progs.test_3; However, you need to turn off autoload for test_2() since it is not assigned to any struct_ops map anymore. Or, it will fails to load test_2(). bpf_program__set_autoload(skel->progs.test_2, false); You can define more struct_ops programs than necessary. However, you need to turn autoload off for unused ones. --- v1: https://lore.kernel.org/all/20240124224130.859921-1-thinker.li@gmail.com/ 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(-)