libbpf: Simplify BPF_CORE_READ_BITFIELD_PROBED usage

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.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Yonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/bpf/20191106201500.2582438-1-andriin@fb.com

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index 11461b2623b02c30a4c500f01fa7083b50a34b3d..7009dc90e012a5851d2d382022e3754f62751b2d 100644 (file)
--- 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 a381f8ac24199302249f025dc670779b02aa84c5..e466e3ab7de4562810377f76c8de6f46d483a8c4 100644 (file)
--- 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;
 }