Re: [PATCHv3 bpf-next 1/9] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h

Jiri Olsa
2023-02-09T08:45:56Z

On Wed, Feb 08, 2023 at 04:20:13PM -0800, Andrii Nakryiko wrote:

SNIP

> > > diff --git a/tools/testing/selftests/bpf/progs/cb_refs.c b/tools/testing/selftests/bpf/progs/cb_refs.c
> > > index 7653df1bc787..823901c1b839 100644
> > > --- a/tools/testing/selftests/bpf/progs/cb_refs.c
> > > +++ b/tools/testing/selftests/bpf/progs/cb_refs.c
> > > @@ -2,6 +2,7 @@
> > > #include <vmlinux.h>
> > > #include <bpf/bpf_tracing.h>
> > > #include <bpf/bpf_helpers.h>
> > > +#include "bpf_testmod/bpf_testmod_kfunc.h"
> >
> > Feel free to ignore if you disagree, but here and elsewhere, should we
> > do this:
> >
> > #include <bpf_testmod/bpf_testmod_kfunc.h>
> >
> > rather than using #include "bpf_testmod/bpf_testmod_kfunc.h". Doesn't
> > matter much, but IMO it's just slightly more readable to use the <> to
> > show that we're relying on -I rather than expecting
> > bpf_testmod/bpf_testmod_kfunc.h to be found at a path relative to the
> > progs. #include "bpf_misc.h" makes more sense because it really is
> > located in the progs/ directory.
>
> We do <> for headers that are expected to be installed in the system
> (even if we cheat with -I sometimes). But in this case it's a local
> header, so using "" makes more sense to me. But shouldn't it be
> "../bpf_testmod/bpf_testmod_kfunc.h"?

I think we have -I<..selftests/bpf> so it works.. but right, we want
to show it's local header, so "../bpf_testmod/bpf_testmod_kfunc.h"
makes sense to me

jirka