diff mbox series

[bpf-next,v2] selftests/bpf: support struct/union presets in veristat

Message ID 20250324123455.35888-1-mykyta.yatsenko5@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next,v2] selftests/bpf: support struct/union presets in veristat | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 26 (+0) this patch: 26 (+0)
netdev/cc_maintainers warning 11 maintainers not CCed: sdf@fomichev.me linux-kselftest@vger.kernel.org jolsa@kernel.org song@kernel.org martin.lau@linux.dev haoluo@google.com mykolal@fb.com john.fastabend@gmail.com yonghong.song@linux.dev shuah@kernel.org kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning CHECK: Please use a blank line after function/struct/union/enum declarations WARNING: 'struc' may be misspelled - perhaps 'struct'? WARNING: Prefer strscpy, strscpy_pad, or __nonstring over strncpy - see: https://github.com/KSPP/linux/issues/90 WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns WARNING: line length of 91 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-12 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-11 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-20 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-21 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-43 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-44 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-51 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-50 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-49 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-47 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-48 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18

Commit Message

Mykyta Yatsenko March 24, 2025, 12:34 p.m. UTC
From: Mykyta Yatsenko <yatsenko@meta.com>

Extend commit e3c9abd0d14b ("selftests/bpf: Implement setting global
variables in veristat") to support applying presets to members of
the global structs or unions in veristat.
For example:
```
./veristat set_global_vars.bpf.o  -G "union1.struct3.var_u8_h = 0xBB"
```

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 .../selftests/bpf/prog_tests/test_veristat.c  |   5 +
 tools/testing/selftests/bpf/progs/prepare.c   |   1 -
 .../selftests/bpf/progs/set_global_vars.c     |  39 +++++
 tools/testing/selftests/bpf/veristat.c        | 141 +++++++++++++++++-
 4 files changed, 178 insertions(+), 8 deletions(-)

Comments

Eduard Zingerman March 27, 2025, 7:39 p.m. UTC | #1
On Mon, 2025-03-24 at 12:34 +0000, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Extend commit e3c9abd0d14b ("selftests/bpf: Implement setting global
> variables in veristat") to support applying presets to members of
> the global structs or unions in veristat.
> For example:
> ```
> ./veristat set_global_vars.bpf.o  -G "union1.struct3.var_u8_h = 0xBB"
> ```
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---

Thank you for addressing comments from my previous review.
Sorry for the delay, didn't look at patches while travelling.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> +static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
> +				  const struct btf_type *t, struct btf_var_secinfo *sinfo)
> +{
> +	char *name = strtok_r(NULL, ".", name_tok);
> +	const struct btf_type *member_type;
> +	const struct btf_member *member;
> +	int member_tid;
> +	__u32 anon_offset = 0;
> +
> +	if (!name)
> +		return 0;
> +
> +	member = btf_find_member(btf, t, name, &anon_offset);
> +	if (IS_ERR(member)) {
> +		fprintf(stderr, "Could not find member %s\n", name);
> +		return -EINVAL;
> +	}
> +
> +	member_tid = btf__resolve_type(btf, member->type);
> +	member_type = btf__type_by_id(btf, member_tid);
> +
> +	if (btf_kflag(t)) {
> +		fprintf(stderr, "Bitfield presets are not supported %s\n", name);
> +		return -EINVAL;
> +	}
> +	sinfo->offset += (member->offset + anon_offset) / 8;
> +	sinfo->size = member_type->size;
> +	sinfo->type = member_tid;

Nit: I'd push this assignment down to `btf_find_member` and avoid
     `&anon_offset` parameter. It would be easier to read this way,
     as all offset manipulations would be in one place.

> +
> +	return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);
> +}
> +
> +static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
> +			      struct btf_var_secinfo *sinfo, const char *var)
> +{
> +	char expr[256], *saveptr;
> +	const struct btf_type *base_type;
> +	int err;
> +
> +	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> +	strncpy(expr, var, 256);

Nit: strncpy does not null terminate if strlen(src) == N.

> +	strtok_r(expr, ".", &saveptr);
> +	err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int set_global_var(struct bpf_object *obj, struct btf *btf,
>  			  struct bpf_map *map, struct btf_var_secinfo *sinfo,
>  			  struct var_preset *preset)
>  {

[...]
Andrii Nakryiko March 28, 2025, 5:03 p.m. UTC | #2
On Mon, Mar 24, 2025 at 5:35 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Extend commit e3c9abd0d14b ("selftests/bpf: Implement setting global
> variables in veristat") to support applying presets to members of
> the global structs or unions in veristat.
> For example:
> ```
> ./veristat set_global_vars.bpf.o  -G "union1.struct3.var_u8_h = 0xBB"
> ```
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  .../selftests/bpf/prog_tests/test_veristat.c  |   5 +
>  tools/testing/selftests/bpf/progs/prepare.c   |   1 -
>  .../selftests/bpf/progs/set_global_vars.c     |  39 +++++
>  tools/testing/selftests/bpf/veristat.c        | 141 +++++++++++++++++-
>  4 files changed, 178 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_veristat.c b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
> index a95b42bf744a..47b56c258f3f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_veristat.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
> @@ -63,6 +63,9 @@ static void test_set_global_vars_succeeds(void)
>             " -G \"var_eb = EB2\" "\
>             " -G \"var_ec = EC2\" "\
>             " -G \"var_b = 1\" "\
> +           " -G \"struct1.struct2.u.var_u8 = 170\" "\
> +           " -G \"union1.struct3.var_u8_l = 0xaa\" "\
> +           " -G \"union1.struct3.var_u8_h = 0xaa\" "\
>             "-vl2 > %s", fix->veristat, fix->tmpfile);
>
>         read(fix->fd, fix->output, fix->sz);
> @@ -78,6 +81,8 @@ static void test_set_global_vars_succeeds(void)
>         __CHECK_STR("_w=12 ", "var_eb = EB2");
>         __CHECK_STR("_w=13 ", "var_ec = EC2");
>         __CHECK_STR("_w=1 ", "var_b = 1");
> +       __CHECK_STR("_w=170 ", "struct1.struct2.u.var_u8 = 170");
> +       __CHECK_STR("_w=0xaaaa ", "union1.var_u16 = 0xaaaa");
>
>  out:
>         teardown_fixture(fix);
> diff --git a/tools/testing/selftests/bpf/progs/prepare.c b/tools/testing/selftests/bpf/progs/prepare.c
> index 1f1dd547e4ee..cfc1f48e0d28 100644
> --- a/tools/testing/selftests/bpf/progs/prepare.c
> +++ b/tools/testing/selftests/bpf/progs/prepare.c
> @@ -2,7 +2,6 @@
>  /* Copyright (c) 2025 Meta */
>  #include <vmlinux.h>
>  #include <bpf/bpf_helpers.h>
> -//#include <bpf/bpf_tracing.h>
>
>  char _license[] SEC("license") = "GPL";
>
> diff --git a/tools/testing/selftests/bpf/progs/set_global_vars.c b/tools/testing/selftests/bpf/progs/set_global_vars.c
> index 9adb5ba4cd4d..0259d290d5f2 100644
> --- a/tools/testing/selftests/bpf/progs/set_global_vars.c
> +++ b/tools/testing/selftests/bpf/progs/set_global_vars.c
> @@ -24,6 +24,42 @@ const volatile enum Enumu64 var_eb = EB1;
>  const volatile enum Enums64 var_ec = EC1;
>  const volatile bool var_b = false;
>
> +struct Struct {
> +       int:16;
> +       __u16 filler;
> +       struct {
> +               __u16 filler2;
> +       };
> +       struct Struct2 {
> +               __u16 filler;
> +               volatile struct {
> +                       __u32 filler2;
> +                       union {
> +                               const volatile __u8 var_u8;
> +                               const volatile __s16 filler3;
> +                       } u;
> +               };
> +       } struct2;
> +};
> +const volatile __u32 struc = 0; /* same prefix as below */
> +const volatile struct Struct struct1 = {.struct2 = {.u = {.var_u8 = 1}}};
> +
> +union Union {
> +       __u16 var_u16;
> +       struct Struct3 {
> +               struct {
> +                       __u8 var_u8_l;
> +               };
> +               struct {
> +                       struct {
> +                               __u8 var_u8_h;
> +                       };
> +               };
> +       } struct3;
> +};
> +
> +const volatile union Union union1 = {.var_u16 = -1};
> +
>  char arr[4] = {0};
>
>  SEC("socket")
> @@ -43,5 +79,8 @@ int test_set_globals(void *ctx)
>         a = var_eb;
>         a = var_ec;
>         a = var_b;
> +       a = struct1.struct2.u.var_u8;
> +       a = union1.var_u16;
> +
>         return a;
>  }
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index a18972ffdeb6..4fb52767ea73 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -23,6 +23,7 @@
>  #include <float.h>
>  #include <math.h>
>  #include <limits.h>
> +#include <linux/err.h>

this is kernel-internal header (not UAPI), so we won't have it in
Github; let's avoid adding this

>
>  #ifndef ARRAY_SIZE
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> @@ -1486,7 +1487,124 @@ static bool is_preset_supported(const struct btf_type *t)
>         return btf_is_int(t) || btf_is_enum(t) || btf_is_enum64(t);
>  }
>
> -static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct btf_type *t,
> +struct btf_anon_stack {
> +       const struct btf_type *type;
> +       __u32 offset;
> +};
> +
> +const struct btf_member *btf_find_member(const struct btf *btf,
> +                                        const struct btf_type *parent_type,
> +                                        const char *member_name,
> +                                        __u32 *anon_offset)
> +{
> +       struct btf_anon_stack *anon_stack;
> +       const struct btf_member *retval = NULL;
> +       __u32 cur_offset = 0;
> +       const char *name;
> +       int top = 0, i;
> +
> +       if (!btf_is_struct(parent_type) && !btf_is_union(parent_type))

we have btf_is_composite() which does exactly this

> +               return ERR_PTR(-EINVAL);
> +
> +       anon_stack = malloc(sizeof(*anon_stack));
> +       if (!anon_stack)
> +               return ERR_PTR(-ENOMEM);
> +
> +       anon_stack[top].type = parent_type;
> +       anon_stack[top++].offset = 0;
> +
> +       do {
> +               parent_type = anon_stack[--top].type;
> +               cur_offset = anon_stack[top].offset;
> +
> +               for (i = 0; i < btf_vlen(parent_type); ++i) {
> +                       const struct btf_member *member;
> +                       const struct btf_type *member_type;
> +                       int member_tid;
> +
> +                       member = btf_members(parent_type) + i;
> +                       member_tid =  btf__resolve_type(btf, member->type);
> +                       if (member_tid < 0) {
> +                               retval = ERR_PTR(-EINVAL);
> +                               goto out;
> +                       }
> +                       member_type = btf__type_by_id(btf, member_tid);
> +                       if (member->name_off) {
> +                               name = btf__name_by_offset(btf, member->name_off);
> +                               if (name && strcmp(member_name, name) == 0) {

let's assume valid BTF and not do these unnecessary name != NULL
checks, we don't do it in many other places anyways

> +                                       *anon_offset = cur_offset;
> +                                       retval = member;
> +                                       goto out;
> +                               }
> +                       } else if (btf_is_struct(member_type) || btf_is_union(member_type)) {
> +                               struct btf_anon_stack *tmp;
> +                               /* Anonymous union/struct: push to stack */
> +                               tmp = realloc(anon_stack, (top + 1) * sizeof(*anon_stack));
> +                               if (!tmp) {
> +                                       retval = ERR_PTR(-ENOMEM);
> +                                       goto out;
> +                               }
> +                               anon_stack = tmp;
> +                               anon_stack[top].type = member_type;
> +                               anon_stack[top++].offset = cur_offset + member->offset;
> +                       }
> +               }
> +       } while (top > 0);
> +out:
> +       free(anon_stack);
> +       return retval;
> +}
> +

why all this dynamic mem allocation for stack? this is user space
code, we have recursion, let's use the benefits of user space here

pw-bot: cr

> +static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
> +                                 const struct btf_type *t, struct btf_var_secinfo *sinfo)
> +{
> +       char *name = strtok_r(NULL, ".", name_tok);
> +       const struct btf_type *member_type;
> +       const struct btf_member *member;
> +       int member_tid;
> +       __u32 anon_offset = 0;
> +
> +       if (!name)
> +               return 0;
> +
> +       member = btf_find_member(btf, t, name, &anon_offset);
> +       if (IS_ERR(member)) {

why using ERR_PTR approach here if you don't really propagate error code?

I'd say instead of returning btf_member, I'd return containing BTF
type ID and member index within it (plus the offset you are returning)

This way you can take advantage of btf_member_bit_offset() and
btf_member_bitfield_size() helpers provided by libbpf in btf.h

and given you have multiple outputs, it's probably easier to return
int error code (or zero on success), and then everything else as out
parameters by pointer (instead of all the ERR_PTR business)

> +               fprintf(stderr, "Could not find member %s\n", name);
> +               return -EINVAL;
> +       }
> +
> +       member_tid = btf__resolve_type(btf, member->type);
> +       member_type = btf__type_by_id(btf, member_tid);
> +
> +       if (btf_kflag(t)) {
> +               fprintf(stderr, "Bitfield presets are not supported %s\n", name);
> +               return -EINVAL;
> +       }
> +       sinfo->offset += (member->offset + anon_offset) / 8;
> +       sinfo->size = member_type->size;
> +       sinfo->type = member_tid;
> +
> +       return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);

tbh, not a big fan of this recursion on tokenizer itself... Why can't
there be an outer loop to get tokens one at a time, and then recursive
btf_find_member() logic inside the loop to find offset and type
information?

Ultimately you need to find one offset and one type, corresponding to
the last member in the `a.b.c.d.e.f` chain, right? There is no
backtracking here, overall (the only backtracking will be hidden
inside btf_find_member due to anonymous fields), so the process is a
simple loop. There is no need to employ recursion, imo (unless I'm
missing some nuance).

Let's keep it simple(r).

> +}
> +
> +static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
> +                             struct btf_var_secinfo *sinfo, const char *var)
> +{
> +       char expr[256], *saveptr;
> +       const struct btf_type *base_type;
> +       int err;
> +
> +       base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> +       strncpy(expr, var, 256);
> +       strtok_r(expr, ".", &saveptr);
> +       err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
> +       if (err)
> +               return err;
> +
> +       return 0;
> +}
> +
> +static int set_global_var(struct bpf_object *obj, struct btf *btf,
>                           struct bpf_map *map, struct btf_var_secinfo *sinfo,
>                           struct var_preset *preset)
>  {
> @@ -1495,9 +1613,9 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
>         long long value = preset->ivalue;
>         size_t size;
>
> -       base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> +       base_type = btf__type_by_id(btf, btf__resolve_type(btf, sinfo->type));
>         if (!base_type) {
> -               fprintf(stderr, "Failed to resolve type %d\n", t->type);
> +               fprintf(stderr, "Failed to resolve type %d\n", sinfo->type);
>                 return -EINVAL;
>         }
>         if (!is_preset_supported(base_type)) {
> @@ -1530,7 +1648,7 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
>                 if (value >= max_val || value < -max_val) {
>                         fprintf(stderr,
>                                 "Variable %s value %lld is out of range [%lld; %lld]\n",
> -                               btf__name_by_offset(btf, t->name_off), value,
> +                               btf__name_by_offset(btf, base_type->name_off), value,
>                                 is_signed ? -max_val : 0, max_val - 1);
>                         return -EINVAL;
>                 }
> @@ -1590,7 +1708,12 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>                         var_name = btf__name_by_offset(btf, var_type->name_off);
>
>                         for (k = 0; k < npresets; ++k) {
> -                               if (strcmp(var_name, presets[k].name) != 0)
> +                               struct btf_var_secinfo tmp_sinfo;
> +                               int var_len = strlen(var_name);

do this once outside of the loop, why recalculating on each iteration?

> +
> +                               if (strncmp(var_name, presets[k].name, var_len) != 0 ||
> +                                   (presets[k].name[var_len] != '\0' &&
> +                                    presets[k].name[var_len] != '.'))
>                                         continue;
>
>                                 if (presets[k].applied) {
> @@ -1598,13 +1721,17 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>                                                 var_name);
>                                         return -EINVAL;
>                                 }
> +                               memcpy(&tmp_sinfo, sinfo, sizeof(*sinfo));
> +                               err = adjust_var_secinfo(btf, var_type,
> +                                                        &tmp_sinfo, presets[k].name);
> +                               if (err)
> +                                       return err;
>
> -                               err = set_global_var(obj, btf, var_type, map, sinfo, presets + k);
> +                               err = set_global_var(obj, btf, map, &tmp_sinfo, presets + k);
>                                 if (err)
>                                         return err;
>
>                                 presets[k].applied = true;
> -                               break;
>                         }
>                 }
>         }
> --
> 2.48.1
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/test_veristat.c b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
index a95b42bf744a..47b56c258f3f 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_veristat.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
@@ -63,6 +63,9 @@  static void test_set_global_vars_succeeds(void)
 	    " -G \"var_eb = EB2\" "\
 	    " -G \"var_ec = EC2\" "\
 	    " -G \"var_b = 1\" "\
+	    " -G \"struct1.struct2.u.var_u8 = 170\" "\
+	    " -G \"union1.struct3.var_u8_l = 0xaa\" "\
+	    " -G \"union1.struct3.var_u8_h = 0xaa\" "\
 	    "-vl2 > %s", fix->veristat, fix->tmpfile);
 
 	read(fix->fd, fix->output, fix->sz);
@@ -78,6 +81,8 @@  static void test_set_global_vars_succeeds(void)
 	__CHECK_STR("_w=12 ", "var_eb = EB2");
 	__CHECK_STR("_w=13 ", "var_ec = EC2");
 	__CHECK_STR("_w=1 ", "var_b = 1");
+	__CHECK_STR("_w=170 ", "struct1.struct2.u.var_u8 = 170");
+	__CHECK_STR("_w=0xaaaa ", "union1.var_u16 = 0xaaaa");
 
 out:
 	teardown_fixture(fix);
diff --git a/tools/testing/selftests/bpf/progs/prepare.c b/tools/testing/selftests/bpf/progs/prepare.c
index 1f1dd547e4ee..cfc1f48e0d28 100644
--- a/tools/testing/selftests/bpf/progs/prepare.c
+++ b/tools/testing/selftests/bpf/progs/prepare.c
@@ -2,7 +2,6 @@ 
 /* Copyright (c) 2025 Meta */
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-//#include <bpf/bpf_tracing.h>
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/set_global_vars.c b/tools/testing/selftests/bpf/progs/set_global_vars.c
index 9adb5ba4cd4d..0259d290d5f2 100644
--- a/tools/testing/selftests/bpf/progs/set_global_vars.c
+++ b/tools/testing/selftests/bpf/progs/set_global_vars.c
@@ -24,6 +24,42 @@  const volatile enum Enumu64 var_eb = EB1;
 const volatile enum Enums64 var_ec = EC1;
 const volatile bool var_b = false;
 
+struct Struct {
+	int:16;
+	__u16 filler;
+	struct {
+		__u16 filler2;
+	};
+	struct Struct2 {
+		__u16 filler;
+		volatile struct {
+			__u32 filler2;
+			union {
+				const volatile __u8 var_u8;
+				const volatile __s16 filler3;
+			} u;
+		};
+	} struct2;
+};
+const volatile __u32 struc = 0; /* same prefix as below */
+const volatile struct Struct struct1 = {.struct2 = {.u = {.var_u8 = 1}}};
+
+union Union {
+	__u16 var_u16;
+	struct Struct3 {
+		struct {
+			__u8 var_u8_l;
+		};
+		struct {
+			struct {
+				__u8 var_u8_h;
+			};
+		};
+	} struct3;
+};
+
+const volatile union Union union1 = {.var_u16 = -1};
+
 char arr[4] = {0};
 
 SEC("socket")
@@ -43,5 +79,8 @@  int test_set_globals(void *ctx)
 	a = var_eb;
 	a = var_ec;
 	a = var_b;
+	a = struct1.struct2.u.var_u8;
+	a = union1.var_u16;
+
 	return a;
 }
diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index a18972ffdeb6..4fb52767ea73 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -23,6 +23,7 @@ 
 #include <float.h>
 #include <math.h>
 #include <limits.h>
+#include <linux/err.h>
 
 #ifndef ARRAY_SIZE
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
@@ -1486,7 +1487,124 @@  static bool is_preset_supported(const struct btf_type *t)
 	return btf_is_int(t) || btf_is_enum(t) || btf_is_enum64(t);
 }
 
-static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct btf_type *t,
+struct btf_anon_stack {
+	const struct btf_type *type;
+	__u32 offset;
+};
+
+const struct btf_member *btf_find_member(const struct btf *btf,
+					 const struct btf_type *parent_type,
+					 const char *member_name,
+					 __u32 *anon_offset)
+{
+	struct btf_anon_stack *anon_stack;
+	const struct btf_member *retval = NULL;
+	__u32 cur_offset = 0;
+	const char *name;
+	int top = 0, i;
+
+	if (!btf_is_struct(parent_type) && !btf_is_union(parent_type))
+		return ERR_PTR(-EINVAL);
+
+	anon_stack = malloc(sizeof(*anon_stack));
+	if (!anon_stack)
+		return ERR_PTR(-ENOMEM);
+
+	anon_stack[top].type = parent_type;
+	anon_stack[top++].offset = 0;
+
+	do {
+		parent_type = anon_stack[--top].type;
+		cur_offset = anon_stack[top].offset;
+
+		for (i = 0; i < btf_vlen(parent_type); ++i) {
+			const struct btf_member *member;
+			const struct btf_type *member_type;
+			int member_tid;
+
+			member = btf_members(parent_type) + i;
+			member_tid =  btf__resolve_type(btf, member->type);
+			if (member_tid < 0) {
+				retval = ERR_PTR(-EINVAL);
+				goto out;
+			}
+			member_type = btf__type_by_id(btf, member_tid);
+			if (member->name_off) {
+				name = btf__name_by_offset(btf, member->name_off);
+				if (name && strcmp(member_name, name) == 0) {
+					*anon_offset = cur_offset;
+					retval = member;
+					goto out;
+				}
+			} else if (btf_is_struct(member_type) || btf_is_union(member_type)) {
+				struct btf_anon_stack *tmp;
+				/* Anonymous union/struct: push to stack */
+				tmp = realloc(anon_stack, (top + 1) * sizeof(*anon_stack));
+				if (!tmp) {
+					retval = ERR_PTR(-ENOMEM);
+					goto out;
+				}
+				anon_stack = tmp;
+				anon_stack[top].type = member_type;
+				anon_stack[top++].offset = cur_offset + member->offset;
+			}
+		}
+	} while (top > 0);
+out:
+	free(anon_stack);
+	return retval;
+}
+
+static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
+				  const struct btf_type *t, struct btf_var_secinfo *sinfo)
+{
+	char *name = strtok_r(NULL, ".", name_tok);
+	const struct btf_type *member_type;
+	const struct btf_member *member;
+	int member_tid;
+	__u32 anon_offset = 0;
+
+	if (!name)
+		return 0;
+
+	member = btf_find_member(btf, t, name, &anon_offset);
+	if (IS_ERR(member)) {
+		fprintf(stderr, "Could not find member %s\n", name);
+		return -EINVAL;
+	}
+
+	member_tid = btf__resolve_type(btf, member->type);
+	member_type = btf__type_by_id(btf, member_tid);
+
+	if (btf_kflag(t)) {
+		fprintf(stderr, "Bitfield presets are not supported %s\n", name);
+		return -EINVAL;
+	}
+	sinfo->offset += (member->offset + anon_offset) / 8;
+	sinfo->size = member_type->size;
+	sinfo->type = member_tid;
+
+	return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);
+}
+
+static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
+			      struct btf_var_secinfo *sinfo, const char *var)
+{
+	char expr[256], *saveptr;
+	const struct btf_type *base_type;
+	int err;
+
+	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
+	strncpy(expr, var, 256);
+	strtok_r(expr, ".", &saveptr);
+	err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int set_global_var(struct bpf_object *obj, struct btf *btf,
 			  struct bpf_map *map, struct btf_var_secinfo *sinfo,
 			  struct var_preset *preset)
 {
@@ -1495,9 +1613,9 @@  static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
 	long long value = preset->ivalue;
 	size_t size;
 
-	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
+	base_type = btf__type_by_id(btf, btf__resolve_type(btf, sinfo->type));
 	if (!base_type) {
-		fprintf(stderr, "Failed to resolve type %d\n", t->type);
+		fprintf(stderr, "Failed to resolve type %d\n", sinfo->type);
 		return -EINVAL;
 	}
 	if (!is_preset_supported(base_type)) {
@@ -1530,7 +1648,7 @@  static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
 		if (value >= max_val || value < -max_val) {
 			fprintf(stderr,
 				"Variable %s value %lld is out of range [%lld; %lld]\n",
-				btf__name_by_offset(btf, t->name_off), value,
+				btf__name_by_offset(btf, base_type->name_off), value,
 				is_signed ? -max_val : 0, max_val - 1);
 			return -EINVAL;
 		}
@@ -1590,7 +1708,12 @@  static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
 			var_name = btf__name_by_offset(btf, var_type->name_off);
 
 			for (k = 0; k < npresets; ++k) {
-				if (strcmp(var_name, presets[k].name) != 0)
+				struct btf_var_secinfo tmp_sinfo;
+				int var_len = strlen(var_name);
+
+				if (strncmp(var_name, presets[k].name, var_len) != 0 ||
+				    (presets[k].name[var_len] != '\0' &&
+				     presets[k].name[var_len] != '.'))
 					continue;
 
 				if (presets[k].applied) {
@@ -1598,13 +1721,17 @@  static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
 						var_name);
 					return -EINVAL;
 				}
+				memcpy(&tmp_sinfo, sinfo, sizeof(*sinfo));
+				err = adjust_var_secinfo(btf, var_type,
+							 &tmp_sinfo, presets[k].name);
+				if (err)
+					return err;
 
-				err = set_global_var(obj, btf, var_type, map, sinfo, presets + k);
+				err = set_global_var(obj, btf, map, &tmp_sinfo, presets + k);
 				if (err)
 					return err;
 
 				presets[k].applied = true;
-				break;
 			}
 		}
 	}