Martin KaFai Lau <martin.lau@xxxxxxxxx> writes: > On 11/10/22 4:10 PM, Toke Høiland-Jørgensen wrote: >>> The problem with AF_XDP is that, IIUC, it doesn't have a data_meta >>> pointer in the userspace. >>> >>> You get an rx descriptor where the address points to the 'data': >>> | 256 bytes headroom where metadata can go | data | >> >> Ah, I was missing the bit where the data pointer actually points at >> data, not the start of the buf. Oops, my bad! >> >>> So you have (at most) 256 bytes of headroom, some of that might be the >>> metadata, but you really don't know where it starts. But you know it >>> definitely ends where the data begins. >>> >>> So if we have the following, we can locate skb_metadata: >>> | 256-sizeof(skb_metadata) headroom | custom metadata | skb_metadata | data | >>> data - sizeof(skb_metadata) will get you there >>> >>> But if it's the other way around, the program has to know >>> sizeof(custom metadata) to locate skb_metadata: >>> | 256-sizeof(skb_metadata) headroom | skb_metadata | custom metadata | data | >>> >>> Am I missing something here? >> >> Hmm, so one could argue that the only way AF_XDP can consume custom >> metadata today is if it knows out of band what the size of it is. And if >> it knows that, it can just skip over it to go back to the skb_metadata, >> no? > > +1 I replied with a similar point in another email. I also think we > can safely assume this. Great! >> >> The only problem left then is if there were multiple XDP programs called >> in sequence (whether before a redirect, or by libxdp chaining or tail >> calls), and the first one resized the metadata area without the last one >> knowing about it. For this, we could add a CLOBBER_PROGRAM_META flag to >> the skb_metadata helper which if set will ensure that the program >> metadata length is reset to 0? > > How is it different from the same xdp prog calling bpf_xdp_adjust_meta() and > bpf_xdp_metadata_export_to_skb() multiple times. The earlier stored > skb_metadata needs to be moved during the latter bpf_xdp_adjust_meta(). The > latter bpf_xdp_metadata_export_to_skb() will overwrite the earlier skb_metadata. Well, it would just be a convenience flag, so instead of doing: metalen = ctx->data - ctx->data_meta; if (metalen) xdp_adjust_meta(-metalen); bpf_xdp_metadata_export_to_skb(ctx); you could just do: bpf_xdp_metadata_export_to_skb(ctx, CLOBBER_PROGRAM_META); and the kernel would do the check+move for you. But, well, the couple of extra instructions to do the check in BPF is probably fine. (I'm talking here about a program that wants to make sure that any custom metadata that may have been added by an earlier program is removed before redirecting to an XSK socket; I expect we'd want to do something like this in the default program in libxdp). -Toke
