Re: [PATCH net-next 06/15] net/mlx5: Introduce node struct and rename group terminology to node

[Simon Horman <horms@kernel.org>]

On Sun, Oct 13, 2024 at 09:45:31AM +0300, Tariq Toukan wrote:
> From: Carolina Jubran <cjubran@nvidia.com>
> 
> Introduce the `mlx5_esw_sched_node` struct, consolidating all rate
> hierarchy related details, including membership and scheduling
> parameters.
> 
> Since the group concept aligns with the `mlx5_esw_sched_node`, replace
> the `mlx5_esw_rate_group` struct with it and rename the "group"
> terminology to "node" throughout the rate hierarchy.
> 
> All relevant code paths and structures have been updated to use the
> "node" terminology accordingly, laying the groundwork for future
> patches that will unify the handling of different types of members
> within the rate hierarchy.
> 
> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>

...

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c

...

> -static struct mlx5_esw_rate_group *
> -__esw_qos_create_vports_rate_group(struct mlx5_eswitch *esw, struct mlx5_esw_rate_group *parent,
> -				   struct netlink_ext_ack *extack)
> +static struct mlx5_esw_sched_node *
> +__esw_qos_create_vports_rate_node(struct mlx5_eswitch *esw, struct mlx5_esw_sched_node *parent,
> +				  struct netlink_ext_ack *extack)
>  {
> -	struct mlx5_esw_rate_group *group;
> +	struct mlx5_esw_sched_node *node;
>  	u32 tsar_ix, err;
>  
> -	err = esw_qos_create_group_sched_elem(esw->dev, esw->qos.root_tsar_ix, &tsar_ix);
> +	err = esw_qos_create_node_sched_elem(esw->dev, esw->qos.root_tsar_ix, &tsar_ix);
>  	if (err) {
> -		NL_SET_ERR_MSG_MOD(extack, "E-Switch create TSAR for group failed");
> +		NL_SET_ERR_MSG_MOD(extack, "E-Switch create TSAR for node failed");
>  		return ERR_PTR(err);
>  	}
>  
> -	group = __esw_qos_alloc_rate_group(esw, tsar_ix, SCHED_NODE_TYPE_VPORTS_TSAR, parent);
> -	if (!group) {
> -		NL_SET_ERR_MSG_MOD(extack, "E-Switch alloc group failed");
> +	node = __esw_qos_alloc_rate_node(esw, tsar_ix, SCHED_NODE_TYPE_VPORTS_TSAR, parent);
> +	if (!node) {
> +		NL_SET_ERR_MSG_MOD(extack, "E-Switch alloc node failed");
>  		err = -ENOMEM;
> -		goto err_alloc_group;
> +		goto err_alloc_node;

Hi Carolina and Tariq,

node is NULL here, but will be dereferenced after jumping to err_alloc_node.

Flagged by Smatch.

>  	}
>  
>  	err = esw_qos_normalize_min_rate(esw, extack);
>  	if (err) {
> -		NL_SET_ERR_MSG_MOD(extack, "E-Switch groups normalization failed");
> +		NL_SET_ERR_MSG_MOD(extack, "E-Switch nodes normalization failed");
>  		goto err_min_rate;
>  	}
> -	trace_mlx5_esw_group_qos_create(esw->dev, group, group->tsar_ix);
> +	trace_mlx5_esw_node_qos_create(esw->dev, node, node->ix);
>  
> -	return group;
> +	return node;
>  
>  err_min_rate:
> -	__esw_qos_free_rate_group(group);
> -err_alloc_group:
> +	__esw_qos_free_node(node);
> +err_alloc_node:
>  	if (mlx5_destroy_scheduling_element_cmd(esw->dev,
>  						SCHEDULING_HIERARCHY_E_SWITCH,
> -						tsar_ix))
> -		NL_SET_ERR_MSG_MOD(extack, "E-Switch destroy TSAR for group failed");
> +						node->ix))
> +		NL_SET_ERR_MSG_MOD(extack, "E-Switch destroy TSAR for node failed");
>  	return ERR_PTR(err);
>  }

...