On Tue, 12 May 2026 19:36:49 +0800
[email protected] wrote:

> From: Jie Liu <[email protected]>
> 
> This patch set addresses the feedback received on the v10 submission
> for the sxe2 PMD. The primary focus is on fixing vector path selection,
> ensuring memory safety during mbuf initialization, and cleaning up
> redundant logic in the configuration functions.
> 
> v13 Changes:
> - Fixed vector Rx burst function being overwritten by scalar selection.
> - Refactored Rx/Tx mode set functions to seed flags from caps 
> first,eliminating tautological checks.
> - Added memset for mbuf_def in vector init to avoid uninitialized reads.
> - Converted pci_map_addr_info to designated initializers.
> - Removed dead Windows-only code in meson.build.
> - Added NULL checks for mbuf free for driver-wide consistency.
> - Updated burst_mode_get to accurately report AVX paths.
> - Adjusted SXE2_ETH_OVERHEAD to match actual VLAN capabilities.
> 
> Jie Liu (10):
>   mailmap: add Jie Liu
>   doc: add sxe2 guide and release notes
>   common/sxe2: add sxe2 basic structures
>   drivers: add base driver skeleton
>   drivers: add base driver probe skeleton
>   drivers: support PCI BAR mapping
>   common/sxe2: add ioctl interface for DMA map and unmap
>   net/sxe2: support queue setup and control
>   drivers: add data path for Rx and Tx
>   net/sxe2: add vectorized Rx and Tx
> 
>  .mailmap                                   |   1 +
>  doc/guides/nics/features/sxe2.ini          |  30 +
>  doc/guides/nics/index.rst                  |   1 +
>  doc/guides/nics/sxe2.rst                   |  34 +
>  doc/guides/rel_notes/release_26_07.rst     |   4 +
>  drivers/common/sxe2/meson.build            |  15 +
>  drivers/common/sxe2/sxe2_common.c          | 685 +++++++++++++++
>  drivers/common/sxe2/sxe2_common.h          |  86 ++
>  drivers/common/sxe2/sxe2_common_log.h      |  83 ++
>  drivers/common/sxe2/sxe2_errno.h           | 110 +++
>  drivers/common/sxe2/sxe2_host_regs.h       | 707 +++++++++++++++
>  drivers/common/sxe2/sxe2_internal_ver.h    |  33 +
>  drivers/common/sxe2/sxe2_ioctl_chnl.c      | 326 +++++++
>  drivers/common/sxe2/sxe2_ioctl_chnl.h      | 141 +++
>  drivers/common/sxe2/sxe2_ioctl_chnl_func.h |  63 ++
>  drivers/common/sxe2/sxe2_osal.h            | 584 +++++++++++++
>  drivers/common/sxe2/sxe2_type.h            |  60 ++
>  drivers/meson.build                        |   1 +
>  drivers/net/meson.build                    |   1 +
>  drivers/net/sxe2/meson.build               |  32 +
>  drivers/net/sxe2/sxe2_cmd_chnl.c           | 319 +++++++
>  drivers/net/sxe2/sxe2_cmd_chnl.h           |  33 +
>  drivers/net/sxe2/sxe2_drv_cmd.h            | 389 +++++++++
>  drivers/net/sxe2/sxe2_ethdev.c             | 941 ++++++++++++++++++++
>  drivers/net/sxe2/sxe2_ethdev.h             | 315 +++++++
>  drivers/net/sxe2/sxe2_irq.h                |  49 ++
>  drivers/net/sxe2/sxe2_queue.c              |  67 ++
>  drivers/net/sxe2/sxe2_queue.h              | 194 +++++
>  drivers/net/sxe2/sxe2_rx.c                 | 579 +++++++++++++
>  drivers/net/sxe2/sxe2_rx.h                 |  34 +
>  drivers/net/sxe2/sxe2_tx.c                 | 447 ++++++++++
>  drivers/net/sxe2/sxe2_tx.h                 |  32 +
>  drivers/net/sxe2/sxe2_txrx.c               | 372 ++++++++
>  drivers/net/sxe2/sxe2_txrx.h               |  22 +
>  drivers/net/sxe2/sxe2_txrx_common.h        | 541 ++++++++++++
>  drivers/net/sxe2/sxe2_txrx_poll.c          | 945 +++++++++++++++++++++
>  drivers/net/sxe2/sxe2_txrx_poll.h          |  17 +
>  drivers/net/sxe2/sxe2_txrx_vec.c           | 197 +++++
>  drivers/net/sxe2/sxe2_txrx_vec.h           |  72 ++
>  drivers/net/sxe2/sxe2_txrx_vec_common.h    | 235 +++++
>  drivers/net/sxe2/sxe2_txrx_vec_sse.c       | 545 ++++++++++++
>  drivers/net/sxe2/sxe2_vsi.c                | 212 +++++
>  drivers/net/sxe2/sxe2_vsi.h                | 205 +++++
>  43 files changed, 9759 insertions(+)
>  create mode 100644 doc/guides/nics/features/sxe2.ini
>  create mode 100644 doc/guides/nics/sxe2.rst
>  create mode 100644 drivers/common/sxe2/meson.build
>  create mode 100644 drivers/common/sxe2/sxe2_common.c
>  create mode 100644 drivers/common/sxe2/sxe2_common.h
>  create mode 100644 drivers/common/sxe2/sxe2_common_log.h
>  create mode 100644 drivers/common/sxe2/sxe2_errno.h
>  create mode 100644 drivers/common/sxe2/sxe2_host_regs.h
>  create mode 100644 drivers/common/sxe2/sxe2_internal_ver.h
>  create mode 100644 drivers/common/sxe2/sxe2_ioctl_chnl.c
>  create mode 100644 drivers/common/sxe2/sxe2_ioctl_chnl.h
>  create mode 100644 drivers/common/sxe2/sxe2_ioctl_chnl_func.h
>  create mode 100644 drivers/common/sxe2/sxe2_osal.h
>  create mode 100644 drivers/common/sxe2/sxe2_type.h
>  create mode 100644 drivers/net/sxe2/meson.build
>  create mode 100644 drivers/net/sxe2/sxe2_cmd_chnl.c
>  create mode 100644 drivers/net/sxe2/sxe2_cmd_chnl.h
>  create mode 100644 drivers/net/sxe2/sxe2_drv_cmd.h
>  create mode 100644 drivers/net/sxe2/sxe2_ethdev.c
>  create mode 100644 drivers/net/sxe2/sxe2_ethdev.h
>  create mode 100644 drivers/net/sxe2/sxe2_irq.h
>  create mode 100644 drivers/net/sxe2/sxe2_queue.c
>  create mode 100644 drivers/net/sxe2/sxe2_queue.h
>  create mode 100644 drivers/net/sxe2/sxe2_rx.c
>  create mode 100644 drivers/net/sxe2/sxe2_rx.h
>  create mode 100644 drivers/net/sxe2/sxe2_tx.c
>  create mode 100644 drivers/net/sxe2/sxe2_tx.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx.c
>  create mode 100644 drivers/net/sxe2/sxe2_txrx.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_common.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_poll.c
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_poll.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec.c
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_common.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_sse.c
>  create mode 100644 drivers/net/sxe2/sxe2_vsi.c
>  create mode 100644 drivers/net/sxe2/sxe2_vsi.h
> 

Still lots of AI review feedback:

Summary of v13 sxe2 PMD review

Most of the mechanical issues from v10 (carried into v11/v12) have
been addressed in v13: duplicate tx_queue_offload_capa is gone,
RTE_LOG_REGISTER_SUFFIX no longer registers Tx as "rx", the
fopen("/var/log/...") path, SXE2_DPDK_DEBUG, FPGA_VER_ASIC and the
debug flags in meson are gone, rx_queue_release / tx_queue_release
are in dev_ops, and mmap() is correctly checked against MAP_FAILED.

What still needs to be fixed:

Errors
------

Patch 06/10 - Inverted error checks in sxe2_dev_pci_map_init().
sxe2_dev_pci_res_seg_map() returns 0 on success, negative on
failure (the definition is in this same patch). Five consecutive
call sites use "if (!ret)" which is true on success, then log
"Failed to map..." and goto cleanup, unmapping the BAR segment
just mapped and returning ret=0 so the caller thinks probe
succeeded. On a real failure, the check is false and the code
proceeds as if the mapping had succeeded. Same bug flagged in
v10, unchanged in v11/v12/v13.

Patch 04/10 - rte_ticketlock_t (busy-wait FIFO lock) held
across blocking ioctl() to the kernel driver in
sxe2_drv_cmd_exec(), sxe2_drv_dev_handshark(), and the DMA
map/unmap entry points added in patch 07. Other lcores trying
to acquire the lock burn CPU spinning until the kernel returns.
Use pthread_mutex_t; the lock is in process-private memory so
PTHREAD_PROCESS_SHARED is not needed.

Patch 06/10 - Non-ASCII characters in source comments. In
sxe2_net_map_addr_info_pf[]: "/* SXE2_PCI_MAP_RES_IRQ_ITR(默认使用ITR0) */".
DPDK source should be ASCII English.

Warnings
--------

Patch 02/10 - Feature matrix overclaims. sxe2.ini lists
"MTU update = Y" but no .mtu_set is registered, and
"Free Tx mbuf on demand = Y" but no .tx_done_cleanup is
registered. Either implement the callbacks or fix the matrix.

Patch 09/10 - Dead "if (ret != SXE2_SUCCESS)" check in the
secondary-process branch of sxe2_dev_init() after two calls to
void-returning functions (sxe2_rx_mode_func_set,
sxe2_tx_mode_func_set). ret cannot change, so the error log
never fires.

Patch 03/10 - Driver-private kernel-style aliases in
sxe2_type.h: u8/s8/u16/.../s64 typedefs, "#define STATIC static",
"#define __le16 u16", "#define __be16 u16". DPDK convention is
to use the standard names directly. The __le*/__be* defines
also erase the endianness annotation rather than preserving
it.

Patch 03/10 - sxe2_osal.h reinvents infrastructure that already
exists in DPDK: BIT/BIT_ULL/GENMASK/set_bit/test_bit/bitmap_weight
(use rte_bitops.h / rte_bitmap.h), LIST_FOR_EACH_ENTRY and
friends (use sys/queue.h TAILQ_* or rte_tailq), COMPILER_BARRIER
(use rte_compiler_barrier), sxe2_*_lock wrappers around
rte_spinlock_*, and udelay/mdelay/msleep aliases for
rte_delay_us. The kernel idioms __iomem and IS_ERR/IS_ERR_VALUE
do not belong in a userspace PMD.

Patch 03/10 - sxe2_errno.h defines a parallel SXE2_ERR_* namespace
that aliases every POSIX errno. The rest of the driver mixes
both spellings (-EFAULT and SXE2_ERR_PERM appear in the same
file). Pick one and use it everywhere.

Patch 08/10 - Redundant "queue_idx >= nb_*_queues" guards at
the top of sxe2_rx_queue_setup / sxe2_tx_queue_setup / queue_start
/ queue_stop. The ethdev layer validates queue_idx before
calling the PMD.

Patches 04 and 06 - Typos in identifiers and log strings:
sxe2_commoin_inited (commoin -> common), sxe2_drv_dev_handshark
(handshark -> handshake; the ioctl SXE2_COM_CMD_HANDSHAKE is
correct, only the wrapper is mistyped), "kernel reseted, need
restart app." (reseted -> was reset).

Info
----

Patch 09/10 - sxe2_rx_mode_func_set always picks a scattered Rx
burst (split or non-split) regardless of dev->data->scattered_rx.
There is no plain single-segment fast path.

Patch 10/10 - The vector capability probe in
sxe2_rx_mode_func_set is gated on RTE_PROC_PRIMARY, but a
secondary process calls the same function and lands on the
scalar path. Since rx_pkt_burst is per-rte_eth_dev and
re-assigned by whichever process calls last, the resulting
mode depends on attach ordering.

Patch 09/10 - Dead "goto l_end_of_tx;" immediately before the
"l_end_of_tx:" label in sxe2_tx_pkts().

Patch 10/10 - The "AVX512/AVX2 is not supported in build env"
log lines fire based on CPU capability, not on any build-time
absence, so the message is misleading.

The v10-flagged PCI map inverted-check bug is still present
after three respins. 

Longer report available if needed.

Reply via email to