All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andriin@fb.com>
To: <bpf@vger.kernel.org>, <netdev@vger.kernel.org>, <ast@fb.com>,
	<daniel@iogearbox.net>
Cc: <andrii.nakryiko@gmail.com>, <kernel-team@fb.com>,
	Andrii Nakryiko <andriin@fb.com>, Yonghong Song <yhs@fb.com>
Subject: [PATCH bpf-next] libbpf: simplify BPF_CORE_READ_BITFIELD_PROBED usage
Date: Wed, 6 Nov 2019 12:15:00 -0800	[thread overview]
Message-ID: <20191106201500.2582438-1-andriin@fb.com> (raw)

Streamline BPF_CORE_READ_BITFIELD_PROBED interface to follow
BPF_CORE_READ_BITFIELD (direct) and BPF_CORE_READ, in general, i.e., just
return read result or 0, if underlying bpf_probe_read() failed.

In practice, real applications rarely check bpf_probe_read() result, because
it has to always work or otherwise it's a bug. So propagating internal
bpf_probe_read() error from this macro hurts usability without providing real
benefits in practice. This patch fixes the issue and simplifies usage,
noticeable even in selftest itself.

Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/bpf_core_read.h                 | 27 ++++++++-----------
 .../progs/test_core_reloc_bitfields_probed.c  | 19 +++++--------
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index 11461b2623b0..7009dc90e012 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -39,32 +39,27 @@ enum bpf_field_info_kind {
 #endif
 
 /*
- * Extract bitfield, identified by src->field, and put its value into u64
- * *res. All this is done in relocatable manner, so bitfield changes such as
+ * Extract bitfield, identified by s->field, and return its value as u64.
+ * All this is done in relocatable manner, so bitfield changes such as
  * signedness, bit size, offset changes, this will be handled automatically.
  * This version of macro is using bpf_probe_read() to read underlying integer
  * storage. Macro functions as an expression and its return type is
  * bpf_probe_read()'s return value: 0, on success, <0 on error.
  */
-#define BPF_CORE_READ_BITFIELD_PROBED(src, field, res) ({		      \
-	unsigned long long val;						      \
+#define BPF_CORE_READ_BITFIELD_PROBED(s, field) ({			      \
+	unsigned long long val = 0;					      \
 									      \
-	*res = 0;							      \
-	val = __CORE_BITFIELD_PROBE_READ(res, src, field);		      \
-	if (!val) {							      \
-		*res <<= __CORE_RELO(src, field, LSHIFT_U64);		      \
-		val = __CORE_RELO(src, field, RSHIFT_U64);		      \
-		if (__CORE_RELO(src, field, SIGNED))			      \
-			*res = ((long long)*res) >> val;		      \
-		else							      \
-			*res = ((unsigned long long)*res) >> val;	      \
-		val = 0;						      \
-	}								      \
+	__CORE_BITFIELD_PROBE_READ(&val, s, field);			      \
+	val <<= __CORE_RELO(s, field, LSHIFT_U64);			      \
+	if (__CORE_RELO(s, field, SIGNED))				      \
+		val = ((long long)val) >> __CORE_RELO(s, field, RSHIFT_U64);  \
+	else								      \
+		val = val >> __CORE_RELO(s, field, RSHIFT_U64);		      \
 	val;								      \
 })
 
 /*
- * Extract bitfield, identified by src->field, and return its value as u64.
+ * Extract bitfield, identified by s->field, and return its value as u64.
  * This version of macro is using direct memory reads and should be used from
  * BPF program types that support such functionality (e.g., typed raw
  * tracepoints).
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_bitfields_probed.c b/tools/testing/selftests/bpf/progs/test_core_reloc_bitfields_probed.c
index a381f8ac2419..e466e3ab7de4 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_bitfields_probed.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_bitfields_probed.c
@@ -37,11 +37,6 @@ struct core_reloc_bitfields_output {
 	int64_t		s32;
 };
 
-#define TRANSFER_BITFIELD(in, out, field)				\
-	if (BPF_CORE_READ_BITFIELD_PROBED(in, field, &res))		\
-		return 1;						\
-	out->field = res
-
 SEC("raw_tracepoint/sys_enter")
 int test_core_bitfields(void *ctx)
 {
@@ -49,13 +44,13 @@ int test_core_bitfields(void *ctx)
 	struct core_reloc_bitfields_output *out = (void *)&data.out;
 	uint64_t res;
 
-	TRANSFER_BITFIELD(in, out, ub1);
-	TRANSFER_BITFIELD(in, out, ub2);
-	TRANSFER_BITFIELD(in, out, ub7);
-	TRANSFER_BITFIELD(in, out, sb4);
-	TRANSFER_BITFIELD(in, out, sb20);
-	TRANSFER_BITFIELD(in, out, u32);
-	TRANSFER_BITFIELD(in, out, s32);
+	out->ub1 = BPF_CORE_READ_BITFIELD_PROBED(in, ub1);
+	out->ub2 = BPF_CORE_READ_BITFIELD_PROBED(in, ub2);
+	out->ub7 = BPF_CORE_READ_BITFIELD_PROBED(in, ub7);
+	out->sb4 = BPF_CORE_READ_BITFIELD_PROBED(in, sb4);
+	out->sb20 = BPF_CORE_READ_BITFIELD_PROBED(in, sb20);
+	out->u32 = BPF_CORE_READ_BITFIELD_PROBED(in, u32);
+	out->s32 = BPF_CORE_READ_BITFIELD_PROBED(in, s32);
 
 	return 0;
 }
-- 
2.17.1


             reply	other threads:[~2019-11-06 20:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-06 20:15 Andrii Nakryiko [this message]
2019-11-06 21:18 ` [PATCH bpf-next] libbpf: simplify BPF_CORE_READ_BITFIELD_PROBED usage Yonghong Song
2019-11-06 21:58   ` Alexei Starovoitov
2019-11-06 22:02     ` Yonghong Song

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=20191106201500.2582438-1-andriin@fb.com \
    --to=andriin@fb.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=yhs@fb.com \
    /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.