Re: [PATCH 2/3] net/ice: skip TC validation if hierarchy committed

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Sep 04, 2026 at 10:51:38AM +0000, Ciara Loftus wrote:
> Currently, ice_tx_queue_start() determines a queue's congestion domain by
> scanning vsi->info.tc_mapping[] for a matching traffic class range,
> rejecting the queue if no match is found.
> 
> The Tx scheduler hierarchies only ever operate on a single traffic class,
> TC0, so every queue managed by a committed TM hierarchy always belongs
> to domain 0. The tc_mapping[] lookup doesn't account for this, and queues
> added after a TM hierarchy is committed can be incorrectly rejected.
> 
> Fix this by skipping the lookup if a hierarchy has been committed as the
> domain is already known to be 0 in that case. Since TM does not support
> DCB's multi-TC queue layout, also reject queue start explicitly if DCB
> has configured more than one TC while a TM hierarchy is committed.
> 
> Fixes: 02b71e570294 ("net/ice: support DCB")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c |  4 ++++
>  drivers/net/intel/ice/ice_ethdev.h |  2 ++
>  drivers/net/intel/ice/ice_rxtx.c   | 33 ++++++++++++++++++------------
>  3 files changed, 26 insertions(+), 13 deletions(-)
> 

Did you hit this in testing, or was it just via code review or AI review?
Fix looks good anyway.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 76b8ff0a72e..fd148848d3c 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -3963,6 +3963,8 @@ ice_dev_configure(struct rte_eth_dev *dev)
>  	ad->rx_func_type = ICE_RX_DEFAULT;
>  	ad->tx_func_type = ICE_TX_DEFAULT;
>  
> +	pf->dcb_num_tcs = 1;
> +
>  	if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG)
>  		dev->data->dev_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
>  
> @@ -3992,6 +3994,8 @@ ice_dev_configure(struct rte_eth_dev *dev)
>  		if (nb_tc_used < 0)
>  			return -EINVAL;
>  
> +		pf->dcb_num_tcs = nb_tc_used;
> +
>  		ctxt.info = vsi->info;
>  		if (rte_le_to_cpu_16(ctxt.info.mapping_flags) == ICE_AQ_VSI_Q_MAP_NONCONTIG) {
>  			PMD_DRV_LOG(ERR, "VSI configured with non contiguous queues, DCB is not supported");
> diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h
> index 7ee3ea8a709..5914454c7c2 100644
> --- a/drivers/net/intel/ice/ice_ethdev.h
> +++ b/drivers/net/intel/ice/ice_ethdev.h
> @@ -609,6 +609,8 @@ struct ice_pf {
>  	uint64_t rss_hf;
>  	struct ice_tm_conf tm_conf;
>  	uint16_t outer_ethertype;
> +	/* Number of TCs requested, 1 if DCB not configured */
> +	uint8_t dcb_num_tcs;
>  	/* lock prevent race condition between lsc interrupt handler
>  	 * and link status update during dev_start.
>  	 */
> diff --git a/drivers/net/intel/ice/ice_rxtx.c b/drivers/net/intel/ice/ice_rxtx.c
> index c4b5454c530..71f9155e588 100644
> --- a/drivers/net/intel/ice/ice_rxtx.c
> +++ b/drivers/net/intel/ice/ice_rxtx.c
> @@ -839,20 +839,27 @@ ice_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
>  	tx_ctx.legacy_int = 1; /* Legacy or Advanced Host Interface */
>  	tx_ctx.tsyn_ena = 1;
>  
> -	/* Mirror RXQ<->CGD association to TXQ<->CGD */
> -	for (int i = 0; i < ICE_MAX_TRAFFIC_CLASS; i++) {
> -		q_base = rte_le_to_cpu_16(vsi->info.tc_mapping[i]) & ICE_AQ_VSI_TC_Q_OFFSET_M;
> -		q_range = 1 << ((rte_le_to_cpu_16(vsi->info.tc_mapping[i]) &
> -			ICE_AQ_VSI_TC_Q_NUM_M) >> ICE_AQ_VSI_TC_Q_NUM_S);
> -
> -		if (q_base <= tx_queue_id && tx_queue_id < q_base + q_range)
> -			break;
> -
> -		cgd_idx++;
> -	}
> +	if (!pf->tm_conf.committed) {
> +		/* Mirror RXQ<->CGD association to TXQ<->CGD */
> +		for (int i = 0; i < ICE_MAX_TRAFFIC_CLASS; i++) {
> +			q_base = rte_le_to_cpu_16(vsi->info.tc_mapping[i]) &
> +							ICE_AQ_VSI_TC_Q_OFFSET_M;
> +			q_range = 1 << ((rte_le_to_cpu_16(vsi->info.tc_mapping[i]) &
> +				ICE_AQ_VSI_TC_Q_NUM_M) >> ICE_AQ_VSI_TC_Q_NUM_S);
> +
> +			if (q_base <= tx_queue_id && tx_queue_id < q_base + q_range)
> +				break;
>  
> -	if (cgd_idx >= ICE_MAX_TRAFFIC_CLASS) {
> -		PMD_DRV_LOG(ERR, "Bad queue mapping configuration");
> +			cgd_idx++;
> +		}
> +		if (cgd_idx >= ICE_MAX_TRAFFIC_CLASS) {
> +			PMD_DRV_LOG(ERR, "Bad queue mapping configuration");
> +			rte_free(txq_elem);
> +			return -EINVAL;
> +		}
> +	} else if (pf->dcb_num_tcs > 1) {
> +		/* TM only manages the TC0 scheduler subtree. */
> +		PMD_DRV_LOG(ERR, "TM hierarchy is not supported together with multi-TC DCB");
>  		rte_free(txq_elem);
>  		return -EINVAL;
>  	}
> -- 
> 2.43.0
>