Re: [PATCH bpf-next v6 03/11] bpf: Populate kfunc BTF ID sets in struct btf
From: Alexei Starovoitov <hidden>
Date: 2022-01-05 06:19:25
Also in:
bpf, netfilter-devel
On Sun, Jan 02, 2022 at 09:51:07PM +0530, Kumar Kartikeya Dwivedi wrote:
+enum btf_kfunc_hook {
+ BTF_KFUNC_HOOK_XDP,
+ BTF_KFUNC_HOOK_TC,
+ BTF_KFUNC_HOOK_STRUCT_OPS,
+ _BTF_KFUNC_HOOK_MAX,Why prefix with _ ?
+enum {
+ BTF_KFUNC_SET_MAX_CNT = 32,
+};...
+ if (set_cnt + add_set->cnt > BTF_KFUNC_SET_MAX_CNT) {
+ ret = -E2BIG;
+ goto end;
+ }This artificial limit wouldn't be needed if you didn't insist on sorting. The later patches don't take advantage of this sorting feature and I don't see a test for sorting either.
+
+ /* Grow set */
+ set = krealloc(tab->sets[hook][type], offsetof(struct btf_id_set, ids[set_cnt + add_set->cnt]),
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!set) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ /* For newly allocated set, initialize set->cnt to 0 */
+ if (!tab->sets[hook][type])
+ set->cnt = 0;
+ tab->sets[hook][type] = set;
+
+ /* Concatenate the two sets */
+ memcpy(set->ids + set->cnt, add_set->ids, add_set->cnt * sizeof(set->ids[0]));
+ set->cnt += add_set->cnt;Without sorting this function would just assign the pointer. No need for krealloc and memcpy.
+ + if (sort_set) + sort(set->ids, set->cnt, sizeof(set->ids[0]), btf_id_cmp_func, NULL);
All that looks like extra code for a dubious feature.
+bool btf_kfunc_id_set_contains(const struct btf *btf,
+ enum bpf_prog_type prog_type,
+ enum btf_kfunc_type type, u32 kfunc_btf_id)
+{
+ enum btf_kfunc_hook hook;
+
+ switch (prog_type) {
+ case BPF_PROG_TYPE_XDP:
+ hook = BTF_KFUNC_HOOK_XDP;
+ break;
+ case BPF_PROG_TYPE_SCHED_CLS:
+ hook = BTF_KFUNC_HOOK_TC;
+ break;
+ case BPF_PROG_TYPE_STRUCT_OPS:
+ hook = BTF_KFUNC_HOOK_STRUCT_OPS;
+ break;
+ default:
+ return false;
+ }So this switch() is necessary only to compress prog_types into smaller hooks to save memory in the struct btf_kfunc_set_tab, right ? If so both kfunc_id_set_contains() and register_btf_kfunc() should probably use prog_type as an argument for symmetry.