ethdev: fix device info getting
Checks
Commit Message
The device information cannot be gotten correctly before
the configuration is set. Because on some NICs the
information has dependence on the configuration.
Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
lib/librte_ethdev/rte_ethdev.c | 47 +++++++++++++++++++++---------------------
1 file changed, 24 insertions(+), 23 deletions(-)
Comments
On 12.07.2018 08:27, Wenzhuo Lu wrote:
> The device information cannot be gotten correctly before
> the configuration is set. Because on some NICs the
> information has dependence on the configuration.
>
> Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> lib/librte_ethdev/rte_ethdev.c | 47 +++++++++++++++++++++---------------------
> 1 file changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 3d556a8..9d60bea 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1017,28 +1017,6 @@ struct rte_eth_dev *
>
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>
> - dev = &rte_eth_devices[port_id];
> -
> - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
> - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
> -
> - rte_eth_dev_info_get(port_id, &dev_info);
> -
> - /* If number of queues specified by application for both Rx and Tx is
> - * zero, use driver preferred values. This cannot be done individually
> - * as it is valid for either Tx or Rx (but not both) to be zero.
> - * If driver does not provide any preferred valued, fall back on
> - * EAL defaults.
> - */
> - if (nb_rx_q == 0 && nb_tx_q == 0) {
> - nb_rx_q = dev_info.default_rxportconf.nb_queues;
> - if (nb_rx_q == 0)
> - nb_rx_q = RTE_ETH_DEV_FALLBACK_RX_NBQUEUES;
> - nb_tx_q = dev_info.default_txportconf.nb_queues;
> - if (nb_tx_q == 0)
> - nb_tx_q = RTE_ETH_DEV_FALLBACK_TX_NBQUEUES;
> - }
> -
> if (nb_rx_q > RTE_MAX_QUEUES_PER_PORT) {
> RTE_ETHDEV_LOG(ERR,
> "Number of RX queues requested (%u) is greater than max supported(%d)\n",
> @@ -1053,6 +1031,11 @@ struct rte_eth_dev *
> return -EINVAL;
> }
>
> + dev = &rte_eth_devices[port_id];
> +
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
> +
> if (dev->data->dev_started) {
> RTE_ETHDEV_LOG(ERR,
> "Port %u must be stopped to allow configuration\n",
> @@ -1060,8 +1043,26 @@ struct rte_eth_dev *
> return -EBUSY;
> }
>
> - /* Copy the dev_conf parameter into the dev structure */
> + /* Copy the dev_conf parameter into the dev structure,
> + * then get the info.
> + */
> memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf));
> + rte_eth_dev_info_get(port_id, &dev_info);
> +
> + /* If number of queues specified by application for both Rx and Tx is
> + * zero, use driver preferred values. This cannot be done individually
> + * as it is valid for either Tx or Rx (but not both) to be zero.
> + * If driver does not provide any preferred valued, fall back on
> + * EAL defaults.
> + */
> + if (nb_rx_q == 0 && nb_tx_q == 0) {
> + nb_rx_q = dev_info.default_rxportconf.nb_queues;
> + if (nb_rx_q == 0)
> + nb_rx_q = RTE_ETH_DEV_FALLBACK_RX_NBQUEUES;
> + nb_tx_q = dev_info.default_txportconf.nb_queues;
> + if (nb_tx_q == 0)
> + nb_tx_q = RTE_ETH_DEV_FALLBACK_TX_NBQUEUES;
Values assigned in this branch are not checked against
RTE_MAX_QUEUES_PER_PORT and RTE_MAX_QUEUES_PER_PORT now
> + }
>
> /*
> * Check that the numbers of RX and TX queues are not greater
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Thursday, July 12, 2018 4:06 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ethdev: fix device info getting
>
> On 12.07.2018 08:27, Wenzhuo Lu wrote:
> > The device information cannot be gotten correctly before the
> > configuration is set. Because on some NICs the information has
> > dependence on the configuration.
> >
> > Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> > lib/librte_ethdev/rte_ethdev.c | 47 +++++++++++++++++++++------------------
> ---
> > 1 file changed, 24 insertions(+), 23 deletions(-)
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.c
> > b/lib/librte_ethdev/rte_ethdev.c index 3d556a8..9d60bea 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -1017,28 +1017,6 @@ struct rte_eth_dev *
> >
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> >
> > - dev = &rte_eth_devices[port_id];
> > -
> > - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -
> ENOTSUP);
> > - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -
> ENOTSUP);
> > -
> > - rte_eth_dev_info_get(port_id, &dev_info);
> > -
> > - /* If number of queues specified by application for both Rx and Tx is
> > - * zero, use driver preferred values. This cannot be done individually
> > - * as it is valid for either Tx or Rx (but not both) to be zero.
> > - * If driver does not provide any preferred valued, fall back on
> > - * EAL defaults.
> > - */
> > - if (nb_rx_q == 0 && nb_tx_q == 0) {
> > - nb_rx_q = dev_info.default_rxportconf.nb_queues;
> > - if (nb_rx_q == 0)
> > - nb_rx_q = RTE_ETH_DEV_FALLBACK_RX_NBQUEUES;
> > - nb_tx_q = dev_info.default_txportconf.nb_queues;
> > - if (nb_tx_q == 0)
> > - nb_tx_q = RTE_ETH_DEV_FALLBACK_TX_NBQUEUES;
> > - }
> > -
> > if (nb_rx_q > RTE_MAX_QUEUES_PER_PORT) {
> > RTE_ETHDEV_LOG(ERR,
> > "Number of RX queues requested (%u) is greater
> than max
> > supported(%d)\n", @@ -1053,6 +1031,11 @@ struct rte_eth_dev *
> > return -EINVAL;
> > }
> >
> > + dev = &rte_eth_devices[port_id];
> > +
> > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -
> ENOTSUP);
> > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -
> ENOTSUP);
> > +
> > if (dev->data->dev_started) {
> > RTE_ETHDEV_LOG(ERR,
> > "Port %u must be stopped to allow configuration\n",
> @@ -1060,8
> > +1043,26 @@ struct rte_eth_dev *
> > return -EBUSY;
> > }
> >
> > - /* Copy the dev_conf parameter into the dev structure */
> > + /* Copy the dev_conf parameter into the dev structure,
> > + * then get the info.
> > + */
> > memcpy(&dev->data->dev_conf, &local_conf,
> > sizeof(dev->data->dev_conf));
> > + rte_eth_dev_info_get(port_id, &dev_info);
> > +
> > + /* If number of queues specified by application for both Rx and Tx is
> > + * zero, use driver preferred values. This cannot be done individually
> > + * as it is valid for either Tx or Rx (but not both) to be zero.
> > + * If driver does not provide any preferred valued, fall back on
> > + * EAL defaults.
> > + */
> > + if (nb_rx_q == 0 && nb_tx_q == 0) {
> > + nb_rx_q = dev_info.default_rxportconf.nb_queues;
> > + if (nb_rx_q == 0)
> > + nb_rx_q = RTE_ETH_DEV_FALLBACK_RX_NBQUEUES;
> > + nb_tx_q = dev_info.default_txportconf.nb_queues;
> > + if (nb_tx_q == 0)
> > + nb_tx_q = RTE_ETH_DEV_FALLBACK_TX_NBQUEUES;
>
> Values assigned in this branch are not checked against
> RTE_MAX_QUEUES_PER_PORT and RTE_MAX_QUEUES_PER_PORT now
O, I supposed the default values cannot be larger than RTE_MAX_QUEUES_PER_PORT. But, yes, it's each NIC's decision. My assumption can be wrong. I'll move the check down here.
>
> > + }
> >
> > /*
> > * Check that the numbers of RX and TX queues are not greater
@@ -1017,28 +1017,6 @@ struct rte_eth_dev *
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
- dev = &rte_eth_devices[port_id];
-
- RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
- RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
-
- rte_eth_dev_info_get(port_id, &dev_info);
-
- /* If number of queues specified by application for both Rx and Tx is
- * zero, use driver preferred values. This cannot be done individually
- * as it is valid for either Tx or Rx (but not both) to be zero.
- * If driver does not provide any preferred valued, fall back on
- * EAL defaults.
- */
- if (nb_rx_q == 0 && nb_tx_q == 0) {
- nb_rx_q = dev_info.default_rxportconf.nb_queues;
- if (nb_rx_q == 0)
- nb_rx_q = RTE_ETH_DEV_FALLBACK_RX_NBQUEUES;
- nb_tx_q = dev_info.default_txportconf.nb_queues;
- if (nb_tx_q == 0)
- nb_tx_q = RTE_ETH_DEV_FALLBACK_TX_NBQUEUES;
- }
-
if (nb_rx_q > RTE_MAX_QUEUES_PER_PORT) {
RTE_ETHDEV_LOG(ERR,
"Number of RX queues requested (%u) is greater than max supported(%d)\n",
@@ -1053,6 +1031,11 @@ struct rte_eth_dev *
return -EINVAL;
}
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
+
if (dev->data->dev_started) {
RTE_ETHDEV_LOG(ERR,
"Port %u must be stopped to allow configuration\n",
@@ -1060,8 +1043,26 @@ struct rte_eth_dev *
return -EBUSY;
}
- /* Copy the dev_conf parameter into the dev structure */
+ /* Copy the dev_conf parameter into the dev structure,
+ * then get the info.
+ */
memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf));
+ rte_eth_dev_info_get(port_id, &dev_info);
+
+ /* If number of queues specified by application for both Rx and Tx is
+ * zero, use driver preferred values. This cannot be done individually
+ * as it is valid for either Tx or Rx (but not both) to be zero.
+ * If driver does not provide any preferred valued, fall back on
+ * EAL defaults.
+ */
+ if (nb_rx_q == 0 && nb_tx_q == 0) {
+ nb_rx_q = dev_info.default_rxportconf.nb_queues;
+ if (nb_rx_q == 0)
+ nb_rx_q = RTE_ETH_DEV_FALLBACK_RX_NBQUEUES;
+ nb_tx_q = dev_info.default_txportconf.nb_queues;
+ if (nb_tx_q == 0)
+ nb_tx_q = RTE_ETH_DEV_FALLBACK_TX_NBQUEUES;
+ }
/*
* Check that the numbers of RX and TX queues are not greater