On Mon, Jan 11, 2021 at 9:34 AM Alan Maguire <alan.maguire(a)oracle.com> wrote:
>
> libbpf already supports a "dumper" API for dumping type information,
> but there is currently no support for dumping typed _data_ via libbpf.
> However this functionality does exist in the kernel, in part to
> facilitate the bpf_snprintf_btf() helper which dumps a string
> representation of the pointer passed in utilizing the BTF type id
> of the data pointed to. For example, the pair of a pointer to
> a "struct sk_buff" and the BTF type id of "struct sk_buff" can be
> used.
>
> Here the kernel code is generalized into btf_show_common.c. For the
> most part, code is identical for userspace and kernel, beyond a few API
> differences and missing functions. The only significant differences are
>
> - the "safe copy" logic used by the kernel to ensure we do not induce a
> crash during BPF operation; and
> - the BTF seq file support that is kernel-only.
>
> The mechanics are to maintain identical btf_show_common.c files in
> kernel/bpf and tools/lib/bpf , and a common header btf_common.h in
> include/linux/ and tools/lib/bpf/. This file duplication seems to
> be the common practice with duplication between kernel and tools/
> so it's the approach taken here.
>
> The common code approach could likely be explored further, but here
> the minimum common code required to support BTF show functionality is
> used.
>
I don't think this approach will work. libbpf and kernel have
considerably different restrictions and styles, I don't think it's
appropriate to take kernel code and try to fit it into libbpf almost
as is, with a bunch of #defines. It would be much cleaner, simpler,
and more maintainable to just re-implement core logic for libbpf, IMO.
> Currently the only "show" function for userspace is to write the
> representation of the typed data to a string via
>
> LIBBPF_API int
> btf__snprintf(struct btf *btf, char *buf, int len, __u32 id, void *obj,
> __u64 flags);
>
> ...but other approaches could be pursued including printf()-based
> show, or even a callback mechanism could be supported to allow
> user-defined show functions.
>
It's strange that you saw btf_dump APIs, and yet decided to go with
this API instead. snprintf() is not a natural "method" of struct btf.
Using char buffer as an output is overly restrictive and inconvenient.
It's appropriate for kernel and BPF program due to their restrictions,
but there is no need to cripple libbpf APIs for that. I think it
should follow btf_dump APIs with custom callback so that it's easy to
just printf() everything, but also user can create whatever elaborate
mechanism they need and that fits their use case.
Code reuse is not the ultimate goal, it should facilitate
maintainability, not harm it. There are times where sharing code
introduces unnecessary coupling and maintainability issues. And I
think this one is a very obvious case of that.
See below a few comments as well. But overall it's really hard to
review such a humongous patch, of course. So I so far just skimmed
through it.
> Here's an example usage, storing a string representation of
> struct sk_buff *skb in buf:
>
> struct btf *btf = libbpf_find_kernel_btf();
> char buf[8192];
> __s32 skb_id;
>
> skb_id = btf__find_by_name_kind(btf, "sk_buff", BTF_KIND_STRUCT);
> if (skb_id < 0)
> fprintf(stderr, "no skbuff, err %d\n", skb_id);
> else
> btf__snprintf(btf, buf, sizeof(buf), skb_id, skb, 0);
>
> Suggested-by: Alexei Starovoitov <ast(a)kernel.org>
> Signed-off-by: Alan Maguire <alan.maguire(a)oracle.com>
> ---
> include/linux/btf.h | 121 +---
> include/linux/btf_common.h | 286 +++++++++
> kernel/bpf/Makefile | 2 +-
> kernel/bpf/arraymap.c | 1 +
> kernel/bpf/bpf_struct_ops.c | 1 +
> kernel/bpf/btf.c | 1215 +-------------------------------------
> kernel/bpf/btf_show_common.c | 1218 +++++++++++++++++++++++++++++++++++++++
> kernel/bpf/core.c | 1 +
> kernel/bpf/hashtab.c | 1 +
> kernel/bpf/local_storage.c | 1 +
> kernel/bpf/verifier.c | 1 +
> kernel/trace/bpf_trace.c | 1 +
> tools/lib/bpf/Build | 2 +-
> tools/lib/bpf/btf.h | 7 +
> tools/lib/bpf/btf_common.h | 286 +++++++++
> tools/lib/bpf/btf_show_common.c | 1218 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.map | 1 +
> 17 files changed, 3044 insertions(+), 1319 deletions(-)
> create mode 100644 include/linux/btf_common.h
> create mode 100644 kernel/bpf/btf_show_common.c
> create mode 100644 tools/lib/bpf/btf_common.h
> create mode 100644 tools/lib/bpf/btf_show_common.c
>
[...]
> +/* For kernel u64 is long long unsigned int... */
> +#define FMT64 "ll"
> +
> +#else
> +/* ...while for userspace it is long unsigned int. These definitions avoid
> + * format specifier warnings.
> + */
that's not true, it depends on the architecture
> +#define FMT64 "l"
> +
> +/* libbpf names differ slightly to in-kernel function names. */
> +#define btf_type_by_id btf__type_by_id
> +#define btf_name_by_offset btf__name_by_offset
> +#define btf_str_by_offset btf__str_by_offset
> +#define btf_resolve_size btf__resolve_size
ugh... good luck navigating the code in libbpf....
> +
> +#endif /* __KERNEL__ */
> +/*
> + * Options to control show behaviour.
> + * - BTF_SHOW_COMPACT: no formatting around type information
> + * - BTF_SHOW_NONAME: no struct/union member names/types
> + * - BTF_SHOW_PTR_RAW: show raw (unobfuscated) pointer values;
> + * equivalent to %px.
> + * - BTF_SHOW_ZERO: show zero-valued struct/union members; they
> + * are not displayed by default
> + * - BTF_SHOW_UNSAFE: skip use of bpf_probe_read() to safely read
> + * data before displaying it.
> + */
> +#define BTF_SHOW_COMPACT BTF_F_COMPACT
> +#define BTF_SHOW_NONAME BTF_F_NONAME
> +#define BTF_SHOW_PTR_RAW BTF_F_PTR_RAW
> +#define BTF_SHOW_ZERO BTF_F_ZERO
> +#define BTF_SHOW_UNSAFE (1ULL << 4)
this (or some subset of them) should be done as opts struct's bool
fields for libbpf
> +
> +/*
> + * Copy len bytes of string representation of obj of BTF type_id into buf.
> + *
> + * @btf: struct btf object
> + * @type_id: type id of type obj points to
> + * @obj: pointer to typed data
> + * @buf: buffer to write to
> + * @len: maximum length to write to buf
> + * @flags: show options (see above)
> + *
> + * Return: length that would have been/was copied as per snprintf, or
> + * negative error.
> + */
> +int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
> + char *buf, int len, u64 flags);
> +
> +#define for_each_member(i, struct_type, member) \
> + for (i = 0, member = btf_type_member(struct_type); \
> + i < btf_type_vlen(struct_type); \
> + i++, member++)
> +
> +#define for_each_vsi(i, datasec_type, member) \
> + for (i = 0, member = btf_type_var_secinfo(datasec_type); \
> + i < btf_type_vlen(datasec_type); \
> + i++, member++)
> +
> +static inline bool btf_type_is_ptr(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_PTR;
> +}
> +
> +static inline bool btf_type_is_int(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_INT;
> +}
> +
> +static inline bool btf_type_is_small_int(const struct btf_type *t)
> +{
> + return btf_type_is_int(t) && t->size <= sizeof(u64);
> +}
> +
> +static inline bool btf_type_is_enum(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_ENUM;
> +}
> +
> +static inline bool btf_type_is_typedef(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF;
> +}
> +
> +static inline bool btf_type_is_func(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC;
> +}
> +
> +static inline bool btf_type_is_func_proto(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC_PROTO;
> +}
> +
> +static inline bool btf_type_is_var(const struct btf_type *t)
> +{
> + return BTF_INFO_KIND(t->info) == BTF_KIND_VAR;
> +}
> +
> +/* union is only a special case of struct:
> + * all its offsetof(member) == 0
> + */
> +static inline bool btf_type_is_struct(const struct btf_type *t)
> +{
> + u8 kind = BTF_INFO_KIND(t->info);
> +
> + return kind == BTF_KIND_STRUCT || kind == BTF_KIND_UNION;
> +}
> +
> +static inline bool btf_type_is_modifier(const struct btf_type *t)
> +{
> + /* Some of them is not strictly a C modifier
> + * but they are grouped into the same bucket
> + * for BTF concern:
> + * A type (t) that refers to another
> + * type through t->type AND its size cannot
> + * be determined without following the t->type.
> + *
> + * ptr does not fall into this bucket
> + * because its size is always sizeof(void *).
> + */
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_TYPEDEF:
> + case BTF_KIND_VOLATILE:
> + case BTF_KIND_CONST:
> + case BTF_KIND_RESTRICT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static inline
> +const struct btf_type *btf_type_skip_modifiers(const struct btf *btf,
> + u32 id, u32 *res_id)
> +{
> + const struct btf_type *t = btf_type_by_id(btf, id);
> +
> + while (btf_type_is_modifier(t)) {
> + id = t->type;
> + t = btf_type_by_id(btf, t->type);
> + }
> +
> + if (res_id)
> + *res_id = id;
> +
> + return t;
> +}
> +
> +static inline u32 btf_type_int(const struct btf_type *t)
> +{
> + return *(u32 *)(t + 1);
> +}
> +
> +static inline const struct btf_array *btf_type_array(const struct btf_type *t)
> +{
> + return (const struct btf_array *)(t + 1);
> +}
> +
> +static inline const struct btf_enum *btf_type_enum(const struct btf_type *t)
> +{
> + return (const struct btf_enum *)(t + 1);
> +}
> +
> +static inline const struct btf_var *btf_type_var(const struct btf_type *t)
> +{
> + return (const struct btf_var *)(t + 1);
> +}
> +
> +static inline u16 btf_type_vlen(const struct btf_type *t)
> +{
> + return BTF_INFO_VLEN(t->info);
> +}
> +
> +static inline u16 btf_func_linkage(const struct btf_type *t)
> +{
> + return BTF_INFO_VLEN(t->info);
> +}
> +
> +/* size can be used */
> +static inline bool btf_type_has_size(const struct btf_type *t)
> +{
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_INT:
> + case BTF_KIND_STRUCT:
> + case BTF_KIND_UNION:
> + case BTF_KIND_ENUM:
> + case BTF_KIND_DATASEC:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static inline const struct btf_member *btf_type_member(const struct btf_type *t)
> +{
> + return (const struct btf_member *)(t + 1);
> +}
> +
> +static inline const struct btf_var_secinfo *btf_type_var_secinfo(
> + const struct btf_type *t)
> +{
> + return (const struct btf_var_secinfo *)(t + 1);
> +}
> +
> +static inline const char *__btf_name_by_offset(const struct btf *btf,
> + u32 offset)
> +{
> + const char *name;
> +
> + if (!offset)
> + return "(anon)";
> +
> + name = btf_str_by_offset(btf, offset);
> + return name ?: "(invalid-name-offset)";
> +}
> +
(almost?) all of the above helpers are already defined in libbpf's
btf.h, no need to add all this duplication
> +/* functions shared between btf.c and btf_show_common.c */
> +void btf_type_ops_show(const struct btf *btf, const struct btf_type *t,
> + __u32 type_id, void *obj, u8 bits_offset,
> + struct btf_show *show);
[...]
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 1c0fd2d..35bd9dc 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -346,6 +346,7 @@ LIBBPF_0.3.0 {
> btf__parse_split;
> btf__new_empty_split;
> btf__new_split;
> + btf__snprintf;
It's LIBBPF_0.4.0 already, I or someone else should send a patch
adding a new section in .map file.
> ring_buffer__epoll_fd;
> xsk_setup_xdp_prog;
> xsk_socket__update_xskmap;
> --
> 1.8.3.1
>
Commit fadb08e7c750 ("kunit: Support for Parameterized Testing")
introduced support but lacks documentation for how to use it.
This patch builds on commit 1f0e943df68a ("Documentation: kunit: provide
guidance for testing many inputs") to show a minimal example of the new
feature.
Signed-off-by: Daniel Latypov <dlatypov(a)google.com>
---
Documentation/dev-tools/kunit/usage.rst | 57 +++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index d9fdc14f0677..650f99590df5 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -522,6 +522,63 @@ There's more boilerplate involved, but it can:
* E.g. if we wanted to also test ``sha256sum``, we could add a ``sha256``
field and reuse ``cases``.
+* be converted to a "parameterized test", see below.
+
+Parameterized Testing
+~~~~~~~~~~~~~~~~~~~~~
+
+The table-driven testing pattern is common enough that KUnit has special
+support for it.
+
+Reusing the same ``cases`` array from above, we can write the test as a
+"parameterized test" with the following.
+
+.. code-block:: c
+
+ // This is copy-pasted from above.
+ struct sha1_test_case {
+ const char *str;
+ const char *sha1;
+ };
+ struct sha1_test_case cases[] = {
+ {
+ .str = "hello world",
+ .sha1 = "2aae6c35c94fcfb415dbe95f408b9ce91ee846ed",
+ },
+ {
+ .str = "hello world!",
+ .sha1 = "430ce34d020724ed75a196dfc2ad67c77772d169",
+ },
+ };
+
+ // Need a helper function to generate a name for each test case.
+ static void case_to_desc(const struct sha1_test_case *t, char *desc)
+ {
+ strcpy(desc, t->str);
+ }
+ // Creates `sha1_gen_params()` to iterate over `cases`.
+ KUNIT_ARRAY_PARAM(sha1, cases, case_to_desc);
+
+ // Looks no different from a normal test.
+ static void sha1_test(struct kunit *test)
+ {
+ // This function can just contain the body of the for-loop.
+ // The former `cases[i]` is accessible under test->param_value.
+ char out[40];
+ struct sha1_test_case *test_param = (struct sha1_test_case *)(test->param_value);
+
+ sha1sum(test_param->str, out);
+ KUNIT_EXPECT_STREQ_MSG(test, (char *)out, test_param->sha1,
+ "sha1sum(%s)", test_param->str);
+ }
+
+ // Instead of KUNIT_CASE, we use KUNIT_CASE_PARAM and pass in the
+ // function declared by KUNIT_ARRAY_PARAM.
+ static struct kunit_case sha1_test_cases[] = {
+ KUNIT_CASE_PARAM(sha1_test, sha1_gen_params),
+ {}
+ };
+
.. _kunit-on-non-uml:
KUnit on non-UML architectures
base-commit: 5f6b99d0287de2c2d0b5e7abcb0092d553ad804a
--
2.29.2.684.gfbc64c5ab5-goog
Hi Linus,
Please pull the following Kselftest fixes update for Linux 5.11-rc4
This Kselftest fixes update for Linux 5.11-rc4 consists of one single
fix to skip BPF selftests by default. BPF selftests have a hard
dependency on cutting edge versions of tools in the BPF ecosystem
including LLVM.
Skipping BPF allows by default will make it easier for users interested
in running kselftest as a whole. Users can include BPF in Kselftest
build by via SKIP_TARGETS variable.
diff is attached.
thanks,
-- Shuah
----------------------------------------------------------------
The following changes since commit e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62:
Linux 5.11-rc2 (2021-01-03 15:55:30 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
tags/linux-kselftest-fixes-5.11-rc4
for you to fetch changes up to 7a6eb7c34a78498742b5f82543b7a68c1c443329:
selftests: Skip BPF seftests by default (2021-01-04 09:27:20 -0700)
----------------------------------------------------------------
linux-kselftest-fixes-5.11-rc4
This Kselftest fixes update for Linux 5.11-rc4 consists of one single
fix to skip BPF selftests by default. BPF selftests have a hard
dependency on cutting edge versions of tools in the BPF ecosystem
including LLVM.
Skipping BPF allows by default will make it easier for users interested
in running kselftest as a whole. Users can include BPF in Kselftest
build by via SKIP_TARGETS variable.
----------------------------------------------------------------
Mark Brown (1):
selftests: Skip BPF seftests by default
tools/testing/selftests/Makefile | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
----------------------------------------------------------------
Add support for pointer to mem register spilling, to allow the verifier
to track pointers to valid memory addresses. Such pointers are returned
for example by a successful call of the bpf_ringbuf_reserve helper.
The patch was partially contributed by CyberArk Software, Inc.
Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
Suggested-by: Yonghong Song <yhs(a)fb.com>
Signed-off-by: Gilad Reti <gilad.reti(a)gmail.com>
---
kernel/bpf/verifier.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 17270b8404f1..36af69fac591 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
case PTR_TO_RDWR_BUF:
case PTR_TO_RDWR_BUF_OR_NULL:
case PTR_TO_PERCPU_BTF_ID:
+ case PTR_TO_MEM:
+ case PTR_TO_MEM_OR_NULL:
return true;
default:
return false;
--
2.27.0
A few fixes for cross-building the sefltests out of tree. This will
enable wider automated testing on various Arm hardware.
Changes since v1 [1]:
* Use wildcard in patch 5
* Move the MAKE_DIRS declaration in patch 1
[1] https://lore.kernel.org/bpf/20210112135959.649075-1-jean-philippe@linaro.or…
Jean-Philippe Brucker (5):
selftests/bpf: Enable cross-building
selftests/bpf: Fix out-of-tree build
selftests/bpf: Move generated test files to $(TEST_GEN_FILES)
selftests/bpf: Fix installation of urandom_read
selftests/bpf: Install btf_dump test cases
tools/testing/selftests/bpf/Makefile | 58 ++++++++++++++++++++--------
1 file changed, 41 insertions(+), 17 deletions(-)
--
2.30.0