- User Since
- May 24 2019, 12:23 PM (209 w, 5 d)
Mar 29 2023
Note, this is not the only transformation that hurts BPF verification, @yonghong-song previously added "undo transformation" logic, but it seems like a not very sustainable and very convoluted solution. So we are asking if we can have a simpler and more reliable opt-out for some small set of transformations.
One goal of LICM is to target-independently canonicalise the IR to enable other IR optimizations, so IMO it would be preferable to avoid disabling parts via target hooks. Also, the currently proposed hook is not really scalable, e.g. what if another target wants to opt out as well?
Mar 28 2023
Sep 8 2022
Aug 18 2022
Are there any restrictions about number of struct arguments that can be passed in? Kernel changes were assuming at most 2, should we have a test that tests passing 3 structs that fit into 5 input registers and another test that passes 3 structs that do not fit in 5 input registers? And what should be the behavior in the latter case -- compilation error or switch to "pass by reference"?
Aug 3 2022
Aug 2 2022
This will severly break BPF users, as we have a heavy reliance on const volatile variables being allocated to .rodata, which are passed to BPF verifier in Linux kernel as read-only values. This is critical for BPF Complie Once - Run Everywhere approach of writing portable BPF applications. We've been relying on this behavior for years now and changes this will break many existing BPF applications.
Dec 15 2021
Nov 11 2021
Oct 19 2021
Great, thank you!
Oct 11 2021
Sep 9 2021
Aug 4 2021
I've hit similar issue in the past and it's a very frustrating experience having to work around that. So thumbs up for preventing this!
Aug 3 2021
I think it's reasonable behavior to just merge all bpf_tags across declaration(s) and a definition. Kind of like __weak behaves, it doesn't matter if it's on actual function or its extern declaration.
Aug 2 2021
LGTM from the BTF definition standpoint.
Jul 27 2021
May 6 2021
But bar is not a field, why can't we just return error for such case? If someone wants to check type existence we have a separate TYPE_EXISTS relocation.
Apr 19 2021
Apr 15 2021
Nice, thanks! This will work for externs with explicit section name (.ksym) and with no section name (externs for static linking), right?
Apr 13 2021
Mar 4 2021
Feb 8 2021
Dec 19 2020
Having an explicit special section for extern functions clearly indicating the intent (that it should match kernel function) is really good. Especially with static linking. Otherwise, this "try local function, otherwise fallback to kernel function" is going to be really painful for users if there is any error, because all such errors will be indicated at the load time instead of at the link time, where they should be detected and reported.
Nov 15 2020
Aug 4 2020
This is different than testing whether a type exists or not where we want to precisely test whether that type exists or not, so we want to preserve typedef type vs. struct t type.
Aug 3 2020
Tested locally. Previously failing tests are now passing. All recorded relocations look correct. Thanks!
LGTM. One question: why didn't we run into the need for SeqNumVal trick with field-based relocations? We seem to need it for all other types (including type ID-based), but not for field-based?
Looks good. I've tested locally all this and it works as expected. Thanks!
Jul 30 2020
looks great, I'll complete libbpf support for all these ASAP
Jul 22 2020
After thinking about this a bit more, I think adding a new TYPE_EXISTENCE (instead of FIELD_EXISTENCE) relocation type is the better way to handle this. It would allow Clang to be stricter as to what is passed into FIELD_EXISTENCE (only fields, not types) vs TYPE_EXISTENCE(only types, no fields). It will make it cleaner on libbpf side as well, because the validation and relocation logic is different between handling fields and types. So overall, I think it will be beneficial on every level of the stack to separate them. I hope it's not too hard to make this change?
Jul 16 2020
Thanks for the fix! LGTM.
Jul 15 2020
This is awesome. I'll apply patches locally and will play with them tonight to see if everything works in libbpf as well. Thanks!
Jul 12 2020
ah, makes sense. LGTM then :)
Jul 11 2020
maybe let's just remove special-casing of .maps section? It's just like any other global variable, should be handled in exactly the same way, no?
Jul 10 2020
I concur with @yonghong-song, making float disappear is much bigger pain.
pahole does a sensible thing, it represents it as BTF_KIND_INT (which maps to DWARF's DW_TAG_base_type), but with no encoding (so it's effectively unsigned integer with proper size).
Jul 6 2020
Awesome, that's exactly what we need for BPF helper availability checks!
Jun 17 2020
Jun 9 2020
V >= Lo && V < Hi --> V - Lo u< Hi - Lo
V < Lo || V >= Hi --> V - Lo u>= Hi - Lo
IntrinsicsBPF.td uses __builtin_bpf_load_u32_to_ptr, while everywhere it is just __builtin_load_u32_to_ptr. I think having bpf prefix there is good, given this is bpf-specific. But either way, probably should be consistent everywhere?
Jun 3 2020
For my purposes, having extern var BTF only for extern const void is just fine. Not crashing Clang for extern void is great as well :)
May 5 2020
May 4 2020
what's the use case for flag==0 (no relocation)? why using built-in at all in such case? Also flag==1 means relocate to local BTF ID or remote (kernel) BTF ID? Do you plan to add flag=2 as well to cover both cases? Or am I misunderstanding the meaning of this flag?
May 1 2020
Let's extend __builtin_btf_type_id() to accept second argument specifying whether it's local BTF ID (from program's BTF) or target BTF ID (from kernel/module BTF)? We can probably make it an enum just like with preserve_access_index() built-in, for easy future extension. WDYT?
Feb 4 2020
looks simple enough, thanks for fixing it fast!
Feb 3 2020
Dec 21 2019
I implemented libbpf relocation patching support for new classes of instructions and tested against kernel selftests and runqslower program. All of those pass. Thanks a lot, this is going to improve BPF CO-RE experience immensely!
Dec 10 2019
Nov 12 2019
looks good, thanks!
Oct 21 2019
hm... size is useful, actually. Ok, let me play with it, it's hard to get a good idea of BTF from LLVM tests. Verifier will definitely reject such variables, though, so we'll need to do sanitization for sure.
if there is no type information available for extern variables, there is little benefit in having them as DATASEC+VAR. No size, no type. We can get the same information from ELF itself without having to teach verifier and libbpf how to handle these new types. Let's drop this for now? The only potential benefit is having a section name associated with extern, but we can either add that later, or even add it in a way that will require no verifier changes - BTF.ext.
Oct 11 2019
Oct 2 2019
Sep 24 2019
Looks good, thanks for fixing so fast!