Re: [PATCH] net/mlx5e: reduce stack usage in mlx5_eswitch_termtbl_create
From: Saeed Mahameed
Date: Mon Jun 17 2019 - 14:07:46 EST
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@xxxxxxxx>
> ---
> .../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).
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);