On Sat, Mar 04, 2023 at 03:29:23PM -0800, Andrii Nakryiko wrote:
> 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.
Sounds good to me. Follow up is fine.
> > 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.
+1
