netdev - Re: [PATCH net-next 1/1] net/sched: Introduce skb hash classifier

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 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:   Sun, 9 Aug 2020 19:41:38 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...nulli.us>,
        Ariel Levkovich <lariel@...lanox.com>
Subject: Re: [PATCH net-next 1/1] net/sched: Introduce skb hash classifier

On 2020-08-09 2:15 p.m., Cong Wang wrote:
> On Fri, Aug 7, 2020 at 3:28 PM Jamal Hadi Salim <jhs@...atatu.com> wrote:
>>
>> From: Jamal Hadi Salim <jhs@...atatu.com>
>>
>> his classifier, in the same spirit as the tc skb mark classifier,
>> provides a generic (fast lookup) approach to filter on the hash value
>> and optional mask.
>>
>> like skb->mark, skb->hash could be set by multiple entities in the
>> datapath including but not limited to hardware offloaded classification
>> policies, hardware RSS (which already sets it today), XDP, ebpf programs
>> and even by other tc actions like skbedit and IFE.
> 
> Looks like a lot of code duplication with cls_fw, is there any way to
> reuse the code?
> 

Yeah, it was the simplest way to code this.
They are very similar since they both use 32 bit keys.
I am not exactly thrilled by the current 255 buckets(limited by rcu
structure) - it limits performance in presence of large number
of entries (since they can only be spread across 255 buckets).
If we have a million entries, there will be a lot of hash collisions.

> And I wonder if it is time to introduce a filter to match multiple skb
> fields, as adding a filter for each skb field clearly does not scale.
> Perhaps something like:
> 
> $TC filter add dev $DEV1 parent ffff: protocol ip prio 3 handle X skb \
> hash Y mark Z flowid 1:12
> 

Interesting idea. Note: my experience is that typical setup is
to have only one of those (from offload perspective). Ariel,
are your use cases requiring say both fields?

 From policy perspective, i think above will get more complex
mostly because you have to deal with either mark or hash
being optional. It also opens doors for more complex matching
requirements. Example "match mark X AND hash Y" and
"match mark X OR hash Y".
The new classifier will have to deal with that semantic.

With fw and hash being the complex/optional semantics are easy:

"match mark X AND hash Y":
$TC filter add dev $DEV1 parent ffff: protocol ip prio 3 handle X 
skbhash flowid 1:12 action continue
$TC filter add dev $DEV1 parent ffff: protocol ip prio 4 handle Y fw 
flowid 1:12 action ok

"match mark X OR hash Y":
$TC filter add dev $DEV1 parent ffff: protocol ip prio 3 handle X 
skbhash flowid 1:12 action ok
$TC filter add dev $DEV1 parent ffff: protocol ip prio 4 handle Y fw 
flowid 1:12 action ok

Then the question is how to implement? is it one hash table for
both or two(one for mark and one for hash), etc.

cheers,
jamal