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

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]

Date:	Sat, 18 Jun 2016 01:54:46 +0300
From:	Saeed Mahameed <saeedm@....mellanox.co.il>
To:	Daniel Borkmann <daniel@...earbox.net>
Cc:	Saeed Mahameed <saeedm@...lanox.com>,
	"David S. Miller" <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Doug Ledford <dledford@...hat.com>,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Maor Gottlieb <maorg@...lanox.com>,
	Huy Nguyen <huyn@...lanox.com>, Tal Alon <talal@...lanox.com>,
	Patrick McHardy <kaber@...sh.net>,
	Eric Dumazet <edumazet@...gle.com>, ast@...com
Subject: 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 <daniel@...earbox.net> wrote:
> On 06/17/2016 04:43 PM, Saeed Mahameed wrote:
>>
>> From: Maor Gottlieb <maorg@...lanox.com>
>>
>> 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 <maorg@...lanox.com>
>> CC: David S. Miller <davem@...emloft.net>
>> CC: Patrick McHardy <kaber@...sh.net>
>> CC: Eric Dumazet <edumazet@...gle.com>
>> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
>> ---
>>   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.