Re: bpf selftests and page size

Hi, Andrii!

On Tue, Mar 2, 2021 at 7:08 AM Andrii Nakryiko
[off-list ref] wrote:

On Mon, Mar 1, 2021 at 1:02 AM Yauheni Kaliuta
[off-list ref] wrote:

quoted

Hi!

Bunch of bpf selftests actually depends of page size and has it
hardcoded to 4K. That causes failures if page shift is configured
to values other than 12. It looks as a known issue since for the
userspace parts sysconf(_SC_PAGE_SIZE) is used, but what would be
the correct way to export it to bpf programs?

Given PAGE_SIZE and PAGE_SHIFT are just #defines, the only way seems
to be to pass it from the user-space as a read-only variable.

I could not find a good example to attach to cgroup. Here is the
draft, could you point me to right direction?

diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c

b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
index d5b44b135c00..7932236a021e 100644

--- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c

@@ -1,8 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 #include "cgroup_helpers.h"
-
 #include <linux/tcp.h>
+#include "sockopt_sk.skel.h"

 #ifndef SOL_TCP
 #define SOL_TCP IPPROTO_TCP

@@ -191,60 +191,33 @@ static int getsetsockopt(void)
     return -1;
 }

-static int prog_attach(struct bpf_object *obj, int cgroup_fd, const

char *title)
-{
-    enum bpf_attach_type attach_type;
-    enum bpf_prog_type prog_type;
-    struct bpf_program *prog;
-    int err;
-
-    err = libbpf_prog_type_by_name(title, &prog_type, &attach_type);
-    if (err) {
-        log_err("Failed to deduct types for %s BPF program", title);
-        return -1;
-    }
-
-    prog = bpf_object__find_program_by_title(obj, title);
-    if (!prog) {
-        log_err("Failed to find %s BPF program", title);
-        return -1;
-    }
-
-    err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd,
-                  attach_type, 0);
-    if (err) {
-        log_err("Failed to attach %s BPF program", title);
-        return -1;
-    }
-
-    return 0;
-}
-
 static void run_test(int cgroup_fd)
 {
-    struct bpf_prog_load_attr attr = {
-        .file = "./sockopt_sk.o",
-    };
-    struct bpf_object *obj;
-    int ignored;
+    struct sockopt_sk *skel;
+    int prog_fd;
+    int duration = 0;
     int err;

-    err = bpf_prog_load_xattr(&attr, &obj, &ignored);
-    if (CHECK_FAIL(err))
-        return;
+    skel = sockopt_sk__open_and_load();
+    if (CHECK(!skel, "skel_load", "sockopt_sk skeleton failed\n"))
+        goto cleanup;
+
+    skel->bss->page_size = getpagesize();

-    err = prog_attach(obj, cgroup_fd, "cgroup/getsockopt");
-    if (CHECK_FAIL(err))
-        goto close_bpf_object;
+    prog_fd = bpf_program__fd(skel->progs._getsockopt);
+    err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_GETSOCKOPT, 0);
+    if (CHECK(err, "attach", "getsockopt attach failed: %d\n", err))
+        goto cleanup;

-    err = prog_attach(obj, cgroup_fd, "cgroup/setsockopt");
-    if (CHECK_FAIL(err))
-        goto close_bpf_object;
+    prog_fd = bpf_program__fd(skel->progs._setsockopt);
+    err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SETSOCKOPT, 0);
+    if (CHECK(err, "attach", "setsockopt attach failed: %d\n", err))
+        goto cleanup;

     CHECK_FAIL(getsetsockopt());

-close_bpf_object:
-    bpf_object__close(obj);
+cleanup:
+    sockopt_sk__destroy(skel);
 }

 void test_sockopt_sk(void)

diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c

b/tools/testing/selftests/bpf/progs/sockopt_sk.c
index d3597f81e6e9..f8b051589681 100644

--- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c

@@ -8,9 +8,7 @@
 char _license[] SEC("license") = "GPL";
 __u32 _version SEC("version") = 1;

-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
+int page_size; /* userspace should set it */

 #ifndef SOL_TCP
 #define SOL_TCP IPPROTO_TCP

@@ -41,7 +39,7 @@ int _getsockopt(struct bpf_sockopt *ctx)
          * let next BPF program in the cgroup chain or kernel
          * handle it.
          */
-        ctx->optlen = 0; /* bypass optval>PAGE_SIZE */
+        ctx->optlen = 0; /* bypass optval>page_size */
         return 1;
     }

@@ -86,11 +84,11 @@ int _getsockopt(struct bpf_sockopt *ctx)
         optval[0] = 0x55;
         ctx->optlen = 1;

-        /* Userspace buffer is PAGE_SIZE * 2, but BPF
-         * program can only see the first PAGE_SIZE
+        /* Userspace buffer is page_size * 2, but BPF
+         * program can only see the first page_size
          * bytes of data.
          */
-        if (optval_end - optval != PAGE_SIZE)
+        if (optval_end - optval != page_size)
             return 0; /* EPERM, unexpected data size */

         return 1;

@@ -131,7 +129,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
          * let next BPF program in the cgroup chain or kernel
          * handle it.
          */
-        ctx->optlen = 0; /* bypass optval>PAGE_SIZE */
+        ctx->optlen = 0; /* bypass optval>page_size */
         return 1;
     }

@@ -160,8 +158,8 @@ int _setsockopt(struct bpf_sockopt *ctx)
     }

     if (ctx->level == SOL_IP && ctx->optname == IP_FREEBIND) {
-        /* Original optlen is larger than PAGE_SIZE. */
-        if (ctx->optlen != PAGE_SIZE * 2)
+        /* Original optlen is larger than page_size. */
+        if (ctx->optlen != page_size * 2)
             return 0; /* EPERM, unexpected data size */

         if (optval + 1 > optval_end)

@@ -171,11 +169,11 @@ int _setsockopt(struct bpf_sockopt *ctx)
         optval[0] = 0;
         ctx->optlen = 1;

-        /* Usepace buffer is PAGE_SIZE * 2, but BPF
-         * program can only see the first PAGE_SIZE
+        /* Usepace buffer is page_size * 2, but BPF
+         * program can only see the first page_size
          * bytes of data.
          */
-        if (optval_end - optval != PAGE_SIZE)
+        if (optval_end - optval != page_size)
             return 0; /* EPERM, unexpected data size */

         return 1;

-- 
WBR, Yauheni