diff mbox series

[v4] ethdev: check if queue setup in queue-related APIs

Message ID 20201013024126.26492-1-huwei013@chinasoftinc.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v4] ethdev: check if queue setup in queue-related APIs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Wei Hu (Xavier) Oct. 13, 2020, 2:41 a.m. UTC 
  From: Chengchang Tang <tangchengchang@huawei.com>

This patch adds checking whether the related Tx or Rx queue has been setup
in the queue-related API functions to avoid illegal address access. And
validity check of the queue_id is also added in the API functions
rte_eth_dev_rx_intr_enable and rte_eth_dev_rx_intr_disable.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
v3 -> v4:
	 1. dropping the 'rte_' prefix from the funcitons names.
	 2. get "struct rte_eth_dev *dev" as parameter and drop the port_id
	    validation in the function named eth_dev_validate_rx_queue and
	    eth_dev_validate_tx_queue.
	 3. replace "setupped" with "setup" in the commit log and titile.
v2 -> v3:
	 don't break lines in format strings.
v1 -> v2:
	 1. replace %"PRIu16" with %u.
	 2. extact two common functions which validate RXQ/TXQ ids and
	    whether it has been set up or not.
---
 lib/librte_ethdev/rte_ethdev.c | 86 ++++++++++++++++++++++++++++++++++--------
 lib/librte_ethdev/rte_ethdev.h |  3 +-
 2 files changed, 72 insertions(+), 17 deletions(-)

Comments

Kalesh A P Oct. 13, 2020, 4:28 a.m. UTC | #1 
Hi Xavier,

Thanks for taking care of the comments. LGTM.

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

Regards,
Kalesh

On Tue, Oct 13, 2020 at 8:11 AM Wei Hu (Xavier) <huwei013@chinasoftinc.com>
wrote:

> From: Chengchang Tang <tangchengchang@huawei.com>
>
> This patch adds checking whether the related Tx or Rx queue has been setup
> in the queue-related API functions to avoid illegal address access. And
> validity check of the queue_id is also added in the API functions
> rte_eth_dev_rx_intr_enable and rte_eth_dev_rx_intr_disable.
>
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v3 -> v4:
>          1. dropping the 'rte_' prefix from the funcitons names.
>          2. get "struct rte_eth_dev *dev" as parameter and drop the port_id
>             validation in the function named eth_dev_validate_rx_queue and
>             eth_dev_validate_tx_queue.
>          3. replace "setupped" with "setup" in the commit log and titile.
> v2 -> v3:
>          don't break lines in format strings.
> v1 -> v2:
>          1. replace %"PRIu16" with %u.
>          2. extact two common functions which validate RXQ/TXQ ids and
>             whether it has been set up or not.
> ---
>  lib/librte_ethdev/rte_ethdev.c | 86
> ++++++++++++++++++++++++++++++++++--------
>  lib/librte_ethdev/rte_ethdev.h |  3 +-
>  2 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c
> b/lib/librte_ethdev/rte_ethdev.c
> index 892c246..04d30f9 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -877,10 +877,53 @@ rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev,
> uint16_t nb_queues)
>         return 0;
>  }
>
> +static inline int
> +eth_dev_validate_rx_queue(struct rte_eth_dev *dev, uint16_t rx_queue_id)
> +{
> +       uint16_t port_id;
> +
> +       if (rx_queue_id >= dev->data->nb_rx_queues) {
> +               RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n",
> rx_queue_id);
> +               return -EINVAL;
> +       }
> +
> +       if (dev->data->rx_queues[rx_queue_id] == NULL) {
> +               port_id = dev->data->port_id;
> +               RTE_ETHDEV_LOG(ERR,
> +                              "Queue %u of device with port_id=%u has not
> been setup\n",
> +                              rx_queue_id, port_id);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static inline int
> +eth_dev_validate_tx_queue(struct rte_eth_dev *dev, uint16_t tx_queue_id)
> +{
> +       uint16_t port_id;
> +
> +       if (tx_queue_id >= dev->data->nb_tx_queues) {
> +               RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n",
> tx_queue_id);
> +               return -EINVAL;
> +       }
> +
> +       if (dev->data->tx_queues[tx_queue_id] == NULL) {
> +               port_id = dev->data->port_id;
> +               RTE_ETHDEV_LOG(ERR,
> +                              "Queue %u of device with port_id=%u has not
> been setup\n",
> +                              tx_queue_id, port_id);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  int
>  rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
>  {
>         struct rte_eth_dev *dev;
> +       int ret;
>
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>
> @@ -892,10 +935,9 @@ rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t
> rx_queue_id)
>                 return -EINVAL;
>         }
>
> -       if (rx_queue_id >= dev->data->nb_rx_queues) {
> -               RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n",
> rx_queue_id);
> -               return -EINVAL;
> -       }
> +       ret = eth_dev_validate_rx_queue(dev, rx_queue_id);
> +       if (ret)
> +               return ret;
>
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_start, -ENOTSUP);
>
> @@ -922,14 +964,15 @@ int
>  rte_eth_dev_rx_queue_stop(uint16_t port_id, uint16_t rx_queue_id)
>  {
>         struct rte_eth_dev *dev;
> +       int ret;
>
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>
>         dev = &rte_eth_devices[port_id];
> -       if (rx_queue_id >= dev->data->nb_rx_queues) {
> -               RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n",
> rx_queue_id);
> -               return -EINVAL;
> -       }
> +
> +       ret = eth_dev_validate_rx_queue(dev, rx_queue_id);
> +       if (ret)
> +               return ret;
>
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_stop, -ENOTSUP);
>
> @@ -955,6 +998,7 @@ int
>  rte_eth_dev_tx_queue_start(uint16_t port_id, uint16_t tx_queue_id)
>  {
>         struct rte_eth_dev *dev;
> +       int ret;
>
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>
> @@ -966,10 +1010,9 @@ rte_eth_dev_tx_queue_start(uint16_t port_id,
> uint16_t tx_queue_id)
>                 return -EINVAL;
>         }
>
> -       if (tx_queue_id >= dev->data->nb_tx_queues) {
> -               RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n",
> tx_queue_id);
> -               return -EINVAL;
> -       }
> +       ret = eth_dev_validate_tx_queue(dev, tx_queue_id);
> +       if (ret)
> +               return ret;
>
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_start, -ENOTSUP);
>
> @@ -994,14 +1037,15 @@ int
>  rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id)
>  {
>         struct rte_eth_dev *dev;
> +       int ret;
>
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>
>         dev = &rte_eth_devices[port_id];
> -       if (tx_queue_id >= dev->data->nb_tx_queues) {
> -               RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n",
> tx_queue_id);
> -               return -EINVAL;
> -       }
> +
> +       ret = eth_dev_validate_tx_queue(dev, tx_queue_id);
> +       if (ret)
> +               return ret;
>
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_stop, -ENOTSUP);
>
> @@ -4458,11 +4502,16 @@ rte_eth_dev_rx_intr_enable(uint16_t port_id,
>                            uint16_t queue_id)
>  {
>         struct rte_eth_dev *dev;
> +       int ret;
>
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>
>         dev = &rte_eth_devices[port_id];
>
> +       ret = eth_dev_validate_rx_queue(dev, queue_id);
> +       if (ret)
> +               return ret;
> +
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_enable,
> -ENOTSUP);
>         return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_enable)(dev,
>                                                                 queue_id));
> @@ -4473,11 +4522,16 @@ rte_eth_dev_rx_intr_disable(uint16_t port_id,
>                             uint16_t queue_id)
>  {
>         struct rte_eth_dev *dev;
> +       int ret;
>
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>
>         dev = &rte_eth_devices[port_id];
>
> +       ret = eth_dev_validate_rx_queue(dev, queue_id);
> +       if (ret)
> +               return ret;
> +
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_disable,
> -ENOTSUP);
>         return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_disable)(dev,
>                                                                 queue_id));
> diff --git a/lib/librte_ethdev/rte_ethdev.h
> b/lib/librte_ethdev/rte_ethdev.h
> index 5bcfbb8..f4cc591 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4721,7 +4721,8 @@ rte_eth_rx_queue_count(uint16_t port_id, uint16_t
> queue_id)
>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>         dev = &rte_eth_devices[port_id];
>         RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_queue_count, -ENOTSUP);
> -       if (queue_id >= dev->data->nb_rx_queues)
> +       if (queue_id >= dev->data->nb_rx_queues ||
> +           dev->data->rx_queues[queue_id] == NULL)
>                 return -EINVAL;
>
>         return (int)(*dev->rx_queue_count)(dev, queue_id);
> --
> 2.9.5
>
>
Andrew Rybchenko Oct. 13, 2020, 8:08 a.m. UTC | #2 
On 10/13/20 5:41 AM, Wei Hu (Xavier) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> This patch adds checking whether the related Tx or Rx queue has been setup

"This patch adds checking" -> "Add checking"

> in the queue-related API functions to avoid illegal address access. And
> validity check of the queue_id is also added in the API functions
> rte_eth_dev_rx_intr_enable and rte_eth_dev_rx_intr_disable.

Thanks for the patch. LGTM with few minor notes.

Based on the "And ..." in the description I'd consider to split
it into two patches since it has few logical changes.
 * Addition of helper without extra check for queue setup in
    it. I.e. just code restructuring without any changes.
    Definitely harmless.
 * Additoin of queue setup check to helpers
 * Addition of queue ID checks to Rx interrupt control routines

> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v3 -> v4:
> 	 1. dropping the 'rte_' prefix from the funcitons names.
> 	 2. get "struct rte_eth_dev *dev" as parameter and drop the port_id
> 	    validation in the function named eth_dev_validate_rx_queue and
> 	    eth_dev_validate_tx_queue.
> 	 3. replace "setupped" with "setup" in the commit log and titile.
> v2 -> v3:
> 	 don't break lines in format strings.
> v1 -> v2:
> 	 1. replace %"PRIu16" with %u.
> 	 2. extact two common functions which validate RXQ/TXQ ids and
> 	    whether it has been set up or not.
> ---
>  lib/librte_ethdev/rte_ethdev.c | 86 ++++++++++++++++++++++++++++++++++--------
>  lib/librte_ethdev/rte_ethdev.h |  3 +-
>  2 files changed, 72 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 892c246..04d30f9 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -877,10 +877,53 @@ rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
>  	return 0;
>  }
>  
> +static inline int

Pleae, remove inline. The compiler should be wise enought to do
the job. Also it is control path helpers, so there is no point
to blow binaries size.

> +eth_dev_validate_rx_queue(struct rte_eth_dev *dev, uint16_t rx_queue_id)

May be add 'const' to dev to highlight that it changes nothing.

> +{
> +	uint16_t port_id;
> +
> +	if (rx_queue_id >= dev->data->nb_rx_queues) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);

RX -> Rx
Please, log port ID as well.
I see that the log is simply copies from previous code,
but let's improve it on the way.

> +		return -EINVAL;
> +	}
> +
> +	if (dev->data->rx_queues[rx_queue_id] == NULL) {
> +		port_id = dev->data->port_id;
> +		RTE_ETHDEV_LOG(ERR,
> +			       "Queue %u of device with port_id=%u has not been setup\n",
> +			       rx_queue_id, port_id);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static inline int

same as above

> +eth_dev_validate_tx_queue(struct rte_eth_dev *dev, uint16_t tx_queue_id)

same as above

> +{
> +	uint16_t port_id;
> +
> +	if (tx_queue_id >= dev->data->nb_tx_queues) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);

TX -> Tx
plus port_id logging

> +		return -EINVAL;
> +	}
> +
> +	if (dev->data->tx_queues[tx_queue_id] == NULL) {
> +		port_id = dev->data->port_id;
> +		RTE_ETHDEV_LOG(ERR,
> +			       "Queue %u of device with port_id=%u has not been setup\n",
> +			       tx_queue_id, port_id);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  int
>  rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int ret;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
> @@ -892,10 +935,9 @@ rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
>  		return -EINVAL;
>  	}
>  
> -	if (rx_queue_id >= dev->data->nb_rx_queues) {
> -		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
> -		return -EINVAL;
> -	}
> +	ret = eth_dev_validate_rx_queue(dev, rx_queue_id);
> +	if (ret)

DPDK coding style requires explicit comparion to 0.
Please, add != 0

> +		return ret;
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_start, -ENOTSUP);
>  
> @@ -922,14 +964,15 @@ int
>  rte_eth_dev_rx_queue_stop(uint16_t port_id, uint16_t rx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int ret;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> -	if (rx_queue_id >= dev->data->nb_rx_queues) {
> -		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
> -		return -EINVAL;
> -	}
> +
> +	ret = eth_dev_validate_rx_queue(dev, rx_queue_id);
> +	if (ret)

same as above, if (ret != 0)

> +		return ret;
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_stop, -ENOTSUP);
>  
> @@ -955,6 +998,7 @@ int
>  rte_eth_dev_tx_queue_start(uint16_t port_id, uint16_t tx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int ret;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
> @@ -966,10 +1010,9 @@ rte_eth_dev_tx_queue_start(uint16_t port_id, uint16_t tx_queue_id)
>  		return -EINVAL;
>  	}
>  
> -	if (tx_queue_id >= dev->data->nb_tx_queues) {
> -		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
> -		return -EINVAL;
> -	}
> +	ret = eth_dev_validate_tx_queue(dev, tx_queue_id);
> +	if (ret)

same as above, if (ret != 0)

> +		return ret;
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_start, -ENOTSUP);
>  
> @@ -994,14 +1037,15 @@ int
>  rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int ret;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> -	if (tx_queue_id >= dev->data->nb_tx_queues) {
> -		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
> -		return -EINVAL;
> -	}
> +
> +	ret = eth_dev_validate_tx_queue(dev, tx_queue_id);
> +	if (ret)

same as above, if (ret != 0)

> +		return ret;
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_stop, -ENOTSUP);
>  
> @@ -4458,11 +4502,16 @@ rte_eth_dev_rx_intr_enable(uint16_t port_id,
>  			   uint16_t queue_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int ret;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  
>  	dev = &rte_eth_devices[port_id];
>  
> +	ret = eth_dev_validate_rx_queue(dev, queue_id);
> +	if (ret)

same as above, if (ret != 0)

> +		return ret;
> +
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_enable, -ENOTSUP);
>  	return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_enable)(dev,
>  								queue_id));
> @@ -4473,11 +4522,16 @@ rte_eth_dev_rx_intr_disable(uint16_t port_id,
>  			    uint16_t queue_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int ret;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  
>  	dev = &rte_eth_devices[port_id];
>  
> +	ret = eth_dev_validate_rx_queue(dev, queue_id);
> +	if (ret)

same as above, if (ret != 0)

> +		return ret;
> +
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_disable, -ENOTSUP);
>  	return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_disable)(dev,
>  								queue_id));
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 5bcfbb8..f4cc591 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4721,7 +4721,8 @@ rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  	dev = &rte_eth_devices[port_id];
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_queue_count, -ENOTSUP);
> -	if (queue_id >= dev->data->nb_rx_queues)
> +	if (queue_id >= dev->data->nb_rx_queues ||
> +	    dev->data->rx_queues[queue_id] == NULL)
>  		return -EINVAL;
>  
>  	return (int)(*dev->rx_queue_count)(dev, queue_id);
>
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 892c246..04d30f9 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -877,10 +877,53 @@  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
 	return 0;
 }
 
+static inline int
+eth_dev_validate_rx_queue(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+	uint16_t port_id;
+
+	if (rx_queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
+		return -EINVAL;
+	}
+
+	if (dev->data->rx_queues[rx_queue_id] == NULL) {
+		port_id = dev->data->port_id;
+		RTE_ETHDEV_LOG(ERR,
+			       "Queue %u of device with port_id=%u has not been setup\n",
+			       rx_queue_id, port_id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static inline int
+eth_dev_validate_tx_queue(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+	uint16_t port_id;
+
+	if (tx_queue_id >= dev->data->nb_tx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
+		return -EINVAL;
+	}
+
+	if (dev->data->tx_queues[tx_queue_id] == NULL) {
+		port_id = dev->data->port_id;
+		RTE_ETHDEV_LOG(ERR,
+			       "Queue %u of device with port_id=%u has not been setup\n",
+			       tx_queue_id, port_id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int
 rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
@@ -892,10 +935,9 @@  rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
 		return -EINVAL;
 	}
 
-	if (rx_queue_id >= dev->data->nb_rx_queues) {
-		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
-		return -EINVAL;
-	}
+	ret = eth_dev_validate_rx_queue(dev, rx_queue_id);
+	if (ret)
+		return ret;
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_start, -ENOTSUP);
 
@@ -922,14 +964,15 @@  int
 rte_eth_dev_rx_queue_stop(uint16_t port_id, uint16_t rx_queue_id)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
-	if (rx_queue_id >= dev->data->nb_rx_queues) {
-		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
-		return -EINVAL;
-	}
+
+	ret = eth_dev_validate_rx_queue(dev, rx_queue_id);
+	if (ret)
+		return ret;
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_stop, -ENOTSUP);
 
@@ -955,6 +998,7 @@  int
 rte_eth_dev_tx_queue_start(uint16_t port_id, uint16_t tx_queue_id)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
@@ -966,10 +1010,9 @@  rte_eth_dev_tx_queue_start(uint16_t port_id, uint16_t tx_queue_id)
 		return -EINVAL;
 	}
 
-	if (tx_queue_id >= dev->data->nb_tx_queues) {
-		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
-		return -EINVAL;
-	}
+	ret = eth_dev_validate_tx_queue(dev, tx_queue_id);
+	if (ret)
+		return ret;
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_start, -ENOTSUP);
 
@@ -994,14 +1037,15 @@  int
 rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
-	if (tx_queue_id >= dev->data->nb_tx_queues) {
-		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
-		return -EINVAL;
-	}
+
+	ret = eth_dev_validate_tx_queue(dev, tx_queue_id);
+	if (ret)
+		return ret;
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_stop, -ENOTSUP);
 
@@ -4458,11 +4502,16 @@  rte_eth_dev_rx_intr_enable(uint16_t port_id,
 			   uint16_t queue_id)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
 	dev = &rte_eth_devices[port_id];
 
+	ret = eth_dev_validate_rx_queue(dev, queue_id);
+	if (ret)
+		return ret;
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_enable, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_enable)(dev,
 								queue_id));
@@ -4473,11 +4522,16 @@  rte_eth_dev_rx_intr_disable(uint16_t port_id,
 			    uint16_t queue_id)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
 	dev = &rte_eth_devices[port_id];
 
+	ret = eth_dev_validate_rx_queue(dev, queue_id);
+	if (ret)
+		return ret;
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_disable, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_disable)(dev,
 								queue_id));
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 5bcfbb8..f4cc591 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4721,7 +4721,8 @@  rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_queue_count, -ENOTSUP);
-	if (queue_id >= dev->data->nb_rx_queues)
+	if (queue_id >= dev->data->nb_rx_queues ||
+	    dev->data->rx_queues[queue_id] == NULL)
 		return -EINVAL;
 
 	return (int)(*dev->rx_queue_count)(dev, queue_id);