On Thu, Dec 12, 2024 at 9:15 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote: > > On 24/12/10 10:18AM, Andrii Nakryiko wrote: > > On Tue, Dec 10, 2024 at 7:57 AM Alexei Starovoitov > > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > > > On Tue, Dec 10, 2024 at 12:56 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote: > > > > > > > > > > > > > > This makes total sense to treat all BPF objects in fd_array the same > > > > > way. With BTFs the problem is that, currently, a btf fd can end up > > > > > either in used_btfs or kfunc_btf_tab. I will take a look at how easy > > > > > it is to merge those two. > > > > > > > > So, currently during program load BTFs are parsed from file > > > > descriptors and are stored in two places: env->used_btfs and > > > > env->prog->aux->kfunc_btf_tab: > > > > > > > > 1) env->used_btfs populated only when a DW load with the > > > > (src_reg == BPF_PSEUDO_BTF_ID) flag set is performed > > > > > > > > 2) kfunc_btf_tab is populated by __find_kfunc_desc_btf(), > > > > and the source is attr->fd_array[offset]. The kfunc_btf_tab is > > > > sorted by offset to allow faster search > > > > > > > > So, to merge them something like this might be done: > > > > > > > > 1) If fd_array_cnt != 0, then on load create a [sorted by offset] > > > > table "used_btfs", formatted similar to kfunc_btf_tab in (2) > > > > above. > > > > > > > > 2) On program load change (1) to add a btf to this new sorted > > > > used_btfs. As there is no corresponding offset, just use > > > > offset=-1 (not literally like this, as bsearch() wants unique > > > > keys, so by offset=-1 an array of btfs, aka, old used_maps, > > > > should be stored) > > > > > > > > Looks like this, conceptually, doesn't change things too much: kfuncs > > > > btfs will still be searchable in log(n) time, the "normal" btfs will > > > > still be searched in used_btfs in linear time. > > > > > > > > (The other way is to just allow kfunc btfs to be loaded from fd_array > > > > if fd_array_cnt != 0, as it is done now, but as you've mentioned > > > > before, you had other use cases in mind, so this won't work.) > > > > > > This is getting a bit too complex. > > > I think Andrii is asking to keep BTFs if they are in fd_array. > > > No need to combine kfunc_btf_tab and used_btfs. > > > I think adding BTFs from fd_array to prog->aux->used_btfs > > > should do it. > > > > Exactly, no need to do major changes, let's just add those BTFs into > > used_btfs, that's all. > > Added. However, I have a question here: how to add proper selftests? The btfs > listed in env->used_btfs are later copied to prog->aux->used_btfs, and are > never actually exposed to user-space in any way. So, one test I can think of is > > * passing a btf fd in fd_array on prog load > * closing this btf fd and checking that id exists before closing the program > (requires to wait until rcu sync to be sure that the btf wasn't destroyed, > but still is refcounted) > > Is this enough? Yeah, I think so, something minimal and simple should do, thanks. > > (I assume exposing used_btfs to user-space is also out of scope of this patch > set, right?) right
