Re: [PATCH v4 24/25] net/intel: add common Rx mbuf recycle

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, May 30, 2025 at 02:57:20PM +0100, Anatoly Burakov wrote:
> Currently, there are duplicate implementations of Rx mbuf recycle in some
> drivers, specifically ixgbe and i40e. Move them into a common header.
> 
> While we're at it, also support no-IOVA-in-mbuf case.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One very minor nit inline below.

>  drivers/net/intel/common/recycle_mbufs.h      | 68 +++++++++++++++++++
>  .../i40e/i40e_recycle_mbufs_vec_common.c      | 37 +---------
>  .../ixgbe/ixgbe_recycle_mbufs_vec_common.c    | 35 +---------
>  3 files changed, 74 insertions(+), 66 deletions(-)
>  create mode 100644 drivers/net/intel/common/recycle_mbufs.h
> 
> diff --git a/drivers/net/intel/common/recycle_mbufs.h b/drivers/net/intel/common/recycle_mbufs.h
> new file mode 100644
> index 0000000000..c32e2ce9b1
> --- /dev/null
> +++ b/drivers/net/intel/common/recycle_mbufs.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2025 Intel Corporation
> + */
> +
> + #ifndef _COMMON_INTEL_RECYCLE_MBUFS_H_
> +#define _COMMON_INTEL_RECYCLE_MBUFS_H_
> +
> +#include <stdint.h>
> +#include <unistd.h>
> +
> +#include <rte_mbuf.h>
> +#include <rte_io.h>
> +#include <ethdev_driver.h>
> +
> +#include "rx.h"
> +#include "tx.h"
> +
> +/**
> + * Recycle mbufs for Rx queue.
> + *
> + * @param rxq Rx queue pointer
> + * @param nb_mbufs number of mbufs to recycle
> + * @param desc_len length of Rx descriptor
> + */
> +static __rte_always_inline void
> +ci_rx_recycle_mbufs(struct ci_rx_queue *rxq, const uint16_t nb_mbufs)
> +{
> +	struct ci_rx_entry *rxep;
> +	volatile union ci_rx_desc *rxdp;
> +	uint16_t rx_id;
> +	uint16_t i;
> +
> +	rxdp = rxq->rx_ring + rxq->rxrearm_start;
> +	rxep = &rxq->sw_ring[rxq->rxrearm_start];
> +
> +	for (i = 0; i < nb_mbufs; i++) {
> +		struct rte_mbuf *mb = rxep[i].mbuf;
> +
> +#if RTE_IOVA_IN_MBUF
> +		const uint64_t paddr = mb->buf_iova + RTE_PKTMBUF_HEADROOM;

Nit, prefer iova to paddr, since it's (hopefully) not an actual physical
address, but an iommu lookup address.

> +		const uint64_t dma_addr = rte_cpu_to_le_64(paddr);
> +#else
> +		const uint64_t vaddr = (uintptr_t)mb->buf_addr + RTE_PKTMBUF_HEADROOM;
> +		const uint64_t dma_addr = rte_cpu_to_le_64(vaddr);
> +#endif
> +
> +		rxdp[i].read.hdr_addr = 0;
> +		rxdp[i].read.pkt_addr = dma_addr;
> +	}
> +
<snip>