On Mon, Sep 20, 2021 at 9:55 PM Dave Marchevsky [off-list ref] wrote:

On 9/20/21 7:43 PM, Andrii Nakryiko wrote:

quoted

Convert almost all SEC("xdp_blah") uses to strict SEC("xdp") to comply
with strict libbpf 1.0 logic of exact section name match for XDP program
types. There is only one exception, which is only tested through
iproute2 and defines multiple XDP programs within the same BPF object.
Given iproute2 still works in non-strict libbpf mode and it doesn't have
means to specify XDP programs by its name (not section name/title),
leave that single file alone for now until iproute2 gains lookup by
function/program name.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---

Aside from a checkpatch nit which you didn't cause, LGTM. Some general
comments follow as well, but aren't directly related to the patch.

Acked-by: Dave Marchevsky <redacted>

quoted

 tools/testing/selftests/bpf/progs/test_map_in_map.c         | 2 +-
 .../selftests/bpf/progs/test_tcp_check_syncookie_kern.c     | 2 +-
 tools/testing/selftests/bpf/progs/test_xdp.c                | 2 +-
 .../testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c | 2 +-
 .../selftests/bpf/progs/test_xdp_adjust_tail_shrink.c       | 4 +---
 tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c | 2 +-
 tools/testing/selftests/bpf/progs/test_xdp_link.c           | 2 +-
 tools/testing/selftests/bpf/progs/test_xdp_loop.c           | 2 +-
 tools/testing/selftests/bpf/progs/test_xdp_noinline.c       | 4 ++--
 .../selftests/bpf/progs/test_xdp_with_cpumap_helpers.c      | 4 ++--
 .../selftests/bpf/progs/test_xdp_with_devmap_helpers.c      | 4 ++--
 tools/testing/selftests/bpf/progs/xdp_dummy.c               | 2 +-
 tools/testing/selftests/bpf/progs/xdp_redirect_multi_kern.c | 4 ++--
 tools/testing/selftests/bpf/progs/xdping_kern.c             | 4 ++--
 tools/testing/selftests/bpf/test_tcp_check_syncookie.sh     | 2 +-
 tools/testing/selftests/bpf/test_xdp_redirect.sh            | 4 ++--
 tools/testing/selftests/bpf/test_xdp_redirect_multi.sh      | 2 +-
 tools/testing/selftests/bpf/test_xdp_veth.sh                | 4 ++--
 tools/testing/selftests/bpf/xdping.c                        | 6 +++---

Doesn't look like the test_...sh's here are run by the CI. Confirmed they
(as well as test_xdping.sh) all passed for me. My test VM isn't doing anything
special networking-wise, so maybe it's not too difficult to add these to CI.

Thanks for confirming. Yes, they are not run in CI, we only run
test_progs, test_verifier and test_maps. Instead of adding all those
small scripts to be run by CI, we are encouraging everyone to converge
on test_progs, because that's where we invest efforts to make it a
universal test runner for BPF needs. So I'd say let's convert them to
test_progs framework instead.

quoted

 19 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_map_in_map.c b/tools/testing/selftests/bpf/progs/test_map_in_map.c
index 1cfeb940cf9f..5f0e0bfc151e 100644
--- a/tools/testing/selftests/bpf/progs/test_map_in_map.c
+++ b/tools/testing/selftests/bpf/progs/test_map_in_map.c

@@ -23,7 +23,7 @@ struct {

[...]

quoted

      void *data_end = (void *)(long)xdp->data_end;

diff --git a/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_shrink.c b/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_shrink.c
index 22065a9cfb25..b7448253d135 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_shrink.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_shrink.c

@@ -9,9 +9,7 @@
 #include <linux/if_ether.h>
 #include <bpf/bpf_helpers.h>

-int _version SEC("version") = 1;
-

Didn't realize this was meant to specify kernel version for compat, and that
it no longer does anything anyways. Maybe this should be removed from all
selftests + examples to make this more obvious?

Yes, it should, but perhaps in a separate clean up series. Except I'd
leave it for BPF static linker testing, of course.

quoted

-SEC("xdp_adjust_tail_shrink")
+SEC("xdp")
 int _xdp_adjust_tail_shrink(struct xdp_md *xdp)
 {
      void *data_end = (void *)(long)xdp->data_end;

diff --git a/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
index b360ba2bd441..807bf895f42c 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c

Note, it's generally a good idea to trim irrelevant parts in your
reply making it easier to see one's replies among the wall of text,
especially outside of gmail UI.

[...]

quoted

diff --git a/tools/testing/selftests/bpf/xdping.c b/tools/testing/selftests/bpf/xdping.c
index 842d9155d36c..f9798ead20a9 100644
--- a/tools/testing/selftests/bpf/xdping.c
+++ b/tools/testing/selftests/bpf/xdping.c

@@ -178,9 +178,9 @@ int main(int argc, char **argv)
              return 1;
      }

-     main_prog = bpf_object__find_program_by_title(obj,
-                                                   server ? "xdpserver" :
-                                                            "xdpclient");
+     main_prog = bpf_object__find_program_by_name(obj,
+                                                   server ? "xdping_server" :
+                                                            "xdping_client");

checkpatch doesn't like the text alignment here, not that you changed it

yeah, me neither, but not worth re-spin just for this :)

quoted

      if (main_prog)
              prog_fd = bpf_program__fd(main_prog);
      if (!main_prog || prog_fd < 0) {