Re: [RFC PATCH 4/6] ethdev: extend VMDq/DCB configuration with queue mapping

[Vladimir Medvedkin <medvedkinv@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 17184 bytes --]

сб, 30 авг. 2025 г. в 21:54, Ivan Malov <ivan.malov@arknetworks.am>:

> Hi Vladimir,
>
> On Sat, 30 Aug 2025, Vladimir Medvedkin wrote:
>
> > Currently DCB Traffic Class queue mapping is only used when calling get
> > DCB info API, while setting up queue mappings is up to each individual
> > driver. Use the tc queue mapping structure to enable setting up custom
> > mapping explicitly.
> >
> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > ---
> > app/test-pmd/testpmd.c             | 57 ++++++++++++++++++------------
> > drivers/net/intel/ice/ice_ethdev.c | 39 ++++++++++----------
> > drivers/net/intel/ice/ice_rxtx.c   | 27 +++++++-------
> > lib/ethdev/rte_ethdev.h            | 43 +++++++++++-----------
> > 4 files changed, 92 insertions(+), 74 deletions(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > index b5a7e7b3ee..3a55434d44 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -4158,6 +4158,16 @@ get_eth_dcb_conf(struct rte_eth_conf *eth_conf,
> enum dcb_mode_enable dcb_mode,
> >               struct rte_eth_dcb_conf *tx_conf =
> >                               &eth_conf->tx_adv_conf.dcb_tx_conf;
> >
> > +             struct rte_eth_dcb_tc_queue_mapping *q_map =
> &eth_conf->q_map;
> > +             memset(q_map, 0, sizeof(*q_map));
> > +             int queues_per_tc = nb_rxq / num_tcs;
> > +             for (i = 0; i < num_tcs; i++) {
> > +                     q_map->tc_rxq[0][i].base = i * queues_per_tc;
> > +                     q_map->tc_txq[0][i].base = i * queues_per_tc;
> > +                     q_map->tc_rxq[0][i].nb_queue = queues_per_tc;
> > +                     q_map->tc_txq[0][i].nb_queue = queues_per_tc;
> > +             }
> > +
> >               for (i = 0; i < RTE_ETH_DCB_NUM_USER_PRIORITIES; i++) {
> >                       dcb_tc_val = prio_tc_en ? prio_tc[i] : i % num_tcs;
> >                       rx_conf->dcb_tc[i] = dcb_tc_val;
> > @@ -4231,6 +4241,30 @@ init_port_dcb_config(portid_t pid,
> >       /* retain the original device configuration. */
> >       memcpy(&port_conf, &rte_port->dev_conf, sizeof(struct
> rte_eth_conf));
> >
> > +     if (num_tcs > 1 && keep_qnum == 0) {
> > +             /* Assume the ports in testpmd have the same dcb capability
> > +              * and has the same number of rxq and txq in dcb mode
> > +              */
> > +             if (dcb_mode == DCB_VT_ENABLED) {
> > +                     if (rte_port->dev_info.max_vfs > 0) {
> > +                             nb_rxq = rte_port->dev_info.nb_rx_queues;
> > +                             nb_txq = rte_port->dev_info.nb_tx_queues;
> > +                     } else {
> > +                             nb_rxq = rte_port->dev_info.max_rx_queues;
> > +                             nb_txq = rte_port->dev_info.max_tx_queues;
> > +                     }
> > +             } else {
> > +                     /*if vt is disabled, use all pf queues */
>
> Missing space between '/*' and 'if'.
>
> > +                     if (rte_port->dev_info.vmdq_pool_base == 0) {
> > +                             nb_rxq = rte_port->dev_info.max_rx_queues;
> > +                             nb_txq = rte_port->dev_info.max_tx_queues;
> > +                     } else {
> > +                             nb_rxq = (queueid_t)num_tcs;
> > +                             nb_txq = (queueid_t)num_tcs;
> > +                     }
> > +             }
> > +     }
> > +
> >       if (num_tcs > 1) {
> >               /* set configuration of DCB in vt mode and DCB in non-vt
> mode */
> >               get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en,
> prio_tc, prio_tc_en);
> > @@ -4267,29 +4301,6 @@ init_port_dcb_config(portid_t pid,
> >               return -1;
> >       }
> >
> > -     if (num_tcs > 1 && keep_qnum == 0) {
> > -             /* Assume the ports in testpmd have the same dcb capability
> > -              * and has the same number of rxq and txq in dcb mode
> > -              */
> > -             if (dcb_mode == DCB_VT_ENABLED) {
> > -                     if (rte_port->dev_info.max_vfs > 0) {
> > -                             nb_rxq = rte_port->dev_info.nb_rx_queues;
> > -                             nb_txq = rte_port->dev_info.nb_tx_queues;
> > -                     } else {
> > -                             nb_rxq = rte_port->dev_info.max_rx_queues;
> > -                             nb_txq = rte_port->dev_info.max_tx_queues;
> > -                     }
> > -             } else {
> > -                     /*if vt is disabled, use all pf queues */
> > -                     if (rte_port->dev_info.vmdq_pool_base == 0) {
> > -                             nb_rxq = rte_port->dev_info.max_rx_queues;
> > -                             nb_txq = rte_port->dev_info.max_tx_queues;
> > -                     } else {
> > -                             nb_rxq = (queueid_t)num_tcs;
> > -                             nb_txq = (queueid_t)num_tcs;
> > -                     }
> > -             }
> > -     }
> >       rx_free_thresh = 64;
> >
> >       memcpy(&rte_port->dev_conf, &port_conf, sizeof(struct
> rte_eth_conf));
> > diff --git a/drivers/net/intel/ice/ice_ethdev.c
> b/drivers/net/intel/ice/ice_ethdev.c
> > index ecde00182f..27559bbe18 100644
> > --- a/drivers/net/intel/ice/ice_ethdev.c
> > +++ b/drivers/net/intel/ice/ice_ethdev.c
> > @@ -3846,7 +3846,7 @@ ice_dev_configure(struct rte_eth_dev *dev)
> >               struct rte_eth_dcb_conf *tx_dcb_conf =
> >                       &dev->data->dev_conf.tx_adv_conf.dcb_tx_conf;
> >               int i;
> > -             int nb_tc_used_rx, nb_tc_used_tx, queues_per_tc;
> > +             int nb_tc_used_rx, nb_tc_used_tx;
> >               uint16_t total_q_nb;
> >
> >               nb_tc_used_rx = check_dcb_conf(ice_get_port_max_cgd(hw) ==
> ICE_4_CGD_PER_PORT,
> > @@ -3865,19 +3865,23 @@ ice_dev_configure(struct rte_eth_dev *dev)
> >               }
> >
> >               total_q_nb = dev->data->nb_rx_queues;
> > -             queues_per_tc = total_q_nb / nb_tc_used_rx;
> > -             if (total_q_nb % nb_tc_used_rx != 0) {
> > -                     PMD_DRV_LOG(ERR, "For DCB, number of queues must
> be evenly divisble by number of used TCs");
> > -                     return -EINVAL;
> > -             } else if (!rte_is_power_of_2(queues_per_tc)) {
> > -                     PMD_DRV_LOG(ERR, "For DCB, number of queues per TC
> must be a power of 2");
> > -                     return -EINVAL;
> > -             }
> > -
> > +             struct rte_eth_dcb_tc_queue_mapping *q_map =
> &dev->data->dev_conf.q_map;
> >               for (i = 0; i < nb_tc_used_rx; i++) {
> > +                     if (q_map->tc_rxq[0][i].nb_queue != 0 &&
>
> How is 'nb_queue == 0' case handled here and below? Should some
> driver-default
> fill-in be used, similar to code being deleted above?
>
> Or is '(0 << ICE_AQ_VSI_TC_Q_OFFSET_S) | (0 << ICE_AQ_VSI_TC_Q_NUM_S)'
> legit?
>

Yes, this is legit. In this case packets arriving to the corresponding TC
will be sent to the default queue (i.e. the first queue).


>
> > +
>  !rte_is_power_of_2(q_map->tc_rxq[0][i].nb_queue)) {
> > +                             PMD_DRV_LOG(ERR, "For DCB, number of
> queues per TC must be a power of 2");
> > +                             return -EINVAL;
> > +                     }
> > +                     if ((q_map->tc_rxq[0][i].base +
> q_map->tc_rxq[0][i].nb_queue) > total_q_nb) {
> > +                             PMD_DRV_LOG(ERR, "Queue range for TC %d
> exceeds total number of queues",
> > +                                     i);
> > +                             return -EINVAL;
> > +                     }
> >                       ctxt.info.tc_mapping[i] =
> > -                             rte_cpu_to_le_16(((i * queues_per_tc) <<
> ICE_AQ_VSI_TC_Q_OFFSET_S) |
> > -                                     (rte_log2_u32(queues_per_tc) <<
> ICE_AQ_VSI_TC_Q_NUM_S));
> > +                             rte_cpu_to_le_16((q_map->tc_rxq[0][i].base
> <<
> > +                                     ICE_AQ_VSI_TC_Q_OFFSET_S) |
> > +
>  (rte_log2_u32(q_map->tc_rxq[0][i].nb_queue) <<
> > +                                     ICE_AQ_VSI_TC_Q_NUM_S));
>
> This seems to always use pool [0] but does not seem to check if 'nb_queue'
> in,
> say, pool [1] is non-zero, to return an error or something. Or is this
> unneeded?
>

Yes, this is unneeded. DCB with VMDq is not supported in ice PMD. In more
detail, we do not support VMDq1, where per-VM queue ranges are "exposed"
like in older ixgbe HW. Instead we support VMDq2, where each per-VM queue
range is covered under VSI (think of it as a virtual interface) abstraction
layer.
For queue mapping I reuse the existing structure, which is used
with rte_eth_dev_get_dcb_info() API. Maybe it is better to create a
dedicated queue mapping structure without all VMDq-related stuff?


>
> >               }
> >
> >               memset(local_dcb_conf, 0, sizeof(*local_dcb_conf));
> > @@ -3941,6 +3945,7 @@ ice_get_dcb_info(struct rte_eth_dev *dev, struct
> rte_eth_dcb_info *dcb_info)
> >       struct ice_dcbx_cfg *dcb_conf = &qos_cfg->local_dcbx_cfg;
> >       struct ice_vsi *vsi = pf->main_vsi;
> >       struct ice_vsi_ctx ctxt = { 0 };
> > +     struct rte_eth_dcb_tc_queue_mapping *q_map =
> &dev->data->dev_conf.q_map;
> >
> >       ctxt.info = vsi->info;
> >       if (rte_le_to_cpu_16(ctxt.info.mapping_flags) ==
> ICE_AQ_VSI_Q_MAP_NONCONTIG) {
> > @@ -3953,13 +3958,11 @@ ice_get_dcb_info(struct rte_eth_dev *dev, struct
> rte_eth_dcb_info *dcb_info)
> >               dcb_info->prio_tc[i] = dcb_conf->etscfg.prio_table[i];
> >               dcb_info->tc_bws[i] = dcb_conf->etscfg.tcbwtable[i];
> >               /* Using VMDQ pool zero since DCB+VMDQ is not supported */
> > -             uint16_t tc_rx_q_map =
> rte_le_to_cpu_16(ctxt.info.tc_mapping[i]);
> > -             dcb_info->tc_queue.tc_rxq[0][i].base = tc_rx_q_map &
> ICE_AQ_VSI_TC_Q_OFFSET_M;
> > -             dcb_info->tc_queue.tc_rxq[0][i].nb_queue =
> > -                     1 << ((tc_rx_q_map & ICE_AQ_VSI_TC_Q_NUM_M) >>
> ICE_AQ_VSI_TC_Q_NUM_S);
> > +             dcb_info->tc_queue.tc_rxq[0][i].base =
> q_map->tc_rxq[0][i].base;
> > +             dcb_info->tc_queue.tc_rxq[0][i].nb_queue =
> q_map->tc_rxq[0][i].nb_queue;
> >
> > -             dcb_info->tc_queue.tc_txq[0][i].base =
> dcb_info->tc_queue.tc_rxq[0][i].base;
> > -             dcb_info->tc_queue.tc_txq[0][i].nb_queue =
> dcb_info->tc_queue.tc_rxq[0][i].nb_queue;
> > +             dcb_info->tc_queue.tc_txq[0][i].base =
> q_map->tc_txq[0][i].base;
> > +             dcb_info->tc_queue.tc_txq[0][i].nb_queue =
> q_map->tc_txq[0][i].nb_queue;
> >       }
> >
> >       return 0;
> > diff --git a/drivers/net/intel/ice/ice_rxtx.c
> b/drivers/net/intel/ice/ice_rxtx.c
> > index 451816affd..55424e7a23 100644
> > --- a/drivers/net/intel/ice/ice_rxtx.c
> > +++ b/drivers/net/intel/ice/ice_rxtx.c
> > @@ -796,6 +796,7 @@ ice_tx_queue_start(struct rte_eth_dev *dev, uint16_t
> tx_queue_id)
> >       int buf_len;
> >       struct ice_adapter *ad =
> ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> >       u16 q_base, q_range, cgd_idx = 0;
> > +     struct rte_eth_dcb_tc_queue_mapping *q_map =
> &dev->data->dev_conf.q_map;
> >
> >       PMD_INIT_FUNC_TRACE();
> >
> > @@ -840,22 +841,22 @@ 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<->CDG */
> > -     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 (dev->data->dev_conf.txmode.mq_mode == RTE_ETH_MQ_TX_DCB) {
> > +             for (int i = 0; i < ICE_MAX_TRAFFIC_CLASS; i++) {
> > +                     q_base = q_map->tc_txq[0][i].base;
> > +                     q_range = q_map->tc_txq[0][i].nb_queue;
> >
> > -             if (q_base <= tx_queue_id && tx_queue_id < q_base +
> q_range)
> > -                     break;
> > +                     if (q_base <= tx_queue_id && tx_queue_id < q_base
> + q_range)
> > +                             break;
> >
> > -             cgd_idx++;
> > -     }
> > +                     cgd_idx++;
> > +             }
> >
> > -     if (cgd_idx >= ICE_MAX_TRAFFIC_CLASS) {
> > -             PMD_DRV_LOG(ERR, "Bad queue mapping configuration");
> > -             rte_free(txq_elem);
> > -             return -EINVAL;
> > +             if (cgd_idx >= ICE_MAX_TRAFFIC_CLASS) {
> > +                     PMD_DRV_LOG(ERR, "Bad queue mapping
> configuration");
> > +                     rte_free(txq_elem);
> > +                     return -EINVAL;
> > +             }
> >       }
> >
> >       tx_ctx.cgd_num = cgd_idx;
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > index c220760043..0bd86e1e7d 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -1472,6 +1472,26 @@ struct rte_eth_intr_conf {
> >
> > #define rte_intr_conf rte_eth_intr_conf
> >
> > +#define RTE_ETH_DCB_NUM_TCS    8
> > +#define RTE_ETH_MAX_VMDQ_POOL  64
> > +
> > +/**
> > + * A structure used to get the information of queue and
> > + * TC mapping on both Tx and Rx paths.
> > + */
> > +struct rte_eth_dcb_tc_queue_mapping {
> > +     /** Rx queues assigned to tc per Pool */
> > +     struct {
> > +             uint16_t base;
> > +             uint16_t nb_queue;
> > +     } tc_rxq[RTE_ETH_MAX_VMDQ_POOL][RTE_ETH_DCB_NUM_TCS];
> > +     /** Rx queues assigned to tc per Pool */
> > +     struct {
> > +             uint16_t base;
> > +             uint16_t nb_queue;
> > +     } tc_txq[RTE_ETH_MAX_VMDQ_POOL][RTE_ETH_DCB_NUM_TCS];
> > +};
> > +
> > /**
> >  * A structure used to configure an Ethernet port.
> >  * Depending upon the Rx multi-queue mode, extra advanced
> > @@ -1492,18 +1512,21 @@ struct rte_eth_conf {
> >                                Read the datasheet of given Ethernet
> controller
> >                                for details. The possible values of this
> field
> >                                are defined in implementation of each
> driver. */
> > +     struct rte_eth_dcb_tc_queue_mapping q_map;
>
> Is the driver supposed to tell 'q_map' is valid by looking at [0][0]
> 'nb_queue'?
>

Could you please clarify what validity criteria for the [0][0] 'nb_queue'
do you mean?


>
> >       struct {
> >               struct rte_eth_rss_conf rss_conf; /**< Port RSS
> configuration */
> >               /** Port DCB Rx configuration. */
> >               struct rte_eth_dcb_conf dcb_rx_conf;
> >               /** Port VMDq Rx configuration. */
> >               struct rte_eth_vmdq_rx_conf vmdq_rx_conf;
> > +             /* VMDQ and DCB Rx queue mapping configuration. */
>
> Perhaps it's better to have just one such comment right before 'q_map'.
>

Oh, my bad, last minute rework, thanks!


>
> Thank you.
>
> >       } rx_adv_conf; /**< Port Rx filtering configuration. */
> >       struct {
> >               /** Port DCB Tx configuration. */
> >               struct rte_eth_dcb_conf dcb_tx_conf;
> >               /** Port VMDq Tx configuration. */
> >               struct rte_eth_vmdq_tx_conf vmdq_tx_conf;
> > +             /* VMDQ and DCB Tx queue mapping configuration. */
> >       } tx_adv_conf; /**< Port Tx DCB configuration (union). */
> >       /** Currently,Priority Flow Control(PFC) are supported,if DCB with
> PFC
> >           is needed,and the variable must be set
> RTE_ETH_DCB_PFC_SUPPORT. */
> > @@ -1930,26 +1953,6 @@ struct rte_eth_xstat_name {
> >       char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */
> > };
> >
> > -#define RTE_ETH_DCB_NUM_TCS    8
> > -#define RTE_ETH_MAX_VMDQ_POOL  64
> > -
> > -/**
> > - * A structure used to get the information of queue and
> > - * TC mapping on both Tx and Rx paths.
> > - */
> > -struct rte_eth_dcb_tc_queue_mapping {
> > -     /** Rx queues assigned to tc per Pool */
> > -     struct {
> > -             uint16_t base;
> > -             uint16_t nb_queue;
> > -     } tc_rxq[RTE_ETH_MAX_VMDQ_POOL][RTE_ETH_DCB_NUM_TCS];
> > -     /** Rx queues assigned to tc per Pool */
> > -     struct {
> > -             uint16_t base;
> > -             uint16_t nb_queue;
> > -     } tc_txq[RTE_ETH_MAX_VMDQ_POOL][RTE_ETH_DCB_NUM_TCS];
> > -};
> > -
> > /**
> >  * A structure used to get the information of DCB.
> >  * It includes TC UP mapping and queue TC mapping.
> > --
> > 2.43.0
> >
> >
>


-- 
Regards,
Vladimir

[-- Attachment #2: Type: text/html, Size: 22358 bytes --]