Thread (7 messages) 7 messages, 2 authors, 2021-12-21

Re: [PATCH v2 bpf-next 5/5] selftests/bpf: Update sockopt_sk test to the use bpf_set_retval

From: Andrii Nakryiko <hidden>
Date: 2021-12-21 23:14:02

On Wed, Dec 15, 2021 at 6:04 PM YiFei Zhu [off-list ref] wrote:
The tests would break without this patch, because at one point it calls
Please fix selftests in the same patch where kernel change breaks it
to ensure a proper "bisectability" of the code.
quoted hunk ↗ jump to hunk
  getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen)
This getsockopt receives the kernel-set -EINVAL. Prior to this patch
series, the eBPF getsockopt hook's -EPERM would override kernel's
-EINVAL, however, after this patch series, return 0's automatic
-EPERM will not; the eBPF prog has to explicitly bpf_set_retval(-EPERM)
if that is wanted.

I also removed the explicit mentions of EPERM in the comments in the
prog.

Signed-off-by: YiFei Zhu <redacted>
Reviewed-by: Stanislav Fomichev <redacted>
---
 .../selftests/bpf/prog_tests/sockopt_sk.c     |  2 +-
 .../testing/selftests/bpf/progs/sockopt_sk.c  | 32 +++++++++----------
 2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
index 4b937e5dbaca..164aa5020bf1 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
@@ -177,7 +177,7 @@ static int getsetsockopt(void)
        optlen = sizeof(buf.zc);
        errno = 0;
        err = getsockopt(fd, SOL_TCP, TCP_ZEROCOPY_RECEIVE, &buf, &optlen);
-       if (errno != EPERM) {
+       if (errno != EINVAL) {
                log_err("Unexpected getsockopt(TCP_ZEROCOPY_RECEIVE) err=%d errno=%d",
                        err, errno);
                goto err;
diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
index 79c8139b63b8..d0298dccedcd 100644
--- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
@@ -73,17 +73,17 @@ int _getsockopt(struct bpf_sockopt *ctx)
                 */

                if (optval + sizeof(struct tcp_zerocopy_receive) > optval_end)
-                       return 0; /* EPERM, bounds check */
+                       return 0; /* bounds check */

                if (((struct tcp_zerocopy_receive *)optval)->address != 0)
-                       return 0; /* EPERM, unexpected data */
+                       return 0; /* unexpected data */

                return 1;
        }

        if (ctx->level == SOL_IP && ctx->optname == IP_FREEBIND) {
                if (optval + 1 > optval_end)
-                       return 0; /* EPERM, bounds check */
+                       return 0; /* bounds check */

                ctx->retval = 0; /* Reset system call return value to zero */
@@ -96,24 +96,24 @@ int _getsockopt(struct bpf_sockopt *ctx)
                 * bytes of data.
                 */
                if (optval_end - optval != page_size)
-                       return 0; /* EPERM, unexpected data size */
+                       return 0; /* unexpected data size */

                return 1;
        }

        if (ctx->level != SOL_CUSTOM)
-               return 0; /* EPERM, deny everything except custom level */
+               return 0; /* deny everything except custom level */

        if (optval + 1 > optval_end)
-               return 0; /* EPERM, bounds check */
+               return 0; /* bounds check */

        storage = bpf_sk_storage_get(&socket_storage_map, ctx->sk, 0,
                                     BPF_SK_STORAGE_GET_F_CREATE);
        if (!storage)
-               return 0; /* EPERM, couldn't get sk storage */
+               return 0; /* couldn't get sk storage */

        if (!ctx->retval)
-               return 0; /* EPERM, kernel should not have handled
+               return 0; /* kernel should not have handled
                           * SOL_CUSTOM, something is wrong!
                           */
        ctx->retval = 0; /* Reset system call return value to zero */
@@ -152,7 +152,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
                /* Overwrite SO_SNDBUF value */

                if (optval + sizeof(__u32) > optval_end)
-                       return 0; /* EPERM, bounds check */
+                       return 0; /* bounds check */

                *(__u32 *)optval = 0x55AA;
                ctx->optlen = 4;
@@ -164,7 +164,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
                /* Always use cubic */

                if (optval + 5 > optval_end)
-                       return 0; /* EPERM, bounds check */
+                       return 0; /* bounds check */

                memcpy(optval, "cubic", 5);
                ctx->optlen = 5;
@@ -175,10 +175,10 @@ 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)
-                       return 0; /* EPERM, unexpected data size */
+                       return 0; /* unexpected data size */

                if (optval + 1 > optval_end)
-                       return 0; /* EPERM, bounds check */
+                       return 0; /* bounds check */

                /* Make sure we can trim the buffer. */
                optval[0] = 0;
@@ -189,21 +189,21 @@ int _setsockopt(struct bpf_sockopt *ctx)
                 * bytes of data.
                 */
                if (optval_end - optval != page_size)
-                       return 0; /* EPERM, unexpected data size */
+                       return 0; /* unexpected data size */

                return 1;
        }

        if (ctx->level != SOL_CUSTOM)
-               return 0; /* EPERM, deny everything except custom level */
+               return 0; /* deny everything except custom level */

        if (optval + 1 > optval_end)
-               return 0; /* EPERM, bounds check */
+               return 0; /* bounds check */

        storage = bpf_sk_storage_get(&socket_storage_map, ctx->sk, 0,
                                     BPF_SK_STORAGE_GET_F_CREATE);
        if (!storage)
-               return 0; /* EPERM, couldn't get sk storage */
+               return 0; /* couldn't get sk storage */

        storage->val = optval[0];
        ctx->optlen = -1; /* BPF has consumed this option, don't call kernel
--
2.34.1.173.g76aa8bc2d0-goog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help