On Wed, Jul 28, 2021 at 8:14 AM Stanislav Fomichev [off-list ref] wrote:

Rewrite to skel and ASSERT macros as well while we are at it.

Signed-off-by: Stanislav Fomichev <redacted>
---

Thanks! Bonus points for skeleton and ASSERT_XXX! ;)

In addition to Yonghong's comments, a few more below. Missed assert()s
require a new revision, unfortunately.

 tools/testing/selftests/bpf/Makefile          |   3 +-
 .../testing/selftests/bpf/prog_tests/netcnt.c |  93 +++++++++++
 tools/testing/selftests/bpf/test_netcnt.c     | 148 ------------------

Usually there is .gitignore clean up as well.

 3 files changed, 94 insertions(+), 150 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/netcnt.c
 delete mode 100644 tools/testing/selftests/bpf/test_netcnt.c

[...]

+
+       skel->links.bpf_nextcnt =
+               bpf_program__attach_cgroup(skel->progs.bpf_nextcnt, cg_fd);
+       if (!ASSERT_OK_PTR(skel->links.bpf_nextcnt,
+                          "attach_cgroup(bpf_nextcnt)"))
+               goto err;
+
+       if (system("which ping6 &>/dev/null") == 0)

see 4cda0c82a34b ("selftests/bpf: Use ping6 only if available in
tc_redirect"), we should probably add some system() + ping -6/ping6
wrapper into network_helpers.c and use that in at least all
test_progs' tests.

+               assert(!system("ping6 ::1 -c 10000 -f -q > /dev/null"));
+       else
+               assert(!system("ping -6 ::1 -c 10000 -f -q > /dev/null"));

no assert() please

+
+       map_fd = bpf_map__fd(skel->maps.netcnt);
+       if (!ASSERT_GE(map_fd, 0, "bpf_map__fd(netcnt)"))
+               goto err;
+
+       percpu_map_fd = bpf_map__fd(skel->maps.percpu_netcnt);
+       if (!ASSERT_GE(percpu_map_fd, 0, "bpf_map__fd(percpu_netcnt)"))
+               goto err;
+
+       if (!ASSERT_OK(bpf_map_get_next_key(map_fd, NULL, &key),
+                      "bpf_map_get_next_key"))

it's ok to use all 100 characters if that helps keeps simple function
invocations on the single line, so don't hesitate to do that

+               goto err;
+
+       if (!ASSERT_OK(bpf_map_lookup_elem(map_fd, &key, &netcnt),
+                      "bpf_map_lookup_elem(netcnt)"))
+               goto err;
+
+       if (!ASSERT_OK(bpf_map_lookup_elem(percpu_map_fd, &key,
+                                          &percpu_netcnt[0]),
+                      "bpf_map_lookup_elem(percpu_netcnt)"))
+               goto err;
+

[...]