Page MenuHomePhabricator

anakryiko (Andrii Nakryiko)
User

Projects

User does not belong to any projects.

User Details

User Since
May 24 2019, 12:23 PM (70 w, 2 d)

Recent Activity

Aug 4 2020

anakryiko added a comment to D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

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.

typedef t and struct t are also different for field relocations (those t is in different namespaces, actually), so it might be a good idea to disambiguate those for field relocations as well?

Just to be clear. The type recorded in relocation will be always valid since we have type_id there. For field exists, due to we may use nullptr as the object, if CSE happens, it is possible the root type id may be the "struct t" instead of "typedef t" (or the other ordering) assuming the definition "typedef struct t t;". But from root type_id, libbpf will be able to correctly trace down to the field and do the relocation properly.

On the other hand, I agree that this is not ideal as we may use a different root type than what user specified in the source. But everything is hidden and invisible to user so I won't have user visible impact.

Aug 4 2020, 11:58 PM · Restricted Project, Restricted Project
anakryiko added a comment to D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

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 4 2020, 11:05 AM · Restricted Project, Restricted Project

Aug 3 2020

anakryiko accepted D85174: BPF: simplify IR generation for __builtin_btf_type_id().

Tested locally. Previously failing tests are now passing. All recorded relocations look correct. Thanks!

Aug 3 2020, 5:52 PM · Restricted Project, Restricted Project
anakryiko accepted D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

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?

Aug 3 2020, 4:29 PM · Restricted Project, Restricted Project
anakryiko accepted D83878: BPF: support type exist/size and enum exist/value relocations.

Looks good. I've tested locally all this and it works as expected. Thanks!

Aug 3 2020, 4:18 PM · Restricted Project

Jul 30 2020

anakryiko added inline comments to D83878: BPF: support type exist/size and enum exist/value relocations.
Jul 30 2020, 12:24 PM · Restricted Project
anakryiko accepted D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

looks great, I'll complete libbpf support for all these ASAP

Jul 30 2020, 12:17 PM · Restricted Project, Restricted Project

Jul 22 2020

anakryiko added a comment to D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

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 22 2020, 12:55 PM · Restricted Project, Restricted Project

Jul 16 2020

anakryiko accepted D84002: BPF: generate .rodata BTF datasec for certain initialized local var's.

Thanks for the fix! LGTM.

Jul 16 2020, 11:00 PM · Restricted Project

Jul 15 2020

anakryiko added a comment to D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

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 15 2020, 1:14 PM · Restricted Project, Restricted Project

Jul 12 2020

anakryiko accepted D83638: BPF: permit .maps section variables with typedef type.

ah, makes sense. LGTM then :)

Jul 12 2020, 12:31 AM · Restricted Project

Jul 11 2020

anakryiko added a comment to D83638: BPF: permit .maps section variables with typedef type.

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 11 2020, 11:17 PM · Restricted Project

Jul 10 2020

anakryiko added a comment to D83289: [BPF] Emit unknown types as byte arrays.

I concur with @yonghong-song, making float disappear is much bigger pain.

Jul 10 2020, 2:59 PM · Restricted Project
anakryiko added a comment to D83289: [BPF] Emit unknown types as byte arrays.

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 10 2020, 1:00 PM · Restricted Project

Jul 6 2020

anakryiko added a comment to D83242: [clang][BPF] support type exist/size and enum exist/value relocations.

Awesome, that's exactly what we need for BPF helper availability checks!

Jul 6 2020, 1:37 PM · Restricted Project, Restricted Project

Jun 17 2020

anakryiko added inline comments to D82041: [BPF] fix a bug for BTF pointee type pruning.
Jun 17 2020, 12:56 PM · Restricted Project

Jun 9 2020

anakryiko added a comment to D72787: [BPF] Adjust optimizations to generate kernel verifier friendly codes.

V >= Lo && V < Hi --> V - Lo u< Hi - Lo
V < Lo || V >= Hi --> V - Lo u>= Hi - Lo

Jun 9 2020, 7:20 PM · Restricted Project
anakryiko added a comment to D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic.

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 9 2020, 2:54 PM · Restricted Project, Restricted Project

Jun 3 2020

anakryiko accepted D81131: [DebugInfo] Fix assertion for extern void type.

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 :)

Jun 3 2020, 11:02 PM · Restricted Project, Restricted Project, debug-info

May 5 2020

anakryiko added a comment to D74668: [Clang][BPF] implement __builtin_btf_type_id() builtin function.

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?

Originally, I thought flag = 0 for the following use case:

e.g., they just want to know the type of a particular local structure for pretty print purpose.
Note that currently only types for global/extern variables, function parameters are recorded in btf.
  int test() {
     struct { int a; int b; ...} ctx;
     btf_id = __builtin_btf_type_id(ctx, 0);
     bpf_seq_write(seq, &btf_id, sizeof(btf_id));
     bpf_seq_write(seq, &ctx, sizeof(ctx));
     ...
  }

But obviously without relocation, this will not work with btf deduplication, future static linking etc.

May 5 2020, 11:53 AM · debug-info, Restricted Project, Restricted Project

May 4 2020

anakryiko added a comment to D74668: [Clang][BPF] implement __builtin_btf_type_id() builtin function.

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 4 2020, 11:57 PM · debug-info, Restricted Project, Restricted Project

May 1 2020

anakryiko added a comment to D74668: [Clang][BPF] implement __builtin_btf_type_id() builtin function.

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?

May 1 2020, 5:11 PM · debug-info, Restricted Project, Restricted Project

Feb 4 2020

anakryiko accepted D73997: [BPF] disable ReduceLoadWidth during SelectionDag phase.

looks simple enough, thanks for fixing it fast!

Feb 4 2020, 3:09 PM · Restricted Project
anakryiko accepted D73902: [BPF] handle typedef of struct/union for CO-RE relocations.
Feb 4 2020, 8:31 AM · Restricted Project

Feb 3 2020

anakryiko accepted D73900: [BPF] use base lvalue type for preserve_{struct,union}_access_index metadata.

lgtm

Feb 3 2020, 10:02 AM · Restricted Project
anakryiko added inline comments to D73902: [BPF] handle typedef of struct/union for CO-RE relocations.
Feb 3 2020, 9:47 AM · Restricted Project

Dec 21 2019

anakryiko accepted D71790: [BPF] Enable relocation location for load/store/shifts.

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 21 2019, 8:31 PM · Restricted Project

Dec 10 2019

anakryiko accepted D71290: [BPF] put not-section-attribute externs into BTF ".extern" data section.

awesome, thanks!

Dec 10 2019, 11:23 AM · Restricted Project

Nov 12 2019

anakryiko accepted D70145: [BPF] generate BTF_KIND_VARs for all non-static globals.

looks good, thanks!

Nov 12 2019, 2:12 PM · Restricted Project

Oct 21 2019

anakryiko added a comment to D68822: [BPF] Support external globals.

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.

Oct 21 2019, 8:41 PM · Restricted Project
anakryiko added a comment to D68822: [BPF] Support external globals.

Also, another thing. we COULD emit debug info type for externs for BPF in clang. We just have to think whether this is really necessary or not.

Oct 21 2019, 8:41 PM · Restricted Project
anakryiko added a comment to D68822: [BPF] Support external globals.

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 21 2019, 8:13 PM · Restricted Project

Oct 11 2019

anakryiko added inline comments to D68822: [BPF] Support external globals.
Oct 11 2019, 3:38 PM · Restricted Project

Oct 2 2019

anakryiko added inline comments to D67980: [BPF] do compile-once run-everywhere relocation for bitfields.
Oct 2 2019, 9:18 PM · Restricted Project, Restricted Project

Sep 24 2019

anakryiko accepted D67979: [BPF] Generate array dimension size properly for zero-size elements.

Looks good, thanks for fixing so fast!

Sep 24 2019, 2:26 PM · Restricted Project