lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <c4427a57-aea9-4acc-a6be-e30cfb1dbaad@gmail.com> Date: Wed, 1 Nov 2023 17:59:30 -0700 From: Kui-Feng Lee <sinquersw@...il.com> To: Martin KaFai Lau <martin.lau@...ux.dev> Cc: kuifeng@...a.com, netdev@...r.kernel.org, bpf@...r.kernel.org, ast@...nel.org, song@...nel.org, kernel-team@...a.com, andrii@...nel.org, thinker.li@...il.com, drosen@...gle.com Subject: Re: [PATCH bpf-next v8 07/10] bpf, net: switch to dynamic registration On 11/1/23 17:17, Martin KaFai Lau wrote: > On 10/31/23 5:19 PM, Kui-Feng Lee wrote: >> >> >> On 10/31/23 17:02, Martin KaFai Lau wrote: >>> On 10/31/23 4:34 PM, Kui-Feng Lee wrote: >>>>>> diff --git a/include/linux/btf.h b/include/linux/btf.h >>>>>> index a8813605f2f6..954536431e0b 100644 >>>>>> --- a/include/linux/btf.h >>>>>> +++ b/include/linux/btf.h >>>>>> @@ -12,6 +12,8 @@ >>>>>> #include <uapi/linux/bpf.h> >>>>>> #define BTF_TYPE_EMIT(type) ((void)(type *)0) >>>>>> +#define BTF_STRUCT_OPS_TYPE_EMIT(type) {((void)(struct type >>>>>> *)0); \ >>>>> >>>>> ((void)(struct type *)0); is new. Why is it needed? >>>> >>>> This is a trick of BTF to force compiler generate type info for >>>> the given type. Without trick, compiler may skip these types if these >>>> type are not used at all in the module. For example, modules usually >>>> don't use value types of struct_ops directly. >>> It is not the value type and value type emit is understood. It is the >>> struct_ops type itself and it is new addition in this patchset >>> afaict. The value type emit is in the next line which was cut out >>> from the context here. >>> >> I mean both of them are required. >> In the case of a dummy implementation, struct_ops type itself properly >> never being used, only being declared by the module. Without this line, > > Other than bpf_dummy_ops, after reg(), the struct_ops->func() must be > used somewhere in the kernel or module. Like tcp must be using the > tcp_congestion_ops after reg(). bpf_dummy_ops is very special and > probably should be moved out to bpf_testmod somehow but this is for > later. Even bpf_dummy_ops does not have an issue now. Why it is needed > after the kmod support change? > > or it is a preemptive addition to be future proof only? > > Addition is fine if it is required to work. I am trying to understand > why this new addition is needed after the kmod support change. The > reason why this is needed after the kmod support change is not obvious > from looking at the code. The commit message didn't mention why and what > broke after this kmod change. If someone wants to clean it up a few > months later, we will need to figure out why it was added in the first > place. It is a future proof. What do you think if I add a comment in the code? > > >> the module developer will fail to load a struct_ops map of the dummy >> type. This line is added to avoid this awful situation. >> >
Powered by blists - more mailing lists