[PATCH] ethdev: remove callback checks from fast path

[PATCH] ethdev: remove callback checks from fast path

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

rte_eth_fp_ops contains ops for fast path APIs. Each API
validates availability of callback and then invoke it.

Removing these NULL checks instead using dummy callbacks.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 lib/ethdev/ethdev_driver.c | 47 ++++++++++++++++++++++
 lib/ethdev/ethdev_driver.h | 82 ++++++++++++++++++++++++++++++++++++++
 lib/ethdev/ethdev_pci.h    | 19 +++++++++
 lib/ethdev/rte_ethdev.h    | 20 +---------
 4 files changed, 150 insertions(+), 18 deletions(-)

diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index ec0c1e1176..75073f98cf 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -847,6 +847,53 @@ rte_eth_pkt_burst_dummy(void *queue __rte_unused,
 	return 0;
 }
 
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_tx_pkt_prepare_dummy)
+uint16_t
+rte_eth_tx_pkt_prepare_dummy(void *queue __rte_unused,
+		struct rte_mbuf **pkts __rte_unused,
+		uint16_t nb_pkts)
+{
+	return nb_pkts;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_rx_queue_count_dummy)
+uint32_t
+rte_eth_rx_queue_count_dummy(void *queue __rte_unused)
+{
+	return -ENOTSUP;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_tx_queue_count_dummy)
+int
+rte_eth_tx_queue_count_dummy(void *queue __rte_unused)
+{
+	return -ENOTSUP;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_descriptor_status_dummy)
+int
+rte_eth_descriptor_status_dummy(void *queue __rte_unused,
+		uint16_t offset __rte_unused)
+{
+	return -ENOTSUP;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_recycle_tx_mbufs_reuse_dummy)
+uint16_t
+rte_eth_recycle_tx_mbufs_reuse_dummy(void *queue __rte_unused,
+		struct rte_eth_recycle_rxq_info *recycle_rxq_info __rte_unused)
+{
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_recycle_rx_descriptors_refill_dummy)
+void
+rte_eth_recycle_rx_descriptors_refill_dummy(void *queue __rte_unused,
+		uint16_t nb __rte_unused)
+{
+
+}
+
 RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_representor_id_get)
 int
 rte_eth_representor_id_get(uint16_t port_id,
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 2b4d2ae9c3..ec00f16ed3 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1874,6 +1874,88 @@ rte_eth_pkt_burst_dummy(void *queue __rte_unused,
 		struct rte_mbuf **pkts __rte_unused,
 		uint16_t nb_pkts __rte_unused);
 
+/**
+ * @internal
+ * Dummy DPDK callback for Tx packet prepare.
+ *
+ * @param queue
+ *  Pointer to Tx queue
+ * @param pkts
+ *  Packet array
+ * @param nb_pkts
+ *  Number of packets in packet array
+ */
+__rte_internal
+uint16_t
+rte_eth_tx_pkt_prepare_dummy(void *queue __rte_unused,
+		struct rte_mbuf **pkts __rte_unused,
+		uint16_t nb_pkts __rte_unused);
+
+/**
+ * @internal
+ * Dummy DPDK callback for Rx queue count.
+ *
+ * @param queue
+ *  Pointer to Rx queue
+ */
+__rte_internal
+uint32_t
+rte_eth_rx_queue_count_dummy(void *queue __rte_unused);
+
+/**
+ * @internal
+ * Dummy DPDK callback for Tx queue count.
+ *
+ * @param queue
+ *  Pointer to Tx queue
+ */
+__rte_internal
+int
+rte_eth_tx_queue_count_dummy(void *queue __rte_unused);
+
+/**
+ * @internal
+ * Dummy DPDK callback for descriptor status.
+ *
+ * @param queue
+ *  Pointer to Rx/Tx queue
+ * @param offset
+ *  The offset of the descriptor starting from tail (0 is the next
+ *  packet to be received by the driver).
+ */
+__rte_internal
+int
+rte_eth_descriptor_status_dummy(void *queue __rte_unused,
+		uint16_t offset __rte_unused);
+
+/**
+ * @internal
+ * Dummy DPDK callback for recycle Tx mbufs reuse.
+ *
+ * @param queue
+ *  Pointer to Tx queue
+ * @param recycle_rxq_info
+ *  Pointer to recycle Rx queue info
+ */
+__rte_internal
+uint16_t
+rte_eth_recycle_tx_mbufs_reuse_dummy(void *queue __rte_unused,
+		struct rte_eth_recycle_rxq_info *recycle_rxq_info __rte_unused);
+
+/**
+ * @internal
+ * Dummy DPDK callback Rx descriptor refill.
+ *
+ * @param queue
+ *  Pointer Rx queue
+ * @param offset
+ *  number of descriptors to refill
+ */
+__rte_internal
+void
+rte_eth_recycle_rx_descriptors_refill_dummy(void *queue __rte_unused,
+		uint16_t nb __rte_unused);
+
 /**
  * Allocate an unique switch domain identifier.
  *
diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
index 2229ffa252..1bd49ab822 100644
--- a/lib/ethdev/ethdev_pci.h
+++ b/lib/ethdev/ethdev_pci.h
@@ -16,6 +16,20 @@
 extern "C" {
 #endif
 
+static inline void
+rte_eth_set_dummy_fops(struct rte_eth_dev *eth_dev)
+{
+	eth_dev->rx_pkt_burst = rte_eth_pkt_burst_dummy;
+	eth_dev->tx_pkt_burst = rte_eth_pkt_burst_dummy;
+	eth_dev->tx_pkt_prepare = rte_eth_tx_pkt_prepare_dummy;
+	eth_dev->rx_queue_count = rte_eth_rx_queue_count_dummy;
+	eth_dev->tx_queue_count = rte_eth_tx_queue_count_dummy;
+	eth_dev->rx_descriptor_status = rte_eth_descriptor_status_dummy;
+	eth_dev->tx_descriptor_status = rte_eth_descriptor_status_dummy;
+	eth_dev->recycle_tx_mbufs_reuse = rte_eth_recycle_tx_mbufs_reuse_dummy;
+	eth_dev->recycle_rx_descriptors_refill = rte_eth_recycle_rx_descriptors_refill_dummy;
+}
+
 /**
  * Copy pci device info to the Ethernet device data.
  * Shared memory (eth_dev->data) only updated by primary process, so it is safe
@@ -147,6 +161,11 @@ rte_eth_dev_pci_generic_probe(struct rte_pci_device *pci_dev,
 	if (!eth_dev)
 		return -ENOMEM;
 
+	/* Update fast path ops with dummy callbacks. Driver will update
+	 * them with required callbacks in the init function.
+	 */
+	rte_eth_set_dummy_fops(eth_dev);
+
 	ret = dev_init(eth_dev);
 	if (ret)
 		rte_eth_dev_release_port(eth_dev);
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index ea7f8c4a1a..aa67b69134 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -6399,8 +6399,6 @@ rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
 		return -EINVAL;
 #endif
 
-	if (p->rx_queue_count == NULL)
-		return -ENOTSUP;
 	return (int)p->rx_queue_count(qd);
 }
 
@@ -6471,8 +6469,6 @@ rte_eth_rx_descriptor_status(uint16_t port_id, uint16_t queue_id,
 	if (qd == NULL)
 		return -ENODEV;
 #endif
-	if (p->rx_descriptor_status == NULL)
-		return -ENOTSUP;
 	return p->rx_descriptor_status(qd, offset);
 }
 
@@ -6542,8 +6538,6 @@ static inline int rte_eth_tx_descriptor_status(uint16_t port_id,
 	if (qd == NULL)
 		return -ENODEV;
 #endif
-	if (p->tx_descriptor_status == NULL)
-		return -ENOTSUP;
 	return p->tx_descriptor_status(qd, offset);
 }
 
@@ -6786,9 +6780,6 @@ rte_eth_tx_prepare(uint16_t port_id, uint16_t queue_id,
 	}
 #endif
 
-	if (!p->tx_pkt_prepare)
-		return nb_pkts;
-
 	return p->tx_pkt_prepare(qd, tx_pkts, nb_pkts);
 }
 
@@ -6985,8 +6976,6 @@ rte_eth_recycle_mbufs(uint16_t rx_port_id, uint16_t rx_queue_id,
 		return 0;
 	}
 #endif
-	if (p1->recycle_tx_mbufs_reuse == NULL)
-		return 0;
 
 #ifdef RTE_ETHDEV_DEBUG_RX
 	if (rx_port_id >= RTE_MAX_ETHPORTS ||
@@ -7010,8 +6999,6 @@ rte_eth_recycle_mbufs(uint16_t rx_port_id, uint16_t rx_queue_id,
 		return 0;
 	}
 #endif
-	if (p2->recycle_rx_descriptors_refill == NULL)
-		return 0;
 
 	/* Copy used *rte_mbuf* buffer pointers from Tx mbuf ring
 	 * into Rx mbuf ring.
@@ -7131,14 +7118,11 @@ rte_eth_tx_queue_count(uint16_t port_id, uint16_t queue_id)
 		goto out;
 	}
 #endif
-	if (fops->tx_queue_count == NULL) {
-		rc = -ENOTSUP;
-		goto out;
-	}
-
 	rc = fops->tx_queue_count(qd);
 
+#ifdef RTE_ETHDEV_DEBUG_TX
 out:
+#endif
 	rte_eth_trace_tx_queue_count(port_id, queue_id, rc);
 	return rc;
 }
-- 
2.43.0

Re: [PATCH] ethdev: remove callback checks from fast path

[Stephen Hemminger]

On Tue, 29 Apr 2025 23:41:30 +0530
<skori@marvell.com> wrote:

> @@ -7131,14 +7118,11 @@ rte_eth_tx_queue_count(uint16_t port_id, uint16_t queue_id)
>  		goto out;
>  	}
>  #endif
> -	if (fops->tx_queue_count == NULL) {
> -		rc = -ENOTSUP;
> -		goto out;
> -	}
> -
>  	rc = fops->tx_queue_count(qd);
>  
> +#ifdef RTE_ETHDEV_DEBUG_TX
>  out:
> +#endif

I think you could just fix the ETHDEV_DEBUG_TX to just return.
Other ethdev functions skip calling tracing for
the case of errors.

Re: [PATCH] ethdev: remove callback checks from fast path

[Stephen Hemminger]

On Tue, 29 Apr 2025 23:41:30 +0530
<skori@marvell.com> wrote:

>  
> +static inline void
> +rte_eth_set_dummy_fops(struct rte_eth_dev *eth_dev)

Not fastpath, do not add inline here

> +{
> +	eth_dev->rx_pkt_burst = rte_eth_pkt_burst_dummy;
> +	eth_dev->tx_pkt_burst = rte_eth_pkt_burst_dummy;
> +	eth_dev->tx_pkt_prepare = rte_eth_tx_pkt_prepare_dummy;
> +	eth_dev->rx_queue_count = rte_eth_rx_queue_count_dummy;
> +	eth_dev->tx_queue_count = rte_eth_tx_queue_count_dummy;
> +	eth_dev->rx_descriptor_status = rte_eth_descriptor_status_dummy;
> +	eth_dev->tx_descriptor_status = rte_eth_descriptor_status_dummy;
> +	eth_dev->recycle_tx_mbufs_reuse = rte_eth_recycle_tx_mbufs_reuse_dummy;
> +	eth_dev->recycle_rx_descriptors_refill = rte_eth_recycle_rx_descriptors_refill_dummy;
> +}
> +
>  /**
>   * Copy pci device info to the Ethernet device data.
>   * Shared memory (eth_dev->data) only updated by primary process, so it is safe
> @@ -147,6 +161,11 @@ rte_eth_dev_pci_generic_probe(struct rte_pci_device *pci_dev,
>  	if (!eth_dev)
>  		return -ENOMEM;
>  
> +	/* Update fast path ops with dummy callbacks. Driver will update
> +	 * them with required callbacks in the init function.
> +	 */
> +	rte_eth_set_dummy_fops(eth_dev);
> +

What about non PCI ethdev's?
And in PCI devices there is lots of duplicated code already initilizes these.
Needs to be consolidated and documented. Actually the whole probe process needs more documentation.

Also need to make sure secondary process setup is not broken.

RE: [PATCH] ethdev: remove callback checks from fast path

[Morten Brørup]

> From: Sunil Kumar Kori <skori@marvell.com>
> Sent: Tuesday, 29 April 2025 20.12
> 
> rte_eth_fp_ops contains ops for fast path APIs. Each API
> validates availability of callback and then invoke it.
> 
> Removing these NULL checks instead using dummy callbacks.

The description should mention the motivation for this patch: mbuf recycling performance optimization.

A few nits below.

> +RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_rx_queue_count_dummy)
> +uint32_t
> +rte_eth_rx_queue_count_dummy(void *queue __rte_unused)
> +{
> +	return -ENOTSUP;
> +}

Instead of type casting back and forth, change the type of the RX queue count callback [1]:
-typedef uint32_t (*eth_rx_queue_count_t)(void *rxq);
+typedef int (*eth_rx_queue_count_t)(void *rxq);

So it resembles the TX queue count callback, eth_tx_queue_count_t, which already returns int.

[1]: https://elixir.bootlin.com/dpdk/v25.03/source/lib/ethdev/rte_ethdev_core.h#L48

Although my suggestion is formally an API change, I suppose changing from unsigned to signed should be acceptable.

> +RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_recycle_rx_descriptors_refill_dummy
> )
> +void
> +rte_eth_recycle_rx_descriptors_refill_dummy(void *queue __rte_unused,
> +		uint16_t nb __rte_unused)
> +{
> +

This empty line looks strange. Perhaps add a comment /* No action. */ to indicate that no code is missing here.

> +}