netdev - Re: [PATCH net-next 7/9] net/mlx5e: TC, store tc action cookies per attr

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: <CALnP8ZY_QUf2euy5aGGSHZjcZrsWuQ3eTO2kryw80vaAFvaJGQ@mail.gmail.com>
Date:   Fri, 3 Feb 2023 08:11:59 -0800
From:   Marcelo Ricardo Leitner <mleitner@...hat.com>
To:     Oz Shlomo <ozsh@...dia.com>
Cc:     netdev@...r.kernel.org, Saeed Mahameed <saeedm@...dia.com>,
        Roi Dayan <roid@...dia.com>, Jiri Pirko <jiri@...dia.com>,
        Simon Horman <simon.horman@...igine.com>,
        Baowen Zheng <baowen.zheng@...igine.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Edward Cree <ecree.xilinx@...il.com>
Subject: Re: [PATCH net-next 7/9] net/mlx5e: TC, store tc action cookies per attr

On Wed, Feb 01, 2023 at 06:10:36PM +0200, Oz Shlomo wrote:
> The tc parse action phase translates the tc actions to mlx5 flow
> attributes data structure that is used during the flow offload phase.
> Currently, the flow offload stage instantiates hw counters while
> associating them to flow cookie. However, flows with branching
> actions are required to associate a hardware counter with its action
> cookies.
>
> Store the parsed tc action cookies on the flow attribute.
> Use the list of cookies in the next patch to associate a tc action cookie
> with its allocated hw counter.
>
> Signed-off-by: Oz Shlomo <ozsh@...dia.com>
> Reviewed-by: Roi Dayan <roid@...dia.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 3 +++
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.h | 2 ++
>  2 files changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 39f75f7d5c8b..a5118da3ed6c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -3797,6 +3797,7 @@ bool mlx5e_same_hw_devs(struct mlx5e_priv *priv, struct mlx5e_priv *peer_priv)
>  	parse_attr->filter_dev = attr->parse_attr->filter_dev;
>  	attr2->action = 0;
>  	attr2->counter = NULL;
> +	attr->tc_act_cookies_count = 0;
>  	attr2->flags = 0;
>  	attr2->parse_attr = parse_attr;
>  	attr2->dest_chain = 0;
> @@ -4160,6 +4161,8 @@ struct mlx5_flow_attr *
>  			goto out_free;
>
>  		parse_state->actions |= attr->action;
> +		if (!tc_act->stats_action)
> +			attr->tc_act_cookies[attr->tc_act_cookies_count++] = act->act_cookie;
>
>  		/* Split attr for multi table act if not the last act. */
>  		if (jump_state.jump_target ||
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
> index ce516dc7f3fd..8aa25d8bac86 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
> @@ -70,6 +70,8 @@ struct mlx5_nic_flow_attr {
>  struct mlx5_flow_attr {
>  	u32 action;
>  	struct mlx5_fc *counter;
> +	unsigned long tc_act_cookies[TCA_ACT_MAX_PRIO];
> +	int tc_act_cookies_count;

This one won't count much, as it is limited by TCA_ACT_MAX_PRIO above
andi which is 32.
Maybe this can be an u8 or u16 instead and be added together with 'prio'?
To save 2 bytes, yes, but with a 1M flows, that's 2Mbytes.
Or below 'action' above, to keep it on the same cache line.

>  	struct mlx5_modify_hdr *modify_hdr;
>  	struct mlx5e_mod_hdr_handle *mh; /* attached mod header instance */
>  	struct mlx5e_mod_hdr_handle *slow_mh; /* attached mod header instance for slow path */
> --
> 1.8.3.1
>