Re: [PATCH net-next v2 0/3] ] TC datapath hash api

[Jamal Hadi Salim <jhs@mojatatu.com>]

Hi,

Several comments:
1) I agree with previous comments that you should
look at incorporating this into skbedit.
Unless incorporating into skbedit introduces huge
complexity, IMO it belongs there.

2) I think it would make sense to create a skb hash classifier
instead of tying this entirely to flower i.e i should not
have to change u32 just so i can support hash classification.
So policy would be something of the sort:

$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 0 proto ip \
flower ip_proto tcp \
action skbedit hash bpf object-file <file> \
action goto chain 2

$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 2 proto ip \
handle 0x0 skbhash  flowid 1:11 mask 0xf  \
action mirred egress redirect dev ens1f0_1

$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 2 proto ip \
handle 0x1 skbhash  flowid 1:11 mask 0xf  \
action mirred egress redirect dev ens1f0_2

IOW, we maintain current modularity as opposed
to dumping everything into flower.
Ive always wanted to write the skbhash classifier but
time was scarce. At one point i had some experiment
where I would copy skb hash into mark in the driver
and use fw classifier for further processing.
It was ugly.

cheers,
jamal

On 2020-07-01 2:47 p.m., Ariel Levkovich wrote:
> Supporting datapath hash allows user to set up rules that provide
> load balancing of traffic across multiple vports and for ECMP path
> selection while keeping the number of rule at minimum.
> 
> Instead of matching on exact flow spec, which requires a rule per
> flow, user can define rules based on hashing on the packet headers
> and distribute the flows to different buckets. The number of rules
> in this case will be constant and equal to the number of buckets.
> 
> The datapath hash functionality is achieved in two steps -
> performing the hash action and then matching on the result, as
> part of the packet's classification.
> 
> The api allows user to define a filter with a tc hash action
> where the hash function can be standard asymetric hashing that Linux
> offers or alternatively user can provide a bpf program that
> performs hash calculation on a packet.
> 
> Usage is as follows:
> 
> $ tc filter add dev ens1f0_0 ingress \
> prio 1 chain 0 proto ip \
> flower ip_proto tcp \
> action hash bpf object-file <file> \
> action goto chain 2
> 
> $ tc filter add dev ens1f0_0 ingress \
> prio 1 chain 0 proto ip \
> flower ip_proto udp \
> action hash bpf asym_l4 basis <basis> \
> action goto chain 2
> 
> $ tc filter add dev ens1f0_0 ingress \
> prio 1 chain 2 proto ip \
> flower hash 0x0/0xf  \
> action mirred egress redirect dev ens1f0_1
> 
> $ tc filter add dev ens1f0_0 ingress \
> prio 1 chain 2 proto ip \
> flower hash 0x1/0xf  \
> action mirred egress redirect dev ens1f0_2
> 
> Ariel Levkovich (3):
>    net/sched: Introduce action hash
>    net/flow_dissector: add packet hash dissection
>    net/sched: cls_flower: Add hash info to flow classification
> 
>   include/linux/skbuff.h              |   4 +
>   include/net/act_api.h               |   2 +
>   include/net/flow_dissector.h        |   9 +
>   include/net/tc_act/tc_hash.h        |  22 ++
>   include/uapi/linux/pkt_cls.h        |   4 +
>   include/uapi/linux/tc_act/tc_hash.h |  32 +++
>   net/core/flow_dissector.c           |  17 ++
>   net/sched/Kconfig                   |  11 +
>   net/sched/Makefile                  |   1 +
>   net/sched/act_hash.c                | 389 ++++++++++++++++++++++++++++
>   net/sched/cls_api.c                 |   1 +
>   net/sched/cls_flower.c              |  16 ++
>   12 files changed, 508 insertions(+)
>   create mode 100644 include/net/tc_act/tc_hash.h
>   create mode 100644 include/uapi/linux/tc_act/tc_hash.h
>   create mode 100644 net/sched/act_hash.c
>