On Tue, 8 Dec 2020, Andrii Nakryiko wrote:
> When Clang emits ldimm64 instruction for BPF_TYPE_ID_TARGET CO-RE relocation,
> put module BTF FD, containing target type, into upper 32 bits of imm64.
>
> Because this FD is internal to libbpf, it's very cumbersome to test this in
> selftests. Manual testing was performed with debug log messages sprinkled
> across selftests and libbpf, confirming expected values are substituted.
> Better testing will be performed as part of the work adding module BTF types
> support to bpf_snprintf_btf() helpers.
>
> v1->v2:
> - fix crash on failing to resolve target spec (Alan).
>
> Cc: Alan Maguire <alan.maguire@xxxxxxxxxx>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
Thanks for this!
Can confirm the segmentation fault has gone away. I tested with the
veth_stats_rx program (though will switch to btf_test module later),
and I still see the issue with a local kind of fwd for veth_stats
leading to an inability to find the target kind in the module BTF:
libbpf: sec 'kprobe/veth_stats_rx': found 5 CO-RE relocations
libbpf: prog 'veth_stats_rx': relo #0: kind <target_type_id> (7), spec is
[20] fwd veth_stats
libbpf: prog 'veth_stats_rx': relo #0: no matching targets found
libbpf: prog 'veth_stats_rx': relo #0: patched insn #3 (LDIMM64) imm64 20
-> 0
libbpf: prog 'veth_stats_rx': relo #1: kind <target_type_id> (7), spec is
[20] fwd veth_stats
libbpf: prog 'veth_stats_rx': relo #1: no matching targets found
libbpf: prog 'veth_stats_rx': relo #1: patched insn #5 (LDIMM64) imm64 20
-> 0
Here's the same debug info with a patch on top of yours that loosens the
constraints on kind matching such that a fwd local type will match a struct
target type:
libbpf: prog 'veth_stats_rx': relo #0: kind <target_type_id> (7), spec is
[20] fwd veth_stats
libbpf: CO-RE relocating [0] fwd veth_stats: found target candidate
[91516] struct veth_stats in [veth]
libbpf: prog 'veth_stats_rx': relo #0: matching candidate #0 [91516]
struct veth_stats
libbpf: prog 'veth_stats_rx': relo #0: patched insn #3 (LDIMM64) imm64 20
-> 450971657596
libbpf: prog 'veth_stats_rx': relo #1: kind <target_type_id> (7), spec is
[20] fwd veth_stats
libbpf: prog 'veth_stats_rx': relo #1: matching candidate #0 [91516]
struct veth_stats
libbpf: prog 'veth_stats_rx': relo #1: patched insn #5 (LDIMM64) imm64 20
-> 450971657596
Patch is below; if it makes sense to support loosening constraints on kind
matching like this feel free to roll it into your patch or I can send a
follow-up, whatever's easiest. Thanks!
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2fb9824..9ead5b3 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4673,6 +4673,23 @@ static void bpf_core_free_cands(struct
core_cand_list *ca
free(cands);
}
+/* module-specific structs may have relo kind set to fwd, so as
+ * well as handling exact matches, a fwd kind has to match
+ * a target struct kind.
+ */
+static bool kind_matches_target(const struct btf_type *local,
+ const struct btf_type *target)
+{
+ __u8 local_kind = btf_kind(local);
+ __u8 target_kind = btf_kind(target);
+
+ if (local_kind == target_kind)
+ return true;
+ if (local_kind == BTF_KIND_FWD && target_kind == BTF_KIND_STRUCT)
+ return true;
+ return false;
+}
+
static int bpf_core_add_cands(struct core_cand *local_cand,
size_t local_essent_len,
const struct btf *targ_btf,
@@ -4689,7 +4706,7 @@ static int bpf_core_add_cands(struct core_cand
*local_cand
n = btf__get_nr_types(targ_btf);
for (i = targ_start_id; i <= n; i++) {
t = btf__type_by_id(targ_btf, i);
- if (btf_kind(t) != btf_kind(local_cand->t))
+ if (!kind_matches_target(local_cand->t, t))
continue;
targ_name = btf__name_by_offset(targ_btf, t->name_off);
@@ -5057,7 +5074,7 @@ static int bpf_core_types_are_compat(const struct
btf *loc
/* caller made sure that names match (ignoring flavor suffix) */
local_type = btf__type_by_id(local_btf, local_id);
targ_type = btf__type_by_id(targ_btf, targ_id);
- if (btf_kind(local_type) != btf_kind(targ_type))
+ if (!kind_matches_target(local_type, targ_type))
return 0;
recur:
@@ -5070,7 +5087,7 @@ static int bpf_core_types_are_compat(const struct
btf *loc
if (!local_type || !targ_type)
return -EINVAL;
- if (btf_kind(local_type) != btf_kind(targ_type))
+ if (!kind_matches_target(local_type, targ_type))
return 0;
switch (btf_kind(local_type)) {
