On Thu, Sep 10, 2020 at 03:34:26PM -0700, Andrii Nakryiko wrote:
SNIP
> > +
> > +void test_trace_ext(void)
> > +{
> > + struct test_trace_ext_tracing *skel_trace = NULL;
> > + struct test_trace_ext_tracing__bss *bss_trace;
> > + const char *file = "./test_pkt_md_access.o";
> > + struct test_trace_ext *skel_ext = NULL;
> > + struct test_trace_ext__bss *bss_ext;
> > + int err, prog_fd, ext_fd;
> > + struct bpf_object *obj;
> > + char buf[100];
> > + __u32 retval;
> > + __u64 len;
> > +
> > + err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
> > + if (CHECK_FAIL(err))
> > + return;
>
> We should avoid using bpf_prog_load() for new code. Can you please
> just skeleton instead? Or at least bpf_object__open_file()?
ok
>
> > +
> > + DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
> > + .attach_prog_fd = prog_fd,
> > + );
>
> DECLARE_LIBBPF_OPTS does declare a variable, so should be together
> with all the other variables above, otherwise some overly strict C89
> mode compiler will start complaining. You can assign
> `opts.attach_prog_fd = prog_fd;` outside of declaration. But I also
> don't think you need this one. Having .attach_prog_fd in open_opts is
> not great, because it's a per-program setting specified at bpf_object
> level. Would bpf_program__set_attach_target() work here?
right, I'll try it, it should be enough
SNIP
> > +
> > +cleanup:
> > + test_trace_ext__destroy(skel_ext);
> > + bpf_object__close(obj);
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/test_trace_ext.c b/tools/testing/selftests/bpf/progs/test_trace_ext.c
> > new file mode 100644
> > index 000000000000..a6318f6b52ee
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_trace_ext.c
> > @@ -0,0 +1,18 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2019 Facebook
> > +#include <linux/bpf.h>
> > +#include <stdbool.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_endian.h>
> > +#include <bpf/bpf_tracing.h>
> > +
> > +volatile __u64 ext_called = 0;
>
> nit: no need for volatile, global variables are not going anywhere;
> same below in two places
ok, thanks
jirka
