On Wed, Dec 15, 2021 at 6:04 PM YiFei Zhu <zhuyifei@xxxxxxxxxx> 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.
> 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 <zhuyifei@xxxxxxxxxx>
> Reviewed-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> ---
> .../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
>
