* [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type @ 2021-01-05 15:39 Jiri Olsa 2021-01-05 19:37 ` Alexei Starovoitov 2021-01-05 21:15 ` Andrii Nakryiko 0 siblings, 2 replies; 6+ messages in thread From: Jiri Olsa @ 2021-01-05 15:39 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Qais Yousef, netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh The kernel image can contain multiple types (structs/unions) with the same name. This causes distinct type hierarchies in BTF data and makes resolve_btfids fail with error like: BTFIDS vmlinux FAILED unresolved symbol udp6_sock as reported by Qais Yousef [1]. This change adds warning when multiple types of the same name are detected: BTFIDS vmlinux WARN: multiple IDs found for 'file' (526, 113351) WARN: multiple IDs found for 'sk_buff' (2744, 113958) We keep the lower ID for the given type instance and let the build continue. [1] https://lore.kernel.org/lkml/20201229151352.6hzmjvu3qh6p2qgg@e107158-lin/ Reported-by: Qais Yousef <qais.yousef@arm.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/bpf/resolve_btfids/main.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index e3ea569ee125..36a3b1024cdc 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -139,6 +139,8 @@ int eprintf(int level, int var, const char *fmt, ...) #define pr_debug2(fmt, ...) pr_debugN(2, pr_fmt(fmt), ##__VA_ARGS__) #define pr_err(fmt, ...) \ eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__) +#define pr_info(fmt, ...) \ + eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__) static bool is_btf_id(const char *name) { @@ -526,8 +528,13 @@ static int symbols_resolve(struct object *obj) id = btf_id__find(root, str); if (id) { - id->id = type_id; - (*nr)--; + if (id->id) { + pr_info("WARN: multiple IDs found for '%s' (%d, %d)\n", + str, id->id, type_id); + } else { + id->id = type_id; + (*nr)--; + } } } -- 2.26.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type 2021-01-05 15:39 [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type Jiri Olsa @ 2021-01-05 19:37 ` Alexei Starovoitov 2021-01-05 19:50 ` Jiri Olsa 2021-01-05 21:15 ` Andrii Nakryiko 1 sibling, 1 reply; 6+ messages in thread From: Alexei Starovoitov @ 2021-01-05 19:37 UTC (permalink / raw) To: Jiri Olsa Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Qais Yousef, Network Development, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Tue, Jan 5, 2021 at 7:39 AM Jiri Olsa <jolsa@kernel.org> wrote: > > The kernel image can contain multiple types (structs/unions) > with the same name. This causes distinct type hierarchies in > BTF data and makes resolve_btfids fail with error like: > > BTFIDS vmlinux > FAILED unresolved symbol udp6_sock > > as reported by Qais Yousef [1]. > > This change adds warning when multiple types of the same name > are detected: > > BTFIDS vmlinux > WARN: multiple IDs found for 'file' (526, 113351) > WARN: multiple IDs found for 'sk_buff' (2744, 113958) > > We keep the lower ID for the given type instance and let the > build continue. I think it would make sense to mention this decision in the warning. 'WARN: multiple IDs' is ambiguous and confusing when action is not specified. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type 2021-01-05 19:37 ` Alexei Starovoitov @ 2021-01-05 19:50 ` Jiri Olsa 2021-01-05 20:02 ` Alexei Starovoitov 0 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2021-01-05 19:50 UTC (permalink / raw) To: Alexei Starovoitov Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Qais Yousef, Network Development, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Tue, Jan 05, 2021 at 11:37:09AM -0800, Alexei Starovoitov wrote: > On Tue, Jan 5, 2021 at 7:39 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > The kernel image can contain multiple types (structs/unions) > > with the same name. This causes distinct type hierarchies in > > BTF data and makes resolve_btfids fail with error like: > > > > BTFIDS vmlinux > > FAILED unresolved symbol udp6_sock > > > > as reported by Qais Yousef [1]. > > > > This change adds warning when multiple types of the same name > > are detected: > > > > BTFIDS vmlinux > > WARN: multiple IDs found for 'file' (526, 113351) > > WARN: multiple IDs found for 'sk_buff' (2744, 113958) > > > > We keep the lower ID for the given type instance and let the > > build continue. > > I think it would make sense to mention this decision in the warning. > 'WARN: multiple IDs' is ambiguous and confusing when action is not specified. ok, how about: WARN: multiple IDs found for 'file': 526, 113351 - using 526 jirka ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type 2021-01-05 19:50 ` Jiri Olsa @ 2021-01-05 20:02 ` Alexei Starovoitov 0 siblings, 0 replies; 6+ messages in thread From: Alexei Starovoitov @ 2021-01-05 20:02 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Qais Yousef, Network Development, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Tue, Jan 5, 2021 at 11:50 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Tue, Jan 05, 2021 at 11:37:09AM -0800, Alexei Starovoitov wrote: > > On Tue, Jan 5, 2021 at 7:39 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > The kernel image can contain multiple types (structs/unions) > > > with the same name. This causes distinct type hierarchies in > > > BTF data and makes resolve_btfids fail with error like: > > > > > > BTFIDS vmlinux > > > FAILED unresolved symbol udp6_sock > > > > > > as reported by Qais Yousef [1]. > > > > > > This change adds warning when multiple types of the same name > > > are detected: > > > > > > BTFIDS vmlinux > > > WARN: multiple IDs found for 'file' (526, 113351) > > > WARN: multiple IDs found for 'sk_buff' (2744, 113958) > > > > > > We keep the lower ID for the given type instance and let the > > > build continue. > > > > I think it would make sense to mention this decision in the warning. > > 'WARN: multiple IDs' is ambiguous and confusing when action is not specified. > > ok, how about: > > WARN: multiple IDs found for 'file': 526, 113351 - using 526 yep. much better imo. Please add a comment to .c file with the suggestion that such types should be renamed to avoid ambiguity. Adding 'please rename' to WARN is probably overkill. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type 2021-01-05 15:39 [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type Jiri Olsa 2021-01-05 19:37 ` Alexei Starovoitov @ 2021-01-05 21:15 ` Andrii Nakryiko 2021-01-05 22:47 ` Jiri Olsa 1 sibling, 1 reply; 6+ messages in thread From: Andrii Nakryiko @ 2021-01-05 21:15 UTC (permalink / raw) To: Jiri Olsa Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Qais Yousef, Networking, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Tue, Jan 5, 2021 at 7:41 AM Jiri Olsa <jolsa@kernel.org> wrote: > > The kernel image can contain multiple types (structs/unions) > with the same name. This causes distinct type hierarchies in > BTF data and makes resolve_btfids fail with error like: > > BTFIDS vmlinux > FAILED unresolved symbol udp6_sock > > as reported by Qais Yousef [1]. > > This change adds warning when multiple types of the same name > are detected: > > BTFIDS vmlinux > WARN: multiple IDs found for 'file' (526, 113351) > WARN: multiple IDs found for 'sk_buff' (2744, 113958) > > We keep the lower ID for the given type instance and let the > build continue. > > [1] https://lore.kernel.org/lkml/20201229151352.6hzmjvu3qh6p2qgg@e107158-lin/ > Reported-by: Qais Yousef <qais.yousef@arm.com> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- see comments below, but otherwise lgtm Acked-by: Andrii Nakryiko <andrii@kernel.org> > tools/bpf/resolve_btfids/main.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index e3ea569ee125..36a3b1024cdc 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c > @@ -139,6 +139,8 @@ int eprintf(int level, int var, const char *fmt, ...) > #define pr_debug2(fmt, ...) pr_debugN(2, pr_fmt(fmt), ##__VA_ARGS__) > #define pr_err(fmt, ...) \ > eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__) > +#define pr_info(fmt, ...) \ > + eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__) how is it different from pr_err? Did you forget to update verboseness levels or it's intentional? > > static bool is_btf_id(const char *name) > { > @@ -526,8 +528,13 @@ static int symbols_resolve(struct object *obj) > > id = btf_id__find(root, str); > if (id) { > - id->id = type_id; > - (*nr)--; > + if (id->id) { > + pr_info("WARN: multiple IDs found for '%s' (%d, %d)\n", > + str, id->id, type_id); > + } else { > + id->id = type_id; > + (*nr)--; btw, there is a nasty shadowing of nr variable, which is used both for the for() loop condition (as int) and as `int *` inside the loop body. It's better to rename inner (or outer) nr, it's extremely confusing as is. > + } > } > } > > -- > 2.26.2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type 2021-01-05 21:15 ` Andrii Nakryiko @ 2021-01-05 22:47 ` Jiri Olsa 0 siblings, 0 replies; 6+ messages in thread From: Jiri Olsa @ 2021-01-05 22:47 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Qais Yousef, Networking, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Tue, Jan 05, 2021 at 01:15:39PM -0800, Andrii Nakryiko wrote: > On Tue, Jan 5, 2021 at 7:41 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > The kernel image can contain multiple types (structs/unions) > > with the same name. This causes distinct type hierarchies in > > BTF data and makes resolve_btfids fail with error like: > > > > BTFIDS vmlinux > > FAILED unresolved symbol udp6_sock > > > > as reported by Qais Yousef [1]. > > > > This change adds warning when multiple types of the same name > > are detected: > > > > BTFIDS vmlinux > > WARN: multiple IDs found for 'file' (526, 113351) > > WARN: multiple IDs found for 'sk_buff' (2744, 113958) > > > > We keep the lower ID for the given type instance and let the > > build continue. > > > > [1] https://lore.kernel.org/lkml/20201229151352.6hzmjvu3qh6p2qgg@e107158-lin/ > > Reported-by: Qais Yousef <qais.yousef@arm.com> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > see comments below, but otherwise lgtm > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > tools/bpf/resolve_btfids/main.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > > index e3ea569ee125..36a3b1024cdc 100644 > > --- a/tools/bpf/resolve_btfids/main.c > > +++ b/tools/bpf/resolve_btfids/main.c > > @@ -139,6 +139,8 @@ int eprintf(int level, int var, const char *fmt, ...) > > #define pr_debug2(fmt, ...) pr_debugN(2, pr_fmt(fmt), ##__VA_ARGS__) > > #define pr_err(fmt, ...) \ > > eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__) > > +#define pr_info(fmt, ...) \ > > + eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__) > > how is it different from pr_err? Did you forget to update verboseness > levels or it's intentional? intentional, I'm using pr_err to print in error paths, so I wanted to add new one for other 'info' messages without -v > > > > > static bool is_btf_id(const char *name) > > { > > @@ -526,8 +528,13 @@ static int symbols_resolve(struct object *obj) > > > > id = btf_id__find(root, str); > > if (id) { > > - id->id = type_id; > > - (*nr)--; > > + if (id->id) { > > + pr_info("WARN: multiple IDs found for '%s' (%d, %d)\n", > > + str, id->id, type_id); > > + } else { > > + id->id = type_id; > > + (*nr)--; > > btw, there is a nasty shadowing of nr variable, which is used both for > the for() loop condition (as int) and as `int *` inside the loop body. > It's better to rename inner (or outer) nr, it's extremely confusing as > is. nice, I'll change that thanks, jirka > > > + } > > } > > } > > > > -- > > 2.26.2 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-05 22:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-05 15:39 [PATCH bpf-next] tools/resolve_btfids: Warn when having multiple IDs for single type Jiri Olsa 2021-01-05 19:37 ` Alexei Starovoitov 2021-01-05 19:50 ` Jiri Olsa 2021-01-05 20:02 ` Alexei Starovoitov 2021-01-05 21:15 ` Andrii Nakryiko 2021-01-05 22:47 ` Jiri Olsa
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.