netdev - Re: [RFC bpf-next v3 6/8] flow_dissector: handle no-skb use case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190323010049.dbywat3n6kfelwth@ast-mbp.dhcp.thefacebook.com>
Date:   Fri, 22 Mar 2019 18:00:50 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
        ast@...nel.org, daniel@...earbox.net, simon.horman@...ronome.com,
        willemb@...gle.com, peterpenkov96@...il.com
Subject: Re: [RFC bpf-next v3 6/8] flow_dissector: handle no-skb use case

On Fri, Mar 22, 2019 at 12:59:01PM -0700, Stanislav Fomichev wrote:
> When called without skb, gather all required data from the
> __skb_flow_dissect's arguments and use recently introduces
> no-skb mode of bpf flow dissector.
> 
> Note: WARN_ON_ONCE(!net) will now trigger for eth_get_headlen users.
> 
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> +			struct bpf_flow_keys flow_keys;
> +			struct bpf_flow_dissector ctx = {
> +				.flow_keys = &flow_keys,
> +				.data = data,
> +				.data_end = data + hlen,
> +				.protocol = proto,
> +			};
> +
> +			if (skb) {
> +				ctx.skb = skb;
> +				ctx.protocol = skb->protocol;
> +				ctx.vlan_tci = skb->vlan_tci;
> +				ctx.vlan_proto = skb->vlan_proto;
> +				ctx.vlan_present = skb->vlan_present;
> +			}

are you suggesting to have vlan* fields that only work properly for skb case?
It means that progs/bpf_flow.c would not work as-is for eth_get_headlen.
And to have unified program that works in both cases the program would need to rely
on above internal implementation detail, like checking that ctx->protocol == 0 ?
imo that is worse than having two different flow dissector program types.

May be remove protocol and vlan* from ctx ?
Then the only thing program can do is look at the packet data which is
eth_get_headlen use case. For skb case the existence of vlan can be retrofitted into
dissector results by the kernel after the program finished.