On Sat, Aug 7, 2021 at 12:50 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Aug 5, 2021 at 9:10 AM Jussi Maki <joamaki@xxxxxxxxx> wrote:
> >
> > Add a test suite to test XDP bonding implementation
> > over a pair of veth devices.
> >
> > Signed-off-by: Jussi Maki <joamaki@xxxxxxxxx>
> > ---
> > .../selftests/bpf/prog_tests/xdp_bonding.c | 520 ++++++++++++++++++
> > 1 file changed, 520 insertions(+)
> >
>
> I don't pretend to understand what's going on in this selftests, but
> it looks good from the generic selftest standpoint. One and half small
> issues below, please double-check (and probably fix the fd close
> issue).
Thanks for the reviews!
> > + if (xdp_attach(skeletons,
> > + skeletons->xdp_redirect_multi_kern->progs.xdp_redirect_map_multi_prog,
> > + "bond2"))
> > + goto out;
> > +
> > + restore_root_netns();
>
> the "goto out" below might call restore_root_netns() again, is that ok?
Yep that's fine.
> > + if (!test__start_subtest("xdp_bonding_redirect_multi"))
> > + test_xdp_bonding_redirect_multi(&skeletons);
> > +
> > +out:
> > + xdp_dummy__destroy(skeletons.xdp_dummy);
> > + xdp_tx__destroy(skeletons.xdp_tx);
> > + xdp_redirect_multi_kern__destroy(skeletons.xdp_redirect_multi_kern);
> > +
> > + libbpf_set_print(old_print_fn);
> > + if (root_netns_fd)
>
> technically, fd could be 0, so for fds we have if (fd >= 0)
> everywhere. Also, if open() above fails, root_netns_fd will be -1 and
> you'll still attempt to close it.
Good catch. Daniel, could you fix this when applying to be "if
(root_netns_fd >= 0)"?
