Re: [PATCH net-next 13/18] net: Add offload kernel net stack packet type

On Fri, Jun 17, 2016 at 6:15 PM, Daniel Borkmann [off-list ref] wrote:

On 06/17/2016 04:43 PM, Saeed Mahameed wrote:

quoted

From: Maor Gottlieb <redacted>

Add new packet type to skip kernel specific protocol handlers.

This is needed so device drivers can pass packets up to user space
(af_packet/tcpdump, etc..) without the need for them to go through
the whole kernel data path.

Signed-off-by: Maor Gottlieb <redacted>
CC: David S. Miller <davem@davemloft.net>
CC: Patrick McHardy <redacted>
CC: Eric Dumazet <edumazet@google.com>
Signed-off-by: Saeed Mahameed <redacted>
---
  include/linux/skbuff.h         | 6 +++---
  include/uapi/linux/if_packet.h | 1 +
  net/core/dev.c                 | 4 ++++
  3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index dc0fca7..359724e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -694,14 +694,14 @@ struct sk_buff {

  /* if you move pkt_type around you also must adapt those constants */
  #ifdef __BIG_ENDIAN_BITFIELD
-#define PKT_TYPE_MAX   (7 << 5)
+#define PKT_TYPE_MAX   (8 << 5)
  #else
-#define PKT_TYPE_MAX   7
+#define PKT_TYPE_MAX   8
  #endif

Aehm ... did you actually test this with BPF ?!

PKT_TYPE_MAX is a mask (naming could be better no doubt), see also function
convert_skb_access():

[...]
        case SKF_AD_PKTTYPE:
                *insn++ = BPF_LDX_MEM(BPF_B, dst_reg, src_reg,
PKT_TYPE_OFFSET());
                *insn++ = BPF_ALU32_IMM(BPF_AND, dst_reg, PKT_TYPE_MAX);
#ifdef __BIG_ENDIAN_BITFIELD
                *insn++ = BPF_ALU32_IMM(BPF_RSH, dst_reg, 5);
#endif
                break;
[...]

Also, dunno if it's worth burning a skb bit for one driver.

oops! it didn't occur to me that it was used as a mask!
does this mean we are out of PKT_TYPE flags ?
Maybe we can use skb->mark instead ? any ideas ?

Also i am not sure it is a one driver thing, i know about other
examples where some device drivers set skb->protocol to 0xffff to
achieve the same ! which i didn't like.