There is a funny logic that seems to be outdated which tries to detect a situation when a user requests a default size of the queue and configures a device specific default value. This seems to be not in line with the DPDK current design that expects the user to go via the rte_eth_dev_info_get() in order to get device specific values. And if the user doesn't care it should pass 0 as a queue length and this case is also being taken care of on the rte_eth level. Signed-off-by: Vlad Zolotarov <vladz@scylladb.com> --- drivers/net/ena/ena_ethdev.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 665afee4f..bf632dee0 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -1221,9 +1221,6 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, return -EINVAL; } - if (nb_desc == RTE_ETH_DEV_FALLBACK_TX_RINGSIZE) - nb_desc = adapter->tx_ring_size; - txq->port_id = dev->data->port_id; txq->next_to_clean = 0; txq->next_to_use = 0; @@ -1292,9 +1289,6 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, return ENA_COM_FAULT; } - if (nb_desc == RTE_ETH_DEV_FALLBACK_RX_RINGSIZE) - nb_desc = adapter->rx_ring_size; - if (!rte_is_power_of_2(nb_desc)) { PMD_DRV_LOG(ERR, "Unsupported size of RX queue: %d is not a power of 2.\n", -- 2.20.1
Igor, Evgeny,
Please, review.
On 3/5/20 5:23 PM, Vlad Zolotarov wrote:
> There is a funny logic that seems to be outdated which tries to detect
> a situation when a user requests a default size of the queue and
> configures a device specific default value.
>
> This seems to be not in line with the DPDK current design that expects
> the user to go via the rte_eth_dev_info_get() in order to get device
> specific values.
>
> And if the user doesn't care it should pass 0 as a queue length and this
> case is also being taken care of on the rte_eth level.
>
> Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
> ---
> drivers/net/ena/ena_ethdev.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 665afee4f..bf632dee0 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1221,9 +1221,6 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
> return -EINVAL;
> }
>
> - if (nb_desc == RTE_ETH_DEV_FALLBACK_TX_RINGSIZE)
> - nb_desc = adapter->tx_ring_size;
> -
> txq->port_id = dev->data->port_id;
> txq->next_to_clean = 0;
> txq->next_to_use = 0;
> @@ -1292,9 +1289,6 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
> return ENA_COM_FAULT;
> }
>
> - if (nb_desc == RTE_ETH_DEV_FALLBACK_RX_RINGSIZE)
> - nb_desc = adapter->rx_ring_size;
> -
> if (!rte_is_power_of_2(nb_desc)) {
> PMD_DRV_LOG(ERR,
> "Unsupported size of RX queue: %d is not a power of 2.\n",
Acked inline From: Vladislav Zolotarov <vladz@scylladb.com> Sent: Friday, March 6, 2020 12:28 AM To: dev@dpdk.org; Schmeilin, Evgeny <evgenys@amazon.com>; Chauskin, Igor <igorch@amazon.com> Subject: RE: [EXTERNAL][dpdk-dev] [PATCH v1] ena_ethdev: don't override the user provided queue length value Igor, Evgeny, Please, review. On 3/5/20 5:23 PM, Vlad Zolotarov wrote: There is a funny logic that seems to be outdated which tries to detect a situation when a user requests a default size of the queue and configures a device specific default value. This seems to be not in line with the DPDK current design that expects the user to go via the rte_eth_dev_info_get() in order to get device specific values. And if the user doesn't care it should pass 0 as a queue length and this case is also being taken care of on the rte_eth level. Signed-off-by: Vlad Zolotarov <vladz@scylladb.com><mailto:vladz@scylladb.com> Acked-by: Igor Chauskin igorch@amazon.com<mailto:igorch@amazon.com> --- drivers/net/ena/ena_ethdev.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 665afee4f..bf632dee0 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -1221,9 +1221,6 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, return -EINVAL; } - if (nb_desc == RTE_ETH_DEV_FALLBACK_TX_RINGSIZE) - nb_desc = adapter->tx_ring_size; - txq->port_id = dev->data->port_id; txq->next_to_clean = 0; txq->next_to_use = 0; @@ -1292,9 +1289,6 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, return ENA_COM_FAULT; } - if (nb_desc == RTE_ETH_DEV_FALLBACK_RX_RINGSIZE) - nb_desc = adapter->rx_ring_size; - if (!rte_is_power_of_2(nb_desc)) { PMD_DRV_LOG(ERR, "Unsupported size of RX queue: %d is not a power of 2.\n",
On 3/9/2020 10:07 AM, Chauskin, Igor wrote: > Acked inline > > From: Vladislav Zolotarov <vladz@scylladb.com> > Sent: Friday, March 6, 2020 12:28 AM > To: dev@dpdk.org; Schmeilin, Evgeny <evgenys@amazon.com>; Chauskin, Igor <igorch@amazon.com> > Subject: RE: [EXTERNAL][dpdk-dev] [PATCH v1] ena_ethdev: don't override the user provided queue length value > > Igor, Evgeny, > Please, review. > On 3/5/20 5:23 PM, Vlad Zolotarov wrote: > > There is a funny logic that seems to be outdated which tries to detect > > a situation when a user requests a default size of the queue and > > configures a device specific default value. > > > > This seems to be not in line with the DPDK current design that expects > > the user to go via the rte_eth_dev_info_get() in order to get device > > specific values. > > > > And if the user doesn't care it should pass 0 as a queue length and this > > case is also being taken care of on the rte_eth level. > > > > Signed-off-by: Vlad Zolotarov <vladz@scylladb.com><mailto:vladz@scylladb.com> > > Acked-by: Igor Chauskin igorch@amazon.com<mailto:igorch@amazon.com> > > --- > > drivers/net/ena/ena_ethdev.c | 6 ------ > > 1 file changed, 6 deletions(-) > > > > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c > > index 665afee4f..bf632dee0 100644 > > --- a/drivers/net/ena/ena_ethdev.c > > +++ b/drivers/net/ena/ena_ethdev.c > > @@ -1221,9 +1221,6 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, > > return -EINVAL; > > } > > > > - if (nb_desc == RTE_ETH_DEV_FALLBACK_TX_RINGSIZE) > > - nb_desc = adapter->tx_ring_size; > > - Logic in ethdev is; /* Use default specified by driver, if nb_tx_desc is zero */ if (nb_tx_desc == 0) { nb_tx_desc = dev_info.default_txportconf.ring_size; /* If driver default is zero, fall back on EAL default */ if (nb_tx_desc == 0) nb_tx_desc = RTE_ETH_DEV_FALLBACK_TX_RINGSIZE; } User may provide, '0' nb_tx_desc to request driver default value. If there is no driver default value 'RTE_ETH_DEV_FALLBACK_TX_RINGSIZE' is used. As driver doesn't support the default value, the ring size still can be set to 'RTE_ETH_DEV_FALLBACK_TX_RINGSIZE'. Wouldn't be good to add driver default value when removing this check? > > txq->port_id = dev->data->port_id; > > txq->next_to_clean = 0; > > txq->next_to_use = 0; > > @@ -1292,9 +1289,6 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, > > return ENA_COM_FAULT; > > } > > > > - if (nb_desc == RTE_ETH_DEV_FALLBACK_RX_RINGSIZE) > > - nb_desc = adapter->rx_ring_size; > > - > > if (!rte_is_power_of_2(nb_desc)) { > > PMD_DRV_LOG(ERR, > > "Unsupported size of RX queue: %d is not a power of 2.\n", >