On Tue, Feb 07, 2023 at 08:38:09AM -0600, David Vernet wrote:
> On Fri, Feb 03, 2023 at 05:23:29PM +0100, Jiri Olsa wrote:
> > Moving test_progs helpers to testing_helpers object so they can be
> > used from test_verifier in following changes.
> >
> > Also adding missing ifndef header guard to testing_helpers.h header.
> >
> > Using stderr instead of env.stderr because un/load_bpf_testmod helpers
> > will be used outside test_progs. Also at the point of calling them
> > in test_progs the std files are not hijacked yet and stderr is the
> > same as env.stderr.
>
> Makes sense. Possibly something to clean up at another time but given
> that we were being inconsistent with env.stdout and env.stderr in
> load_bpf_testmod() in the first place, this seems totally fine.
ok
>
> Acked-by: David Vernet <void@xxxxxxxxxxxxx>
>
> Left one question about kern_sync_rcu() below that need not block this
> patch series, and can be addressed in a follow-up if it's even relevant.
SNIP
> > +void unload_bpf_testmod(bool verbose)
> > +{
> > + if (kern_sync_rcu())
> > + fprintf(stderr, "Failed to trigger kernel-side RCU sync!\n");
>
> I realize there's no behavior change here, but out of curiosity, do you
> know why we need a synchronize_rcu() here? In general this feels kind of
> sketchy, and like something we should just put in bpf_testmod_exit() if
> it's really needed for something in the kernel.
it's explained in here:
635599bace25 selftests/bpf: Sync RCU before unloading bpf_testmod
If some of the subtests use module BTFs through ksyms, they will cause
bpf_prog to take a refcount on bpf_testmod module, which will prevent it from
successfully unloading. Module's refcnt is decremented when bpf_prog is freed,
which generally happens in RCU callback. So we need to trigger
syncronize_rcu() in the kernel, which can be achieved nicely with
membarrier(MEMBARRIER_CMD_SHARED) or membarrier(MEMBARRIER_CMD_GLOBAL) syscall.
So do that in kernel_sync_rcu() and make it available to other test inside the
test_progs. This synchronize_rcu() is called before attempting to unload
bpf_testmod.
jirka
>
> > + if (delete_module("bpf_testmod", 0)) {
> > + if (errno == ENOENT) {
> > + if (verbose)
> > + fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
> > + return;
> > + }
> > + fprintf(stderr, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
> > + return;
> > + }
> > + if (verbose)
> > + fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
> > +}
> > +
SNIP
