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: <9efb76f79369b8577ef425c7f6e694132719353e.camel@mellanox.com>
Date:   Mon, 17 Jun 2019 18:02:39 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "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>,
        Mark Bloch <markb@...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 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).

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

Powered by Openwall GNU/*/Linux Powered by OpenVZ