On 03/17/2015 06:56 PM, Alexei Starovoitov wrote:
On 3/17/15 2:22 AM, Daniel Borkmann wrote:
On 03/17/2015 02:06 AM, Alexei Starovoitov wrote:
...
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).
yeah. we don't have to. Somehow I felt that keeping ntohs will make
it easier for folks moving from classic to extended, but I guess
they're different enough, so no point wasting run time cycles.
Yes, I think that case seems reasonable in my opinion.
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?
yes. I thought about the same. Currently VLAN_TAG_PRESENT bit is not
officially exposed to user space, but implicitly, since that bit
is always cleared when we return tci to user space and it's always
set when drivers indicate that vlan header was present in the packet.
Right.
So I think we can return skb->vlan_tci as-is, since it will save
one load in bpf program which will be able to do
if (skb->vlan_tci != 0) /* vlan header is present */
vid = skb->vlan_tci & 0x0fff;
compiler will optimize above two accesses into single load and will
reuse the register in 2nd line.
Ok, I'm not sure what's best in the vlan_tci case. I think both
options are a possible way to move forward. If the compiler can
further optimize the latter, it might be the better option. I'll
leave that to you. ;)
Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
