Em Sat, Feb 18, 2023 at 11:12:50AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Feb 17, 2023 at 11:10:29PM +0000, Alan Maguire escreveu:
> > It has been observed [1] that the recent dwarves changes
> > that skip BTF encoding for functions that have optimized-out
> > parameters are too aggressive, leading to missing kfuncs
> > which generate warnings and a BPF selftest failure.
> >
> > Here a different approach is used; we observe that
> > just because a function does not _use_ a parameter,
> > it does not mean it was not passed to it. What we
> > are really keen to detect are cases where the calling
> > conventions are violated such that a function will
> > not have parameter values in the expected registers.
> > In such cases, tracing and kfunc behaviour will be
> > unstable. We are not worried about parameters being
> > optimized out, provided that optimization does not
> > lead to other parameters being passed via
> > unexpected registers.
> >
> > So this series attempts to detect such cases by
> > examining register (DW_OP_regX) values for
> > parameters where available; if these match
> > expectations, the function is deemed safe to add to
> > BTF, even if parameters are optimized out.
> >
> > Using this approach, the only functions that
> > BTF generation is suppressed for are
> >
> > 1. those with parameters that violate calling
> > conventions where present; and
> > 2. those which have multiple inconsistent prototypes.
>
> This sounds sensible at first sight, I've applied the patches so that it
> can go thru further testing on the libbpf CI, for now its just on the
> 'next' branch, the testing I did so far:
>
> make allmodconfig + enablig BTF on bpf-next reverting that revert so
> that we use the new options,
>
> Now I'm building and booting a kernel with a fedora-ish config without
> using the new options and then with them (reverting the revert) to
> compare the tools/testing/selftests/bpf/ ./test-progs output
> before/after.
-Summary: 274/1609 PASSED, 24 SKIPPED, 17 FAILED
+Summary: 276/1612 PASSED, 24 SKIPPED, 15 FAILED
Well, we're passing _more_ tests :-)
And no messages about that kernel module, etc. Will redo with clang as
time permits.
- Arnaldo
> Its an extended holiday down here, so I'll be spotty but want to get
> this moving forward,
>
> Thanks!
>
> - Arnaldo
>
> > With these changes, running pahole on a gcc-built
> > vmlinux skips
> >
> > - 1164 functions due to multiple inconsistent function
> > prototypes. Most of these are "."-suffixed optimized
> > fuctions.
> > - 331 functions due to unexpected register usage
> >
> > For a clang-built kernel, the numbers are
> >
> > - 539 functions with inconsistent prototypes are skipped
> > - 209 functions with unexpected register usage are skipped
> >
> > One complication is that functions that are passed
> > structs (or typedef structs) can use multiple registers
> > to pass those structures. Examples include
> > bpf_lsm_socket_getpeersec_stream() (passing a typedef
> > struct sockptr_t) and the bpf_testmod_test_struct_arg_1
> > function in bpf_testmod. Because multiple registers
> > are used to represent the structure, this throws
> > off expectations for any subsequent parameter->register
> > mappings. To handle this, simply exempt functions
> > that have struct (or typedef struct) parameters from
> > our register checks.
> >
> > Note to test this series on bpf-next, the following
> > commit should be reverted (reverting the revert
> > so that the flags are added to BTF encoding when
> > using pahole v1.25):
> >
> > commit 1f5dfcc78ab4 ("Revert "bpf: Add --skip_encoding_btf_inconsistent_proto, --btf_gen_optimized to pahole flags for v1.25"")
> >
> > With these changes we also see tracing_struct now pass:
> >
> > $ sudo ./test_progs -t tracing_struct
> > #233 tracing_struct:OK
> > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> >
> > Further testing is needed - along with support for additional
> > parameter index -> DWARF reg for more platforms.
> >
> > Future work could also add annotations for optimized-out
> > parameters via BTF tags to help guide tracing.
> >
> > [1] https://lore.kernel.org/bpf/CAADnVQ+hfQ9LEmEFXneB7hm17NvRniXSShrHLaM-1BrguLjLQw@xxxxxxxxxxxxxx/
> >
> > Alan Maguire (4):
> > dwarf_loader: mark functions that do not use expected registers for
> > params
> > btf_encoder: exclude functions with unexpected param register use not
> > optimizations
> > pahole: update descriptions for btf_gen_optimized,
> > skip_encoding_btf_inconsistent_proto
> > pahole: update man page for options also
> >
> > btf_encoder.c | 24 +++++++---
> > dwarf_loader.c | 109 ++++++++++++++++++++++++++++++++++++++++++---
> > dwarves.h | 5 +++
> > man-pages/pahole.1 | 4 +-
> > pahole.c | 4 +-
> > 5 files changed, 129 insertions(+), 17 deletions(-)
> >
> > --
> > 2.31.1
> >
>
> --
>
> - Arnaldo
--
- Arnaldo
