Re: [patch net-next 01/11] devlink: introduce nested devlink entity for line card

[Ido Schimmel <idosch@nvidia.com>]

On Tue, Jun 14, 2022 at 02:33:16PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> For the purpose of exposing device info and allow flash updated which is

s/updated/update/

> going to be implemented in follow-up patches, introduce a possibility
> for a line card to expose relation to nested devlink entity. The nested
> devlink entity represents the line card.
> 
> Example:
> 
> $ devlink lc show pci/0000:01:00.0 lc 1
> pci/0000:01:00.0:
>   lc 1 state active type 16x100G nested_devlink auxiliary/mlxsw_core.lc.0
>     supported_types:
>        16x100G
> $ devlink dev show auxiliary/mlxsw_core.lc.0
> auxiliary/mlxsw_core.lc.0
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  include/net/devlink.h        |  2 ++
>  include/uapi/linux/devlink.h |  2 ++
>  net/core/devlink.c           | 42 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
> 
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 2a2a2a0c93f7..83e62943e1d4 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1584,6 +1584,8 @@ void devlink_linecard_provision_clear(struct devlink_linecard *linecard);
>  void devlink_linecard_provision_fail(struct devlink_linecard *linecard);
>  void devlink_linecard_activate(struct devlink_linecard *linecard);
>  void devlink_linecard_deactivate(struct devlink_linecard *linecard);
> +void devlink_linecard_nested_dl_set(struct devlink_linecard *linecard,
> +				    struct devlink *nested_devlink);
>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>  			u32 size, u16 ingress_pools_count,
>  			u16 egress_pools_count, u16 ingress_tc_count,
> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index b3d40a5d72ff..541321695f52 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -576,6 +576,8 @@ enum devlink_attr {
>  	DEVLINK_ATTR_LINECARD_TYPE,		/* string */
>  	DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES,	/* nested */
>  
> +	DEVLINK_ATTR_NESTED_DEVLINK,		/* nested */
> +
>  	/* add new attributes above here, update the policy in devlink.c */
>  
>  	__DEVLINK_ATTR_MAX,
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index db61f3a341cb..a5953cfe1baa 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -87,6 +87,7 @@ struct devlink_linecard {
>  	const char *type;
>  	struct devlink_linecard_type *types;
>  	unsigned int types_count;
> +	struct devlink *nested_devlink;
>  };
>  
>  /**
> @@ -796,6 +797,24 @@ static int devlink_nl_put_handle(struct sk_buff *msg, struct devlink *devlink)
>  	return 0;
>  }
>  
> +static int devlink_nl_put_nested_handle(struct sk_buff *msg, struct devlink *devlink)
> +{
> +	struct nlattr *nested_attr;
> +
> +	nested_attr = nla_nest_start(msg, DEVLINK_ATTR_NESTED_DEVLINK);
> +	if (!nested_attr)
> +		return -EMSGSIZE;
> +	if (devlink_nl_put_handle(msg, devlink))
> +		goto nla_put_failure;
> +
> +	nla_nest_end(msg, nested_attr);
> +	return 0;
> +
> +nla_put_failure:
> +	nla_nest_cancel(msg, nested_attr);
> +	return -EMSGSIZE;
> +}
> +
>  struct devlink_reload_combination {
>  	enum devlink_reload_action action;
>  	enum devlink_reload_limit limit;
> @@ -2100,6 +2119,10 @@ static int devlink_nl_linecard_fill(struct sk_buff *msg,
>  		nla_nest_end(msg, attr);
>  	}
>  
> +	if (linecard->nested_devlink &&
> +	    devlink_nl_put_nested_handle(msg, linecard->nested_devlink))
> +		goto nla_put_failure;
> +
>  	genlmsg_end(msg, hdr);
>  	return 0;
>  
> @@ -10335,6 +10358,7 @@ EXPORT_SYMBOL_GPL(devlink_linecard_provision_set);
>  void devlink_linecard_provision_clear(struct devlink_linecard *linecard)
>  {
>  	mutex_lock(&linecard->state_lock);
> +	WARN_ON(linecard->nested_devlink);
>  	linecard->state = DEVLINK_LINECARD_STATE_UNPROVISIONED;
>  	linecard->type = NULL;
>  	devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW);
> @@ -10353,6 +10377,7 @@ EXPORT_SYMBOL_GPL(devlink_linecard_provision_clear);
>  void devlink_linecard_provision_fail(struct devlink_linecard *linecard)
>  {
>  	mutex_lock(&linecard->state_lock);
> +	WARN_ON(linecard->nested_devlink);
>  	linecard->state = DEVLINK_LINECARD_STATE_PROVISIONING_FAILED;
>  	devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW);
>  	mutex_unlock(&linecard->state_lock);
> @@ -10400,6 +10425,23 @@ void devlink_linecard_deactivate(struct devlink_linecard *linecard)
>  }
>  EXPORT_SYMBOL_GPL(devlink_linecard_deactivate);
>  
> +/**
> + *	devlink_linecard_nested_dl_set - Attach/detach nested delink

s/delink/devlink/

> + *					 instance to linecard.
> + *
> + *	@linecard: devlink linecard
> + *      @nested_devlink: devlink instance to attach or NULL to detach

The alignment looks off

> + */
> +void devlink_linecard_nested_dl_set(struct devlink_linecard *linecard,
> +				    struct devlink *nested_devlink)
> +{
> +	mutex_lock(&linecard->state_lock);
> +	linecard->nested_devlink = nested_devlink;
> +	devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW);
> +	mutex_unlock(&linecard->state_lock);
> +}
> +EXPORT_SYMBOL_GPL(devlink_linecard_nested_dl_set);
> +
>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>  			u32 size, u16 ingress_pools_count,
>  			u16 egress_pools_count, u16 ingress_tc_count,
> -- 
> 2.35.3
>