On Fri, May 28, 2021 at 4:16 AM Jesper Dangaard Brouer <brouer@xxxxxxxxxx> wrote: > > On Wed, 26 May 2021 15:39:17 -0700 > Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > > On Wed, May 26, 2021 at 1:20 PM Jesper Dangaard Brouer > > <brouer@xxxxxxxxxx> wrote: > > > > > > On Wed, 26 May 2021 12:12:09 -0700 > > > Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > > > On Wed, May 26, 2021 at 3:59 AM Jesper Dangaard Brouer > > > > <brouer@xxxxxxxxxx> wrote: > > > > > > > > > > Hi All, > > > > > > > > > > I see a need for a driver to use different XDP metadata layout on a per > > > > > packet basis. E.g. PTP packets contains a hardware timestamp. E.g. VLAN > > > > > offloading and associated metadata as only relevant for packets using > > > > > VLANs. (Reserving room for every possible HW-hint is against the idea > > > > > of BTF). > > > > > > > > > > The question is how to support multiple BTF types on per packet basis? > > > > > (I need input from BTF experts, to tell me if I'm going in the wrong > > > > > direction with below ideas). > > > > > > > > I'm trying to follow all three threads, but still, can someone dumb it > > > > down for me and use few very specific examples to show how all this is > > > > supposed to work end-to-end. I.e., how the C definition for those > > > > custom BTF layouts might look like and how they are used in BPF > > > > programs, etc. I'm struggling to put all the pieces together, even > > > > ignoring all the netdev-specific configuration questions. > > > > > > I admit that this thread is pushing the boundaries and "ask" too much. > > > I think we need some steps in-between to get the ball rolling first. I > > > myself need to learn more of what is possible today with BTF, before I > > > ask for more features and multiple simultaneous BTF IDs. > > > > > > I will go read Andrii's excellent docs [1]+[2] *again*, and perhaps[3]. > > > Do you recommend other BTF docs? > > > > BTF in itself, at least as related to type definitions, is a super > > lightweight and straightforward DWARF replacement. I'd recommend to > > just play around with building a simple BPF code with various types > > defined (use `clang -target bpf -g`) and then dump BTF info in both > > raw format (just `bpftool btf dump file <path>` and in C format > > (`bpftool btf dump file <path> format c`). That should be plenty to > > get the feel for BTF. > > I've played with this and I think I get this part now :-) ok, great > > > As for how libbpf and BPF CO-RE use BTF, I guess the below blog post > > is a good start, I'm not sure we have another dedicated post > > describing how CO-RE relocations work. > > > > > > > > [1] https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html > > > [2] https://nakryiko.com/posts/bpf-portability-and-co-re/ > > > > Choose [2], it's slightly more updated, but otherwise is the same as [1]. > > > > > [3] https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html > > > > It's up to you, but I wouldn't bother reading the BTF dedup > > description in order to understand BTF in general :) > > Yes, I think I'll skip that dedup part ;-) > > > > > > > > As for BTF on a per-packet basis. This means that BTF itself is not > > > > known at the BPF program verification time, so there will be some sort > > > > of if/else if/else conditions to handle all recognized BTF IDs? Is > > > > that right? > > > > > > I do want libbpf CO-RE and BPF program verification to work. I'm > > > asking for a BPF-program that can supply multiple BTF struct layouts > > > and get all of them CO-RE offset adjusted. > > > > > > The XDP/BPF-prog itself have if/else conditions on BPF-IDs to handle > > > all the BPF IDs it knows. When loading the BPF-prog the offset > > > relocation are done for the code (as usual I presume). > > > > Ok, so assuming kernel/driver somehow defines these two C structs: > > > > struct xdp_meta_1 { int x; char y[32]; } __attribute__((preserve_access_index)); > > > > struct xdp_meta_2 { void *z; int q[4]; } __attribute__((preserve_access_index)); > > > > on BPF program side, you should be able to do something like this: > > > > int xdp_btf_id = xdp_ctx->btf_id; > > void *xdp_meta = xdp_ctx->meta; > > > > if (xdp_btf_id == bpf_core_type_id_kernel(struct xdp_meta_1)) { > > struct xdp_meta_1 *m = xdp_meta; > > > > return m->x + m->y[7] * 3; > > } else if (xdp_btf_id == bpf_core_type_id_kernel(struct xdp_meta_2)) { > > struct xdp_meta_2 *m = xdp_meta; > > > > return m->z - m->q[2]; > > } else { > > /* we don't know what metadata layout we are working with */ > > return XDP_DROP; > > } > > Yes, I think this is the gist of what I was thinking. > > Cool that we have a bpf_core_type_id_kernel() macro (if others want to > follow in tools/lib/bpf/bpf_core_read.h). That looks VERY helpful for > what I'm looking for. > > /* > * Convenience macro to get BTF type ID of a target kernel's type that matches > * specified local type. > * Returns: > * - valid 32-bit unsigned type ID in kernel BTF; > * - 0, if no matching type was found in a target kernel BTF. > */ > #define bpf_core_type_id_kernel(type) \ > __builtin_btf_type_id(*(typeof(type) *)0, BPF_TYPE_ID_TARGET) > > > At what point in "time" is this bpf_core_type_id_kernel() resolved? During the load time (i.e., bpf_object__load() or skeleton's xxx__load()). It's done completely on the libbpf side before the kernel sees the final relocated BPF instructions. > > > > What I'm struggling a bit is how xdp_meta_1 and xdp_meta_2 come to be, > > how they get to users building BPF application, etc. For example, if > > those xdp_meta_x structs are dumped on the target kernel and the > > program is compiled right there, you don't really need CO-RE because > > you know exact layout and you are compiling on the fly BCC-style. > > > > I guess one way to allow pre-compilation and still let hardware define > > the actual memory layout would be to have a pre-defined struct > > xdp_meta___mega for BPF program, something like: > > > > struct xdp_meta___mega { int x; char y[32]; void *z; int q[4]; } > > __attribute__((preserve_access_index)); > > > > I.e., it defines all potentially possible fields. But then driver > > knows that only, say, x and q are actually present, so in kernel we > > have > > > > struct xdp_meta { int q[4]; int x; } > > > > With that, libbpf will match local xdp_meta___mega (___suffix is > > ignored) to actual kernel definition, x and q offsets will be > > relocated. If BPF program is trying to access y or z, though, it will > > result in an error. > > Wow, this is also a cool trick that I didn't know we could do. > Having a 'xdp_meta_generic_common___mega' could be very useful. > Yes, this triple underscore suffix allows to have multiple versions of types in kernel BTF (which can happen during dedup due to struct name collisions, like it happened before with two different struct ring_buffer until they got renamed) and/or on BPF program side, e.g., to have struct layouts for multiple kernel versions or configurations all in one compilable application. > > CO-RE also allows to check the presence of y and z > > and deal with that, so you have flexibility to do "feature detection" > > right in BPF code: > > > > if (bpf_core_field_exists(m->z)) { > > return m->z; > > } else { > > /* deal with lack of m->z */ > > } > > The bpf_core_field_exists() also look like an interesting and useful > feature. I assume this bpf_core_field_exists() happens at libbpf > load-time, or even at BPF-syscall load-time. Right? > yep > > > Hopefully that gives a bit clearer picture of what CO-RE is about. I > > guess I can also suggest reading [0] for a few more uses of CO-RE, > > just for general understanding. > > Thanks a lot for educating me :-) sure, you are welcome! > > > [0] https://nakryiko.com/posts/bpf-tips-printk/ > > > > > > > > Maybe it is worth pointing out, that the reason for requiring the > > > BPF-prog to check the BPF-ID match, is to solve the netdev HW feature > > > update problem. I'm basically evil and say we can update the netdev HW > > > features anytime, because it is the BPF programmers responsibility to > > > check if BTF info changed (after prog was loaded). (The BPF programmer > > > can solve this via requesting all the possible BTF IDs the driver can > > > change between, or choose she is only interested in a single variant). > > > > Ok, see above, if you know all possible BTF IDs ahead of time, then > > yes, you can do this. You'll pay the price of doing a bunch of if/else > > for BTF ID comparison, of course, but not the price of accessing those > > fields. > > It sounds great that this is basically already possible today. > > I'm willing to pay the if/else for BTF ID comparison cost, only because > it solves the problem of allowing the kernel/driver to change the > BTF-layout for the XDP metadata area dynamically, AFTER the BPF-prog > have been loaded (and after all the nice libbpf relocations). Ok, cool. This assumes you can know all possible BTF IDs, don't know how realistic that is in each particular use case. > > > > > > > By this, I'm trying to avoid loading an XDP-prog locks down what > > > hardware features can be enabled/disabled. It would be sad running > > > tcpdump (-j adapter_unsynced) that request for HW RX-timestamp is > > > blocked due to XDP being loaded. > > I think we need to included this as part of our XDP-hints design. Yes, > there is an annoying overhead in the XDP-prog to check bpf_id's, but it > will be even more annoying/user-unfriendly to lock-down any hardware > changed when an XDP-prog is loaded. > > If sysadm knows that nobody will ever run those commands that change > hardware state and affect XDP-metadata layout, then end-user can remove > this bpf_id check from their BPF-prog and get back the performance. > > > > > > > > > Fake but specific code would help (at least me) to actually join the > > > > discussion. Thanks. > > > > > > I agree, I actually want to code-up a simple example that use BTF CO-RE > > > and then try to follow the libbpf code that adjust the offsets. I > > > admit I need to understand BTF better myself, before I ask for > > > new/advanced features ;-) > > > > > > Thanks Andrii for giving us feedback, I do need to learn more about BTF > > > myself to join the discussion myself. > > > > You are welcome. I left a few breadcrumbs above, I hope that helps a bit. > > Thanks for all the breadcrumbs, really appreciate it! > ... I'm trying to follow them, and the rabbit hole is deep :-) > The good thing is that CO-RE seems pretty stable and complete and hasn't changed much over a pretty long time now, so it probably will pay off to learn that now and use in the future. > > > > > > > > > > > > > > Let me describe a possible/proposed packet flow (feel free to > > > > > disagree): > > > > > > > > > > When driver RX e.g. a PTP packet it knows HW is configured for > > > > > PTP-TS and when it sees a TS is available, then it chooses a code > > > > > path that use the BTF layout that contains RX-TS. To communicate > > > > > what BTF-type the XDP-metadata contains, it simply store the BTF-ID > > > > > in xdp_buff->btf_id. > > > > > > > > > > When redirecting the xdp_buff is converted to xdp_frame, and also > > > > > contains the btf_id member. When converting xdp_frame to SKB, then > > > > > netcore-code checks if this BTF-ID have been registered, if so > > > > > there is a (callback or BPF-hook) registered to handle this > > > > > BTF-type that transfer the fields from XDP-metadata area into SKB > > > > > fields. > > > > > > > > > > The XDP-prog also have access to this ctx->btf_id and can > > > > > multiplex on this in the BPF-code itself. Or use other methods like > > > > > parsing PTP packet and extract TS as expected BTF offset in XDP > > > > > metadata (perhaps add a sanity check if metadata-size match). > > > > > > > > > > > > > > > I talked to AF_XDP people (Magnus, Bjørn and William) about this > > > > > idea, and they pointed out that AF_XDP also need to know what > > > > > BTF-layout is used. As Magnus wrote in other thread; there is only > > > > > 32-bit left in AF_XDP descriptor option. We could store the BTF-ID > > > > > in this field, but it would block for other use-cases. Bjørn came > > > > > up with the idea of storing the BTF-ID in the BTF-layout itself, > > > > > but as the last-member (to have fixed offset to check in userspace > > > > > AF_XDP program). Then we only need to use a single bit in AF_XDP > > > > > descriptor option to say XDP-metadata is BTF described. > > > > > > > > > > In the AF_XDP userspace program, the programmers can have a similar > > > > > callback system per known BTF-ID. This way they can compile > > > > > efficient code per ID via requesting the BTF layout from the > > > > > kernel. (Hint: `bpftool btf dump id 42 format c`). > > > > > > > > > > Please let me know if this it the right or wrong direction? > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer >
