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] [day] [month] [year] [list]

Message-ID: <607dcb67-40e9-9720-ea22-32cad299b022@nvidia.com>
Date:   Sun, 5 Feb 2023 15:13:24 +0200
From:   Oz Shlomo <ozsh@...dia.com>
To:     Marcelo Ricardo Leitner <mleitner@...hat.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 03/02/2023 18:11, Marcelo Ricardo Leitner wrote:
> 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.
I agree
>>   	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
>>