netdev - Re: [PATCH net-next v3 2/4] net/sched: Introduce action hash

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] [day] [month] [year] [list]

Message-ID: <99dfaa15-c35d-19b9-302c-0946ae32a792@mellanox.com>
Date:   Thu, 16 Jul 2020 11:40:02 -0400
From:   Ariel Levkovich <lariel@...lanox.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Cong Wang <xiyou.wangcong@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...nulli.us>,
        Jakub Kicinski <kuba@...nel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: [PATCH net-next v3 2/4] net/sched: Introduce action hash

On 7/15/20 2:49 PM, Daniel Borkmann wrote:
> On 7/15/20 3:30 PM, Ariel Levkovich wrote:
>> On 7/15/20 2:12 AM, Cong Wang wrote:
>>> On Mon, Jul 13, 2020 at 8:17 PM Ariel Levkovich 
>>> <lariel@...lanox.com> wrote:
>>>> On 7/13/20 6:04 PM, Cong Wang wrote:
>>>>> On Sat, Jul 11, 2020 at 2:28 PM Ariel Levkovich 
>>>>> <lariel@...lanox.com> wrote:
>>>>>> Allow user to set a packet's hash value using a bpf program.
>>>>>>
>>>>>> The user provided BPF program is required to compute and return
>>>>>> a hash value for the packet which is then stored in skb->hash.
>>>>> Can be done by act_bpf, right?
>>>> Right. We already agreed on that.
>>>>
>>>> Nevertheless, as I mentioned, act_bpf is not offloadable.
>>>>
>>>> Device driver has no clue what the program does.
>>> What about offloading act_skbedit? You care about offloading
>>> the skb->hash computation, not about bpf.
>>>
>>> Thanks.
>>
>> That's true but act_skedit provides (according to the current design) 
>> hash
>>
>> computation using a kernel implemented algorithm.
>>
>> HW not necessarily can offload this kernel based jhash function and 
>> therefore
>>
>> we introduce the bpf option. With bpf the user can provide an 
>> implemenation
>>
>> of a hash function that the HW can actually offload and that way user
>>
>> maintains consistency between SW hash calculation and HW.
>>
>> For example, in cases where offload is added dynamically as traffic 
>> flows, like
>>
>> in the OVS case, first packets will go to SW and hash will be 
>> calculated on them
>>
>> using bpf that emulates the HW hash so that this packet will get the 
>> same hash
>>
>> result that it will later get in HW when the flow is offloaded.
>>
>>
>> If there's a strong objection to adding a new action,
>>
>> IMO, we can include the bpf option in act_skbedit - action skbedit 
>> hash bpf <bpf.o>
>>
>> What do u think?
>
> Please don't. From a BPF pov this is all very misleading since it 
> might wrongly suggest
> to the user that existing means aka {cls,act}_bpf in tc are not 
> capable of already doing
> this. They are capable for several years already though. Also, it is 
> very confusing that
> act_hash or 'skbedit hash bpf' can do everything that {cls,act}_bpf 
> can do already, so
> much beyond setting a hash value (e.g. you could set tunnel keys etc 
> from there). Given
> act_hash is only about offloading but nothing else, did you consider 
> for the BPF alternative
> to just use plain old classic BPF given you only need to parse the pkt 
> and calc the hash
> val but nothing more?

You can do almost everything with act_bpf and yet there are explicit 
actions to set a tunnel

key and add/remove MPLS header (and more...).

What do u mean by classic BPF? How will that help with the offload?

It will still go via act_bpf without any indication on what type of 
program is this, won't it?

Thanks,

Ariel