|SUCCESS| 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/163588

_AI Code Review_

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

Reviewing patch: net/ice: fix TM node ID validation against configured queues

git sha: ff53f2acfd7ee4070007 (parent commit referenced)
Author: Ciara Loftus <ciara.loftus at intel.com>
Subject: [PATCH] net/ice: fix TM node ID validation against configured queues

This patch fixes incorrect boundary checking for TM (Traffic Management)
leaf node IDs by replacing a compile-time constant with the runtime
configured queue count. It also adds validation to reject leaf-range IDs
for non-leaf nodes.

> diff --git a/drivers/net/intel/ice/ice_tm.c b/drivers/net/intel/ice/ice_tm.c
> index ff53f2acfd..7ee4070007 100644
> --- a/drivers/net/intel/ice/ice_tm.c
> +++ b/drivers/net/intel/ice/ice_tm.c

[ ... ]

> @@ -88,6 +88,7 @@ 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)

The ice_node_param_check() function signature now includes nb_txq to
enable runtime validation. This change propagates to the two call sites
in ice_tm_node_add().

>  {
>  	/* checked all the unsupported parameter */
> @@ -123,6 +124,11 @@ ice_node_param_check(uint32_t node_id,
>  
>  	/* for non-leaf node */
>  	if (!is_leaf) {
> +		if (node_id < nb_txq) {
> +			error->type = RTE_TM_ERROR_TYPE_NODE_ID;
> +			error->message = "node ID is reserved for leaf nodes";
> +			return -EINVAL;
> +		}

This adds a new validation path for non-leaf nodes. Does this code handle
the case where nb_txq is zero? If nb_txq is 0, would all node IDs pass
this check even when no queues are configured?

>  		if (params->nonleaf.wfq_weight_mode) {
>  			error->type =
>  				RTE_TM_ERROR_TYPE_NODE_PARAMS_WFQ_WEIGHT_MODE;
> @@ -146,7 +152,7 @@ ice_node_param_check(uint32_t node_id,
>  	}
>  
>  	/* for leaf node */
> -	if (node_id >= RTE_MAX_QUEUES_PER_PORT) {
> +	if (node_id >= nb_txq) {

This replaces the static constant with the runtime queue count for leaf
validation. However, does this code handle the nb_txq == 0 case correctly?
When nb_txq is 0, would any node_id >= 0 fail this check, preventing any
leaf nodes from being added?

> [ ... ]
> @@ -440,7 +446,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);

The indentation here places the closing parenthesis at the start of a new
line. Does this match DPDK's coding style for multiline function calls?
According to the style guide, continuation lines should be indented to
align with the opening bracket or use a consistent indentation level.

> [ ... ]
> @@ -479,7 +486,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 alignment concern here. The continuation should maintain consistent
indentation with the opening parenthesis or follow a clear indentation
pattern.

Overall concerns:

1. Does this code properly handle the nb_txq == 0 edge case in both the
   leaf and non-leaf validation paths?

2. Do the multiline function call continuations follow DPDK's indentation
   rules for alignment?