[PATCH bpf-next v5 0/2] selftests/bpf: implement setting global variables in veristat

[PATCH bpf-next v5 0/2] selftests/bpf: implement setting global variables in veristat

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

To better verify some complex BPF programs by veristat, it would be useful
to preset global variables. This patch set implements this functionality
and introduces tests for veristat.

v4->v5
  * Rework parsing to use sscanf for integers
  * Addressing nits

v3->v4:
  * Fixing bug in set_global_var introduced by refactoring in previous patch set
  * Addressed nits from Eduard

v2->v3:
  * Reworked parsing of the presets, using sscanf to split into variable and
  value, but still use strtoll/strtoull to support range checks when parsing
  integers
  * Fix test failures for no_alu32 & cpuv4 by checking if veristat binary is in
  parent folder
  * Introduce __CHECK_STR macro for simplifying checks in test
  * Modify tests into sub-tests

Mykyta Yatsenko (2):
  selftests/bpf: implement setting global variables in veristat
  selftests/bpf: introduce veristat test

 tools/testing/selftests/bpf/Makefile          |   1 +
 .../selftests/bpf/prog_tests/test_veristat.c  | 139 ++++++++
 .../selftests/bpf/progs/set_global_vars.c     |  47 +++
 tools/testing/selftests/bpf/test_progs.h      |   8 +
 tools/testing/selftests/bpf/veristat.c        | 309 +++++++++++++++++-
 5 files changed, 503 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_veristat.c
 create mode 100644 tools/testing/selftests/bpf/progs/set_global_vars.c

-- 
2.48.1

[PATCH bpf-next v5 1/2] selftests/bpf: implement setting global variables in veristat

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

To better verify some complex BPF programs we'd like to preset global
variables.
This patch introduces CLI argument `--set-global-vars` or `-G` to
veristat, that allows presetting values to global variables defined
in BPF program. For example:

prog.c:
```
enum Enum { ELEMENT1 = 0, ELEMENT2 = 5 };
const volatile __s64 a = 5;
const volatile __u8 b = 5;
const volatile enum Enum c = ELEMENT2;
const volatile bool d = false;

char arr[4] = {0};

SEC("tp_btf/sched_switch")
int BPF_PROG(...)
{
	bpf_printk("%c\n", arr[a]);
	bpf_printk("%c\n", arr[b]);
	bpf_printk("%c\n", arr[c]);
	bpf_printk("%c\n", arr[d]);
	return 0;
}
```
By default verification of the program fails:
```
./veristat prog.bpf.o
```
By presetting global variables, we can make verification pass:
```
./veristat wq.bpf.o  -G "a = 0" -G "b = 1" -G "c = 2" -G "d = 3"
```

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/testing/selftests/bpf/veristat.c | 309 ++++++++++++++++++++++++-
 1 file changed, 308 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 06af5029885b..43f77f946501 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -154,6 +154,16 @@ struct filter {
 	bool abs;
 };
 
+struct var_preset {
+	char *name;
+	enum { INTEGRAL, ENUMERATOR } type;
+	union {
+		long long ivalue;
+		char *svalue;
+	};
+	bool applied;
+};
+
 static struct env {
 	char **filenames;
 	int filename_cnt;
@@ -195,6 +205,8 @@ static struct env {
 	int progs_processed;
 	int progs_skipped;
 	int top_src_lines;
+	struct var_preset *presets;
+	int npresets;
 } env;
 
 static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va_list args)
@@ -246,12 +258,15 @@ static const struct argp_option opts[] = {
 	{ "test-reg-invariants", 'r', NULL, 0,
 	  "Force BPF verifier failure on register invariant violation (BPF_F_TEST_REG_INVARIANTS program flag)" },
 	{ "top-src-lines", 'S', "N", 0, "Emit N most frequent source code lines" },
+	{ "set-global-vars", 'G', "GLOBAL", 0, "Set global variables provided in the expression, for example \"var1 = 1\"" },
 	{},
 };
 
 static int parse_stats(const char *stats_str, struct stat_specs *specs);
 static int append_filter(struct filter **filters, int *cnt, const char *str);
 static int append_filter_file(const char *path);
+static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr);
+static int append_var_preset_file(const char *filename);
 
 static error_t parse_arg(int key, char *arg, struct argp_state *state)
 {
@@ -363,6 +378,19 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 			return -ENOMEM;
 		env.filename_cnt++;
 		break;
+	case 'G': {
+		if (arg[0] == '@') {
+			if (append_var_preset_file(arg + 1)) {
+				fprintf(stderr, "Could not parse global variables preset: %s\n",
+					arg);
+				argp_usage(state);
+			}
+		} else if (append_var_preset(&env.presets, &env.npresets, arg)) {
+			fprintf(stderr, "Could not parse global variables preset: %s\n", arg);
+			argp_usage(state);
+		}
+		break;
+	}
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
@@ -632,7 +660,7 @@ static int append_filter_file(const char *path)
 	f = fopen(path, "r");
 	if (!f) {
 		err = -errno;
-		fprintf(stderr, "Failed to open filters in '%s': %d\n", path, err);
+		fprintf(stderr, "Failed to open filters in '%s': %s\n", path, strerror(err));
 		return err;
 	}
 
@@ -1292,6 +1320,268 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
 	return 0;
 };
 
+static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr)
+{
+	void *tmp;
+	struct var_preset *cur;
+	char var[256], val[256];
+	long long value;
+	int r, n, val_len;
+
+	tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
+	if (!tmp)
+		return -ENOMEM;
+	*presets = tmp;
+	cur = &(*presets)[*cnt];
+	cur->applied = false;
+
+	if (sscanf(expr, "%s = %s\n", var, val) != 2) {
+		fprintf(stderr, "Could not parse expression %s\n", expr);
+		return -EINVAL;
+	}
+
+	val_len = strlen(val);
+	errno = 0;
+	r = sscanf(val, "%lli %n", &value, &n);
+	if (r == 1 && n == val_len) {
+		if (errno == ERANGE) {
+			/* Try parsing as unsigned */
+			errno = 0;
+			r = sscanf(val, "%llu %n", &value, &n);
+			/* Try hex if not all chars consumed */
+			if (n != val_len) {
+				errno = 0;
+				r = sscanf(val, "%llx %n", &value, &n);
+			}
+		}
+		if (errno || r != 1 || n != val_len) {
+			fprintf(stderr, "Could not parse value %s\n", val);
+			return -EINVAL;
+		}
+		cur->ivalue = value;
+		cur->type = INTEGRAL;
+	} else {
+		/* If not a number, consider it enum value */
+		cur->svalue = strdup(val);
+		if (!cur->svalue)
+			return -ENOMEM;
+
+		cur->type = ENUMERATOR;
+	}
+
+	cur->name = strdup(var);
+	if (!cur->name)
+		return -ENOMEM;
+
+	(*cnt)++;
+	return 0;
+}
+
+static int append_var_preset_file(const char *filename)
+{
+	char buf[1024];
+	FILE *f;
+	int err = 0;
+
+	f = fopen(filename, "rt");
+	if (!f) {
+		err = -errno;
+		fprintf(stderr, "Failed to open presets in '%s': %s\n", filename, strerror(err));
+		return -EINVAL;
+	}
+
+	while (fscanf(f, " %1023[^\n]\n", buf) == 1) {
+		if (buf[0] == '\0' || buf[0] == '#')
+			continue;
+
+		err = append_var_preset(&env.presets, &env.npresets, buf);
+		if (err)
+			goto cleanup;
+	}
+
+cleanup:
+	fclose(f);
+	return err;
+}
+
+static bool is_signed_type(const struct btf_type *t)
+{
+	if (btf_is_int(t))
+		return btf_int_encoding(t) & BTF_INT_SIGNED;
+	if (btf_is_any_enum(t))
+		return btf_kflag(t);
+	return true;
+}
+
+static int enum_value_from_name(const struct btf *btf, const struct btf_type *t,
+				const char *evalue, long long *retval)
+{
+	if (btf_is_enum(t)) {
+		struct btf_enum *e = btf_enum(t);
+		int i, n = btf_vlen(t);
+
+		for (i = 0; i < n; ++i, ++e) {
+			const char *cur_name = btf__name_by_offset(btf, e->name_off);
+
+			if (strcmp(cur_name, evalue) == 0) {
+				*retval = e->val;
+				return 0;
+			}
+		}
+	} else if (btf_is_enum64(t)) {
+		struct btf_enum64 *e = btf_enum64(t);
+		int i, n = btf_vlen(t);
+
+		for (i = 0; i < n; ++i, ++e) {
+			const char *cur_name = btf__name_by_offset(btf, e->name_off);
+			__u64 value =  btf_enum64_value(e);
+
+			if (strcmp(cur_name, evalue) == 0) {
+				*retval = value;
+				return 0;
+			}
+		}
+	}
+	return -EINVAL;
+}
+
+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 bpf_map *map, struct btf_var_secinfo *sinfo,
+			  struct var_preset *preset)
+{
+	const struct btf_type *base_type;
+	void *ptr;
+	long long value = preset->ivalue;
+	size_t size;
+
+	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
+	if (!base_type) {
+		fprintf(stderr, "Could not resolve type %d\n", t->type);
+		return -EINVAL;
+	}
+	if (!is_preset_supported(base_type)) {
+		fprintf(stderr, "Setting value for type %s is not supported\n",
+			btf__name_by_offset(btf, base_type->name_off));
+		return -EINVAL;
+	}
+
+	if (preset->type == ENUMERATOR) {
+		if (btf_is_any_enum(base_type)) {
+			if (enum_value_from_name(btf, base_type, preset->svalue, &value) != 0) {
+				fprintf(stderr,
+					"Could not find integer value for enum element %s\n",
+					preset->svalue);
+				return -EINVAL;
+			}
+		} else {
+			fprintf(stderr, "Value %s is not supported for type %s\n",
+				preset->svalue, btf__name_by_offset(btf, base_type->name_off));
+			return -EINVAL;
+		}
+	}
+
+	/* Check if value fits into the target variable size */
+	if  (sinfo->size < sizeof(value)) {
+		bool is_signed = is_signed_type(base_type);
+		__u32 unsigned_bits = sinfo->size * 8 - (is_signed ? 1 : 0);
+		long long max_val = 1ll << unsigned_bits;
+
+		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,
+				is_signed ? -max_val : 0, max_val - 1);
+			return -EINVAL;
+		}
+	}
+
+	ptr = bpf_map__initial_value(map, &size);
+	if (!ptr || sinfo->offset + sinfo->size > size)
+		return -EINVAL;
+
+	if (__BYTE_ORDER == __LITTLE_ENDIAN) {
+		memcpy(ptr + sinfo->offset, &value, sinfo->size);
+	} else { /* __BYTE_ORDER == __BIG_ENDIAN */
+		__u8 src_offset = sizeof(value) - sinfo->size;
+
+		memcpy(ptr + sinfo->offset, (void *)&value + src_offset, sinfo->size);
+	}
+	return 0;
+}
+
+static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, int npresets)
+{
+	struct btf_var_secinfo *sinfo;
+	const char *sec_name;
+	const struct btf_type *t;
+	struct bpf_map *map;
+	struct btf *btf;
+	int i, j, k, n, cnt, err = 0;
+
+	if (npresets == 0)
+		return 0;
+
+	btf = bpf_object__btf(obj);
+	if (!btf)
+		return -EINVAL;
+
+	cnt = btf__type_cnt(btf);
+	for (i = 1; i != cnt; ++i) {
+		t = btf__type_by_id(btf, i);
+
+		if (!btf_is_datasec(t))
+			continue;
+
+		sinfo = btf_var_secinfos(t);
+		sec_name = btf__name_by_offset(btf, t->name_off);
+		map = bpf_object__find_map_by_name(obj, sec_name);
+		if (!map)
+			continue;
+
+		n = btf_vlen(t);
+		for (j = 0; j < n; ++j, ++sinfo) {
+			const struct btf_type *var_type = btf__type_by_id(btf, sinfo->type);
+			const char *var_name;
+
+			if (!btf_is_var(var_type))
+				continue;
+
+			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)
+					continue;
+
+				if (presets[k].applied) {
+					fprintf(stderr, "Variable %s is set more than once",
+						var_name);
+					return -EINVAL;
+				}
+
+				err = set_global_var(obj, btf, var_type, map, sinfo, presets + k);
+				if (err)
+					return err;
+
+				presets[k].applied = true;
+				break;
+			}
+		}
+	}
+	for (i = 0; i < npresets; ++i) {
+		if (!presets[i].applied) {
+			fprintf(stderr, "Global variable preset %s has not been applied\n",
+				presets[i].name);
+		}
+		presets[i].applied = false;
+	}
+	return err;
+}
+
 static int process_obj(const char *filename)
 {
 	const char *base_filename = basename(strdupa(filename));
@@ -1341,6 +1631,11 @@ static int process_obj(const char *filename)
 	if (prog_cnt == 1) {
 		prog = bpf_object__next_program(obj, NULL);
 		bpf_program__set_autoload(prog, true);
+		err = set_global_vars(obj, env.presets, env.npresets);
+		if (err) {
+			fprintf(stderr, "Failed to set global variables %d\n", err);
+			goto cleanup;
+		}
 		process_prog(filename, obj, prog);
 		goto cleanup;
 	}
@@ -1355,6 +1650,12 @@ static int process_obj(const char *filename)
 			goto cleanup;
 		}
 
+		err = set_global_vars(tobj, env.presets, env.npresets);
+		if (err) {
+			fprintf(stderr, "Failed to set global variables %d\n", err);
+			goto cleanup;
+		}
+
 		lprog = NULL;
 		bpf_object__for_each_program(tprog, tobj) {
 			const char *tprog_name = bpf_program__name(tprog);
@@ -2460,5 +2761,11 @@ int main(int argc, char **argv)
 		free(env.deny_filters[i].prog_glob);
 	}
 	free(env.deny_filters);
+	for (i = 0; i < env.npresets; ++i) {
+		free(env.presets[i].name);
+		if (env.presets[i].type == ENUMERATOR)
+			free(env.presets[i].svalue);
+	}
+	free(env.presets);
 	return -err;
 }
-- 
2.48.1

[PATCH bpf-next v5 2/2] selftests/bpf: introduce veristat test

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Introducing test for veristat, part of test_progs.
Test cases cover functionality of setting global variables in BPF
program.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/testing/selftests/bpf/Makefile          |   1 +
 .../selftests/bpf/prog_tests/test_veristat.c  | 139 ++++++++++++++++++
 .../selftests/bpf/progs/set_global_vars.c     |  47 ++++++
 tools/testing/selftests/bpf/test_progs.h      |   8 +
 4 files changed, 195 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_veristat.c
 create mode 100644 tools/testing/selftests/bpf/progs/set_global_vars.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 5dc9c84ed30f..abfb450c26bb 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -688,6 +688,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)			\
 			     $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ)		\
 			     $(RESOLVE_BTFIDS)				\
 			     $(TRUNNER_BPFTOOL)				\
+			     $(OUTPUT)/veristat				\
 			     | $(TRUNNER_BINARY)-extras
 	$$(call msg,BINARY,,$$@)
 	$(Q)$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) $$(LDFLAGS) -o $$@
diff --git a/tools/testing/selftests/bpf/prog_tests/test_veristat.c b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
new file mode 100644
index 000000000000..a95b42bf744a
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include <string.h>
+#include <stdio.h>
+
+#define __CHECK_STR(str, name)					    \
+	do {							    \
+		if (!ASSERT_HAS_SUBSTR(fix->output, (str), (name))) \
+			goto out;				    \
+	} while (0)
+
+struct fixture {
+	char tmpfile[80];
+	int fd;
+	char *output;
+	size_t sz;
+	char veristat[80];
+};
+
+static struct fixture *init_fixture(void)
+{
+	struct fixture *fix = malloc(sizeof(struct fixture));
+
+	/* for no_alu32 and cpuv4 veristat is in parent folder */
+	if (access("./veristat", F_OK) == 0)
+		strcpy(fix->veristat, "./veristat");
+	else if (access("../veristat", F_OK) == 0)
+		strcpy(fix->veristat, "../veristat");
+	else
+		PRINT_FAIL("Can't find veristat binary");
+
+	snprintf(fix->tmpfile, sizeof(fix->tmpfile), "/tmp/test_veristat.XXXXXX");
+	fix->fd = mkstemp(fix->tmpfile);
+	fix->sz = 1000000;
+	fix->output = malloc(fix->sz);
+	return fix;
+}
+
+static void teardown_fixture(struct fixture *fix)
+{
+	free(fix->output);
+	close(fix->fd);
+	remove(fix->tmpfile);
+	free(fix);
+}
+
+static void test_set_global_vars_succeeds(void)
+{
+	struct fixture *fix = init_fixture();
+
+	SYS(out,
+	    "%s set_global_vars.bpf.o"\
+	    " -G \"var_s64 = 0xf000000000000001\" "\
+	    " -G \"var_u64 = 0xfedcba9876543210\" "\
+	    " -G \"var_s32 = -0x80000000\" "\
+	    " -G \"var_u32 = 0x76543210\" "\
+	    " -G \"var_s16 = -32768\" "\
+	    " -G \"var_u16 = 60652\" "\
+	    " -G \"var_s8 = -128\" "\
+	    " -G \"var_u8 = 255\" "\
+	    " -G \"var_ea = EA2\" "\
+	    " -G \"var_eb = EB2\" "\
+	    " -G \"var_ec = EC2\" "\
+	    " -G \"var_b = 1\" "\
+	    "-vl2 > %s", fix->veristat, fix->tmpfile);
+
+	read(fix->fd, fix->output, fix->sz);
+	__CHECK_STR("_w=0xf000000000000001 ", "var_s64 = 0xf000000000000001");
+	__CHECK_STR("_w=0xfedcba9876543210 ", "var_u64 = 0xfedcba9876543210");
+	__CHECK_STR("_w=0x80000000 ", "var_s32 = -0x80000000");
+	__CHECK_STR("_w=0x76543210 ", "var_u32 = 0x76543210");
+	__CHECK_STR("_w=0x8000 ", "var_s16 = -32768");
+	__CHECK_STR("_w=0xecec ", "var_u16 = 60652");
+	__CHECK_STR("_w=128 ", "var_s8 = -128");
+	__CHECK_STR("_w=255 ", "var_u8 = 255");
+	__CHECK_STR("_w=11 ", "var_ea = EA2");
+	__CHECK_STR("_w=12 ", "var_eb = EB2");
+	__CHECK_STR("_w=13 ", "var_ec = EC2");
+	__CHECK_STR("_w=1 ", "var_b = 1");
+
+out:
+	teardown_fixture(fix);
+}
+
+static void test_set_global_vars_from_file_succeeds(void)
+{
+	struct fixture *fix = init_fixture();
+	char input_file[80];
+	const char *vars = "var_s16 = -32768\nvar_u16 = 60652";
+	int fd;
+
+	snprintf(input_file, sizeof(input_file), "/tmp/veristat_input.XXXXXX");
+	fd = mkstemp(input_file);
+	if (!ASSERT_GE(fd, 0, "valid fd"))
+		goto out;
+
+	write(fd, vars, strlen(vars));
+	syncfs(fd);
+	SYS(out, "%s set_global_vars.bpf.o -G \"@%s\" -vl2 > %s",
+	    fix->veristat, input_file, fix->tmpfile);
+	read(fix->fd, fix->output, fix->sz);
+	__CHECK_STR("_w=0x8000 ", "var_s16 = -32768");
+	__CHECK_STR("_w=0xecec ", "var_u16 = 60652");
+
+out:
+	close(fd);
+	remove(input_file);
+	teardown_fixture(fix);
+}
+
+static void test_set_global_vars_out_of_range(void)
+{
+	struct fixture *fix = init_fixture();
+
+	SYS_FAIL(out,
+		 "%s set_global_vars.bpf.o -G \"var_s32 = 2147483648\" -vl2 2> %s",
+		 fix->veristat, fix->tmpfile);
+
+	read(fix->fd, fix->output, fix->sz);
+	__CHECK_STR("is out of range [-2147483648; 2147483647]", "out of range");
+
+out:
+	teardown_fixture(fix);
+}
+
+void test_veristat(void)
+{
+	if (test__start_subtest("set_global_vars_succeeds"))
+		test_set_global_vars_succeeds();
+
+	if (test__start_subtest("set_global_vars_out_of_range"))
+		test_set_global_vars_out_of_range();
+
+	if (test__start_subtest("set_global_vars_from_file_succeeds"))
+		test_set_global_vars_from_file_succeeds();
+}
+
+#undef __CHECK_STR
diff --git a/tools/testing/selftests/bpf/progs/set_global_vars.c b/tools/testing/selftests/bpf/progs/set_global_vars.c
new file mode 100644
index 000000000000..9adb5ba4cd4d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/set_global_vars.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#include "bpf_experimental.h"
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+#include <stdbool.h>
+
+char _license[] SEC("license") = "GPL";
+
+enum Enum { EA1 = 0, EA2 = 11 };
+enum Enumu64 {EB1 = 0llu, EB2 = 12llu };
+enum Enums64 { EC1 = 0ll, EC2 = 13ll };
+
+const volatile __s64 var_s64 = -1;
+const volatile __u64 var_u64 = 0;
+const volatile __s32 var_s32 = -1;
+const volatile __u32 var_u32 = 0;
+const volatile __s16 var_s16 = -1;
+const volatile __u16 var_u16 = 0;
+const volatile __s8 var_s8 = -1;
+const volatile __u8 var_u8 = 0;
+const volatile enum Enum var_ea = EA1;
+const volatile enum Enumu64 var_eb = EB1;
+const volatile enum Enums64 var_ec = EC1;
+const volatile bool var_b = false;
+
+char arr[4] = {0};
+
+SEC("socket")
+int test_set_globals(void *ctx)
+{
+	volatile __s8 a;
+
+	a = var_s64;
+	a = var_u64;
+	a = var_s32;
+	a = var_u32;
+	a = var_s16;
+	a = var_u16;
+	a = var_s8;
+	a = var_u8;
+	a = var_ea;
+	a = var_eb;
+	a = var_ec;
+	a = var_b;
+	return a;
+}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 404d0d4915d5..870694f2a359 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -427,6 +427,14 @@ void hexdump(const char *prefix, const void *buf, size_t len);
 			goto goto_label;				\
 	})
 
+#define SYS_FAIL(goto_label, fmt, ...)					\
+	({								\
+		char cmd[1024];						\
+		snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__);		\
+		if (!ASSERT_NEQ(0, system(cmd), cmd))			\
+			goto goto_label;				\
+	})
+
 #define ALL_TO_DEV_NULL " >/dev/null 2>&1"
 
 #define SYS_NOFAIL(fmt, ...)						\
-- 
2.48.1

Re: [PATCH bpf-next v5 2/2] selftests/bpf: introduce veristat test

[Eduard Zingerman]

On Tue, 2025-02-25 at 16:31 +0000, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Introducing test for veristat, part of test_progs.
> Test cases cover functionality of setting global variables in BPF
> program.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---

Please don't drop acks.

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

[...]

Re: [PATCH bpf-next v5 1/2] selftests/bpf: implement setting global variables in veristat

[Eduard Zingerman]

On Tue, 2025-02-25 at 16:31 +0000, Mykyta Yatsenko wrote:

New warning for trying to set non-enums from enumerators works fine.
This still can be abused if numeric value outside of the supported
range is specified, but that's fine, I think.

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

[...]

> @@ -1292,6 +1320,268 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>  	return 0;
>  };
>  
> +static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr)
> +{
> +	void *tmp;
> +	struct var_preset *cur;
> +	char var[256], val[256];
> +	long long value;
> +	int r, n, val_len;
> +
> +	tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
> +	if (!tmp)
> +		return -ENOMEM;
> +	*presets = tmp;
> +	cur = &(*presets)[*cnt];
> +	cur->applied = false;
> +
> +	if (sscanf(expr, "%s = %s\n", var, val) != 2) {
> +		fprintf(stderr, "Could not parse expression %s\n", expr);
> +		return -EINVAL;
> +	}
> +
> +	val_len = strlen(val);
> +	errno = 0;
> +	r = sscanf(val, "%lli %n", &value, &n);
> +	if (r == 1 && n == val_len) {
> +		if (errno == ERANGE) {
> +			/* Try parsing as unsigned */
> +			errno = 0;
> +			r = sscanf(val, "%llu %n", &value, &n);
> +			/* Try hex if not all chars consumed */
> +			if (n != val_len) {
> +				errno = 0;
> +				r = sscanf(val, "%llx %n", &value, &n);

The discrepancy between %lli accepting 0x but %llu not accepting 0x is
annoying unfortunate. Does not look simpler then before, but let's
merge this already.

> +			}
> +		}
> +		if (errno || r != 1 || n != val_len) {
> +			fprintf(stderr, "Could not parse value %s\n", val);
> +			return -EINVAL;
> +		}
> +		cur->ivalue = value;
> +		cur->type = INTEGRAL;
> +	} else {
> +		/* If not a number, consider it enum value */
> +		cur->svalue = strdup(val);
> +		if (!cur->svalue)
> +			return -ENOMEM;
> +
> +		cur->type = ENUMERATOR;
> +	}
> +
> +	cur->name = strdup(var);
> +	if (!cur->name)
> +		return -ENOMEM;
> +
> +	(*cnt)++;
> +	return 0;
> +}

[...]

Re: [PATCH bpf-next v5 1/2] selftests/bpf: implement setting global variables in veristat

[Andrii Nakryiko]

On Tue, Feb 25, 2025 at 11:04 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Tue, 2025-02-25 at 16:31 +0000, Mykyta Yatsenko wrote:
>
> New warning for trying to set non-enums from enumerators works fine.
> This still can be abused if numeric value outside of the supported
> range is specified, but that's fine, I think.
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
> [...]
>
> > @@ -1292,6 +1320,268 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
> >       return 0;
> >  };
> >
> > +static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr)
> > +{
> > +     void *tmp;
> > +     struct var_preset *cur;
> > +     char var[256], val[256];
> > +     long long value;
> > +     int r, n, val_len;
> > +
> > +     tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
> > +     if (!tmp)
> > +             return -ENOMEM;
> > +     *presets = tmp;
> > +     cur = &(*presets)[*cnt];
> > +     cur->applied = false;
> > +
> > +     if (sscanf(expr, "%s = %s\n", var, val) != 2) {
> > +             fprintf(stderr, "Could not parse expression %s\n", expr);
> > +             return -EINVAL;
> > +     }
> > +
> > +     val_len = strlen(val);
> > +     errno = 0;
> > +     r = sscanf(val, "%lli %n", &value, &n);
> > +     if (r == 1 && n == val_len) {
> > +             if (errno == ERANGE) {
> > +                     /* Try parsing as unsigned */
> > +                     errno = 0;
> > +                     r = sscanf(val, "%llu %n", &value, &n);
> > +                     /* Try hex if not all chars consumed */
> > +                     if (n != val_len) {
> > +                             errno = 0;
> > +                             r = sscanf(val, "%llx %n", &value, &n);
>
> The discrepancy between %lli accepting 0x but %llu not accepting 0x is
> annoying unfortunate. Does not look simpler then before, but let's
> merge this already.

yeah... with sscanf() we at least get whitespace trimming at end, so
don't need to check that. But I agree, doesn't look better.

>
> > +                     }
> > +             }
> > +             if (errno || r != 1 || n != val_len) {
> > +                     fprintf(stderr, "Could not parse value %s\n", val);
> > +                     return -EINVAL;
> > +             }
> > +             cur->ivalue = value;
> > +             cur->type = INTEGRAL;
> > +     } else {
> > +             /* If not a number, consider it enum value */
> > +             cur->svalue = strdup(val);
> > +             if (!cur->svalue)
> > +                     return -ENOMEM;
> > +
> > +             cur->type = ENUMERATOR;
> > +     }
> > +
> > +     cur->name = strdup(var);
> > +     if (!cur->name)
> > +             return -ENOMEM;
> > +
> > +     (*cnt)++;
> > +     return 0;
> > +}
>
> [...]
>

Re: [PATCH bpf-next v5 1/2] selftests/bpf: implement setting global variables in veristat

[Andrii Nakryiko]

On Tue, Feb 25, 2025 at 1:56 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Feb 25, 2025 at 11:04 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Tue, 2025-02-25 at 16:31 +0000, Mykyta Yatsenko wrote:
> >
> > New warning for trying to set non-enums from enumerators works fine.
> > This still can be abused if numeric value outside of the supported
> > range is specified, but that's fine, I think.
> >
> > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> >
> > [...]
> >
> > > @@ -1292,6 +1320,268 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
> > >       return 0;
> > >  };
> > >
> > > +static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr)
> > > +{
> > > +     void *tmp;
> > > +     struct var_preset *cur;
> > > +     char var[256], val[256];
> > > +     long long value;
> > > +     int r, n, val_len;
> > > +
> > > +     tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
> > > +     if (!tmp)
> > > +             return -ENOMEM;
> > > +     *presets = tmp;
> > > +     cur = &(*presets)[*cnt];
> > > +     cur->applied = false;
> > > +
> > > +     if (sscanf(expr, "%s = %s\n", var, val) != 2) {
> > > +             fprintf(stderr, "Could not parse expression %s\n", expr);
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     val_len = strlen(val);
> > > +     errno = 0;
> > > +     r = sscanf(val, "%lli %n", &value, &n);
> > > +     if (r == 1 && n == val_len) {
> > > +             if (errno == ERANGE) {
> > > +                     /* Try parsing as unsigned */
> > > +                     errno = 0;
> > > +                     r = sscanf(val, "%llu %n", &value, &n);
> > > +                     /* Try hex if not all chars consumed */
> > > +                     if (n != val_len) {
> > > +                             errno = 0;
> > > +                             r = sscanf(val, "%llx %n", &value, &n);
> >
> > The discrepancy between %lli accepting 0x but %llu not accepting 0x is
> > annoying unfortunate. Does not look simpler then before, but let's
> > merge this already.
>
> yeah... with sscanf() we at least get whitespace trimming at end, so
> don't need to check that. But I agree, doesn't look better.

While applying I realized that space trimming is not necessary,
because earlier sscanf() does that for us. So I rewrote this back to
something that looks like the earlier version. I think it's cleaner. I
ended up with this implementation of append_var_preset:

static int append_var_preset(struct var_preset **presets, int *cnt,
const char *expr)
{
        void *tmp;
        struct var_preset *cur;
        char var[256], val[256], *val_end;
        long long value;
        int n;

        tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
        if (!tmp)
                return -ENOMEM;
        *presets = tmp;
        cur = &(*presets)[*cnt];
        memset(cur, 0, sizeof(*cur));
        (*cnt)++;

        if (sscanf(expr, "%s = %s %n", var, val, &n) != 2 || n !=
strlen(expr)) {
                fprintf(stderr, "Failed to parse expression '%s'\n", expr);
                return -EINVAL;
        }

        if (val[0] == '-' || isdigit(val[0])) {
                /* must be a number */
                errno = 0;
                value = strtoll(val, &val_end, 0);
                if (errno == ERANGE) {
                        errno = 0;
                        value = strtoull(val, &val_end, 0);
                }
                if (errno || *val_end != '\0') {
                        fprintf(stderr, "Failed to parse value '%s'\n", val);
                        return -EINVAL;
                }
                cur->ivalue = value;
                cur->type = INTEGRAL;
        } else {
                /* if not a number, consider it enum value */
                cur->svalue = strdup(val);
                if (!cur->svalue)
                        return -ENOMEM;
                cur->type = ENUMERATOR;
        }

        cur->name = strdup(var);
        if (!cur->name)
                return -ENOMEM;

        return 0;
}


I added memset() and early cnt++ to make error handling more correct
(we could have leaked cur->svalue if strdup(var) fails). And I added
%n to initial sscanf() to validate we parsed the entire string.

Oh, I also added back '-' or is_digit() detection, as otherwise users
can be confused due to a small typo in their integer (they'd get
something about enum, very confusing).

So that's what I applied before pushing to bpf-next. Yell if I messed
something up.

>
> >
> > > +                     }
> > > +             }
> > > +             if (errno || r != 1 || n != val_len) {
> > > +                     fprintf(stderr, "Could not parse value %s\n", val);
> > > +                     return -EINVAL;
> > > +             }
> > > +             cur->ivalue = value;
> > > +             cur->type = INTEGRAL;
> > > +     } else {
> > > +             /* If not a number, consider it enum value */
> > > +             cur->svalue = strdup(val);
> > > +             if (!cur->svalue)
> > > +                     return -ENOMEM;
> > > +
> > > +             cur->type = ENUMERATOR;
> > > +     }
> > > +
> > > +     cur->name = strdup(var);
> > > +     if (!cur->name)
> > > +             return -ENOMEM;
> > > +
> > > +     (*cnt)++;
> > > +     return 0;
> > > +}
> >
> > [...]
> >

Re: [PATCH bpf-next v5 0/2] selftests/bpf: implement setting global variables in veristat

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue, 25 Feb 2025 16:30:59 +0000 you wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> To better verify some complex BPF programs by veristat, it would be useful
> to preset global variables. This patch set implements this functionality
> and introduces tests for veristat.
> 
> v4->v5
>   * Rework parsing to use sscanf for integers
>   * Addressing nits
> 
> [...]

Here is the summary with links:
  - [bpf-next,v5,1/2] selftests/bpf: implement setting global variables in veristat
    https://git.kernel.org/bpf/bpf-next/c/e3c9abd0d14b
  - [bpf-next,v5,2/2] selftests/bpf: introduce veristat test
    https://git.kernel.org/bpf/bpf-next/c/3d1033caf056

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html