All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jakub@cloudflare.com>
To: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	kernel-team@cloudflare.com,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>
Subject: [PATCH bpf-next] selftests/bpf: Fix implementation-defined behavior in sk_lookup test
Date: Mon, 21 Feb 2022 19:03:58 +0100	[thread overview]
Message-ID: <20220221180358.169101-1-jakub@cloudflare.com> (raw)

Shifting 16-bit type by 16 bits is implementation-defined for BPF programs.
Don't rely on it in case it is causing the test failures we are seeing on
s390x z15 target.

Fixes: 2ed0dc5937d3 ("selftests/bpf: Cover 4-byte load from remote_port in bpf_sk_lookup")
Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---

I don't have a dev env for s390x/z15 set up yet, so can't definitely confirm the fix.
That said, it seems worth fixing either way.

 tools/testing/selftests/bpf/progs/test_sk_lookup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/test_sk_lookup.c b/tools/testing/selftests/bpf/progs/test_sk_lookup.c
index bf5b7caefdd0..7d47276a8964 100644
--- a/tools/testing/selftests/bpf/progs/test_sk_lookup.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_lookup.c
@@ -65,6 +65,7 @@ static const __u32 KEY_SERVER_A = SERVER_A;
 static const __u32 KEY_SERVER_B = SERVER_B;
 
 static const __u16 SRC_PORT = bpf_htons(8008);
+static const __u32 SRC_PORT_U32 = bpf_htonl(8008U << 16);
 static const __u32 SRC_IP4 = IP4(127, 0, 0, 2);
 static const __u32 SRC_IP6[] = IP6(0xfd000000, 0x0, 0x0, 0x00000002);
 
@@ -421,7 +422,7 @@ int ctx_narrow_access(struct bpf_sk_lookup *ctx)
 
 	/* Load from remote_port field with zero padding (backward compatibility) */
 	val_u32 = *(__u32 *)&ctx->remote_port;
-	if (val_u32 != bpf_htonl(bpf_ntohs(SRC_PORT) << 16))
+	if (val_u32 != SRC_PORT_U32)
 		return SK_DROP;
 
 	/* Narrow loads from local_port field. Expect DST_PORT. */
-- 
2.35.1


             reply	other threads:[~2022-02-21 18:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-21 18:03 Jakub Sitnicki [this message]
2022-02-21 21:39 ` [PATCH bpf-next] selftests/bpf: Fix implementation-defined behavior in sk_lookup test Ilya Leoshkevich
2022-02-22  0:43   ` Ilya Leoshkevich
2022-02-22  2:22     ` Ilya Leoshkevich
2022-02-22 14:53       ` Jakub Sitnicki
2022-02-22 17:42         ` Ilya Leoshkevich
2022-02-22 18:24           ` Jakub Sitnicki
2022-02-22 21:51             ` Ilya Leoshkevich
2022-02-25 10:01               ` Jakub Sitnicki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220221180358.169101-1-jakub@cloudflare.com \
    --to=jakub@cloudflare.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.