On Sat, Mar 4, 2023 at 12:39 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Mar 02, 2023 at 03:50:14PM -0800, Andrii Nakryiko wrote:
> > +
> > +#ifdef REAL_TEST
>
> Looks like REAL_TEST is never set.
>
> and all bpf_printk-s in tests are never executed, because the test are 'load-only'
> to check the verifier?
>
> It looks like all of them can be run (once printks are removed and converted to if-s).
> That would nicely complement patch 17 runners.
>
Yes, it's a bit sloppy. I used these also as manual tests during
development. I did have an ad-hoc test that attaches and triggers
these programs. And I just manually looked at printk output in
trace_pipe to confirm it does actually work as expected.
And I felt sorry to drop all that, so just added that REAL_TEST hack
to make program code simpler (no extra states for those pid
conditions), it was simpler to debug verification failures, less
states to consider.
I did try to quickly extend RUN_TESTS with the ability to specify a
callback that will be called on success, but it's not trivial if we
want to preserve skeletons, so I abandoned that effort, trying to save
a bit of time. I still want to have RUN_TESTS with ability to specify
callback in the form of:
static void on_success(struct <my_skeleton_type> *skel, struct
bpf_program *prog) {
...
}
but it needs more thought and macro magic (or something else), so I
postponed it and wrote simple number iterator tests in patch #17.
> It can be a follow up, of course.
yep, let's keep bpf_printks, as they currently serve as consumers of
variables, preventing the compiler from optimizing loops too much.
This shouldn't be a problem for verification-only kind of tests. And
then with RUN_TESTS() additions, we can actually start executing this.
>
> Great stuff overall!
Thanks!
