On Thu, Sep 13, 2018 at 3:45 PM Alexei Starovoitov
<alexei.starovoi...@gmail.com> wrote:
>
> On Thu, Sep 13, 2018 at 10:45:53AM -0700, Petar Penkov wrote:
> > From: Petar Penkov <ppen...@google.com>
> >
> > Adds a hook for programs of type BPF_PROG_TYPE_FLOW_DISSECTOR and
> > attach type BPF_FLOW_DISSECTOR that is executed in the flow dissector
> > path. The BPF program is per-network namespace.
> >
> > Signed-off-by: Petar Penkov <ppen...@google.com>
> > Signed-off-by: Willem de Bruijn <will...@google.com>
> ...
> > @@ -2333,6 +2335,7 @@ struct __sk_buff {
> > /* ... here. */
> >
> > __u32 data_meta;
> > + struct bpf_flow_keys *flow_keys;
>
> the bpf prog form patch 4 looks much better now. Thanks!
>
> > };
> >
> > struct bpf_tunnel_key {
> > @@ -2778,4 +2781,27 @@ enum bpf_task_fd_type {
> > BPF_FD_TYPE_URETPROBE, /* filename + offset */
> > };
> >
> > +struct bpf_flow_keys {
> > + __u16 nhoff;
> > + __u16 thoff;
> > + __u16 addr_proto; /* ETH_P_* of valid addrs */
> > + __u8 is_frag;
> > + __u8 is_first_frag;
> > + __u8 is_encap;
> > + __be16 n_proto;
> > + __u8 ip_proto;
> > + union {
> > + struct {
> > + __be32 ipv4_src;
> > + __be32 ipv4_dst;
> > + };
> > + struct {
> > + __u32 ipv6_src[4]; /* in6_addr; network order */
> > + __u32 ipv6_dst[4]; /* in6_addr; network order */
> > + };
> > + };
> > + __be16 sport;
> > + __be16 dport;
> > +};
>
> can you please pack it?
> struct bpf_flow_keys {
> __u16 nhoff; /* 0 2 */
> __u16 thoff; /* 2 2 */
> __u16 addr_proto; /* 4 2 */
> __u8 is_frag; /* 6 1 */
> __u8 is_first_frag; /* 7 1 */
> __u8 is_encap; /* 8 1 */
>
> /* XXX 1 byte hole, try to pack */
>
> __be16 n_proto; /* 10 2 */
> __u8 ip_proto; /* 12 1 */
>
> /* XXX 3 bytes hole, try to pack */
>
> union {
>
> also is_frag and other fields are not used by the kernel and
> only used by the prog to pass data between tail_calls ?
No, these are mapped directly onto fields in struct flow_keys
on return from the BPF program in __skb_flow_bpf_to_target.
For is_frag, for instance:
if (flow_keys->is_frag)
key_control->flags |= FLOW_DIS_IS_FRAGMENT;
This is true for all fields in the struct except nhoff.
> In such case reserve some space in bpf_flow_keys similar to skb->cb
> so it can contain any fields and accommodate for inevitable changes
> to bpf flow dissector prog in the future.
Do you mean a second scratch space akin to cb[], just a few
reserved padding bytes?
We have given some thought to forward compatibility. The existing
fields cannot be changed, but it should be fine if we need to expand
the struct later.
