netdev - Re: [PATCH net-next] bpf: allow BPF programs access 'protocol' and 'vlan

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: <5507F253.9000806@iogearbox.net>
Date:	Tue, 17 Mar 2015 10:22:27 +0100
From:	Daniel Borkmann <daniel@...earbox.net>
To:	Alexei Starovoitov <ast@...mgrid.com>,
	"David S. Miller" <davem@...emloft.net>
CC:	Thomas Graf <tgraf@...g.ch>, linux-api@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] bpf: allow BPF programs access 'protocol' and
 'vlan_tci' fields

On 03/17/2015 02:06 AM, Alexei Starovoitov wrote:
> as a follow on to patch 70006af95515 ("bpf: allow eBPF access skb fields")
> this patch allows 'protocol' and 'vlan_tci' fields to be accessible
> from extended BPF programs.
>
> The usage of 'protocol', 'vlan_present' and 'vlan_tci' fields is the same as
> corresponding SKF_AD_PROTOCOL, SKF_AD_VLAN_TAG_PRESENT and SKF_AD_VLAN_TAG
> accesses in classic BPF.
>
> Signed-off-by: Alexei Starovoitov <ast@...mgrid.com>

Ok, code looks good to me.

> 1.
> I was thinking to drop ntohs() from 'protocol' field for extended BPF, since
> the programs could do:
> if (skb->protocol == htons(ETH_P_IP))
> which would have saved one or two cpu cycles.
> But having similar behavior between classic and extended seems to be better.

I'm thinking that skb->protocol == htons(ETH_P_IP) might actually
be more obvious, and, as you mentioned, the compiler can already
resolve the htons() during compile time instead of runtime, which
would be another plus.

Either behavior we should document later anyway.

The question to me here is, do we need to keep similar behavior?

After all, the way of programming both from a user perspective is
quite different (i.e. bpf_asm versus C/LLVM).

Similarly, I was wondering, if just exporting raw skb->vlan_tci is
already sufficient, and the user can e.g. write helpers to extract
bits himself from that protocol field?

> 2.
> 'vlan_tci' name is picked to match real sk_buff->vlan_tci field
> and matches tpacket's tp_vlan_tci field.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html