On 02/05, Willem de Bruijn wrote:
> On Tue, Feb 5, 2019 at 12:57 PM Stanislav Fomichev <s...@google.com> wrote:
> >
> > When flow_dissector is called without skb (with only data and hlen),
> > construct on-stack skb (which has a linear chunk of data passed
> > to the flow dissector). This should let us handle eth_get_headlen
> > case where only data is provided and we don't want to (yet) allocate
> > an skb.
> >
> > Since this on-stack skb doesn't allocate its own data, we can't
> > add shinfo and need to be careful to avoid any code paths that use
> > it. Flow dissector BPF programs can only call bpf_skb_load_bytes helper,
> > which doesn't touch shinfo in our case (skb->len is the length of the
> > linear header so it exits early).
> >
> > Signed-off-by: Stanislav Fomichev <s...@google.com>
> > ---
> > include/linux/skbuff.h | 5 +++
> > net/core/flow_dissector.c | 95 +++++++++++++++++++++++++++++----------
> > 2 files changed, 76 insertions(+), 24 deletions(-)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index aa9a9983de80..5f1c085cb34c 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -1227,6 +1227,11 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> > const struct sk_buff *skb,
> > struct flow_dissector *flow_dissector,
> > struct bpf_flow_keys *flow_keys);
> > +bool __flow_bpf_dissect(struct bpf_prog *prog,
> > + void *data, __be16 proto,
> > + int nhoff, int hlen,
> > + struct flow_dissector *flow_dissector,
> > + struct bpf_flow_keys *flow_keys);
>
> nit: please use more descriptive name. Perhaps bpf_flow_dissect_raw
> and rename __skb_flow_bpf_dissect to bpf_flow_dissect_skb.
Agreed.
> > +bool __flow_bpf_dissect(struct bpf_prog *prog,
> > + void *data, __be16 proto,
> > + int nhoff, int hlen,
> > + struct flow_dissector *flow_dissector,
> > + struct bpf_flow_keys *flow_keys)
> > +{
> > + struct bpf_skb_data_end *cb;
> > + struct sk_buff skb;
> > + u32 result;
> > +
> > + __init_skb(&skb, data, hlen);
> > + skb_put(&skb, hlen);
> > + skb.protocol = proto;
> > +
> > + init_flow_keys(flow_keys, &skb, nhoff);
> > +
> > + cb = (struct bpf_skb_data_end *)skb.cb;
> > + cb->data_meta = skb.data;
> > + cb->data_end = skb.data + skb_headlen(&skb);
> > +
> > + result = BPF_PROG_RUN(prog, &skb);
> > +
> > + clamp_flow_keys(flow_keys, hlen);
> >
> > return result == BPF_OK;
> > }
>
> Can__flow_bpf_dissect just construct an skb and then call
> __skb_flow_bpf_dissect?
__skb_flow_bpf_dissect calls bpf_compute_data_pointers which calls
skb_metadata_len which touches shinfo. And I don't think I have a
clever way to handle that.
>
> It will unnecessarily save and restore the control block, but that is
> a relatively small cost (compared to, say, zeroing the entire skb).
