| Message ID | 20220719132430.19993-2-memxor@gmail.com |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | New nf_conntrack kfuncs for insertion, changing timeout, status | expand |
On Tue, Jul 19, 2022 at 03:24:18PM +0200, Kumar Kartikeya Dwivedi wrote: > > +#define ____BTF_ID_FLAGS_LIST(_0, _1, _2, _3, _4, _5, N, ...) _1##_##_2##_##_3##_##_4##_##_5##__ > +#define __BTF_ID_FLAGS_LIST(...) ____BTF_ID_FLAGS_LIST(0x0, ##__VA_ARGS__, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0) > + > +#define __FLAGS(prefix, ...) \ > + __PASTE(prefix, __BTF_ID_FLAGS_LIST(__VA_ARGS__)) > + > +#define BTF_ID_FLAGS(prefix, name, ...) \ > + BTF_ID(prefix, name) \ > + __BTF_ID(__ID(__FLAGS(__BTF_ID__flags__, ##__VA_ARGS__))) > + > /* > * The BTF_ID_LIST macro defines pure (unsorted) list > * of BTF IDs, with following layout: > @@ -145,10 +164,53 @@ asm( \ > ".popsection; \n"); \ > extern struct btf_id_set name; > > +/* > + * The BTF_SET8_START/END macros pair defines sorted list of > + * BTF IDs and their flags plus its members count, with the > + * following layout: > + * > + * BTF_SET8_START(list) > + * BTF_ID_FLAGS(type1, name1, flags...) > + * BTF_ID_FLAGS(type2, name2, flags...) > + * BTF_SET8_END(list) > + * > + * __BTF_ID__set8__list: > + * .zero 8 > + * list: > + * __BTF_ID__type1__name1__3: > + * .zero 4 > + * __BTF_ID__flags__0x0_0x0_0x0_0x0_0x0__4: > + * .zero 4 Overall looks great, but why encode flags into a name? Why reuse ____BTF_ID for flags and complicate resolve_btfid? Instead of .zero 4 insert the actual flags as .word ? The usage will be slightly different. Instead of: BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE, KF_RET_NULL) it will be BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE | KF_RET_NULL)
On Tue, 19 Jul 2022 at 20:37, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Jul 19, 2022 at 03:24:18PM +0200, Kumar Kartikeya Dwivedi wrote: > > > > +#define ____BTF_ID_FLAGS_LIST(_0, _1, _2, _3, _4, _5, N, ...) _1##_##_2##_##_3##_##_4##_##_5##__ > > +#define __BTF_ID_FLAGS_LIST(...) ____BTF_ID_FLAGS_LIST(0x0, ##__VA_ARGS__, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0) > > + > > +#define __FLAGS(prefix, ...) \ > > + __PASTE(prefix, __BTF_ID_FLAGS_LIST(__VA_ARGS__)) > > + > > +#define BTF_ID_FLAGS(prefix, name, ...) \ > > + BTF_ID(prefix, name) \ > > + __BTF_ID(__ID(__FLAGS(__BTF_ID__flags__, ##__VA_ARGS__))) > > + > > /* > > * The BTF_ID_LIST macro defines pure (unsorted) list > > * of BTF IDs, with following layout: > > @@ -145,10 +164,53 @@ asm( \ > > ".popsection; \n"); \ > > extern struct btf_id_set name; > > > > +/* > > + * The BTF_SET8_START/END macros pair defines sorted list of > > + * BTF IDs and their flags plus its members count, with the > > + * following layout: > > + * > > + * BTF_SET8_START(list) > > + * BTF_ID_FLAGS(type1, name1, flags...) > > + * BTF_ID_FLAGS(type2, name2, flags...) > > + * BTF_SET8_END(list) > > + * > > + * __BTF_ID__set8__list: > > + * .zero 8 > > + * list: > > + * __BTF_ID__type1__name1__3: > > + * .zero 4 > > + * __BTF_ID__flags__0x0_0x0_0x0_0x0_0x0__4: > > + * .zero 4 > > Overall looks great, > but why encode flags into a name? > Why reuse ____BTF_ID for flags and complicate resolve_btfid? > Instead of .zero 4 insert the actual flags as .word ? > > The usage will be slightly different. > Instead of: > BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE, KF_RET_NULL) > it will be > BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE | KF_RET_NULL) Nice, I didn't know you could do complex expressions like this for asm directives like .word, but it makes sense now. TIL. I'm not very well versed with GNU as. I will rework this and drop the resolve_btfids changes for flags. Thanks a lot!
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h index 252a4befeab1..c8055631fb7b 100644 --- a/include/linux/btf_ids.h +++ b/include/linux/btf_ids.h @@ -8,6 +8,15 @@ struct btf_id_set { u32 ids[]; }; +struct btf_id_set8 { + u32 cnt; + u32 flags; + struct { + u32 id; + u32 flags; + } pairs[]; +}; + #ifdef CONFIG_DEBUG_INFO_BTF #include <linux/compiler.h> /* for __PASTE */ @@ -48,6 +57,16 @@ asm( \ #define BTF_ID(prefix, name) \ __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__)) +#define ____BTF_ID_FLAGS_LIST(_0, _1, _2, _3, _4, _5, N, ...) _1##_##_2##_##_3##_##_4##_##_5##__ +#define __BTF_ID_FLAGS_LIST(...) ____BTF_ID_FLAGS_LIST(0x0, ##__VA_ARGS__, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0) + +#define __FLAGS(prefix, ...) \ + __PASTE(prefix, __BTF_ID_FLAGS_LIST(__VA_ARGS__)) + +#define BTF_ID_FLAGS(prefix, name, ...) \ + BTF_ID(prefix, name) \ + __BTF_ID(__ID(__FLAGS(__BTF_ID__flags__, ##__VA_ARGS__))) + /* * The BTF_ID_LIST macro defines pure (unsorted) list * of BTF IDs, with following layout: @@ -145,10 +164,53 @@ asm( \ ".popsection; \n"); \ extern struct btf_id_set name; +/* + * The BTF_SET8_START/END macros pair defines sorted list of + * BTF IDs and their flags plus its members count, with the + * following layout: + * + * BTF_SET8_START(list) + * BTF_ID_FLAGS(type1, name1, flags...) + * BTF_ID_FLAGS(type2, name2, flags...) + * BTF_SET8_END(list) + * + * __BTF_ID__set8__list: + * .zero 8 + * list: + * __BTF_ID__type1__name1__3: + * .zero 4 + * __BTF_ID__flags__0x0_0x0_0x0_0x0_0x0__4: + * .zero 4 + * __BTF_ID__type2__name2__5: + * .zero 4 + * __BTF_ID__flags__0x0_0x0_0x0_0x0_0x0__6: + * .zero 4 + * + */ +#define __BTF_SET8_START(name, scope) \ +asm( \ +".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \ +"." #scope " __BTF_ID__set8__" #name "; \n" \ +"__BTF_ID__set8__" #name ":; \n" \ +".zero 8 \n" \ +".popsection; \n"); + +#define BTF_SET8_START(name) \ +__BTF_ID_LIST(name, local) \ +__BTF_SET8_START(name, local) + +#define BTF_SET8_END(name) \ +asm( \ +".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \ +".size __BTF_ID__set8__" #name ", .-" #name " \n" \ +".popsection; \n"); \ +extern struct btf_id_set8 name; + #else #define BTF_ID_LIST(name) static u32 __maybe_unused name[5]; #define BTF_ID(prefix, name) +#define BTF_ID_FLAGS(prefix, name, ...) #define BTF_ID_UNUSED #define BTF_ID_LIST_GLOBAL(name, n) u32 __maybe_unused name[n]; #define BTF_ID_LIST_SINGLE(name, prefix, typename) static u32 __maybe_unused name[1]; @@ -156,6 +218,8 @@ extern struct btf_id_set name; #define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 }; #define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 }; #define BTF_SET_END(name) +#define BTF_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 }; +#define BTF_SET8_END(name) static struct btf_id_set8 __maybe_unused name = { 0 }; #endif /* CONFIG_DEBUG_INFO_BTF */
Introduce support for defining flags for kfuncs using a new set of macros, BTF_SET8_START/BTF_SET8_END, which define a set which contains 8 byte elements (each of which consists of a pair of BTF ID and flags), using a new BTF_ID_FLAGS macro. This will be used to tag kfuncs registered for a certain program type as acquire, release, sleepable, ret_null, etc. without having to create more and more sets which was proving to be an unscalable solution. Now, when looking up whether a kfunc is allowed for a certain program, we can also obtain its kfunc flags in the same call and avoid further lookups. The way flags for BTF IDs are constructed requires substituting the flag value as a string into the symbol name, concatenating the set of flags (currently limited to 5, but easily extendable to a higher limit), and then consuming the list from resolve_btfids which can then store the final flag value by ORing each individual flag into the final result. The resolve_btfids change is split into a separate patch. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- include/linux/btf_ids.h | 64 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)