> > >
> > > - split nb_q_per_pool to nb_rx_q_per_pool and nb_tx_q_per_pool
> > >
> > > Rationale:
> > >
> > > rx and tx number of queue might be different if RX and TX are
> > >
> > > configured in different mode. This allow to inform VF about
> > >
> > > proper number of queues.
> >
> >
> > Nice move! Ouyang, this is a nice answer to my recent remarks about your
> > PATCH4 in "Enable VF RSS for Niantic" series.
>
> After I respond your last comments, I see this, :-), I am sure we both agree
> it is
> the right way to resolve it in vmdq dcb case.
>
I am now dividing this patch with your suggestions and I am little confused.
In this (DCB in SRIOV) case the primary cause for spliting nb_q_per_pool into
nb_rx_q_per_pool and nb_tx_q_per_pool was because of this code:
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index af9e261..be3afe4 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -537,8 +537,8 @@
default: /* ETH_MQ_RX_VMDQ_ONLY or ETH_MQ_RX_NONE */
/* if nothing mq mode configure, use default scheme */
dev->data->dev_conf.rxmode.mq_mode =
ETH_MQ_RX_VMDQ_ONLY;
- if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1)
- RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = 1;
+ if (RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool > 1)
+ RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool = 1;
break;
}
@@ -553,17 +553,18 @@
default: /* ETH_MQ_TX_VMDQ_ONLY or ETH_MQ_TX_NONE */
/* if nothing mq mode configure, use default scheme */
dev->data->dev_conf.txmode.mq_mode =
ETH_MQ_TX_VMDQ_ONLY;
- if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1)
- RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = 1;
+ if (RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool > 1)
+ RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool = 1;
break;
}
/* check valid queue number */
- if ((nb_rx_q > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) ||
- (nb_tx_q > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool)) {
+ if ((nb_rx_q > RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool) ||
+ (nb_tx_q > RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool)) {
PMD_DEBUG_TRACE("ethdev port_id=%d SRIOV active, "
- "queue number must less equal to %d\n",
- port_id,
RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool);
+ "rx/tx queue number must less equal to
%d/%d\n",
+ port_id,
RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool,
+
RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool);
return (-EINVAL);
}
} else {
--
This introduced an issue when RX and TX was configure in different way. The
problem was
that the RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool as common for RX and TX and it is
changed. So I did the above. But when testpmd was adjusted for DCB in SRIOV
there
was another issue. Testpmd is pre-configuring ports by default and since
nb_rx_q_per_pool and nb_tx_q_per_pool was already reset to 1 there was no way
to
use it for DCB in SRIOV. So I did another modification:
> + uint16_t nb_rx_q_per_pool =
> RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool;
> + uint16_t nb_tx_q_per_pool =
> RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool;
> +
> switch (dev_conf->rxmode.mq_mode) {
> - case ETH_MQ_RX_VMDQ_RSS:
> case ETH_MQ_RX_VMDQ_DCB:
> + break;
> + case ETH_MQ_RX_VMDQ_RSS:
> case ETH_MQ_RX_VMDQ_DCB_RSS:
> - /* DCB/RSS VMDQ in SRIOV mode, not implement yet */
> + /* RSS, DCB+RSS VMDQ in SRIOV mode, not implement yet */
> PMD_DEBUG_TRACE("ethdev port_id=%" PRIu8
> " SRIOV active, "
> "unsupported VMDQ mq_mode rx %u\n",
> @@ -537,37 +560,32 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t
> nb_rx_q, uint16_t nb_tx_q,
> default: /* ETH_MQ_RX_VMDQ_ONLY or ETH_MQ_RX_NONE */
> /* if nothing mq mode configure, use default scheme */
> dev->data->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_VMDQ_ONLY;
> - if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1)
> - RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = 1;
> + if (nb_rx_q_per_pool > 1)
> + nb_rx_q_per_pool = 1;
> break;
> }
>
> switch (dev_conf->txmode.mq_mode) {
> - case ETH_MQ_TX_VMDQ_DCB:
> - /* DCB VMDQ in SRIOV mode, not implement yet */
> - PMD_DEBUG_TRACE("ethdev port_id=%" PRIu8
> - " SRIOV active, "
> - "unsupported VMDQ mq_mode tx %u\n",
> - port_id, dev_conf->txmode.mq_mode);
> - return (-EINVAL);
> + case ETH_MQ_TX_VMDQ_DCB: /* DCB VMDQ in SRIOV mode*/
> + break;
> default: /* ETH_MQ_TX_VMDQ_ONLY or ETH_MQ_TX_NONE */
> /* if nothing mq mode configure, use default scheme */
> dev->data->dev_conf.txmode.mq_mode =
> ETH_MQ_TX_VMDQ_ONLY;
> - if (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool > 1)
> - RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = 1;
> + if (nb_tx_q_per_pool > 1)
> + nb_tx_q_per_pool = 1;
> break;
> }
>
> /* check valid queue number */
> - if ((nb_rx_q > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) ||
> - (nb_tx_q > RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool)) {
> + if (nb_rx_q > nb_rx_q_per_pool || nb_tx_q > nb_tx_q_per_pool) {
> PMD_DEBUG_TRACE("ethdev port_id=%d SRIOV active, "
> - "queue number must less equal to %d\n",
> - port_id,
> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool);
> + "rx/tx queue number must less equal to
> %d/%d\n",
> + port_id,
> RTE_ETH_DEV_SRIOV(dev).nb_rx_q_per_pool,
> +
> RTE_ETH_DEV_SRIOV(dev).nb_tx_q_per_pool);
> return (-EINVAL);
> }
For this point I think that splitting RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool
might be not
needed. From my point of view (DCB), since nb_q_per_pool is untouched, I think
I can stay with:
> + uint16_t nb_rx_q_per_pool =
> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool;
> + uint16_t nb_tx_q_per_pool =
> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool;
> +
What do you think? I noticed that you was discussing some issue about
nb_q_per_pool in face
of RSS functionality. Can you spoke about my doubts in face of that RSS?
Pawel
