Re: [PATCH v13 net-next 3/9] devlink: pass param values by pointer

[Petr Machata]

Ratheesh Kannoth <rkannoth@xxxxxxxxxxx> writes:

> union devlink_param_value grows substantially once U64 array
> parameters are added to devlink (from 32 bytes to over 264 bytes).
> devlink_nl_param_value_fill_one() and devlink_nl_param_value_put()
> copy the union by value in several places. Passing two instances as
> value arguments alone consumes over 528 bytes of stack; combined with
> deeper call chains the parameter stack can approach 800 bytes and trip
> CONFIG_FRAME_WARN more easily.
>
> Switch internal helpers and exported driver APIs to pass pointers to
> union devlink_param_value rather than passing the union by value.
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

Reviewed-by: Petr Machata <petrm@xxxxxxxxxx> # for mlxsw

> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
> index d76246301f67..308d8a94865f 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
> @@ -1306,11 +1306,11 @@ static int mlxsw_core_fw_flash_update(struct mlxsw_core *mlxsw_core,
>  }
>  
>  static int mlxsw_core_devlink_param_fw_load_policy_validate(struct devlink *devlink, u32 id,
> -							    union devlink_param_value val,
> +							    union devlink_param_value *val,
>  							    struct netlink_ext_ack *extack)
>  {
> -	if (val.vu8 != DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_DRIVER &&
> -	    val.vu8 != DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_FLASH) {
> +	if (val->vu8 != DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_DRIVER &&
> +	    val->vu8 != DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_FLASH) {
>  		NL_SET_ERR_MSG_MOD(extack, "'fw_load_policy' must be 'driver' or 'flash'");
>  		return -EINVAL;
>  	}
> @@ -1337,7 +1337,7 @@ static int mlxsw_core_fw_params_register(struct mlxsw_core *mlxsw_core)
>  	value.vu8 = DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_DRIVER;
>  	devl_param_driverinit_value_set(devlink,
>  					DEVLINK_PARAM_GENERIC_ID_FW_LOAD_POLICY,
> -					value);
> +					&value);
>  	return 0;
>  }
>