All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rafael David Tinoco <rafaeldtinoco@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: "Mauricio Vásquez Bernal" <mauricio@kinvolk.io>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Quentin Monnet" <quentin@isovalent.com>,
	"Lorenzo Fontana" <lorenzo.fontana@elastic.co>,
	"Leonardo Di Donato" <leonardo.didonato@elastic.co>
Subject: Re: [PATCH bpf-next v5 6/9] bpftool: Implement relocations recording for BTFGen
Date: Fri, 4 Feb 2022 03:20:22 -0300	[thread overview]
Message-ID: <8846F5AD-CFD3-4F32-B9C5-E36AB38C37DF@gmail.com> (raw)
In-Reply-To: <CAEf4BzagOBmVbrPOnSwthOxt7CYoqNTuojbtmgskNa_Ad=8EVQ@mail.gmail.com>

>>> As in, do you substitute forward declarations for types that are
>>> never directly used? If not, that's going to be very suboptimal for
>>> something like task_struct and any other type that's part of a big
>>> cluster of types.

>> We decided to include the whole types and all direct and indirect
>> types referenced from a structure field for type-based relocations.
>> Our reasoning is that we don't know if the matching algorithm of
>> libbpf could be changed to require more information in the future and
>> type-based relocations are few compared to field based relocations.

> It will depend on application and which type is used in relocation.
> task_struct reaches tons of types and will add a very noticeable size
> to minimized BTF, for no good reason, IMO. If we discover that we do
> need those types, we'll update bpftool to generate more.

Just to see if I understood this part correctly. IIRC, we started type
based relocations support in btfgen because of this particular case:

	union kernfs_node_id {
	    struct {
	        u32 ino;
	        u32 generation;
	    };
	    u64 id;
	};

	struct kernfs_node___older_v55 {
	    const char *name;
	    union kernfs_node_id id;
	};

	struct kernfs_node___rh8 {
	    const char *name;
	    union {
	        u64 id;
	        struct {
	            union kernfs_node_id id;
	        } rh_kabi_hidden_172;
	        union { };
	    };
	};

So we have 3 situations:

(struct kernfs_node *)->id as u64

	[29] STRUCT 'kernfs_node' size=128 vlen=1
	        'id' type_id=42 bits_offset=832
	[42] TYPEDEF 'u64' type_id=10

(struct kernfs_node___older_v55 *)->id as u64 (union kernfs_node_id)->id

	[79] STRUCT 'kernfs_node' size=128 vlen=1
	        'id' type_id=69 bits_offset=832
	[69] UNION 'kernfs_node_id' size=8 vlen=2
	        '(anon)' type_id=132 bits_offset=0
	        'id' type_id=40 bits_offset=0
	[40] TYPEDEF 'u64' type_id=12

(struct kernfs_node___rh8 *)->id = (anon union)->id

	[56] STRUCT 'kernfs_node' size=128 vlen=1
	        '(anon)' type_id=24 bits_offset=832
	[24] UNION '(anon)' size=8 vlen=1
	        'id' type_id=40 bits_offset=0
	[40] TYPEDEF 'u64' type_id=11

We're finding needed BTF types, that should be added to generated BTF,
based on fields/members of CORE relo info. How we would know we had to
add the anon union of the last case if it does not exist in the local
BTF ? What is your suggestion ?

Thanks!

-rafaeldtinoco

  reply	other threads:[~2022-02-04  6:20 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-28 22:33 [PATCH bpf-next v5 0/9] libbpf: Implement BTFGen Mauricio Vásquez
2022-01-28 22:33 ` [PATCH bpf-next v5 1/9] libbpf: Implement changes needed for BTFGen in bpftool Mauricio Vásquez
2022-02-01 20:57   ` Quentin Monnet
2022-02-03 16:08     ` Mauricio Vásquez Bernal
2022-02-02 18:54   ` Andrii Nakryiko
2022-02-02 19:02     ` Andrii Nakryiko
2022-02-03 16:09     ` Mauricio Vásquez Bernal
2022-01-28 22:33 ` [PATCH bpf-next v5 2/9] bpftool: Add gen min_core_btf command Mauricio Vásquez
2022-02-02 17:58   ` Andrii Nakryiko
2022-02-03 16:07     ` Mauricio Vásquez Bernal
2022-02-03 17:21       ` Andrii Nakryiko
2022-01-28 22:33 ` [PATCH bpf-next v5 3/9] bpftool: Implement btf_save_raw() Mauricio Vásquez
2022-02-02 18:48   ` Andrii Nakryiko
2022-02-03 16:07     ` Mauricio Vásquez Bernal
2022-02-03 17:23       ` Andrii Nakryiko
2022-01-28 22:33 ` [PATCH bpf-next v5 4/9] bpftool: Add struct definitions and helpers for BTFGen Mauricio Vásquez
2022-02-02 18:54   ` Andrii Nakryiko
2022-02-03 16:08     ` Mauricio Vásquez Bernal
2022-02-03 17:24       ` Andrii Nakryiko
2022-01-28 22:33 ` [PATCH bpf-next v5 5/9] bpftool: Implement btfgen() Mauricio Vásquez
2022-02-01 20:57   ` Quentin Monnet
2022-02-03 19:10     ` Mauricio Vásquez Bernal
2022-02-02 19:14   ` Andrii Nakryiko
2022-02-03 16:09     ` Mauricio Vásquez Bernal
2022-01-28 22:33 ` [PATCH bpf-next v5 6/9] bpftool: Implement relocations recording for BTFGen Mauricio Vásquez
2022-02-02 19:31   ` Andrii Nakryiko
2022-02-03 16:40     ` Mauricio Vásquez Bernal
2022-02-03 17:30       ` Andrii Nakryiko
2022-02-04  6:20         ` Rafael David Tinoco [this message]
2022-02-04 18:41           ` Andrii Nakryiko
2022-02-02 22:55   ` Andrii Nakryiko
2022-02-04 19:44     ` Mauricio Vásquez Bernal
2022-01-28 22:33 ` [PATCH bpf-next v5 7/9] bpftool: Implement btfgen_get_btf() Mauricio Vásquez
2022-02-02 19:36   ` Andrii Nakryiko
2022-02-03 16:10     ` Mauricio Vásquez Bernal
2022-02-03 17:31       ` Andrii Nakryiko
2022-01-28 22:33 ` [PATCH bpf-next v5 8/9] bpftool: gen min_core_btf explanation and examples Mauricio Vásquez
2022-02-01 20:57   ` Quentin Monnet
2022-01-28 22:33 ` [PATCH bpf-next v5 9/9] selftest/bpf: Implement tests for bpftool gen min_core_btf Mauricio Vásquez
2022-01-28 23:23   ` Mauricio Vásquez Bernal
2022-02-01 20:58     ` Quentin Monnet
2022-02-02 19:50     ` Andrii Nakryiko
2022-02-03 21:17       ` Mauricio Vásquez Bernal
2022-02-04 20:05         ` Andrii Nakryiko
2022-02-01 20:57   ` Quentin Monnet

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=8846F5AD-CFD3-4F32-B9C5-E36AB38C37DF@gmail.com \
    --to=rafaeldtinoco@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=leonardo.didonato@elastic.co \
    --cc=lorenzo.fontana@elastic.co \
    --cc=mauricio@kinvolk.io \
    --cc=netdev@vger.kernel.org \
    --cc=quentin@isovalent.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.