Re: [PATCH bpf-next 2/2] selftests/bpf: exercise veristat filtering logic in a selftest

> Test cases for veristat file/prog name filtering logic.
> Check various formulations for any (*foo*), file (*foo*/),
> prog (/*bar*) and file/prog (*foo*/bar) filters, alongside
> errorneous filters and mixed allow/deny filter expressions.

This isn't a bug, but there's a typo: "errorneous" should be "erroneous".
Also, is the prog filter form meant to be "/bar" rather than "/*bar*" to
match the cases in the table?

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_veristat.c b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
> index 9aff08ac55c0..e1eda4ae755e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_veristat.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
> @@ -37,6 +37,13 @@ static struct fixture *init_fixture(void)
>  	return fix;
>  }
>
> +static void read_output(struct fixture *fix)
> +{
> +	ssize_t len = pread(fix->fd, fix->output, fix->sz - 1, 0);
> +
> +	fix->output[len < 0 ? 0 : len] = 0;
> +}
> +

This isn't a bug, but since read_output() also terminates the buffer, would
it make sense to switch the existing read()/pread() call sites in this file
over to it as well? The new helper is safer than the bare reads used in the
other subtests.

> +/*
> + * Name filter tests below run veristat on veristat_foo.bpf.o and
> + * veristat_bar.bpf.o, both defining programs 'foo', 'bar' and 'buz'.
> + * Every entry describes a single (filters, file, prog) combination and
> + * tells whether that program is expected in the veristat output:
> + * 'true' if it is, 'false' if it is not and -1 if veristat is expected
> + * to reject the filter.
> + */
> +#define FILTER_OBJS "veristat_foo.bpf.o veristat_bar.bpf.o"
> +
> +static const struct name_filter_case {
> +	const char *filters;
> +	const char *file;
> +	const char *prog;
> +	int included;
> +} name_filter_cases[] = {
> +	/* no filters, every program is processed */
> +	{ "",			"foo", "foo", true  },
> +	{ "",			"foo", "bar", true  },
> +	{ "",			"foo", "buz", true  },
> +	{ "",			"bar", "foo", true  },
> +	{ "",			"bar", "bar", true  },
> +	{ "",			"bar", "buz", true  },
> +	/* deny filters */
> +	{ "-f '!*foo*'",	"foo", "bar", false },
> +	{ "-f '!*foo*'",	"bar", "foo", false },

A subsystem pattern flags this as potentially concerning: the table rows for
'-f !*foo*' both assert 'false' (row should be absent), but there is no row
that asserts 'true' (row should be present) for the same filter. Every other
filter with 'false' rows also has a 'true' row for the same filter string.

For example:
  '!*foo*/bar' has (foo,buz)/(bar,bar) true
  '!*foo*/' has (bar,bar) true
  '!/bar' has (foo,foo) true
  '*foo*/bar' has (foo,bar) true

Adding one positive row for '-f !*foo*', such as { "-f '!*foo*'", "bar",
"bar", true }, would make this filter's coverage consistent with the rest
of the table. A regression that made the any-glob deny form reject everything
would still satisfy both existing rows plus ASSERT_OK(err), because absence
of a row is all that is checked.

The expectations themselves are correct (they match veristat's implementation
and the changelog table of the preceding fix commit), so this is a
test-completeness observation rather than a functional bug.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/veristat_bar.c b/tools/testing/selftests/bpf/progs/veristat_bar.c
> new file mode 100644
> index 000000000000..83d2a2a1dfc9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/veristat_bar.c
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include "veristat_foo.c"

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31521742364