[an error occurred while processing this directive]
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZMEMseIyJH9ctdKA@google.com>
Date: Wed, 26 Jul 2023 12:08:17 +0000
From: Matt Bobrowski <mattbobrowski@...gle.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Andrii Nakryiko <andrii@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <martin.lau@...nel.org>,
David Vernet <void@...ifault.com>,
Dave Marchevsky <davemarchevsky@...a.com>,
Tejun Heo <tj@...nel.org>,
Kumar Kartikeya Dwivedi <memxor@...il.com>,
Network Development <netdev@...r.kernel.org>,
bpf <bpf@...r.kernel.org>, Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs.
On Tue, Jul 25, 2023 at 02:00:40PM -0700, Alexei Starovoitov wrote:
> On Tue, Jul 25, 2023 at 1:45 PM Matt Bobrowski <mattbobrowski@...gle.com> wrote:
> >
> > Hey Alexei/Andrii,
> >
> > On Fri, Mar 17, 2023 at 01:19:16PM -0700, Alexei Starovoitov wrote:
> > > From: Alexei Starovoitov <ast@...nel.org>
> > >
> > > Allow BPF programs detect at load time whether particular kfunc exists.
> >
> > So, I'm running a GCC built 6.3.7 Linux kernel and I'm attempting to
> > detect whether a specific kfunc i.e. bpf_rcu_read_lock/unlock() exists
> > using the bpf_ksym_exists() macro. However, I'm running into several
> > BPF verifier constraints that I'm not entirely sure how to work around
> > on the aforementioned Linux kernel version, and hence why I'm reaching
> > out for some guidance.
> >
> > The first BPF verifier constraint that I'm running into is that prior
> > to commit 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to
> > kfunc"), it seems that the ld_imm64 instruction with BPF_PSEUDO_BTF_ID
> > can only hold a ksym address for the kind KIND_VAR. However, when
> > attempting to use the kfuncs bpf_rcu_read_lock/unlock() from a BPF
> > program, the kind associated with the BPF_PSEUDO_BTF_ID is actually
> > KIND_FUNC, and therefore trips over this BPF verifier.
> >
> > The code within the example BPF program is along the lines of the
> > following:
> > ```
> > ...
> > void bpf_rcu_read_lock(void) __ksym __weak;
> > void bpf_rcu_read_unlock(void) __ksym __weak;
> > ...
> > if (bpf_ksym_exists(bpf_rcu_read_lock)) {
> > bpf_rcu_read_lock();
> > }
> > ...
> > if (bpf_ksym_exists(bpf_rcu_read_unlock)) {
> > bpf_rcu_read_unlock();
> > }
> > ...
> > ```
> >
> > The BPF verifier error message that is generated on a 6.3.7 Linux
> > kernel when attempting to load a BPF program that makes use of the
> > above approach is as follows:
> > * "pseudo btf_id {BTF_ID} in ldimm64 isn't KIND_VAR"
> >
> > The second BPF verifier constraint comes from attempting to work
> > around the first BPF verifier constraint that I've mentioned
> > above. This is trivially by dropping the conditionals that contain the
> > bpf_ksym_exists() check and unconditionally calling the kfuncs
> > bpf_rcu_read_lock/unlock().
> >
> > The code within the example BPF program is along the lines of the
> > following:
> > ```
> > ...
> > void bpf_rcu_read_lock(void) __ksym __weak;
> > void bpf_rcu_read_unlock(void) __ksym __weak;
> > ...
> > bpf_rcu_read_lock();
> > ...
> > bpf_rcu_read_unlock();
> > ...
> > ```
> >
> > However, in this case the BPF verifier error message that is generated
> > on a 6.3.7 Linux kernel is as follows:
> > * "no vmlinux btf rcu tag support for kfunc bpf_rcu_read_lock"
> >
> > This approach would be suboptimal anyway as the BPF program would fail
> > to load on older Linux kernels complaining that the kfunc is
> > referenced but couldn't be resolved.
> >
> > Having said this, what's the best way to resolve this on a 6.3.7 Linux
> > kernel? The first BPF program I mentioned above making use of the
> > bpf_ksym_exists() macro works on a 6.4 Linux kernel with commit
> > 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to kfunc")
> > applied. Also, the first BPF program I mentioned above works on a
> > 6.1.* Linux kernel...
>
> Backport of that commit to 6.3.x is probably the only way.
Ah, that's very unfortunate. Should we consider sending this patch
series to linux-stable so that it can be considered for 6.3.x?
/M
Powered by blists - more mailing lists