[PATCH] ethdev: rte_eth_rx_queue_count is a dataplane function

[PATCH] ethdev: rte_eth_rx_queue_count is a dataplane function

[Morten Brørup]

Applications may use rte_eth_rx_queue_count() in the RX stage of the
dataplane, so only check the function parameters if built with
RTE_ETHDEV_DEBUG_RX.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ethdev/rte_ethdev.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index de9e970d4d..8d5d9b42bf 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5681,6 +5681,10 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
 /**
  * Get the number of used descriptors of a Rx queue
  *
+ * Since it's a dataplane function, no check is performed on port_id and
+ * queue_id. The caller must therefore ensure that the port is enabled
+ * and the queue is configured and running.
+ *
  * @param port_id
  *  The port identifier of the Ethernet device.
  * @param queue_id
@@ -5688,8 +5692,8 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
  * @return
  *  The number of used descriptors in the specific queue, or:
  *   - (-ENODEV) if *port_id* is invalid.
- *     (-EINVAL) if *queue_id* is invalid
- *     (-ENOTSUP) if the device does not support this function
+ *   - (-EINVAL) if *queue_id* is invalid
+ *   - (-ENOTSUP) if the device does not support this function
  */
 static inline int
 rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
@@ -5697,6 +5701,7 @@ rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
 	struct rte_eth_fp_ops *p;
 	void *qd;
 
+#ifdef RTE_ETHDEV_DEBUG_RX
 	if (port_id >= RTE_MAX_ETHPORTS ||
 			queue_id >= RTE_MAX_QUEUES_PER_PORT) {
 		RTE_ETHDEV_LOG(ERR,
@@ -5704,16 +5709,19 @@ rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
 			port_id, queue_id);
 		return -EINVAL;
 	}
+#endif
 
 	/* fetch pointer to queue data */
 	p = &rte_eth_fp_ops[port_id];
 	qd = p->rxq.data[queue_id];
 
+#ifdef RTE_ETHDEV_DEBUG_RX
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-	RTE_FUNC_PTR_OR_ERR_RET(*p->rx_queue_count, -ENOTSUP);
 	if (qd == NULL)
 		return -EINVAL;
+#endif
 
+	RTE_FUNC_PTR_OR_ERR_RET(*p->rx_queue_count, -ENOTSUP);
 	return (int)(*p->rx_queue_count)(qd);
 }
 
-- 
2.17.1

Re: [PATCH] ethdev: rte_eth_rx_queue_count is a dataplane function

[Ferruh Yigit]

On 8/18/2022 10:37 AM, Morten Brørup wrote:
> Applications may use rte_eth_rx_queue_count() in the RX stage of the
> dataplane, so only check the function parameters if built with
> RTE_ETHDEV_DEBUG_RX.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>

I don't remember if those checks done by default intentionally, but I 
don't think so,

And since other relevant functions do the same thing:

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

Re: [PATCH] ethdev: rte_eth_rx_queue_count is a dataplane function

[Andrew Rybchenko]

On 8/18/22 15:23, Ferruh Yigit wrote:
> On 8/18/2022 10:37 AM, Morten Brørup wrote:
>> Applications may use rte_eth_rx_queue_count() in the RX stage of the
>> dataplane, so only check the function parameters if built with
>> RTE_ETHDEV_DEBUG_RX.
>>
>> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> 
> I don't remember if those checks done by default intentionally, but I 
> don't think so,
> 
> And since other relevant functions do the same thing:
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> 

The patch itself looks good to me.
Summary should be fixed to be not a statement and should not
mention function name. Other than that:

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

RE: [PATCH] ethdev: rte_eth_rx_queue_count is a dataplane function

[Morten Brørup]

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Monday, 12 September 2022 16.47
> 
> On 8/18/22 15:23, Ferruh Yigit wrote:
> > On 8/18/2022 10:37 AM, Morten Brørup wrote:
> >> Applications may use rte_eth_rx_queue_count() in the RX stage of the
> >> dataplane, so only check the function parameters if built with
> >> RTE_ETHDEV_DEBUG_RX.
> >>
> >> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > I don't remember if those checks done by default intentionally, but I
> > don't think so,
> >
> > And since other relevant functions do the same thing:
> >
> > Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> >
> 
> The patch itself looks good to me.
> Summary should be fixed to be not a statement and should not
> mention function name.

Please feel free to fix when merging. :-)

> Other than that:
> 
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Thank you.

-Morten

Re: [PATCH] ethdev: rte_eth_rx_queue_count is a dataplane function

[Andrew Rybchenko]

On 9/22/22 16:46, Morten Brørup wrote:
>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>> Sent: Monday, 12 September 2022 16.47
>>
>> On 8/18/22 15:23, Ferruh Yigit wrote:
>>> On 8/18/2022 10:37 AM, Morten Brørup wrote:
>>>> Applications may use rte_eth_rx_queue_count() in the RX stage of the
>>>> dataplane, so only check the function parameters if built with
>>>> RTE_ETHDEV_DEBUG_RX.
>>>>
>>>> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
>>>
>>> I don't remember if those checks done by default intentionally, but I
>>> don't think so,
>>>
>>> And since other relevant functions do the same thing:
>>>
>>> Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>>
>>
>> The patch itself looks good to me.
>> Summary should be fixed to be not a statement and should not
>> mention function name.
> 
> Please feel free to fix when merging. :-)
> 
>> Other than that:
>>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> Thank you.
> 
> -Morten
> 

Applied to dpdk-next-net/main, thanks.