Re: [PATCH v4 03/35] net/intel: create common post-Tx cleanup function

Burakov, Anatoly
2026-02-10T12:19:07Z

On 2/9/2026 5:45 PM, Bruce Richardson wrote:
> The code used in ice, iavf, idpf and i40e for doing cleanup of mbufs
> after they had been transmitted was identical. Therefore deduplicate it
> by moving to common and remove the driver-specific versions.
>
> Rather than having all Tx code in the one file, which could start
> getting rather long, create a new header file for scalar datapath
> functions.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

<snip>

> +static __rte_always_inline int
> +ci_tx_xmit_cleanup(struct ci_tx_queue *txq)
> +{
> + struct ci_tx_entry *sw_ring = txq->sw_ring;
> + volatile struct ci_tx_desc *txd = txq->ci_tx_ring;
> + uint16_t last_desc_cleaned = txq->last_desc_cleaned;
> + uint16_t nb_tx_desc = txq->nb_tx_desc;
> + uint16_t desc_to_clean_to;
> + uint16_t nb_tx_to_clean;
> +
> + /* Determine the last descriptor needing to be cleaned */
> + desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
> + if (desc_to_clean_to >= nb_tx_desc)
> + desc_to_clean_to = (uint16_t)(desc_to_clean_to - nb_tx_desc);
> +
> + /* Check if descriptor is done - all drivers use 0xF as done value in bits 3:0 */
> + desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
> + if ((txd[desc_to_clean_to].cmd_type_offset_bsz & rte_cpu_to_le_64(0x0FUL)) !=
> + rte_cpu_to_le_64(0x0FUL))

Kind of a nitpick - we do this kind of "(value & mask) == mask" in a lot
of places, and occasionally it needs these byteswaps and whatnot, which
IMO hurts readability as there's too much going on on a single line.

I think it would be good to add a helper function e.g.

uint64_t desc_done_msk = rte_cpu_to_le_64(0x0FUL);
if (mask_is_set(txd[desc_to_clean_to].cmd_type_offset_bsz,
desc_done_msk)) ....

and use it where we can to make the code a bit more semantically meaningful.

That said, not a biggie, so

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

--
Thanks,
Anatoly