[PATCH] app/testpmd: fix segment fault with invalid queue id

[PATCH] app/testpmd: fix segment fault with invalid queue id

[Dengdui Huang]

When input queue id is invalid, it will lead to
Segmentation fault, like:

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 txq/rxq 99 desc 0 status
Segmentation fault

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 rxq 99 desc used count
Segmentation fault

This patch fixes it.

In addition, this patch add the check for the offset
of the descriptor in case of other anomalies.

Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-pmd/cmdline.c | 50 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7b20bef4e9..624752c320 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -12273,14 +12273,27 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 		__rte_unused void *data)
 {
 	struct cmd_show_rx_tx_desc_status_result *res = parsed_result;
+	struct rte_eth_rxq_info rxq_info;
+	struct rte_eth_txq_info txq_info;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
-		return;
-	}
-
 	if (!strcmp(res->cmd_keyword, "rxq")) {
+		if (rte_eth_rx_queue_info_get(res->cmd_pid, res->cmd_qid, &rxq_info)) {
+			fprintf(stderr, "Failed to get port %u Rx queue %u info\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
+
+		if (rxq_info.queue_state != RTE_ETH_QUEUE_STATE_STARTED) {
+			fprintf(stderr, "Rx queue %u not started\n", res->cmd_qid);
+			return;
+		}
+
+		if (res->cmd_did >= rxq_info.nb_desc) {
+			fprintf(stderr, "Invalid desc id %u\n", res->cmd_did);
+			return;
+		}
+
 		rc = rte_eth_rx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12296,6 +12309,22 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 		else
 			printf("Desc status = UNAVAILABLE\n");
 	} else if (!strcmp(res->cmd_keyword, "txq")) {
+		if (rte_eth_tx_queue_info_get(res->cmd_pid, res->cmd_qid, &txq_info)) {
+			fprintf(stderr, "Failed to get port %u Tx queue %u info\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
+
+		if (txq_info.queue_state != RTE_ETH_QUEUE_STATE_STARTED) {
+			fprintf(stderr, "Tx queue %u not started\n", res->cmd_qid);
+			return;
+		}
+
+		if (res->cmd_did >= txq_info.nb_desc) {
+			fprintf(stderr, "Invalid desc id %u\n", res->cmd_did);
+			return;
+		}
+
 		rc = rte_eth_tx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12373,10 +12402,17 @@ cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
 		__rte_unused void *data)
 {
 	struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
+	struct rte_eth_rxq_info rxq_info;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
+	if (rte_eth_rx_queue_info_get(res->cmd_pid, res->cmd_qid, &rxq_info)) {
+		fprintf(stderr, "Failed to get port %u Rx queue %u info\n",
+			res->cmd_pid, res->cmd_qid);
+		return;
+	}
+
+	if (rxq_info.queue_state != RTE_ETH_QUEUE_STATE_STARTED) {
+		fprintf(stderr, "Rx queue %u not started\n", res->cmd_qid);
 		return;
 	}
 
-- 
2.33.0

Re: [PATCH] app/testpmd: fix segment fault with invalid queue id

[Stephen Hemminger]

On Tue, 16 May 2023 19:00:21 +0800
Dengdui Huang <huangdengdui@huawei.com> wrote:

> When input queue id is invalid, it will lead to
> Segmentation fault, like:
> 
> dpdk-testpmd -a 0000:01:00.0 -- -i
> testpmd> show port 0 txq/rxq 99 desc 0 status  
> Segmentation fault
> 
> dpdk-testpmd -a 0000:01:00.0 -- -i
> testpmd> show port 0 rxq 99 desc used count  
> Segmentation fault
> 
> This patch fixes it.
> 
> In addition, this patch add the check for the offset
> of the descriptor in case of other anomalies.
> 
> Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
> Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>

What is the backtrace and device driver?  The problem is that other users
besides testpmd might hit same problem.

It would make sense to have a function to test for valid rx and tx queue id
in rte_ethdev. Similar to existing rte_eth_dev_is_valid_port() rather than
open coding it.  Maybe rte_eth_dev_is_valid_rxq(port_id, queue_id)?

here was talk that the existing rx queue descriptor status is racy, and
unused by any real application; and therefore would be good candidate for
future removal.

Re: [PATCH] app/testpmd: fix segment fault with invalid queue id

[huangdengdui]

On 2023/5/16 23:12, Stephen Hemminger wrote:
> On Tue, 16 May 2023 19:00:21 +0800
> Dengdui Huang <huangdengdui@huawei.com> wrote:
> 
>> When input queue id is invalid, it will lead to
>> Segmentation fault, like:
>>
>> dpdk-testpmd -a 0000:01:00.0 -- -i
>> testpmd> show port 0 txq/rxq 99 desc 0 status  
>> Segmentation fault
>>
>> dpdk-testpmd -a 0000:01:00.0 -- -i
>> testpmd> show port 0 rxq 99 desc used count  
>> Segmentation fault
>>
>> This patch fixes it.
>>
>> In addition, this patch add the check for the offset
>> of the descriptor in case of other anomalies.
>>
>> Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
>> Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> 
> What is the backtrace and device driver?  The problem is that other users
> besides testpmd might hit same problem.
> 
> It would make sense to have a function to test for valid rx and tx queue id
> in rte_ethdev. Similar to existing rte_eth_dev_is_valid_port() rather than
> open coding it.  Maybe rte_eth_dev_is_valid_rxq(port_id, queue_id)?
> 
> here was talk that the existing rx queue descriptor status is racy, and
> unused by any real application; and therefore would be good candidate for
> future removal.
Hi, Stephen

OK.

Agreed. But these APIs are still in dpdk, so testpmd as user side should use them correctly.

Thanks,
Dengdui

[PATCH v2 0/2] add API and use it to fix a bug

[Dengdui Huang]

add API to check queue ID validity and use it to fix a bug

Dengdui Huang (2):
  ethdev: add API to check queue ID validity
  app/testpmd: fix segment fault with invalid queue id

 app/test-pmd/cmdline.c                 | 23 +++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  5 ++++
 lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  4 +++
 5 files changed, 91 insertions(+), 7 deletions(-)

-- 
2.33.0

[PATCH v2 1/2] ethdev: add API to check queue ID validity

[Dengdui Huang]

The API rte_eth_dev_is_valid_rxq/txq checks
the port ID validity and then the Rx/Tx queue ID is valid.

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 doc/guides/rel_notes/release_23_07.rst |  5 ++++
 lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  4 +++
 4 files changed, 75 insertions(+)

diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..19e645156f 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -56,6 +56,11 @@ New Features
      =======================================================
 
 
+* **Added ethdev Rx/Tx queue id check API.**
+
+  Added ethdev Rx/Tx queue id check API which provides functions
+  for check if Rx/Tx queue id is valid.
+
 Removed Items
 -------------
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 4d03255683..3d85218127 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
 	return is_valid;
 }
 
+int
+rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues ||
+			dev->data->rx_queues[queue_id] == NULL)
+		return -EINVAL;
+
+	return 0;
+}
+
+int
+rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_tx_queues ||
+			dev->data->tx_queues[queue_id] == NULL)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int
 eth_is_valid_owner_id(uint64_t owner_id)
 {
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 99fe9e238b..abda95b27f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2665,6 +2665,42 @@ int rte_eth_dev_socket_id(uint16_t port_id);
  */
 int rte_eth_dev_is_valid_port(uint16_t port_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Rx queue ID is valid.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the receive queue.
+ * @return
+ *   - -ENODEV: if *port_id* is invalid.
+ *   - -EINVAL: if queue is out of range or not been setup.
+ *   - 0 if queue ID is valid.
+ */
+__rte_experimental
+int rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Tx queue ID is valid.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the transmit queue.
+ * @return
+ *   - -ENODEV: if *port_id* is invalid.
+ *   - -EINVAL: if queue is out of range or not been setup.
+ *   - 0 if Tx queue ID is valid.
+ */
+__rte_experimental
+int rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id);
+
 /**
  * Start specified Rx queue of a port. It is used when rx_deferred_start
  * flag of the specified queue is true.
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 357d1a88c0..d8797310d2 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -299,6 +299,10 @@ EXPERIMENTAL {
 	rte_flow_action_handle_query_update;
 	rte_flow_async_action_handle_query_update;
 	rte_flow_async_create_by_index;
+
+	# added in 23.07
+	rte_eth_dev_is_valid_rxq;
+	rte_eth_dev_is_valid_txq;
 };
 
 INTERNAL {
-- 
2.33.0

[PATCH v2 2/2] app/testpmd: fix segment fault with invalid queue id

[Dengdui Huang]

When input queue id is invalid, it will lead to
Segmentation fault, like:

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 txq/rxq 99 desc 0 status
Segmentation fault

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 rxq 99 desc used count
Segmentation fault

This patch fixes it.

Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7b20bef4e9..334434497c 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -12275,12 +12275,13 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 	struct cmd_show_rx_tx_desc_status_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
-		return;
-	}
-
 	if (!strcmp(res->cmd_keyword, "rxq")) {
+		if (rte_eth_dev_is_valid_rxq(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_rx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12296,6 +12297,12 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 		else
 			printf("Desc status = UNAVAILABLE\n");
 	} else if (!strcmp(res->cmd_keyword, "txq")) {
+		if (rte_eth_dev_is_valid_txq(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_tx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12375,8 +12382,10 @@ cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
 	struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
+	if (rte_eth_dev_is_valid_rxq(res->cmd_pid, res->cmd_qid) != 0) {
+		fprintf(stderr,
+			"Invalid input: port id = %d, queue id = %d\n",
+			res->cmd_pid, res->cmd_qid);
 		return;
 	}
 
-- 
2.33.0

Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity

[Andrew Rybchenko]

On 5/22/23 16:09, Dengdui Huang wrote:
> The API rte_eth_dev_is_valid_rxq/txq checks
> the port ID validity and then the Rx/Tx queue ID is valid.

What is valid Tx/Rx queue? It depends on on caller
expectations. Some functions are satisfied with just
check vs configured number of queues. Some require
the queue to be setup. May be some should require
the queue to be started.

So, I suggest to avoid term "valid" and be more precise
here and API naming.

> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
>   doc/guides/rel_notes/release_23_07.rst |  5 ++++
>   lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
>   lib/ethdev/version.map                 |  4 +++
>   4 files changed, 75 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
> index a9b1293689..19e645156f 100644
> --- a/doc/guides/rel_notes/release_23_07.rst
> +++ b/doc/guides/rel_notes/release_23_07.rst
> @@ -56,6 +56,11 @@ New Features
>        =======================================================
>   
>   
> +* **Added ethdev Rx/Tx queue id check API.**
> +
> +  Added ethdev Rx/Tx queue id check API which provides functions

id -> ID

> +  for check if Rx/Tx queue id is valid.

id -> ID

> +

It should be two empty lines here and just one above.

>   Removed Items
>   -------------
>   
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 4d03255683..3d85218127 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
>   	return is_valid;
>   }
>   
> +int
> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (queue_id >= dev->data->nb_rx_queues ||
> +			dev->data->rx_queues[queue_id] == NULL)

We already have internal eth_dev_validate_tx_queue(). Shouldn't
it be used here?

Also, some functions check that queues array is not NULL.
If the the is excessive after queue number check, it
should be consistent everywhere and corresponding check
of the array pointer vs NULL should be removed in a separate
cleanup patch. If the check is required in some corner cases
(I hope no), it should be here as well.

> +		return -EINVAL;
> +
> +	return 0;
> +}

[snip]

Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity

[huangdengdui]

On 2023/5/22 21:58, Andrew Rybchenko wrote:
> On 5/22/23 16:09, Dengdui Huang wrote:
>> The API rte_eth_dev_is_valid_rxq/txq checks
>> the port ID validity and then the Rx/Tx queue ID is valid.
> 
> What is valid Tx/Rx queue? It depends on on caller
> expectations. Some functions are satisfied with just
> check vs configured number of queues. Some require
> the queue to be setup. May be some should require
> the queue to be started.
> 
> So, I suggest to avoid term "valid" and be more precise
> here and API naming.
> 
Hi Andrew,
Thanks for your review, Accepted and fixed in v3.
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>   doc/guides/rel_notes/release_23_07.rst |  5 ++++
>>   lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
>>   lib/ethdev/version.map                 |  4 +++
>>   4 files changed, 75 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
>> index a9b1293689..19e645156f 100644
>> --- a/doc/guides/rel_notes/release_23_07.rst
>> +++ b/doc/guides/rel_notes/release_23_07.rst
>> @@ -56,6 +56,11 @@ New Features
>>        =======================================================
>>     +* **Added ethdev Rx/Tx queue id check API.**
>> +
>> +  Added ethdev Rx/Tx queue id check API which provides functions
> 
> id -> ID
> 
>> +  for check if Rx/Tx queue id is valid.
> 
> id -> ID
> 
>> +
> 
> It should be two empty lines here and just one above.
> 
Thanks, will fix in v3
>>   Removed Items
>>   -------------
>>   diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 4d03255683..3d85218127 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
>>       return is_valid;
>>   }
>>   +int
>> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
>> +{
>> +    struct rte_eth_dev *dev;
>> +
>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +    dev = &rte_eth_devices[port_id];
>> +
>> +    if (queue_id >= dev->data->nb_rx_queues ||
>> +            dev->data->rx_queues[queue_id] == NULL)
> 
> We already have internal eth_dev_validate_tx_queue(). Shouldn't
> it be used here?
> 
Thanks, will do in v3.
> Also, some functions check that queues array is not NULL.
> If the the is excessive after queue number check, it
> should be consistent everywhere and corresponding check
> of the array pointer vs NULL should be removed in a separate
> cleanup patch. If the check is required in some corner cases
> (I hope no), it should be here as well.
> 
I cannot understand what you mean by that.
>> +        return -EINVAL;
>> +
>> +    return 0;
>> +}
> 
> [snip]
> 

Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity

[Andrew Rybchenko]

On 5/24/23 10:38, huangdengdui wrote:
> On 2023/5/22 21:58, Andrew Rybchenko wrote:
>> On 5/22/23 16:09, Dengdui Huang wrote:
>>> The API rte_eth_dev_is_valid_rxq/txq checks
>>> the port ID validity and then the Rx/Tx queue ID is valid.
>>
>> What is valid Tx/Rx queue? It depends on on caller
>> expectations. Some functions are satisfied with just
>> check vs configured number of queues. Some require
>> the queue to be setup. May be some should require
>> the queue to be started.
>>
>> So, I suggest to avoid term "valid" and be more precise
>> here and API naming.
>>
> Hi Andrew,
> Thanks for your review, Accepted and fixed in v3.
>>>
>>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>>> ---
>>>    doc/guides/rel_notes/release_23_07.rst |  5 ++++
>>>    lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
>>>    lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
>>>    lib/ethdev/version.map                 |  4 +++
>>>    4 files changed, 75 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
>>> index a9b1293689..19e645156f 100644
>>> --- a/doc/guides/rel_notes/release_23_07.rst
>>> +++ b/doc/guides/rel_notes/release_23_07.rst
>>> @@ -56,6 +56,11 @@ New Features
>>>         =======================================================
>>>      +* **Added ethdev Rx/Tx queue id check API.**
>>> +
>>> +  Added ethdev Rx/Tx queue id check API which provides functions
>>
>> id -> ID
>>
>>> +  for check if Rx/Tx queue id is valid.
>>
>> id -> ID
>>
>>> +
>>
>> It should be two empty lines here and just one above.
>>
> Thanks, will fix in v3
>>>    Removed Items
>>>    -------------
>>>    diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index 4d03255683..3d85218127 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
>>>        return is_valid;
>>>    }
>>>    +int
>>> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
>>> +{
>>> +    struct rte_eth_dev *dev;
>>> +
>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>> +    dev = &rte_eth_devices[port_id];
>>> +
>>> +    if (queue_id >= dev->data->nb_rx_queues ||
>>> +            dev->data->rx_queues[queue_id] == NULL)
>>
>> We already have internal eth_dev_validate_tx_queue(). Shouldn't
>> it be used here?
>>
> Thanks, will do in v3.
>> Also, some functions check that queues array is not NULL.
>> If the the is excessive after queue number check, it
>> should be consistent everywhere and corresponding check
>> of the array pointer vs NULL should be removed in a separate
>> cleanup patch. If the check is required in some corner cases
>> (I hope no), it should be here as well.

dev->data->rx_queues != NULL ???

>>
> I cannot understand what you mean by that.
>>> +        return -EINVAL;
>>> +
>>> +    return 0;
>>> +}
>>
>> [snip]
>>

[PATCH v3 0/2] add Rx/Tx queue ID check API and use it to fix a bug

[Dengdui Huang]

This series add a commom API to check queue id
and use it to fix a bug.

v2->v3
update API name and use the internal function
eth_dev_validate_tx_queue() to check queue id

v1->v2
add a commom API to check queue id

Dengdui Huang (2):
  ethdev: add API to check if queue is available
  app/testpmd: fix segment fault with invalid queue ID

 app/test-pmd/cmdline.c                 | 23 +++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  6 +++++
 lib/ethdev/rte_ethdev.c                | 22 ++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  4 +++
 5 files changed, 84 insertions(+), 7 deletions(-)

-- 
2.33.0

[PATCH v3 1/2] ethdev: add API to check if queue is available

[Dengdui Huang]

The API rte_eth_dev_rxq/txq_avail which
is used to check if Rx/Tx queue is available.

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 doc/guides/rel_notes/release_23_07.rst |  6 +++++
 lib/ethdev/rte_ethdev.c                | 22 ++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  4 +++
 4 files changed, 68 insertions(+)

diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..399fe342db 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -56,6 +56,12 @@ New Features
      =======================================================
 
 
+* **Added ethdev Rx/Tx queue ID check API.**
+
+  Added ethdev Rx/Tx queue ID check API which provides functions
+  for check if Rx/Tx queue is available.
+
+
 Removed Items
 -------------
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 4d03255683..5534bcce23 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -771,6 +771,28 @@ eth_dev_validate_tx_queue(const struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	return 0;
 }
 
+int
+rte_eth_dev_rxq_avail(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	return eth_dev_validate_rx_queue(dev, queue_id);
+}
+
+int
+rte_eth_dev_txq_avail(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	return eth_dev_validate_tx_queue(dev, queue_id);
+}
+
 int
 rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
 {
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 99fe9e238b..13b71f75f3 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2665,6 +2665,42 @@ int rte_eth_dev_socket_id(uint16_t port_id);
  */
 int rte_eth_dev_is_valid_port(uint16_t port_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Rx queue is available.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the receive queue.
+ * @return
+ *   - -ENODEV: if *port_id* is invalid.
+ *   - -EINVAL: if queue is out of range or not been setup.
+ *   - 0 if Rx queue is available.
+ */
+__rte_experimental
+int rte_eth_dev_rxq_avail(uint16_t port_id, uint16_t queue_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Tx queue is available.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the transmit queue.
+ * @return
+ *   - -ENODEV: if *port_id* is invalid.
+ *   - -EINVAL: if queue is out of range or not been setup.
+ *   - 0 if Tx queue is available.
+ */
+__rte_experimental
+int rte_eth_dev_txq_avail(uint16_t port_id, uint16_t queue_id);
+
 /**
  * Start specified Rx queue of a port. It is used when rx_deferred_start
  * flag of the specified queue is true.
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 357d1a88c0..4b51f821cb 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -299,6 +299,10 @@ EXPERIMENTAL {
 	rte_flow_action_handle_query_update;
 	rte_flow_async_action_handle_query_update;
 	rte_flow_async_create_by_index;
+
+	# added in 23.07
+	rte_eth_dev_rxq_avail;
+	rte_eth_dev_txq_avail;
 };
 
 INTERNAL {
-- 
2.33.0

[PATCH v3 2/2] app/testpmd: fix segment fault with invalid queue ID

[Dengdui Huang]

When input queue ID is invalid, it will lead to
Segmentation fault, like:

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 txq/rxq 99 desc 0 status
Segmentation fault

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 rxq 99 desc used count
Segmentation fault

This patch fixes it.

Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7b20bef4e9..2863b5ebda 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -12275,12 +12275,13 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 	struct cmd_show_rx_tx_desc_status_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
-		return;
-	}
-
 	if (!strcmp(res->cmd_keyword, "rxq")) {
+		if (rte_eth_dev_rxq_avail(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_rx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12296,6 +12297,12 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 		else
 			printf("Desc status = UNAVAILABLE\n");
 	} else if (!strcmp(res->cmd_keyword, "txq")) {
+		if (rte_eth_dev_txq_avail(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_tx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12375,8 +12382,10 @@ cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
 	struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
+	if (rte_eth_dev_rxq_avail(res->cmd_pid, res->cmd_qid) != 0) {
+		fprintf(stderr,
+			"Invalid input: port id = %d, queue id = %d\n",
+			res->cmd_pid, res->cmd_qid);
 		return;
 	}
 
-- 
2.33.0

Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity

[Ferruh Yigit]

On 5/22/2023 2:58 PM, Andrew Rybchenko wrote:
> On 5/22/23 16:09, Dengdui Huang wrote:
>> The API rte_eth_dev_is_valid_rxq/txq checks
>> the port ID validity and then the Rx/Tx queue ID is valid.
> 
> What is valid Tx/Rx queue? It depends on on caller
> expectations. Some functions are satisfied with just
> check vs configured number of queues. Some require
> the queue to be setup. May be some should require
> the queue to be started.
> 
> So, I suggest to avoid term "valid" and be more precise
> here and API naming.
> 

I understand the concern 'valid' keyword, but we already have an API as
'rte_eth_dev_is_valid_port()', which does similar checks,

so 'rte_eth_dev_is_valid_rxq()' & 'rte_eth_dev_is_valid_txq()' looks
consistent with it.

v3 has API names, 'rte_eth_dev_rxq_avail()' & 'rte_eth_dev_txq_avail()',
I am not sure about these naming too, it feels like queues are valid but
it maybe in available and not available states.


@Andrew, do you have any suggestion on the API naming?
If not I am for going with rte_eth_dev_is_valid_rxq()' &
'rte_eth_dev_is_valid_txq()' mainly because of existing
'rte_eth_dev_is_valid_port()' API.

Perhaps we can elaborate what 'valid' means in API documentation to help
users.

>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>   doc/guides/rel_notes/release_23_07.rst |  5 ++++
>>   lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
>>   lib/ethdev/version.map                 |  4 +++
>>   4 files changed, 75 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_23_07.rst
>> b/doc/guides/rel_notes/release_23_07.rst
>> index a9b1293689..19e645156f 100644
>> --- a/doc/guides/rel_notes/release_23_07.rst
>> +++ b/doc/guides/rel_notes/release_23_07.rst
>> @@ -56,6 +56,11 @@ New Features
>>        =======================================================
>>     +* **Added ethdev Rx/Tx queue id check API.**
>> +
>> +  Added ethdev Rx/Tx queue id check API which provides functions
> 
> id -> ID
> 
>> +  for check if Rx/Tx queue id is valid.
> 
> id -> ID
> 
>> +
> 
> It should be two empty lines here and just one above.
> 
>>   Removed Items
>>   -------------
>>   diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 4d03255683..3d85218127 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
>>       return is_valid;
>>   }
>>   +int
>> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
>> +{
>> +    struct rte_eth_dev *dev;
>> +
>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +    dev = &rte_eth_devices[port_id];
>> +
>> +    if (queue_id >= dev->data->nb_rx_queues ||
>> +            dev->data->rx_queues[queue_id] == NULL)
> 
> We already have internal eth_dev_validate_tx_queue(). Shouldn't
> it be used here?
> 
> Also, some functions check that queues array is not NULL.
> If the the is excessive after queue number check, it
> should be consistent everywhere and corresponding check
> of the array pointer vs NULL should be removed in a separate
> cleanup patch. If the check is required in some corner cases
> (I hope no), it should be here as well.
> 
>> +        return -EINVAL;
>> +
>> +    return 0;
>> +}
> 
> [snip]
> 

Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity

[Ferruh Yigit]

On 5/31/2023 5:31 PM, Ferruh Yigit wrote:
> On 5/22/2023 2:58 PM, Andrew Rybchenko wrote:
>> On 5/22/23 16:09, Dengdui Huang wrote:
>>> The API rte_eth_dev_is_valid_rxq/txq checks
>>> the port ID validity and then the Rx/Tx queue ID is valid.
>>
>> What is valid Tx/Rx queue? It depends on on caller
>> expectations. Some functions are satisfied with just
>> check vs configured number of queues. Some require
>> the queue to be setup. May be some should require
>> the queue to be started.
>>
>> So, I suggest to avoid term "valid" and be more precise
>> here and API naming.
>>
> 
> I understand the concern 'valid' keyword, but we already have an API as
> 'rte_eth_dev_is_valid_port()', which does similar checks,
> 
> so 'rte_eth_dev_is_valid_rxq()' & 'rte_eth_dev_is_valid_txq()' looks
> consistent with it.
> 
> v3 has API names, 'rte_eth_dev_rxq_avail()' & 'rte_eth_dev_txq_avail()',
> I am not sure about these naming too, it feels like queues are valid but
> it maybe in available and not available states.
> 
> 
> @Andrew, do you have any suggestion on the API naming?
> If not I am for going with rte_eth_dev_is_valid_rxq()' &
> 'rte_eth_dev_is_valid_txq()' mainly because of existing
> 'rte_eth_dev_is_valid_port()' API.
> 
> Perhaps we can elaborate what 'valid' means in API documentation to help
> users.
> 

Hi Dengdui,

It looks like there is no better suggestion, lets not block this patch
more and continue with
'rte_eth_dev_is_valid_rxq()' & 'rte_eth_dev_is_valid_txq()' API names.

Can you please send a v4, with changes in v3 but API names as above, and
more description in the API documentation for what 'valid' means?


>>>
>>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>>> ---
>>>   doc/guides/rel_notes/release_23_07.rst |  5 ++++
>>>   lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
>>>   lib/ethdev/version.map                 |  4 +++
>>>   4 files changed, 75 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/release_23_07.rst
>>> b/doc/guides/rel_notes/release_23_07.rst
>>> index a9b1293689..19e645156f 100644
>>> --- a/doc/guides/rel_notes/release_23_07.rst
>>> +++ b/doc/guides/rel_notes/release_23_07.rst
>>> @@ -56,6 +56,11 @@ New Features
>>>        =======================================================
>>>     +* **Added ethdev Rx/Tx queue id check API.**
>>> +
>>> +  Added ethdev Rx/Tx queue id check API which provides functions
>>
>> id -> ID
>>
>>> +  for check if Rx/Tx queue id is valid.
>>
>> id -> ID
>>
>>> +
>>
>> It should be two empty lines here and just one above.
>>
>>>   Removed Items
>>>   -------------
>>>   diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index 4d03255683..3d85218127 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
>>>       return is_valid;
>>>   }
>>>   +int
>>> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
>>> +{
>>> +    struct rte_eth_dev *dev;
>>> +
>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>> +    dev = &rte_eth_devices[port_id];
>>> +
>>> +    if (queue_id >= dev->data->nb_rx_queues ||
>>> +            dev->data->rx_queues[queue_id] == NULL)
>>
>> We already have internal eth_dev_validate_tx_queue(). Shouldn't
>> it be used here?
>>
>> Also, some functions check that queues array is not NULL.
>> If the the is excessive after queue number check, it
>> should be consistent everywhere and corresponding check
>> of the array pointer vs NULL should be removed in a separate
>> cleanup patch. If the check is required in some corner cases
>> (I hope no), it should be here as well.
>>
>>> +        return -EINVAL;
>>> +
>>> +    return 0;
>>> +}
>>
>> [snip]
>>
> 

Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity

[huangdengdui]

On 2023/6/2 6:13, Ferruh Yigit wrote:
> On 5/31/2023 5:31 PM, Ferruh Yigit wrote:
>> On 5/22/2023 2:58 PM, Andrew Rybchenko wrote:
>>> On 5/22/23 16:09, Dengdui Huang wrote:
>>>> The API rte_eth_dev_is_valid_rxq/txq checks
>>>> the port ID validity and then the Rx/Tx queue ID is valid.
>>>
>>> What is valid Tx/Rx queue? It depends on on caller
>>> expectations. Some functions are satisfied with just
>>> check vs configured number of queues. Some require
>>> the queue to be setup. May be some should require
>>> the queue to be started.
>>>
>>> So, I suggest to avoid term "valid" and be more precise
>>> here and API naming.
>>>
>>
>> I understand the concern 'valid' keyword, but we already have an API as
>> 'rte_eth_dev_is_valid_port()', which does similar checks,
>>
>> so 'rte_eth_dev_is_valid_rxq()' & 'rte_eth_dev_is_valid_txq()' looks
>> consistent with it.
>>
>> v3 has API names, 'rte_eth_dev_rxq_avail()' & 'rte_eth_dev_txq_avail()',
>> I am not sure about these naming too, it feels like queues are valid but
>> it maybe in available and not available states.
>>
>>
>> @Andrew, do you have any suggestion on the API naming?
>> If not I am for going with rte_eth_dev_is_valid_rxq()' &
>> 'rte_eth_dev_is_valid_txq()' mainly because of existing
>> 'rte_eth_dev_is_valid_port()' API.
>>
>> Perhaps we can elaborate what 'valid' means in API documentation to help
>> users.
>>
> 
> Hi Dengdui,
> 
> It looks like there is no better suggestion, lets not block this patch
> more and continue with
> 'rte_eth_dev_is_valid_rxq()' & 'rte_eth_dev_is_valid_txq()' API names.
> 
> Can you please send a v4, with changes in v3 but API names as above, and
> more description in the API documentation for what 'valid' means?
> Hi Ferruh,
Thanks for your review, I will do in v4.
> 
>>>>
>>>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>>>> ---
>>>>   doc/guides/rel_notes/release_23_07.rst |  5 ++++
>>>>   lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++
>>>>   lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++
>>>>   lib/ethdev/version.map                 |  4 +++
>>>>   4 files changed, 75 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/release_23_07.rst
>>>> b/doc/guides/rel_notes/release_23_07.rst
>>>> index a9b1293689..19e645156f 100644
>>>> --- a/doc/guides/rel_notes/release_23_07.rst
>>>> +++ b/doc/guides/rel_notes/release_23_07.rst
>>>> @@ -56,6 +56,11 @@ New Features
>>>>        =======================================================
>>>>     +* **Added ethdev Rx/Tx queue id check API.**
>>>> +
>>>> +  Added ethdev Rx/Tx queue id check API which provides functions
>>>
>>> id -> ID
>>>
>>>> +  for check if Rx/Tx queue id is valid.
>>>
>>> id -> ID
>>>
>>>> +
>>>
>>> It should be two empty lines here and just one above.
>>>
>>>>   Removed Items
>>>>   -------------
>>>>   diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>>> index 4d03255683..3d85218127 100644
>>>> --- a/lib/ethdev/rte_ethdev.c
>>>> +++ b/lib/ethdev/rte_ethdev.c
>>>> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
>>>>       return is_valid;
>>>>   }
>>>>   +int
>>>> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
>>>> +{
>>>> +    struct rte_eth_dev *dev;
>>>> +
>>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>>> +    dev = &rte_eth_devices[port_id];
>>>> +
>>>> +    if (queue_id >= dev->data->nb_rx_queues ||
>>>> +            dev->data->rx_queues[queue_id] == NULL)
>>>
>>> We already have internal eth_dev_validate_tx_queue(). Shouldn't
>>> it be used here?
>>>
>>> Also, some functions check that queues array is not NULL.
>>> If the the is excessive after queue number check, it
>>> should be consistent everywhere and corresponding check
>>> of the array pointer vs NULL should be removed in a separate
>>> cleanup patch. If the check is required in some corner cases
>>> (I hope no), it should be here as well.
>>>
>>>> +        return -EINVAL;
>>>> +
>>>> +    return 0;
>>>> +}
>>>
>>> [snip]
>>>
>>
> 

[PATCH v4 0/2] add Rx/Tx queue ID check API and use it to fix a bug

[Dengdui Huang]

This series add a commom API to check queue id
and use it to fix a bug.

v3->v4
update API name and uptate description in the API documentation

v2->v3
update API name and use the internal function
eth_dev_validate_tx_queue() to check queue id

v1->v2
add a commom API to check queue id

Dengdui Huang (2):
  ethdev: add API to check if queue is valid
  app/testpmd: fix segment fault with invalid queue ID

 app/test-pmd/cmdline.c                 | 23 +++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  6 ++++
 lib/ethdev/rte_ethdev.c                | 22 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 38 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 5 files changed, 84 insertions(+), 7 deletions(-)

-- 
2.33.0

[PATCH v4 1/2] ethdev: add API to check if queue is valid

[Dengdui Huang]

The API rte_eth_dev_is_valid_rxq/txq which
is used to check if Rx/Tx queue is valid.
If the queue has been setup, it is considered valid.

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 doc/guides/rel_notes/release_23_07.rst |  6 ++++
 lib/ethdev/rte_ethdev.c                | 22 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 38 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 4 files changed, 68 insertions(+)

diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 0d3cada5d0..1332fa3a5a 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -83,6 +83,12 @@ New Features
   for new capability registers, large passthrough BAR and some
   performance enhancements for UPT.
 
+* **Added ethdev Rx/Tx queue ID check API.**
+
+  Added ethdev Rx/Tx queue ID check API which provides functions
+  for check if Rx/Tx queue is valid. If the queue has been setup,
+  it is considered valid.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index d46e74504e..a134928c8a 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -771,6 +771,28 @@ eth_dev_validate_tx_queue(const struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	return 0;
 }
 
+int
+rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	return eth_dev_validate_rx_queue(dev, queue_id);
+}
+
+int
+rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	return eth_dev_validate_tx_queue(dev, queue_id);
+}
+
 int
 rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
 {
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 9932413c33..4ef803c244 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2666,6 +2666,44 @@ int rte_eth_dev_socket_id(uint16_t port_id);
  */
 int rte_eth_dev_is_valid_port(uint16_t port_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Rx queue is valid. If the queue has been setup,
+ * it is considered valid.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the receive queue.
+ * @return
+ *   - -ENODEV: if *port_id* is valid.
+ *   - -EINVAL: if queue is out of range or not been setup.
+ *   - 0 if Rx queue is valid.
+ */
+__rte_experimental
+int rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Tx queue is valid. If the queue has been setup,
+ * it is considered valid.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the transmit queue.
+ * @return
+ *   - -ENODEV: if *port_id* is valid.
+ *   - -EINVAL: if queue is out of range or not been setup.
+ *   - 0 if Tx queue is valid.
+ */
+__rte_experimental
+int rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id);
+
 /**
  * Start specified Rx queue of a port. It is used when rx_deferred_start
  * flag of the specified queue is true.
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 9d418091ef..3aa6bce156 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -309,6 +309,8 @@ EXPERIMENTAL {
 	rte_flow_async_action_list_handle_destroy;
 	rte_flow_async_action_list_handle_query_update;
 	rte_flow_async_actions_update;
+	rte_eth_dev_is_valid_rxq;
+	rte_eth_dev_is_valid_txq;
 };
 
 INTERNAL {
-- 
2.33.0

[PATCH v4 2/2] app/testpmd: fix segment fault with invalid queue ID

[Dengdui Huang]

When input queue ID is invalid, it will lead to
Segmentation fault, like:

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 txq/rxq 99 desc 0 status
Segmentation fault

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 rxq 99 desc used count
Segmentation fault

This patch fixes it.

Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5333ba72c3..a15a442a06 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -12282,12 +12282,13 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 	struct cmd_show_rx_tx_desc_status_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
-		return;
-	}
-
 	if (!strcmp(res->cmd_keyword, "rxq")) {
+		if (rte_eth_dev_is_valid_rxq(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_rx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12303,6 +12304,12 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 		else
 			printf("Desc status = UNAVAILABLE\n");
 	} else if (!strcmp(res->cmd_keyword, "txq")) {
+		if (rte_eth_dev_is_valid_txq(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_tx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12382,8 +12389,10 @@ cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
 	struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
+	if (rte_eth_dev_is_valid_rxq(res->cmd_pid, res->cmd_qid) != 0) {
+		fprintf(stderr,
+			"Invalid input: port id = %d, queue id = %d\n",
+			res->cmd_pid, res->cmd_qid);
 		return;
 	}
 
-- 
2.33.0

Re: [PATCH v4 1/2] ethdev: add API to check if queue is valid

[Ferruh Yigit]

On 6/2/2023 8:52 AM, Dengdui Huang wrote:
> The API rte_eth_dev_is_valid_rxq/txq which
> is used to check if Rx/Tx queue is valid.
> If the queue has been setup, it is considered valid.
> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
>  doc/guides/rel_notes/release_23_07.rst |  6 ++++
>  lib/ethdev/rte_ethdev.c                | 22 +++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 38 ++++++++++++++++++++++++++
>  lib/ethdev/version.map                 |  2 ++
>  4 files changed, 68 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
> index 0d3cada5d0..1332fa3a5a 100644
> --- a/doc/guides/rel_notes/release_23_07.rst
> +++ b/doc/guides/rel_notes/release_23_07.rst
> @@ -83,6 +83,12 @@ New Features
>    for new capability registers, large passthrough BAR and some
>    performance enhancements for UPT.
>  
> +* **Added ethdev Rx/Tx queue ID check API.**
> +
> +  Added ethdev Rx/Tx queue ID check API which provides functions
> +  for check if Rx/Tx queue is valid. If the queue has been setup,
> +  it is considered valid.
> +
>  

Can you please move the release note update to up, before rte_flow
updates, the expected order is documented in section comment.

>  Removed Items
>  -------------
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index d46e74504e..a134928c8a 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -771,6 +771,28 @@ eth_dev_validate_tx_queue(const struct rte_eth_dev *dev, uint16_t tx_queue_id)
>  	return 0;
>  }
>  
> +int
> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	return eth_dev_validate_rx_queue(dev, queue_id);
> +}
> +
> +int
> +rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	return eth_dev_validate_tx_queue(dev, queue_id);
> +}
> +
>  int
>  rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
>  {
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 9932413c33..4ef803c244 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -2666,6 +2666,44 @@ int rte_eth_dev_socket_id(uint16_t port_id);
>   */
>  int rte_eth_dev_is_valid_port(uint16_t port_id);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Check if Rx queue is valid. If the queue has been setup,
> + * it is considered valid.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param queue_id
> + *  The index of the receive queue.
> + * @return
> + *   - -ENODEV: if *port_id* is valid.

s/valid/invalid ?

> + *   - -EINVAL: if queue is out of range or not been setup.

"if queue_id is out of range or queue is not been setup" ?

> + *   - 0 if Rx queue is valid.
> + */
> +__rte_experimental
> +int rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Check if Tx queue is valid. If the queue has been setup,
> + * it is considered valid.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param queue_id
> + *  The index of the transmit queue.
> + * @return
> + *   - -ENODEV: if *port_id* is valid.

s/valid/invalid ?

> + *   - -EINVAL: if queue is out of range or not been setup.

"if queue_id is out of range or queue is not been setup" ?

> + *   - 0 if Tx queue is valid.
> + */
> +__rte_experimental
> +int rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id);
> +
>  /**
>   * Start specified Rx queue of a port. It is used when rx_deferred_start
>   * flag of the specified queue is true.
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 9d418091ef..3aa6bce156 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -309,6 +309,8 @@ EXPERIMENTAL {
>  	rte_flow_async_action_list_handle_destroy;
>  	rte_flow_async_action_list_handle_query_update;
>  	rte_flow_async_actions_update;
> +	rte_eth_dev_is_valid_rxq;
> +	rte_eth_dev_is_valid_txq;

Can you please sort '23.07' block alphabetically?

>  };
>  
>  INTERNAL {

Re: [PATCH v4 2/2] app/testpmd: fix segment fault with invalid queue ID

[Ferruh Yigit]

On 6/2/2023 8:52 AM, Dengdui Huang wrote:
> When input queue ID is invalid, it will lead to
> Segmentation fault, like:
> 
> dpdk-testpmd -a 0000:01:00.0 -- -i
> testpmd> show port 0 txq/rxq 99 desc 0 status
> Segmentation fault
> 
> dpdk-testpmd -a 0000:01:00.0 -- -i
> testpmd> show port 0 rxq 99 desc used count
> Segmentation fault
> 
> This patch fixes it.
> 
> Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
> Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [PATCH v4 1/2] ethdev: add API to check if queue is valid

[huangdengdui]

On 2023/6/2 20:47, Ferruh Yigit wrote:
> On 6/2/2023 8:52 AM, Dengdui Huang wrote:
>> The API rte_eth_dev_is_valid_rxq/txq which
>> is used to check if Rx/Tx queue is valid.
>> If the queue has been setup, it is considered valid.
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>  doc/guides/rel_notes/release_23_07.rst |  6 ++++
>>  lib/ethdev/rte_ethdev.c                | 22 +++++++++++++++
>>  lib/ethdev/rte_ethdev.h                | 38 ++++++++++++++++++++++++++
>>  lib/ethdev/version.map                 |  2 ++
>>  4 files changed, 68 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
>> index 0d3cada5d0..1332fa3a5a 100644
>> --- a/doc/guides/rel_notes/release_23_07.rst
>> +++ b/doc/guides/rel_notes/release_23_07.rst
>> @@ -83,6 +83,12 @@ New Features
>>    for new capability registers, large passthrough BAR and some
>>    performance enhancements for UPT.
>>  
>> +* **Added ethdev Rx/Tx queue ID check API.**
>> +
>> +  Added ethdev Rx/Tx queue ID check API which provides functions
>> +  for check if Rx/Tx queue is valid. If the queue has been setup,
>> +  it is considered valid.
>> +
>>  
> 
> Can you please move the release note update to up, before rte_flow
> updates, the expected order is documented in section comment.
> 
>>  Removed Items
>>  -------------
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index d46e74504e..a134928c8a 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -771,6 +771,28 @@ eth_dev_validate_tx_queue(const struct rte_eth_dev *dev, uint16_t tx_queue_id)
>>  	return 0;
>>  }
>>  
>> +int
>> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
>> +{
>> +	struct rte_eth_dev *dev;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +
>> +	return eth_dev_validate_rx_queue(dev, queue_id);
>> +}
>> +
>> +int
>> +rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id)
>> +{
>> +	struct rte_eth_dev *dev;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +
>> +	return eth_dev_validate_tx_queue(dev, queue_id);
>> +}
>> +
>>  int
>>  rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
>>  {
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 9932413c33..4ef803c244 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -2666,6 +2666,44 @@ int rte_eth_dev_socket_id(uint16_t port_id);
>>   */
>>  int rte_eth_dev_is_valid_port(uint16_t port_id);
>>  
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Check if Rx queue is valid. If the queue has been setup,
>> + * it is considered valid.
>> + *
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @param queue_id
>> + *  The index of the receive queue.
>> + * @return
>> + *   - -ENODEV: if *port_id* is valid.
> 
> s/valid/invalid ?
> 
>> + *   - -EINVAL: if queue is out of range or not been setup.
> 
> "if queue_id is out of range or queue is not been setup" ?
> 
>> + *   - 0 if Rx queue is valid.
>> + */
>> +__rte_experimental
>> +int rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Check if Tx queue is valid. If the queue has been setup,
>> + * it is considered valid.
>> + *
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @param queue_id
>> + *  The index of the transmit queue.
>> + * @return
>> + *   - -ENODEV: if *port_id* is valid.
> 
> s/valid/invalid ?
> 
>> + *   - -EINVAL: if queue is out of range or not been setup.
> 
> "if queue_id is out of range or queue is not been setup" ?
> 
>> + *   - 0 if Tx queue is valid.
>> + */
>> +__rte_experimental
>> +int rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id);
>> +
>>  /**
>>   * Start specified Rx queue of a port. It is used when rx_deferred_start
>>   * flag of the specified queue is true.
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 9d418091ef..3aa6bce156 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -309,6 +309,8 @@ EXPERIMENTAL {
>>  	rte_flow_async_action_list_handle_destroy;
>>  	rte_flow_async_action_list_handle_query_update;
>>  	rte_flow_async_actions_update;
>> +	rte_eth_dev_is_valid_rxq;
>> +	rte_eth_dev_is_valid_txq;
> 
> Can you please sort '23.07' block alphabetically?
> 
>>  };
>>  
>>  INTERNAL {
> 
Hi Ferruh,
Thanks for your review, I will do in v5.

[PATCH v5 0/2] add Rx/Tx queue ID check API and use it to fix a bug

[Dengdui Huang]

This series add a commom API to check queue id
and use it to fix a bug.

v4->v5
update document order and correcting typos

v3->v4
update API name and uptate description in the API documentation

v2->v3
update API name and use the internal function
eth_dev_validate_tx_queue() to check queue id

v1->v2
add a commom API to check queue id

Dengdui Huang (2):
  ethdev: add API to check if queue is valid
  app/testpmd: fix segment fault with invalid queue ID

 app/test-pmd/cmdline.c                 | 23 +++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  6 ++++
 lib/ethdev/rte_ethdev.c                | 22 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 38 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 5 files changed, 84 insertions(+), 7 deletions(-)

-- 
2.33.0

[PATCH v5 1/2] ethdev: add API to check if queue is valid

[Dengdui Huang]

The API rte_eth_dev_is_valid_rxq/txq which
is used to check if Rx/Tx queue is valid.
If the queue has been setup, it is considered valid.

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 doc/guides/rel_notes/release_23_07.rst |  6 ++++
 lib/ethdev/rte_ethdev.c                | 22 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 38 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 4 files changed, 68 insertions(+)

diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 0b5561a6c6..e351be59e4 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -65,6 +65,12 @@ New Features
 
   Added RTE_ETH_FEC_LLRS to rte_eth_fec_mode.
 
+* **Added ethdev Rx/Tx queue ID check API.**
+
+  Added ethdev Rx/Tx queue ID check API which provides functions
+  for check if Rx/Tx queue is valid. If the queue has been setup,
+  it is considered valid.
+
 * **Added flow matching of tx queue.**
 
   Added ``RTE_FLOW_ITEM_TYPE_TX_QUEUE`` rte_flow pattern to match tx queue of
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index d46e74504e..a134928c8a 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -771,6 +771,28 @@ eth_dev_validate_tx_queue(const struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	return 0;
 }
 
+int
+rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	return eth_dev_validate_rx_queue(dev, queue_id);
+}
+
+int
+rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	return eth_dev_validate_tx_queue(dev, queue_id);
+}
+
 int
 rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id)
 {
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 9932413c33..a37c0bdf76 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2666,6 +2666,44 @@ int rte_eth_dev_socket_id(uint16_t port_id);
  */
 int rte_eth_dev_is_valid_port(uint16_t port_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Rx queue is valid. If the queue has been setup,
+ * it is considered valid.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the receive queue.
+ * @return
+ *   - -ENODEV: if *port_id* is invalid.
+ *   - -EINVAL: if queue_id is out of range or queue is not been setup.
+ *   - 0 if Rx queue is valid.
+ */
+__rte_experimental
+int rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Check if Tx queue is valid. If the queue has been setup,
+ * it is considered valid.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *  The index of the transmit queue.
+ * @return
+ *   - -ENODEV: if *port_id* is invalid.
+ *   - -EINVAL: if queue_id is out of range or queue is not been setup.
+ *   - 0 if Tx queue is valid.
+ */
+__rte_experimental
+int rte_eth_dev_is_valid_txq(uint16_t port_id, uint16_t queue_id);
+
 /**
  * Start specified Rx queue of a port. It is used when rx_deferred_start
  * flag of the specified queue is true.
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 9d418091ef..1a33d72668 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -301,6 +301,8 @@ EXPERIMENTAL {
 	rte_flow_async_create_by_index;
 
 	# added in 23.07
+	rte_eth_dev_is_valid_rxq;
+	rte_eth_dev_is_valid_txq;
 	rte_flow_action_list_handle_create;
 	rte_flow_action_list_handle_destroy;
 	rte_flow_action_list_handle_query_update;
-- 
2.33.0

[PATCH v5 2/2] app/testpmd: fix segment fault with invalid queue ID

[Dengdui Huang]

When input queue ID is invalid, it will lead to
Segmentation fault, like:

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 txq/rxq 99 desc 0 status
Segmentation fault

dpdk-testpmd -a 0000:01:00.0 -- -i
testpmd> show port 0 rxq 99 desc used count
Segmentation fault

This patch fixes it.

Fixes: fae9aa717d6c ("app/testpmd: support checking descriptor status")
Fixes: 3f9acb5c83bb ("ethdev: avoid non-dataplane checks in Rx queue count")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5333ba72c3..a15a442a06 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -12282,12 +12282,13 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 	struct cmd_show_rx_tx_desc_status_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
-		return;
-	}
-
 	if (!strcmp(res->cmd_keyword, "rxq")) {
+		if (rte_eth_dev_is_valid_rxq(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_rx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12303,6 +12304,12 @@ cmd_show_rx_tx_desc_status_parsed(void *parsed_result,
 		else
 			printf("Desc status = UNAVAILABLE\n");
 	} else if (!strcmp(res->cmd_keyword, "txq")) {
+		if (rte_eth_dev_is_valid_txq(res->cmd_pid, res->cmd_qid) != 0) {
+			fprintf(stderr,
+				"Invalid input: port id = %d, queue id = %d\n",
+				res->cmd_pid, res->cmd_qid);
+			return;
+		}
 		rc = rte_eth_tx_descriptor_status(res->cmd_pid, res->cmd_qid,
 					     res->cmd_did);
 		if (rc < 0) {
@@ -12382,8 +12389,10 @@ cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
 	struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
 	int rc;
 
-	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
-		fprintf(stderr, "invalid port id %u\n", res->cmd_pid);
+	if (rte_eth_dev_is_valid_rxq(res->cmd_pid, res->cmd_qid) != 0) {
+		fprintf(stderr,
+			"Invalid input: port id = %d, queue id = %d\n",
+			res->cmd_pid, res->cmd_qid);
 		return;
 	}
 
-- 
2.33.0

Re: [PATCH v5 1/2] ethdev: add API to check if queue is valid

[Ferruh Yigit]

On 6/5/2023 3:27 AM, Dengdui Huang wrote:
> The API rte_eth_dev_is_valid_rxq/txq which
> is used to check if Rx/Tx queue is valid.
> If the queue has been setup, it is considered valid.
> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [PATCH v5 0/2] add Rx/Tx queue ID check API and use it to fix a bug

[Ferruh Yigit]

On 6/5/2023 3:27 AM, Dengdui Huang wrote:
> This series add a commom API to check queue id
> and use it to fix a bug.
> 
> v4->v5
> update document order and correcting typos
> 
> v3->v4
> update API name and uptate description in the API documentation
> 
> v2->v3
> update API name and use the internal function
> eth_dev_validate_tx_queue() to check queue id
> 
> v1->v2
> add a commom API to check queue id
> 
> Dengdui Huang (2):
>   ethdev: add API to check if queue is valid
>   app/testpmd: fix segment fault with invalid queue ID
>

Series applied to dpdk-next-net/main, thanks.