|SUCCESS| [v2] net/ice: fix TM node ID validation against configured queues

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164213

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-20

Commit: (SHA not available from patch file)
Author: Ciara Loftus <ciara.loftus at intel.com>
Subject: net/ice: fix TM node ID validation against configured queues

This patch fixes validation of Traffic Manager (TM) node IDs by checking
them against the actual number of configured Tx queues rather than the
compile-time constant RTE_MAX_QUEUES_PER_PORT. It also adds validation to
reject leaf-reserved IDs for non-leaf nodes.

> diff --git a/drivers/net/intel/ice/ice_tm.c b/drivers/net/intel/ice/ice_tm.c
> index 015a827d7a..bf2ac117b1 100644
> --- a/drivers/net/intel/ice/ice_tm.c
> +++ b/drivers/net/intel/ice/ice_tm.c
> @@ -88,8 +88,11 @@ ice_node_param_check(uint32_t node_id,
>  		      uint32_t priority, uint32_t weight,
>  		      const struct rte_tm_node_params *params,
>  		      bool is_leaf,
> +		      uint16_t nb_txq,
>  		      struct rte_tm_error *error)
>  {
> +	uint32_t max_leaf_id = (nb_txq != 0) ? nb_txq : RTE_MAX_QUEUES_PER_PORT;

Does this ternary operator comply with the coding style requirement to use
explicit comparisons? The coding style states that implicit tests like
"if (x)" should be avoided in favor of explicit comparisons.

> +
>  	/* checked all the unsupported parameter */
>  	if (node_id == RTE_TM_NODE_ID_NULL) {
>  		error->type = RTE_TM_ERROR_TYPE_NODE_ID;
> @@ -123,6 +126,11 @@ ice_node_param_check(uint32_t node_id,
>  
>  	/* for non-leaf node */
>  	if (!is_leaf) {

The comment uses "for non-leaf node" but coding style discourages C99-style
"//" comments for non-trivial comments. Should this be converted to the
standard "/* */" format consistently?

> +		if (node_id < max_leaf_id) {
> +			error->type = RTE_TM_ERROR_TYPE_NODE_ID;
> +			error->message = "node ID is reserved for leaf nodes";
> +			return -EINVAL;
> +		}
>  		if (params->nonleaf.wfq_weight_mode) {
>  			error->type =
>  				RTE_TM_ERROR_TYPE_NODE_PARAMS_WFQ_WEIGHT_MODE;
> @@ -146,7 +154,7 @@ ice_node_param_check(uint32_t node_id,
>  	}
>  
>  	/* for leaf node */
> -	if (node_id >= RTE_MAX_QUEUES_PER_PORT) {
> +	if (node_id >= max_leaf_id) {
>  		error->type = RTE_TM_ERROR_TYPE_NODE_ID;
>  		error->message = "Node ID out of range for a leaf node.";
>  		return -EINVAL;

[ ... ]

> @@ -440,7 +448,8 @@ ice_tm_node_add(struct rte_eth_dev *dev, uint32_t node_id,
>  			return -EINVAL;
>  		}
>  
> -		ret = ice_node_param_check(node_id, priority, weight, params, false, error);
> +		ret = ice_node_param_check(node_id, priority, weight, params, false,
> +				dev->data->nb_tx_queues, error);

Is it guaranteed that dev->data is non-NULL at this point? Does this code
risk a NULL pointer dereference in ice_tm_node_add() if dev->data has not
been initialized?

>  		if (ret)
>  			return ret;
>  
> @@ -481,7 +490,8 @@ ice_tm_node_add(struct rte_eth_dev *dev, uint32_t node_id,
>  	}
>  
>  	ret = ice_node_param_check(node_id, priority, weight,
> -			params, level_id == ice_get_leaf_level(pf), error);
> +			params, level_id == ice_get_leaf_level(pf),
> +			dev->data->nb_tx_queues, error);

Same question here regarding potential NULL dereference of dev->data.

>  	if (ret)
>  		return ret;
>