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

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

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