mbox series

[bpf-next,0/3] API to access btf_dump emit queue and print single type

Message ID 20240516230443.3436233-1-eddyz87@gmail.com (mailing list archive)
Headers show
Series API to access btf_dump emit queue and print single type | expand

Message

Eduard Zingerman May 16, 2024, 11:04 p.m. UTC
This is a follow-up to the following discussion:
https://lore.kernel.org/bpf/20240503111836.25275-1-jose.marchesi@oracle.com/

As suggested by Andrii, this series adds several API functions to
allow more flexibility with btf dump:
- a function to add a type and all its dependencies to the emit queue;
- functions to provide access to the emit queue owned by btf_dump object;
- a function to print a given type (skipping any dependencies).

This should allow filtering printed types and also adding
attributes / pre-processor statements to specific types.

There are several ways to add such API:
1. The simplest in terms of code changes is to refactor
   btf_dump_emit_type() to push types and forward declarations
   to the emit queue, instead of printing those directly;
2. More intrusive: refactor btf_dump_emit_type() and
   btf_dump_order_type() to avoid doing topological sorting twice and
   put forward declarations to the emit queue at once.

This series opts for (2) as it seems to simplify library code a bit.
For the sake of discussion, source code for option (1) as available at:
https://github.com/eddyz87/bpf/tree/libbpf-sort-for-dump-api-simple

Also, this series opts for the following new API function:

  int btf_dump__dump_one_type(struct btf_dump *d, __u32 id, bool fwd);

As adding _opts variant of btf_dump__dump_type seems a bit clumsy:

  struct btf_dump_opts {
	size_t sz;
	bool fwd;
	bool skip_dependencies;
	bool skip_last_semi;
  };

  int btf_dump__dump_type_opts(struct btf_dump *d, __u32 id,
			       struct btf_dump_opts *opts);

but maybe community would prefer the later variant.

Eduard Zingerman (3):
  libbpf: put forward declarations to btf_dump->emit_queue
  libbpf: API to access btf_dump emit queue and print single type
  selftests/bpf: tests for btf_dump emit queue API

 tools/lib/bpf/btf.h                           |  33 ++
 tools/lib/bpf/btf_dump.c                      | 363 ++++++++----------
 tools/lib/bpf/libbpf.map                      |   4 +
 .../selftests/bpf/prog_tests/btf_dump.c       |  86 +++++
 4 files changed, 289 insertions(+), 197 deletions(-)

Comments

Eduard Zingerman May 17, 2024, 12:02 a.m. UTC | #1
On Thu, 2024-05-16 at 16:04 -0700, Eduard Zingerman wrote:
> This is a follow-up to the following discussion:
> https://lore.kernel.org/bpf/20240503111836.25275-1-jose.marchesi@oracle.com/
> 
> As suggested by Andrii, this series adds several API functions to
> allow more flexibility with btf dump:
> - a function to add a type and all its dependencies to the emit queue;
> - functions to provide access to the emit queue owned by btf_dump object;
> - a function to print a given type (skipping any dependencies).

The series fails on the CI despite passing local testing,
I'll submit v2 when ready.

[...]
Eduard Zingerman May 17, 2024, 2:03 a.m. UTC | #2
On Thu, 2024-05-16 at 17:02 -0700, Eduard Zingerman wrote:
> On Thu, 2024-05-16 at 16:04 -0700, Eduard Zingerman wrote:
> > This is a follow-up to the following discussion:
> > https://lore.kernel.org/bpf/20240503111836.25275-1-jose.marchesi@oracle.com/
> > 
> > As suggested by Andrii, this series adds several API functions to
> > allow more flexibility with btf dump:
> > - a function to add a type and all its dependencies to the emit queue;
> > - functions to provide access to the emit queue owned by btf_dump object;
> > - a function to print a given type (skipping any dependencies).
> 
> The series fails on the CI despite passing local testing,
> I'll submit v2 when ready.
> 
> [...]

The bug was caused by a logical error in typedefs handling.
Do not mark typedef as ORDERED, always emit a forward declaration for
it instead. Otherwise the following situation would be troublesome:

  typedef struct foo foo_alias;

  struct foo {};

  struct root {
     foo_alias *a;    <-- foo->a leads to forward declaration for 'foo',
     foo_alias b;         if 'foo_alias' is marked ORDERED
  };                      foo->b will not traverse to generate
                          full declaration for 'foo'.

The patch below fixes error.
CI builds seem ok:
https://github.com/kernel-patches/bpf/pull/7052

---

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index cb233f891582..7e845ad9ca9e 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -653,12 +653,9 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, __u32 cont_id, bool
                if (err < 0)
                        return err;
 
-               /* typedef is always a named definition */
-               err = btf_dump_add_emit_queue_id(d, id);
+               err = btf_dump_add_emit_queue_fwd(d, id);
                if (err)
                        return err;
-
-               d->type_states[id].order_state = ORDERED;
                return 0;
        }
        case BTF_KIND_VOLATILE: