| 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
| ||
|
Date: Mon, 17 Jun 2019 18:24:22 +0000
From: Mark Bloch <markb@...lanox.com>
To: Saeed Mahameed <saeedm@...lanox.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"arnd@...db.de" <arnd@...db.de>,
"leon@...nel.org" <leon@...nel.org>
CC: "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
Or Gerlitz <ogerlitz@...lanox.com>,
Oz Shlomo <ozsh@...lanox.com>,
Paul Blakey <paulb@...lanox.com>,
Maor Gottlieb <maorg@...lanox.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Eli Britstein <elibr@...lanox.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] net/mlx5e: reduce stack usage in
mlx5_eswitch_termtbl_create
On 6/17/19 11:02 AM, Saeed Mahameed wrote:
> On Mon, 2019-06-17 at 13:08 +0200, Arnd Bergmann wrote:
>> Putting an empty 'mlx5_flow_spec' structure on the stack is a bit
>> wasteful and causes a warning on 32-bit architectures when building
>> with clang -fsanitize-coverage:
>>
>> drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c:
>> In function 'mlx5_eswitch_termtbl_create':
>> drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c:90
>> :1: error: the frame size of 1032 bytes is larger than 1024 bytes [-
>> Werror=frame-larger-than=]
>>
>> Since the structure is never written to, we can statically allocate
>> it to avoid the stack usage. To be on the safe side, mark all
>> subsequent function arguments that we pass it into as 'const'
>> as well.
>>
>> Fixes: 10caabdaad5a ("net/mlx5e: Use termination table for VLAN push
>> actions")
>> Signed-off-by: Arnd Bergmann <arnd@...db.de>
>> ---
>> .../mlx5/core/eswitch_offloads_termtbl.c | 2 +-
>> .../net/ethernet/mellanox/mlx5/core/fs_core.c | 20 +++++++++------
>> ----
>> include/linux/mlx5/fs.h | 2 +-
>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git
>> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
>> index cb7d8ebe2c95..171f3d4ef9ac 100644
>> ---
>> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
>> +++
>> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
>> @@ -50,7 +50,7 @@ mlx5_eswitch_termtbl_create(struct mlx5_core_dev
>> *dev,
>> struct mlx5_flow_act *flow_act)
>> {
>> struct mlx5_flow_namespace *root_ns;
>> - struct mlx5_flow_spec spec = {};
>> + static const struct mlx5_flow_spec spec = {};
>
> LGTM, just make sure please to have a reverse xmas tree here.
>
> Mark, please let me know if you are ok with such API constrain to flow
> steering (spec must be const).
I don't think the steering core layer should change the matching asked by the user.
Even without the frame size issue I like this change, thanks!
LGTM
Thanks,
Mark
>
> Thanks,
> Saeed.
>
>> int prio, flags;
>> int err;
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> index fe76c6fd6d80..739123e1363b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> @@ -584,7 +584,7 @@ static int insert_fte(struct mlx5_flow_group *fg,
>> struct fs_fte *fte)
>> }
>>
>> static struct fs_fte *alloc_fte(struct mlx5_flow_table *ft,
>> - u32 *match_value,
>> + const u32 *match_value,
>> struct mlx5_flow_act *flow_act)
>> {
>> struct mlx5_flow_steering *steering = get_steering(&ft->node);
>> @@ -612,7 +612,7 @@ static void dealloc_flow_group(struct
>> mlx5_flow_steering *steering,
>>
>> static struct mlx5_flow_group *alloc_flow_group(struct
>> mlx5_flow_steering *steering,
>> u8
>> match_criteria_enable,
>> - void *match_criteria,
>> + const void
>> *match_criteria,
>> int start_index,
>> int end_index)
>> {
>> @@ -642,7 +642,7 @@ static struct mlx5_flow_group
>> *alloc_flow_group(struct mlx5_flow_steering *steer
>>
>> static struct mlx5_flow_group *alloc_insert_flow_group(struct
>> mlx5_flow_table *ft,
>> u8
>> match_criteria_enable,
>> - void
>> *match_criteria,
>> + const void
>> *match_criteria,
>> int start_index,
>> int end_index,
>> struct list_head
>> *prev)
>> @@ -1285,7 +1285,7 @@ add_rule_fte(struct fs_fte *fte,
>> }
>>
>> static struct mlx5_flow_group *alloc_auto_flow_group(struct
>> mlx5_flow_table *ft,
>> - struct
>> mlx5_flow_spec *spec)
>> + const struct
>> mlx5_flow_spec *spec)
>> {
>> struct list_head *prev = &ft->node.children;
>> struct mlx5_flow_group *fg;
>> @@ -1451,7 +1451,7 @@ static int check_conflicting_ftes(struct fs_fte
>> *fte, const struct mlx5_flow_act
>> }
>>
>> static struct mlx5_flow_handle *add_rule_fg(struct mlx5_flow_group
>> *fg,
>> - u32 *match_value,
>> + const u32 *match_value,
>> struct mlx5_flow_act
>> *flow_act,
>> struct
>> mlx5_flow_destination *dest,
>> int dest_num,
>> @@ -1536,7 +1536,7 @@ static void free_match_list(struct
>> match_list_head *head)
>>
>> static int build_match_list(struct match_list_head *match_head,
>> struct mlx5_flow_table *ft,
>> - struct mlx5_flow_spec *spec)
>> + const struct mlx5_flow_spec *spec)
>> {
>> struct rhlist_head *tmp, *list;
>> struct mlx5_flow_group *g;
>> @@ -1589,7 +1589,7 @@ static u64 matched_fgs_get_version(struct
>> list_head *match_head)
>>
>> static struct fs_fte *
>> lookup_fte_locked(struct mlx5_flow_group *g,
>> - u32 *match_value,
>> + const u32 *match_value,
>> bool take_write)
>> {
>> struct fs_fte *fte_tmp;
>> @@ -1622,7 +1622,7 @@ lookup_fte_locked(struct mlx5_flow_group *g,
>> static struct mlx5_flow_handle *
>> try_add_to_existing_fg(struct mlx5_flow_table *ft,
>> struct list_head *match_head,
>> - struct mlx5_flow_spec *spec,
>> + const struct mlx5_flow_spec *spec,
>> struct mlx5_flow_act *flow_act,
>> struct mlx5_flow_destination *dest,
>> int dest_num,
>> @@ -1715,7 +1715,7 @@ try_add_to_existing_fg(struct mlx5_flow_table
>> *ft,
>>
>> static struct mlx5_flow_handle *
>> _mlx5_add_flow_rules(struct mlx5_flow_table *ft,
>> - struct mlx5_flow_spec *spec,
>> + const struct mlx5_flow_spec *spec,
>> struct mlx5_flow_act *flow_act,
>> struct mlx5_flow_destination *dest,
>> int dest_num)
>> @@ -1823,7 +1823,7 @@ static bool fwd_next_prio_supported(struct
>> mlx5_flow_table *ft)
>>
>> struct mlx5_flow_handle *
>> mlx5_add_flow_rules(struct mlx5_flow_table *ft,
>> - struct mlx5_flow_spec *spec,
>> + const struct mlx5_flow_spec *spec,
>> struct mlx5_flow_act *flow_act,
>> struct mlx5_flow_destination *dest,
>> int num_dest)
>> diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
>> index 2ddaa97f2179..c0c029664527 100644
>> --- a/include/linux/mlx5/fs.h
>> +++ b/include/linux/mlx5/fs.h
>> @@ -200,7 +200,7 @@ struct mlx5_flow_act {
>> */
>> struct mlx5_flow_handle *
>> mlx5_add_flow_rules(struct mlx5_flow_table *ft,
>> - struct mlx5_flow_spec *spec,
>> + const struct mlx5_flow_spec *spec,
>> struct mlx5_flow_act *flow_act,
>> struct mlx5_flow_destination *dest,
>> int num_dest);
Powered by blists - more mailing lists