From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
Date: Thu, 21 Apr 2022 19:17:11 +0200
> Larysa Zaremba <larysa.zaremba@xxxxxxxxx> writes:
>
> > Dear all,
> > Our team has encountered a need of accessing data_meta in a following way:
> >
> > int xdp_meta_prog(struct xdp_md *ctx)
> > {
> > void *data_meta_ptr = (void *)(long)ctx->data_meta;
> > void *data_end = (void *)(long)ctx->data_end;
> > void *data = (void *)(long)ctx->data;
> > u64 data_size = sizeof(u32);
> > u32 magic_meta;
> > u8 offset;
> >
> > offset = (u8)((s64)data - (s64)data_meta_ptr);
> > if (offset < data_size) {
> > bpf_printk("invalid offset: %ld\n", offset);
> > return XDP_DROP;
> > }
> >
> > data_meta_ptr += offset;
> > data_meta_ptr -= data_size;
> >
> > if (data_meta_ptr + data_size > data) {
> > return XDP_DROP;
> > }
> >
> > magic_meta = *((u32 *)data);
> > bpf_printk("Magic: %d\n", magic_meta);
> > return XDP_PASS;
> > }
> >
> > Unfortunately, verifier claims this code attempts to access packet with
> > an offset of -2 (a constant part) and negative offset is generally forbidden.
> >
> > For now we have 2 solutions, one is using bpf_xdp_adjust_meta(),
> > which is pretty good, but not ideal for the hot path.
> > The second one is the patch at the end.
> >
> > Do you see any other way of accessing memory from the end of data_meta/data?
> > What do you think about both suggested solutions?
>
> The problem is that the compiler is generating code that the verifier
> doesn't understand. It's notoriously hard to get LLVM to produce code
> that preserves the right bounds checks which is why projects like Cilium
> use helpers with inline ASM to produce the right loads, like in [0].
>
> Adapting that cilium helper to load from the metadata area, your example
> can be rewritten as follows (which works just fine with no verifier
> changes):
>
> static __always_inline int
> xdp_load_meta_bytes(const struct xdp_md *ctx, __u64 off, void *to, const __u64 len)
> {
> void *from;
> int ret;
> /* LLVM tends to generate code that verifier doesn't understand,
> * so force it the way we want it in order to open up a range
> * on the reg.
> */
> asm volatile("r1 = *(u32 *)(%[ctx] +8)\n\t"
> "r2 = *(u32 *)(%[ctx] +0)\n\t"
> "%[off] &= %[offmax]\n\t"
> "r1 += %[off]\n\t"
> "%[from] = r1\n\t"
> "r1 += %[len]\n\t"
> "if r1 > r2 goto +2\n\t"
> "%[ret] = 0\n\t"
> "goto +1\n\t"
> "%[ret] = %[errno]\n\t"
> : [ret]"=r"(ret), [from]"=r"(from)
> : [ctx]"r"(ctx), [off]"r"(off), [len]"ri"(len),
> [offmax]"i"(__CTX_OFF_MAX), [errno]"i"(-EINVAL)
> : "r1", "r2");
> if (!ret)
> __builtin_memcpy(to, from, len);
> return ret;
> }
>
>
> SEC("xdp")
> int xdp_meta_prog(struct xdp_md *ctx)
> {
> void *data_meta_ptr = (void *)(long)ctx->data_meta;
> void *data = (void *)(long)ctx->data;
> __u32 magic_meta;
> __u8 offset;
> int ret;
>
> offset = (__u8)((__s64)data - (__s64)data_meta_ptr);
> ret = xdp_load_meta_bytes(ctx, offset - 4, &magic_meta, sizeof(magic_meta));
> if (ret) {
> bpf_printk("load bytes failed: %d\n", ret);
> return XDP_DROP;
> }
>
> bpf_printk("Magic: %d\n", magic_meta);
> return XDP_PASS;
> }
At the moment, we use this (based on Cilium's and your), it works
just like we want C code to work previously:
#define __CTX_OFF_MAX 0xff
static __always_inline void *
can_i_access_meta_please(const struct xdp_md *ctx, __u64 off, const __u64 len)
{
void *ret;
/* LLVM tends to generate code that verifier doesn't understand,
* so force it the way we want it in order to open up a range
* on the reg.
*/
asm volatile("r1 = *(u32 *)(%[ctx] +8)\n\t"
"r2 = *(u32 *)(%[ctx] +0)\n\t"
"%[off] &= %[offmax]\n\t"
"r1 += %[off]\n\t"
"%[ret] = r1\n\t"
"r1 += %[len]\n\t"
"if r1 > r2 goto +1\n\t"
"goto +1\n\t"
"%[ret] = %[null]\n\t"
: [ret]"=r"(ret)
: [ctx]"r"(ctx), [off]"r"(off), [len]"ri"(len),
[offmax]"i"(__CTX_OFF_MAX), [null]"i"(NULL)
: "r1", "r2");
return ret;
}
SEC("xdp")
int xdp_prognum_n0_meta(struct xdp_md *ctx)
{
void *data_meta = (void *)(__s64)ctx->data_meta;
void *data = (void *)(__s64)ctx->data;
struct xdp_meta_generic *md;
__u64 offset;
offset = (__u64)((__s64)data - (__s64)data_meta);
md = can_i_access_meta_please(ctx, offset, sizeof(*md));
if (__builtin_expect(!md, 0)) {
bpf_printk("No you can't\n");
return XDP_DROP;
}
bpf_printk("Magic: 0x%04x\n", md->magic_id);
return XDP_PASS;
}
Thanks for the help! It's a shame LLVM still suck on generating
correct object code from C.
I guess we'll define a helper above in one of the headers to not
copy-paste it back and forth between each program wanting to
access only the generic part of the metadata (which is always being
placed at the end).
>
> -Toke
>
>
> [0] https://github.com/cilium/cilium/blob/master/bpf/include/bpf/ctx/xdp.h#L35
Thanks,
Al
