Re: [RFC 1/3] bpf/wireless: add wifimon program type

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2017-04-14 at 11:51 -0700, Alexei Starovoitov wrote:
> 
> so today only 'len' field is needed,

Correct, the other __sk_buff fields don't apply.

It's more of an XDP model (with data/data_end), but as the SKB might be
non-linear at this point I prefer to have the SKB so that skb data
access (skb_copy_bits equivalent) works.

> but the plan to add wifi specific
> stuff to bpf context?

Maybe, maybe not.

> If so, adding these very wifi specific fields to __sk_buff will
> confuse other program types,

I don't think this is true - the verifier still checks that you can't
actually use them. It might confuse authors though, if not documented
well.

> so new program type (like you did) and new 'struct bpf_wifi_md' are
> probably cleaner.

The problem is that I still need struct __wifi_sk_buff or so, and need
to internally let it operate like an SKB, since I need
bpf_skb_load_bytes().

Now, bpf_helpers declares bpf_skb_load_bytes() to take an argument of
type "struct __sk_buff *", and thus I either need to provide an alias
to it, or cast every time I use this function.

> Physically the R1 register to bpf program will still be 'struct
> sk_buff', but from bpf program side it will be:
> struct bpf_wifi_md {
>   __u32 len;
>   __u32 wifi_things;
> };

Right, that would work immediately if it's only about the extra fields.
But it's more about being able to call bpf_skb_load_bytes() easily.

I don't even know if I want to add *any* wifi_things here at all. I
don't actually have much data available directly at this point, or at
least not in a format that would be useful, so I think it'd be better
to have a BPF helper function to obtain wifi_things outside of the
struct itself, passing the struct bpf_wifi_md * (and thus getting
struct sk_buff * in the kernel code) to it.


> At the same time if most of the __sk_buff fields can be useful to
> wifimon, then just use it as-is (without restricting to 'len' only)
> and add wifi specific fields to it with a comment.

No, I don't think they can ever be useful.

johannes