netdev - Re: [RFC net-next] net/flow_offload: allow user to offload tc action to net device

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]

Message-ID: <2d2e4ae2-5de0-8c58-cd7a-ee3c841afd45@mojatatu.com>
Date:   Mon, 10 May 2021 18:37:41 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Simon Horman <simon.horman@...ronome.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...lanox.com>
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com,
        Baowen Zheng <baowen.zheng@...igine.com>,
        Louis Peens <louis.peens@...ronome.com>
Subject: Re: [RFC net-next] net/flow_offload: allow user to offload tc action
 to net device

On 2021-04-29 4:10 a.m., Simon Horman wrote:
> From: Baowen Zheng <baowen.zheng@...igine.com>
> 
> Allow use of flow_indr_dev_register/flow_indr_dev_setup_offload to offload
> tc actions independent of flows.
> 
> The motivation for this work is to prepare for using TC police action
> instances to provide hardware offload of OVS metering feature - which calls
> for policers that may be used my multiple flows and whose lifecycle is
> independent of any flows that use them.
> 

Finally! ;->
Happy to see this effort.

>   /* These structures hold the attributes of bpf state that are being passed
> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> index dc5c1e69cd9f..5fd8b5600b4a 100644
> --- a/include/net/flow_offload.h
> +++ b/include/net/flow_offload.h
> @@ -551,6 +551,21 @@ struct flow_cls_offload {
>   	u32 classid;
>   };
>   
> +enum flow_act_command {
> +	FLOW_ACT_REPLACE,
> +	FLOW_ACT_DESTROY,
> +	FLOW_ACT_STATS,
> +};
> +

Should that be CREATE as well as REPLACE/UPDATE?
Missing GET?

> +struct flow_offload_action {
> +	struct netlink_ext_ack *extack;
> +	enum flow_act_command command;
> +	struct flow_stats stats;
> +	struct flow_action action;
> +};

On this:

On 2021-04-29 12:49 p.m., Ido Schimmel wrote:
 > Can you share an example of the tc commands? You might be able to
 > achieve what you want by patching NFP to take into account the policer
 > index in 'struct flow_action_entry'. Seems that it currently assumes
 > that each policer is a new policer.
 >
 > FTR, I do support the overall plan to offload actions independent of
 > flows and to associate stats with actions.

I think Ido may be saying the same thing, but let me provide
my $0.02 Canadiana:

We generally need to identify the instance of a specific action i.e
its general attributes. In tc currently this maps to an index;

I would categorize two types of actions:
1) Stateless, example:

A lot of actions currently fall under this category.

--
sudo tc actions add action drop index 10
--

And later bound to say two flow entries by just specifying
the action {type id,index}:

---
sudo tc filter add dev XX parent ffff: protocol ip prio 8 \
u32 match ip dst 19.0.0.8/32 flowid 1:10 action gact index 10
#
sudo tc filter add dev XX parent ffff: protocol ip prio 7 \
u32 match ip src 10.0.0.0/24 flowid 1:11 action gact index 10
--

In such a case, in hardware this would typically map the specified
index to some hardware counter table index with the same index (10).

If user doesnt specify the index then either the hardware or the kernel
provides one. And user should be able to retrieve it with a GET.
[I have some patches that would actually return the index in the
extack somewhere]

2) stateful

There are other attributes other than counters that are updated
maintained. In this case i think the attribute id aka "index"
may be attribute specific:
Only example i can think of right now is policer (meters).

in hardware such an index will likely map to a meter index.
So for stateless actions i when i enter two commands

Same deal with config, i can create the policer, get,
update, delete it etc independently and maintain the binding
connection separately.

In both cases, dumping just the actions reduces the amount
of data crossing from hw->kernel->userspace.
If you have a lot of flows this becomes extremely valuable.

cheers,
jamal