ethdev: rte_eth_rx_queue_count is a dataplane function
Checks
Commit Message
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(-)
Comments
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>
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>
> 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
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.
@@ -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);
}