Re: [PATCH net-next 1/6] devlink: extend devlink_param *set pointer

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Tony Nguyen <anthony.l.nguyen@intel.com>
Date: Mon, 22 Apr 2024 13:39:06 -0700

> From: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> 
> Extend devlink_param *set function pointer to take extack as a param.
> Sometimes it is needed to pass information to the end user from set
> function. It is more proper to use for that netlink instead of passing
> message to dmesg.
> 
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

[...]

> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index d31769a116ce..35eb0f884386 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -483,7 +483,8 @@ struct devlink_param {
>  	int (*get)(struct devlink *devlink, u32 id,
>  		   struct devlink_param_gset_ctx *ctx);
>  	int (*set)(struct devlink *devlink, u32 id,
> -		   struct devlink_param_gset_ctx *ctx);
> +		   struct devlink_param_gset_ctx *ctx,
> +		   struct netlink_ext_ack *extack);

Sorry for the late comment. Can't we embed extack to
devlink_param_gset_ctx instead? It would take much less lines.

>  	int (*validate)(struct devlink *devlink, u32 id,
>  			union devlink_param_value val,
>  			struct netlink_ext_ack *extack);
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 7edfd8de8882..a6ef7e4c503f 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -1258,7 +1258,8 @@ struct dsa_switch_ops {
>  int dsa_devlink_param_get(struct devlink *dl, u32 id,
>  			  struct devlink_param_gset_ctx *ctx);
>  int dsa_devlink_param_set(struct devlink *dl, u32 id,
> -			  struct devlink_param_gset_ctx *ctx);
> +			  struct devlink_param_gset_ctx *ctx,
> +			  struct netlink_ext_ack *extack);
>  int dsa_devlink_params_register(struct dsa_switch *ds,
>  				const struct devlink_param *params,
>  				size_t params_count);
> diff --git a/net/devlink/param.c b/net/devlink/param.c
> index 22bc3b500518..dcf0d1ccebba 100644
> --- a/net/devlink/param.c
> +++ b/net/devlink/param.c
> @@ -158,11 +158,12 @@ static int devlink_param_get(struct devlink *devlink,
>  
>  static int devlink_param_set(struct devlink *devlink,
>  			     const struct devlink_param *param,
> -			     struct devlink_param_gset_ctx *ctx)
> +			     struct devlink_param_gset_ctx *ctx,
> +			     struct netlink_ext_ack *extack)
>  {
>  	if (!param->set)
>  		return -EOPNOTSUPP;
> -	return param->set(devlink, param->id, ctx);
> +	return param->set(devlink, param->id, ctx, extack);
>  }
>  
>  static int
> @@ -571,7 +572,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
>  			return -EOPNOTSUPP;
>  		ctx.val = value;
>  		ctx.cmode = cmode;
> -		err = devlink_param_set(devlink, param, &ctx);
> +		err = devlink_param_set(devlink, param, &ctx, info->extack);
>  		if (err)
>  			return err;
>  	}
> diff --git a/net/dsa/devlink.c b/net/dsa/devlink.c
> index 431bf52290a1..0aac887d0098 100644
> --- a/net/dsa/devlink.c
> +++ b/net/dsa/devlink.c
> @@ -194,7 +194,8 @@ int dsa_devlink_param_get(struct devlink *dl, u32 id,
>  EXPORT_SYMBOL_GPL(dsa_devlink_param_get);
>  
>  int dsa_devlink_param_set(struct devlink *dl, u32 id,
> -			  struct devlink_param_gset_ctx *ctx)
> +			  struct devlink_param_gset_ctx *ctx,
> +			  struct netlink_ext_ack *extack)
>  {
>  	struct dsa_switch *ds = dsa_devlink_to_ds(dl);

Thanks,
Olek