Re: [PATCH bpf-next v1 2/6] libbpf: Add typeless and weak ksym support to gen_loader

From: Song Liu <song@kernel.org>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	Networking <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next v1 2/6] libbpf: Add typeless and weak ksym support to gen_loader
Date: Thu, 7 Oct 2021 14:45:10 -0700	[thread overview]
Message-ID: <CAPhsuW6nCQK71aeyR1YthvMWGNgH--RwbLnA0_rhi071juTsYg@mail.gmail.com> (raw)
In-Reply-To: <20211006002853.308945-3-memxor@gmail.com>

On Tue, Oct 5, 2021 at 5:29 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> This patch adds typeless and weak ksym support to BTF_KIND_VAR
> relocation code in gen_loader. For typeless ksym, we use the newly added
> bpf_kallsyms_lookup_name helper.
>
> For weak ksym, we simply skip error check, and fix up the srg_reg for
> the insn, as keeping it as BPF_PSEUDO_BTF_ID for weak ksym with its
> insn[0].imm and insn[1].imm set as 0 will cause a failure.  This is
> consistent with how libbpf relocates these two cases of BTF_KIND_VAR.
>
> We also modify cleanup_relos to check for typeless ksyms in fd closing
> loop, since those have no fd associated with the ksym. For this we can
> reuse the unused 'off' member of ksym_desc.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
[...]

Everything above (trimmed) makes sense to me.

> +/* Expects:
> + * BPF_REG_8 - pointer to instruction
> + */
> +static void emit_relo_ksym_btf(struct bpf_gen *gen, struct ksym_relo_desc *relo, int insn)
> +{

But I don't quite follow why we need these changes to emit_relo_ksym_btf.
Maybe we should have these changes in a separate patch and add some
more explanations?

Thanks,
Song

[...]