On Wed, May 18, 2022 at 5:30 AM Eugene Syromiatnikov esyr@redhat.com wrote:
On Wed, May 18, 2022 at 01:24:56PM +0200, Jiri Olsa wrote:
On Tue, May 17, 2022 at 02:34:55PM -0700, Yonghong Song wrote:
On 5/17/22 1:03 PM, Jiri Olsa wrote:
On Tue, May 17, 2022 at 02:30:50PM +0200, Eugene Syromiatnikov wrote:
On Tue, May 17, 2022 at 11:12:34AM +0200, Jiri Olsa wrote:
On Tue, May 17, 2022 at 09:36:47AM +0200, Eugene Syromiatnikov wrote: > With the interface as defined, it is impossible to pass 64-bit kernel > addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI, > which severly limits the useability of the interface, change the ABI > to accept an array of u64 values instead of (kernel? user?) longs. > Interestingly, the rest of the libbpf infrastructure uses 64-bit values > for kallsyms addresses already, so this patch also eliminates > the sym_addr cast in tools/lib/bpf/libbpf.c:resolve_kprobe_multi_cb().
so the problem is when we have 32bit user sace on 64bit kernel right?
I think we should keep addrs as longs in uapi and have kernel to figure out if it needs to read u32 or u64, like you did for symbols in previous patch
No, it's not possible here, as addrs are kernel addrs and not user space addrs, so user space has to explicitly pass 64-bit addresses on 64-bit kernels (or have a notion whether it is running on a 64-bit or 32-bit kernel, and form the passed array accordingly, which is against the idea of compat layer that tries to abstract it out).
hum :-\ I'll need to check on compat layer.. there must be some other code doing this already somewhere, right?
so the 32bit application running on 64bit kernel using libbpf won't work at the moment, right? because it sees:
bpf_kprobe_multi_opts::addrs as its 'unsigned long'
which is 4 bytes and it needs to put there 64bits kernel addresses
if we force the libbpf interface to use u64, then we should be fine
Yes, that's correct.
I am not familiar with all these compatibility thing. But if we have 64-bit pointer for **syms, maybe we could also have 64-bit pointer for *syms for consistency?
right, perhaps we could have one function to read both syms and addrs arrays
The distinction here it that syms are user space pointers (so they are naturally 32-bit for 32-bit applications) and addrs are kernel-space pointers (so they may be 64-bit even when the application is 32-bit). Nothing prevents from changing the interface so that syms is an array of 64-bit values treated as user space pointers, of course.
I agree. User-space pointers should stay pointers in libbpf API , while kernel addresses are not really pointers for user-space app, so marking it as __u64 seems right.
we'll need to fix also bpf_kprobe_multi_cookie_swap because it assumes 64bit user space pointers
if we have both addresses and cookies 64 then this should be ok
would be gret if we could have selftest for this
let's add selftest for this
Sure, I'll try to write one.