On 08/04/2016 12:36 AM, Alexei Starovoitov wrote:
On Wed, Aug 03, 2016 at 12:06:55PM -0700, Tom Herbert wrote:
[...]
This is not at all obvious to XDP programmer. The type of ctx
structure is xdp_md and the definition of that structure in
uapi/linux/bpf.h says that the fields in the that structure are __u32.
So when I, as a user naive the inner workings of the verifier, read
this code it sure looks like we are upcasting a 32 bit value to a 64
bit value-- that does not seem right at all and the compiler
apparently concurs my point of view. If the code ends up being correct
anyway, then the obvious answer to have an explicit cast that points
out the special nature of this cast. Blindly casting to u32 to long
for the purposes of assigning to a pointer is only going to confuse
more people as it has me.
Agree. Would be nice to have few helpers. The question is whether
they belong in bpf.h. Probably not, since they're not kernel abi.
Yeah, fully agree, they don't belong into uapi.
For the same reasons we didn't include instruction building macros
like BPF_ALU64_REG and instead kept them in samples/bpf/libbpf.h
+1
Here probably four static inline functions are needed. Two for __sk_buff
and two for xpd_md. That should make xdp*_kern.c examples a bit easier
to read.
Yeah, all these bits should go into some library headers that the test
cases make use of (and make them easier to parse for developers).
