On Tue, May 11, 2021 at 9:05 PM Alexei Starovoitov <ast@xxxxxx> wrote:
>
> On 5/11/21 3:45 PM, Andrii Nakryiko wrote:
> > On Fri, May 7, 2021 at 8:48 PM Alexei Starovoitov
> > <alexei.starovoitov@xxxxxxxxx> wrote:
> >>
> >> From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >>
> >> Improve selftest to check that btf_load is working from bpf program.
> >>
> >> Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> >> ---
> >> tools/testing/selftests/bpf/progs/syscall.c | 48 +++++++++++++++++++++
> >> 1 file changed, 48 insertions(+)
> >>
> >
> > [...]
> >
> >> SEC("syscall")
> >> int bpf_prog(struct args *ctx)
> >> {
> >> @@ -33,6 +73,8 @@ int bpf_prog(struct args *ctx)
> >> .map_type = BPF_MAP_TYPE_HASH,
> >> .key_size = 8,
> >> .value_size = 8,
> >> + .btf_key_type_id = 1,
> >> + .btf_value_type_id = 2,
> >> };
> >> static union bpf_attr map_update_attr = { .map_fd = 1, };
> >> static __u64 key = 12;
> >> @@ -43,7 +85,13 @@ int bpf_prog(struct args *ctx)
> >> };
> >> int ret;
> >>
> >> + ret = btf_load();
> >
> > Maybe let's move patch #11 (bpf_sys_close() helper) in front of these
> > selftests and call bpf_sys_close() appropriately on error and (if
> > success) after map is created?
>
> Interesting idea. I took a stab at it, but it's not unit-test like.
> That bpf_sys_close is going to be used assuming it's working.
> I'd rather add explicit test for bpf_sys_close eventually
> instead of mixing the two.
> Since your concern is fd leak I've added btf_fd to context instead
> and added explicit close() in user space.
Ok, that's fine. And yes, my concern was FD leak. But also having BPF
selftests that demonstrates how, when you create FD in a "syscall" BPF
program, such FDs can be closed inside "syscall" program as well.
