Re: [PATCH v2 00/36] combine multiple Intel scalar Tx paths

Bruce Richardson
2026-01-26T09:02:57Z

On Thu, Jan 22, 2026 at 10:26:33PM -0800, Stephen Hemminger wrote:
> On Tue, 13 Jan 2026 15:14:24 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > The scalar Tx paths, with support for offloads and multiple mbufs
> > per packet, are almost identical across drivers ice, i40e, iavf and
> > the single-queue mode of idpf. Therefore, we can do some rework to
> > combine these code paths into a single function which is parameterized
> > by compile-time constants, allowing code saving to give us a single
> > path to optimize and maintain - apart from edge cases like IPSec
> > support in iavf.
> >
> > The ixgbe driver has a number of similarities too, which we take
> > advantage of where we can, but the overall descriptor format is
> > sufficiently different that its main scalar code path is kept
> > separate.
> >
> > Once merged, we can then optimize the drivers a bit to improve
> > performance, and also easily extend some drivers to use additional
> > paths for better performance, e.g. add the "simple scalar" path
> > to IDPF driver for better performance on platforms without AVX.
> >
> > V2:
> > - reworked the simple-scalar path as well as full scalar one
> > - added simple scalar path support to idpf driver
> > - small cleanups, e.g. issues flagged by checkpatch
> >
> > Bruce Richardson (36):
> > net/intel: create common Tx descriptor structure
> > net/intel: use common Tx ring structure
> > net/intel: create common post-Tx cleanup function
> > net/intel: consolidate definitions for Tx desc fields
> > net/intel: create separate header for Tx scalar fns
> > net/intel: add common fn to calculate needed descriptors
> > net/ice: refactor context descriptor handling
> > net/i40e: refactor context descriptor handling
> > net/idpf: refactor context descriptor handling
> > net/intel: consolidate checksum mask definition
> > net/intel: create common checksum Tx offload function
> > net/intel: create a common scalar Tx function
> > net/i40e: use common scalar Tx function
> > net/intel: add IPsec hooks to common Tx function
> > net/intel: support configurable VLAN tag insertion on Tx
> > net/iavf: use common scalar Tx function
> > net/i40e: document requirement for QinQ support
> > net/idpf: use common scalar Tx function
> > net/intel: avoid writing the final pkt descriptor twice
> > eal: add macro for marking assumed alignment
> > net/intel: write descriptors using non-volatile pointers
> > net/intel: remove unnecessary flag clearing
> > net/intel: mark mid-burst ring cleanup as unlikely
> > net/intel: add special handling for single desc packets
> > net/intel: use separate array for desc status tracking
> > net/ixgbe: use separate array for desc status tracking
> > net/intel: drop unused Tx queue used count
> > net/intel: remove index for tracking end of packet
> > net/intel: merge ring writes in simple Tx for ice and i40e
> > net/intel: consolidate ice and i40e buffer free function
> > net/intel: complete merging simple Tx paths
> > net/intel: use non-volatile stores in simple Tx function
> > net/intel: align scalar simple Tx path with vector logic
> > net/intel: use vector SW ring entry for simple path
> > net/intel: use vector mbuf cleanup from simple scalar path
> > net/idpf: enable simple Tx function
> >
> > doc/guides/nics/i40e.rst | 18 +
> > drivers/net/intel/common/tx.h | 116 ++-
> > drivers/net/intel/common/tx_scalar_fns.h | 595 ++++++++++++++
> > drivers/net/intel/cpfl/cpfl_rxtx.c | 8 +-
> > drivers/net/intel/i40e/i40e_fdir.c | 34 +-
> > drivers/net/intel/i40e/i40e_rxtx.c | 670 +++-------------
> > drivers/net/intel/i40e/i40e_rxtx.h | 16 -
> > .../net/intel/i40e/i40e_rxtx_vec_altivec.c | 25 +-
> > drivers/net/intel/i40e/i40e_rxtx_vec_avx2.c | 36 +-
> > drivers/net/intel/i40e/i40e_rxtx_vec_avx512.c | 52 +-
> > drivers/net/intel/i40e/i40e_rxtx_vec_common.h | 6 +-
> > drivers/net/intel/i40e/i40e_rxtx_vec_neon.c | 25 +-
> > drivers/net/intel/iavf/iavf_rxtx.c | 642 ++++-----------
> > drivers/net/intel/iavf/iavf_rxtx.h | 30 +-
> > drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c | 55 +-
> > drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c | 104 +--
> > drivers/net/intel/iavf/iavf_rxtx_vec_common.h | 36 +-
> > drivers/net/intel/ice/ice_dcf_ethdev.c | 10 +-
> > drivers/net/intel/ice/ice_rxtx.c | 737 ++++--------------
> > drivers/net/intel/ice/ice_rxtx.h | 15 -
> > drivers/net/intel/ice/ice_rxtx_vec_avx2.c | 55 +-
> > drivers/net/intel/ice/ice_rxtx_vec_avx512.c | 53 +-
> > drivers/net/intel/ice/ice_rxtx_vec_common.h | 43 +-
> > drivers/net/intel/idpf/idpf_common_device.h | 2 +
> > drivers/net/intel/idpf/idpf_common_rxtx.c | 315 ++------
> > drivers/net/intel/idpf/idpf_common_rxtx.h | 24 +-
> > .../net/intel/idpf/idpf_common_rxtx_avx2.c | 53 +-
> > .../net/intel/idpf/idpf_common_rxtx_avx512.c | 55 +-
> > drivers/net/intel/idpf/idpf_rxtx.c | 43 +-
> > drivers/net/intel/idpf/idpf_rxtx_vec_common.h | 6 +-
> > drivers/net/intel/ixgbe/ixgbe_rxtx.c | 103 ++-
> > .../net/intel/ixgbe/ixgbe_rxtx_vec_common.c | 3 +-
> > lib/eal/include/rte_common.h | 6 +
> > 33 files changed, 1565 insertions(+), 2426 deletions(-)
> > create mode 100644 drivers/net/intel/common/tx_scalar_fns.h
> >
>
> Series-Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>
> Love to see common code and "fix it once"
> It was too large for the batch scripts, but here is AI review.
> It sees only minor stuff which you could fix (or skip).
>

Thanks for running the AI review.

> # DPDK Patch Review: Intel Tx Consolidation Series
>
> **Series:** [PATCH v2 01-36/36] Intel Tx driver consolidation
> **Author:** Bruce Richardson <bruce.richardson@intel.com>
> **Review Date:** 2026-01-22
> **Review Against:** AGENTS.md (DPDK Code Review Guidelines)
>
> ---
>
> ## Executive Summary
>
> This 36-patch series consolidates Tx descriptor handling across Intel network drivers (i40e, iavf, ice, idpf, ixgbe). Overall the patches are **well-structured** with proper code organization, but there are several **issues requiring attention** before merge.
>
> | Severity | Count | Summary |
> |----------|-------|---------|
> | **Error** | 1 | Source code line exceeds 100 characters |
> | **Warning** | 5 | Commit message style issues, double blank lines |
> | **Info** | 2 | Minor style observations |
>
> ---
>
> ## Detailed Findings
>
> ### ERRORS (Must Fix)
>
> #### 1. Source Code Line Length Violation
> **Patch 17/36:** `net/i40e: document requirement for QinQ support`
> **Location:** `drivers/net/intel/i40e/i40e_rxtx.c`
> **Issue:** Line exceeds 100-character limit (135 characters)
>
> ```c
> PMD_DRV_LOG(WARNING, "Double VLAN insertion may not work correctly without RTE_ETH_RX_OFFLOAD_VLAN_EXTEND set in Rx configuration.");
> ```
>
> **Fix:** Split the log message across multiple lines or into multiple log calls:
> ```c
> PMD_DRV_LOG(WARNING, "Double VLAN insertion may not work correctly "
> "without RTE_ETH_RX_OFFLOAD_VLAN_EXTEND in Rx config.");
> ```
>

This is one where the AI guidelines need an update. We need to teach it
that it's better to have a long line than to split error messages.

/Bruce