Re: [PATCH bpf-next v4 2/3] selftests/bpf: support array presets in veristat

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2025-06-18 at 21:39 +0100, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Implement support for presetting values for array elements in veristat.
> For example:
> ```
> sudo ./veristat set_global_vars.bpf.o -G "arr[3] = 1"
> ```
> Arrays of structures and structure of arrays work, but each individual
> scalar value has to be set separately: `foo[1].bar[2] = value`.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  tools/testing/selftests/bpf/veristat.c | 226 ++++++++++++++++++++-----
>  1 file changed, 180 insertions(+), 46 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index 483442c08ecf..9942adbda411 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c

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

[...]

> @@ -1670,7 +1706,7 @@ static int append_var_preset(struct var_preset **presets, int *cnt, const char *
>  	memset(cur, 0, sizeof(*cur));
>  	(*cnt)++;
>  
> -	if (sscanf(expr, "%s = %s %n", var, val, &n) != 2 || n != strlen(expr)) {
> +	if (sscanf(expr, "%[][a-zA-Z0-9_.] = %s %n", var, val, &n) != 2 || n != strlen(expr)) {

Out of curiosity, won't match if the pattern would remain "%s = %s %n"?

>  		fprintf(stderr, "Failed to parse expression '%s'\n", expr);
>  		return -EINVAL;
>  	}
> @@ -1763,17 +1799,103 @@ 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 find_enum_value(const struct btf *btf, const char *name, long long *value)
> +{
> +	const struct btf_type *t;
> +	int cnt, i;
> +	long long lvalue;
> +
> +	cnt = btf__type_cnt(btf);
> +	for (i = 1; i != cnt; ++i) {
> +		t = btf__type_by_id(btf, i);
> +
> +		if (!btf_is_any_enum(t))
> +			continue;
> +
> +		if (enum_value_from_name(btf, t, name, &lvalue) == 0) {
> +			*value = lvalue;
> +			return 0;
> +		}
> +	}
> +	return -ESRCH;
> +}
> +

[...]

> @@ -1815,26 +1938,29 @@ const int btf_find_member(const struct btf *btf,
>  static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
>  			      struct btf_var_secinfo *sinfo, struct var_preset *preset)
>  {
> -	const struct btf_type *base_type, *member_type;
> -	int err, member_tid, i;
> -	__u32 member_offset = 0;
> -
> -	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> -
> -	for (i = 1; i < preset->atom_count; ++i) {
> -		err = btf_find_member(btf, base_type, 0, preset->atoms[i].name,
> -				      &member_tid, &member_offset);
> -		if (err) {
> -			fprintf(stderr, "Could not find member %s for variable %s\n",
> -				preset->atoms[i].name, preset->atoms[i - 1].name);
> -			return err;
> +	const struct btf_type *base_type;
> +	int err, i = 1, n;
> +	int tid;
> +
> +	tid = btf__resolve_type(btf, t->type);
> +	base_type = btf__type_by_id(btf, tid);
> +
> +	while (i < preset->atom_count) {
> +		if (preset->atoms[i].type == ARRAY_INDEX) {
> +			n = adjust_var_secinfo_array(btf, tid, preset, i, sinfo);
> +			if (n < 0)
> +				return n;
> +			i += n;

Having a nested loop to consume all indices looks annoying.
On the other hand, there is not much one can do w/o some kind of
btf__type_physical_size.

> +		} else {
> +			err = btf_find_member(btf, base_type, 0, preset->atoms[i].name, sinfo);
> +			if (err)
> +				return err;
> +			i++;
>  		}
> -		member_type = btf__type_by_id(btf, member_tid);
> -		sinfo->offset += member_offset / 8;
> -		sinfo->size = member_type->size;
> -		sinfo->type = member_tid;
> -		base_type = member_type;
> +		base_type = btf__type_by_id(btf, sinfo->type);
> +		tid = sinfo->type;
>  	}
> +
>  	return 0;
>  }
>  

[...]