| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <483e9a808bcc400a80e8c9a752fa0163b2d0dd54.camel@gmail.com>
Date: Mon, 01 Dec 2025 10:27:02 -0800
From: Eduard Zingerman <eddyz87@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>, Alexei Starovoitov
<ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko
<andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu
<song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, John Fastabend
<john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Stanislav
Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa
<jolsa@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Nicolas Schier
<nicolas.schier@...ux.dev>, Nick Desaulniers
<nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>, Justin
Stitt <justinstitt@...gle.com>, Alan Maguire <alan.maguire@...cle.com>,
Donglin Peng <dolinux.peng@...il.com>
Cc: bpf@...r.kernel.org, dwarves@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 3/4] resolve_btfids: introduce enum
btf_id_kind
On Thu, 2025-11-27 at 10:52 -0800, Ihor Solodrai wrote:
> Instead of using multiple flags, make struct btf_id tagged with an
> enum value indicating its kind in the context of resolve_btfids.
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@...ux.dev>
> ---
Just a few nits, looks good to me overall.
> tools/bpf/resolve_btfids/main.c | 62 ++++++++++++++++++++++-----------
> 1 file changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index b4caae1170dd..c60d303ca6ed 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -98,6 +98,13 @@
> # error "Unknown machine endianness!"
> #endif
>
> +enum btf_id_kind {
> + BTF_ID_KIND_NONE,
> + BTF_ID_KIND_SYM,
> + BTF_ID_KIND_SET,
> + BTF_ID_KIND_SET8
> +};
> +
> struct btf_id {
> struct rb_node rb_node;
> char *name;
> @@ -105,9 +112,8 @@ struct btf_id {
> int id;
> int cnt;
> };
> - int addr_cnt;
> - bool is_set;
> - bool is_set8;
> + enum btf_id_kind kind:8;
> + int addr_cnt:8;
^^
Nit: these bitfields are not really necessary:
$ pahole -C btf_id ./tools/bpf/resolve_btfids/resolve_btfids
struct btf_id {
struct rb_node rb_node __attribute__((__aligned__(8))); /* 0 24 */
char * name; /* 24 8 */
union {
int id; /* 32 4 */
int cnt; /* 32 4 */
}; /* 32 4 */
enum btf_id_kind kind:8; /* 36: 0 4 */
int addr_cnt:8; /* 36: 8 4 */
/* XXX 16 bits hole, try to pack */
Elf64_Addr addr[100]; /* 40 800 */
/* size: 840, cachelines: 14, members: 6 */
/* sum members: 836 */
/* sum bitfield members: 16 bits, bit holes: 1, sum bit holes: 16 bits */
/* forced alignments: 1 */
/* last cacheline: 8 bytes */
} __attribute__((__aligned__(8)));
> Elf64_Addr addr[ADDR_CNT];
> };
>
> @@ -260,26 +266,33 @@ static char *get_id(const char *prefix_end)
> return id;
> }
>
> -static struct btf_id *add_set(struct object *obj, char *name, bool is_set8)
> +static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind kind)
> {
> /*
> * __BTF_ID__set__name
> * name = ^
> * id = ^
> */
> - char *id = name + (is_set8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__")) - 1;
> + int prefixlen = kind == BTF_ID_KIND_SET8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__");
Nit: Should prefixlen be an input parameter as well? (Or adjust the 'name' at the callsite?)
Otherwise the parameter is still a boolean logically.
> + char *id = name + prefixlen - 1;
> int len = strlen(name);
> + struct btf_id *btf_id;
>
> if (id >= name + len) {
> pr_err("FAILED to parse set name: %s\n", name);
> return NULL;
> }
>
> - return btf_id__add(&obj->sets, id, true);
> + btf_id = btf_id__add(&obj->sets, id, true);
> + if (btf_id)
> + btf_id->kind = kind;
> +
> + return btf_id;
> }
>
> static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
> {
> + struct btf_id *btf_id;
> char *id;
>
> id = get_id(name + size);
> @@ -288,7 +301,11 @@ static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
> return NULL;
> }
>
> - return btf_id__add(root, id, false);
> + btf_id = btf_id__add(root, id, false);
> + if (btf_id)
> + btf_id->kind = BTF_ID_KIND_SYM;
> +
> + return btf_id;
Agree with Andrii regarding 'kind' being a btf_id__add() parameter.
> }
>
> /* Older libelf.h and glibc elf.h might not yet define the ELF compression types. */
> @@ -491,28 +508,24 @@ static int symbols_collect(struct object *obj)
> id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1);
> /* set8 */
> } else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) {
> - id = add_set(obj, prefix, true);
> + id = add_set(obj, prefix, BTF_ID_KIND_SET8);
> /*
> * SET8 objects store list's count, which is encoded
> * in symbol's size, together with 'cnt' field hence
> * that - 1.
> */
> - if (id) {
> + if (id)
> id->cnt = sym.st_size / sizeof(uint64_t) - 1;
> - id->is_set8 = true;
> - }
> /* set */
> } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
> - id = add_set(obj, prefix, false);
> + id = add_set(obj, prefix, BTF_ID_KIND_SET);
> /*
> * SET objects store list's count, which is encoded
> * in symbol's size, together with 'cnt' field hence
> * that - 1.
> */
> - if (id) {
> + if (id)
> id->cnt = sym.st_size / sizeof(int) - 1;
> - id->is_set = true;
> - }
> } else {
> pr_err("FAILED unsupported prefix %s\n", prefix);
> return -1;
> @@ -643,7 +656,7 @@ static int id_patch(struct object *obj, struct btf_id *id)
> int i;
>
> /* For set, set8, id->id may be 0 */
> - if (!id->id && !id->is_set && !id->is_set8) {
> + if (!id->id && id->kind == BTF_ID_KIND_SYM) {
> pr_err("WARN: resolve_btfids: unresolved symbol %s\n", id->name);
> warnings++;
> }
> @@ -696,6 +709,7 @@ static int sets_patch(struct object *obj)
> {
> Elf_Data *data = obj->efile.idlist;
> struct rb_node *next;
> + int cnt;
>
> next = rb_first(&obj->sets);
> while (next) {
> @@ -715,11 +729,15 @@ static int sets_patch(struct object *obj)
> return -1;
> }
>
> - if (id->is_set) {
> + switch (id->kind) {
> + case BTF_ID_KIND_SET:
> set = data->d_buf + off;
> + cnt = set->cnt;
> qsort(set->ids, set->cnt, sizeof(set->ids[0]), cmp_id);
> - } else {
> + break;
> + case BTF_ID_KIND_SET8:
> set8 = data->d_buf + off;
> + cnt = set8->cnt;
> /*
> * Make sure id is at the beginning of the pairs
> * struct, otherwise the below qsort would not work.
> @@ -744,10 +762,14 @@ static int sets_patch(struct object *obj)
> bswap_32(set8->pairs[i].flags);
> }
> }
> + break;
> + case BTF_ID_KIND_SYM:
> + default:
Nit: just default, no need for `case BTF_ID_KIND_SYM:`?
> + pr_err("Unexpected btf_id_kind %d for set '%s'\n", id->kind, id->name);
> + return -1;
> }
>
> - pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
> - off, id->is_set ? set->cnt : set8->cnt, id->name);
> + pr_debug("sorting addr %5lu: cnt %6d [%s]\n", off, cnt, id->name);
>
> next = rb_next(next);
> }
Powered by blists - more mailing lists