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

Jamal Hadi Salim
2020-07-03T11:22:53Z

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
>