Re: [bug report] net/mlx5: E-switch, Allow setting share/max tx rate limits of rate groups

[Dmytro Linkin <dlinkin@nvidia.com>]

On 8/25/21 11:56 AM, Dan Carpenter wrote:
> Hello Dmytro Linkin,
> 
> The patch f47e04eb96e0: "net/mlx5: E-switch, Allow setting share/max
> tx rate limits of rate groups" from May 31, 2021, leads to the
> following Smatch static checker warning:
> 
> 	drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:483 esw_qos_create_rate_group()
> 	warn: passing zero to 'ERR_PTR'
> 
> drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
>     434 static struct mlx5_esw_rate_group *
>     435 esw_qos_create_rate_group(struct mlx5_eswitch *esw, struct netlink_ext_ack *extack)
>     436 {
>     437 	u32 tsar_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
>     438 	struct mlx5_esw_rate_group *group;
>     439 	u32 divider;
>     440 	int err;
>     441 
>     442 	if (!MLX5_CAP_QOS(esw->dev, log_esw_max_sched_depth))
>     443 		return ERR_PTR(-EOPNOTSUPP);
>     444 
>     445 	group = kzalloc(sizeof(*group), GFP_KERNEL);
>     446 	if (!group)
>     447 		return ERR_PTR(-ENOMEM);
>     448 
>     449 	MLX5_SET(scheduling_context, tsar_ctx, parent_element_id,
>     450 		 esw->qos.root_tsar_ix);
>     451 	err = mlx5_create_scheduling_element_cmd(esw->dev,
>     452 						 SCHEDULING_HIERARCHY_E_SWITCH,
>     453 						 tsar_ctx,
>     454 						 &group->tsar_ix);
>     455 	if (err) {
>     456 		NL_SET_ERR_MSG_MOD(extack, "E-Switch create TSAR for group failed");
>     457 		goto err_sched_elem;
>     458 	}
>     459 
>     460 	list_add_tail(&group->list, &esw->qos.groups);
>     461 
>     462 	divider = esw_qos_calculate_min_rate_divider(esw, group, true);
>     463 	if (divider) {
>     464 		err = esw_qos_normalize_groups_min_rate(esw, divider, extack);
>     465 		if (err) {
>     466 			NL_SET_ERR_MSG_MOD(extack, "E-Switch groups normalization failed");
>     467 			goto err_min_rate;
> 
> Wouldn't we want to we want to propagate this error code
> 
> 
>     468 		}
>     469 	}
>     470 	trace_mlx5_esw_group_qos_create(esw->dev, group, group->tsar_ix);
>     471 
>     472 	return group;
>     473 
>     474 err_min_rate:
>     475 	list_del(&group->list);
>     476 	err = mlx5_destroy_scheduling_element_cmd(esw->dev,
>     477 						  SCHEDULING_HIERARCHY_E_SWITCH,
>     478 						  group->tsar_ix);
> 
> instead of this one?  Also if this succeeds we return succeess?
> 
>     479 	if (err)
>     480 		NL_SET_ERR_MSG_MOD(extack, "E-Switch destroy TSAR for group failed");
>     481 err_sched_elem:
>     482 	kfree(group);
> --> 483 	return ERR_PTR(err);
>     484 }
> 
> regards,
> dan carpenter
> 

Hello Dan,
Thanks for reporting.

Do I need additional reference to Your report except Reported-by?

Regards,
Dmytro