From: Stephen Hemminger <stephen@networkplumber.org>
To: liujie5@linkdatatechnology.com
Cc: dev@dpdk.org
Subject: Re: [PATCH v16 00/11] net/sxe2: fix logic errors and address feedback
Date: Mon, 18 May 2026 10:25:11 -0700 [thread overview]
Message-ID: <20260518102511.3888a02c@phoenix.local> (raw)
In-Reply-To: <20260518091405.3295896-1-liujie5@linkdatatechnology.com>
On Mon, 18 May 2026 17:13:54 +0800
liujie5@linkdatatechnology.com wrote:
> From: Jie Liu <liujie5@linkdatatechnology.com>
>
> 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.
>
> v16 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 (11):
> 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
> net/sxe2: implement Tx done cleanup
>
> .mailmap | 1 +
> doc/guides/nics/features/sxe2.ini | 29 +
> 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 | 683 +++++++++++++
> drivers/common/sxe2/sxe2_common.h | 85 ++
> drivers/common/sxe2/sxe2_common_log.h | 81 ++
> drivers/common/sxe2/sxe2_host_regs.h | 707 +++++++++++++
> drivers/common/sxe2/sxe2_internal_ver.h | 33 +
> drivers/common/sxe2/sxe2_ioctl_chnl.c | 325 ++++++
> drivers/common/sxe2/sxe2_ioctl_chnl.h | 130 +++
> drivers/common/sxe2/sxe2_ioctl_chnl_func.h | 62 ++
> drivers/common/sxe2/sxe2_osal.h | 153 +++
> drivers/meson.build | 1 +
> drivers/net/meson.build | 1 +
> drivers/net/sxe2/meson.build | 32 +
> drivers/net/sxe2/sxe2_cmd_chnl.c | 323 ++++++
> drivers/net/sxe2/sxe2_cmd_chnl.h | 37 +
> drivers/net/sxe2/sxe2_drv_cmd.h | 388 ++++++++
> drivers/net/sxe2/sxe2_ethdev.c | 968 ++++++++++++++++++
> drivers/net/sxe2/sxe2_ethdev.h | 318 ++++++
> drivers/net/sxe2/sxe2_irq.h | 48 +
> drivers/net/sxe2/sxe2_queue.c | 66 ++
> drivers/net/sxe2/sxe2_queue.h | 195 ++++
> drivers/net/sxe2/sxe2_rx.c | 559 +++++++++++
> drivers/net/sxe2/sxe2_rx.h | 34 +
> drivers/net/sxe2/sxe2_tx.c | 420 ++++++++
> drivers/net/sxe2/sxe2_tx.h | 32 +
> drivers/net/sxe2/sxe2_txrx.c | 362 +++++++
> drivers/net/sxe2/sxe2_txrx.h | 23 +
> drivers/net/sxe2/sxe2_txrx_common.h | 540 ++++++++++
> drivers/net/sxe2/sxe2_txrx_poll.c | 1044 ++++++++++++++++++++
> drivers/net/sxe2/sxe2_txrx_poll.h | 20 +
> drivers/net/sxe2/sxe2_txrx_vec.c | 201 ++++
> drivers/net/sxe2/sxe2_txrx_vec.h | 73 ++
> drivers/net/sxe2/sxe2_txrx_vec_common.h | 235 +++++
> drivers/net/sxe2/sxe2_txrx_vec_sse.c | 549 ++++++++++
> drivers/net/sxe2/sxe2_vsi.c | 214 ++++
> drivers/net/sxe2/sxe2_vsi.h | 204 ++++
> 41 files changed, 9230 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_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/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
>
Getting much closer to merge, still time for 26.07.
Review of [PATCH v16 00/11] sxe2 PMD
====================================
Overall comments
----------------
All Error/Warning-level issues from the v15 review are addressed
in v16:
- sxe2_tx_done_cleanup(): tx_queue NULL check now runs before
dereferencing txq->vsi->adapter.
- Vector mode probe is still primary-only, but secondaries now
inherit the decision via adapter->q_ctxt.{rx,tx}_mode_flags
rather than silently falling back to scalar.
- BITS_TO_uint32_t macro renamed to BITS_TO_U32.
- Dead "goto l_end; l_end:" pattern in sxe2_rx_mode_func_set
removed in favour of "return;".
- sxe2_drv_dev_handshake function name correctly spelled in
the patch that introduces it.
Two new concerns and one carryover:
Patch 09/11: drivers: add data path for Rx and Tx
-------------------------------------------------
Warning: asymmetric secondary-process fallback in
sxe2_tx_mode_func_set() versus sxe2_rx_mode_func_set().
Rx (correct):
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
...
adapter->q_ctxt.rx_mode_flags = rx_mode_flags;
} else {
rx_mode_flags = adapter->q_ctxt.rx_mode_flags;
}
Tx (overrides primary's decision):
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
...
adapter->q_ctxt.tx_mode_flags = tx_mode_flags;
} else {
tx_mode_flags = adapter->q_ctxt.tx_mode_flags;
if (tx_mode_flags == 0)
tx_mode_flags = SXE2_TX_MODE_SIMPLE_BATCH;
}
If the primary genuinely settled on tx_mode_flags = 0 (no
vector, no simple-batch — i.e. the full sxe2_tx_pkts /
sxe2_tx_pkts_prepare path), the secondary will pick
sxe2_tx_pkts_simple instead. Primary and secondary then run
different burst functions against the same queue, with different
offload-handling semantics. This breaks the multi-process
contract.
If the fallback is meant to handle "secondary attached before
primary configured anything", that case should be detected
explicitly (e.g. a separate "configured" flag). Otherwise just
mirror the Rx side: trust whatever the primary put in shared
memory.
Warning: SXE2_{RX,TX}_MODE_VEC_SSE bit is never set.
sxe2_{rx,tx}_vec_support_check() always seeds *vec_flags with
either SXE2_*_MODE_VEC_SIMPLE or SXE2_*_MODE_VEC_OFFLOAD, both
of which are in SXE2_*_MODE_VEC_SET_MASK. In the mode-set
function:
tx_mode_flags = vec_flags;
#ifdef RTE_ARCH_X86
if ((tx_mode_flags & SXE2_TX_MODE_VEC_SET_MASK) == 0)
tx_mode_flags |= SXE2_TX_MODE_VEC_SSE;
#endif
the condition is already false because vec_flags set at least
the SIMPLE bit, so VEC_SSE is never OR'd in. The Rx side has
the identical pattern, with the additional dead duplicate
"rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128" check
that the surrounding if already verified. Either the
*_vec_support_check() helpers should seed only the OFFLOAD bit
and let the mode-set function pick SSE/AVX2/AVX512 from CPU
features, or the dead conditional should be removed.
As-is, the bursts dev->tx_pkt_burst = sxe2_tx_pkts_vec_sse and
dev->rx_pkt_burst = sxe2_rx_pkts_scattered_vec_sse_offload get
selected purely on the SET_MASK presence — the SSE-specific bit
never participates. This works on x86 (it's gated by
#ifdef RTE_ARCH_X86), but the VEC_SSE / VEC_AVX2 / VEC_AVX512 /
VEC_NEON bit definitions are then misleading: no code path
actually distinguishes them.
Series hygiene
--------------
Info: log strings "Open fd=%d to handshark with kernel" and
"Failed to handshark, ..." are introduced in patch 03 with the
typo, and corrected to "handshake" in patch 05. The function
itself is correctly named sxe2_drv_dev_handshake throughout,
but the messages inside it disagree with the function name in
patches 03 and 04. Two options: fold the typo fix into patch
03, or just write the log strings correctly the first time.
Info: dead "if (ret != 0) PMD_LOG_ERR(...)" check after two
void-returning sxe2_{rx,tx}_mode_func_set() calls was added in
patch 08 and removed in patch 09 of v15; in v16 the same pattern
appears in patch 08 (added) and patch 09 (removed). Same
within-series churn as before — please collapse.
next prev parent reply other threads:[~2026-05-18 17:25 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-14 2:01 [PATCH v13 0/5] Support add/remove memory region and get-max-slots pravin.bathija
2026-05-14 2:01 ` [PATCH v13 1/5] vhost: add user to mailmap and define to vhost hdr pravin.bathija
2026-05-14 2:01 ` [PATCH v13 2/5] vhost_user: header defines for add/rem mem region pravin.bathija
2026-05-14 2:01 ` [PATCH v13 3/5] vhost_user: support function defines for back-end pravin.bathija
2026-05-14 2:01 ` [PATCH v13 4/5] vhost_user: Function defs for add/rem mem regions pravin.bathija
2026-05-14 2:01 ` [PATCH v13 5/5] vhost_user: enable configure memory slots pravin.bathija
2026-05-16 2:55 ` [PATCH v14 00/11] net/sxe2: fix logic errors and address feedback liujie5
2026-05-16 2:55 ` [PATCH v14 01/11] mailmap: add Jie Liu liujie5
2026-05-16 2:55 ` [PATCH v14 02/11] doc: add sxe2 guide and release notes liujie5
2026-05-16 2:55 ` [PATCH v14 03/11] common/sxe2: add sxe2 basic structures liujie5
2026-05-16 2:55 ` [PATCH v14 04/11] drivers: add base driver skeleton liujie5
2026-05-16 2:55 ` [PATCH v14 05/11] drivers: add base driver probe skeleton liujie5
2026-05-16 2:55 ` [PATCH v14 06/11] drivers: support PCI BAR mapping liujie5
2026-05-16 2:55 ` [PATCH v14 07/11] common/sxe2: add ioctl interface for DMA map and unmap liujie5
2026-05-16 2:55 ` [PATCH v14 08/11] net/sxe2: support queue setup and control liujie5
2026-05-16 2:55 ` [PATCH v14 09/11] drivers: add data path for Rx and Tx liujie5
2026-05-16 2:55 ` [PATCH v14 10/11] net/sxe2: add vectorized " liujie5
2026-05-16 2:55 ` [PATCH v14 11/11] net/sxe2: implement Tx done cleanup liujie5
2026-05-16 7:46 ` [PATCH v15 00/11] net/sxe2: fix logic errors and address feedback liujie5
2026-05-16 7:46 ` [PATCH v15 01/11] mailmap: add Jie Liu liujie5
2026-05-16 7:46 ` [PATCH v15 02/11] doc: add sxe2 guide and release notes liujie5
2026-05-16 7:46 ` [PATCH v15 03/11] common/sxe2: add sxe2 basic structures liujie5
2026-05-16 7:46 ` [PATCH v15 04/11] drivers: add base driver skeleton liujie5
2026-05-16 7:46 ` [PATCH v15 05/11] drivers: add base driver probe skeleton liujie5
2026-05-16 7:46 ` [PATCH v15 06/11] drivers: support PCI BAR mapping liujie5
2026-05-16 7:46 ` [PATCH v15 07/11] common/sxe2: add ioctl interface for DMA map and unmap liujie5
2026-05-16 7:46 ` [PATCH v15 08/11] net/sxe2: support queue setup and control liujie5
2026-05-16 7:46 ` [PATCH v15 09/11] drivers: add data path for Rx and Tx liujie5
2026-05-16 7:46 ` [PATCH v15 10/11] net/sxe2: add vectorized " liujie5
2026-05-16 7:46 ` [PATCH v15 11/11] net/sxe2: implement Tx done cleanup liujie5
2026-05-18 9:13 ` [PATCH v16 00/11] net/sxe2: fix logic errors and address feedback liujie5
2026-05-18 9:13 ` [PATCH v16 01/11] mailmap: add Jie Liu liujie5
2026-05-18 9:13 ` [PATCH v16 02/11] doc: add sxe2 guide and release notes liujie5
2026-05-18 9:13 ` [PATCH v16 03/11] common/sxe2: add sxe2 basic structures liujie5
2026-05-18 9:13 ` [PATCH v16 04/11] drivers: add base driver skeleton liujie5
2026-05-18 9:13 ` [PATCH v16 05/11] drivers: add base driver probe skeleton liujie5
2026-05-18 9:14 ` [PATCH v16 06/11] drivers: support PCI BAR mapping liujie5
2026-05-18 9:14 ` [PATCH v16 07/11] common/sxe2: add ioctl interface for DMA map and unmap liujie5
2026-05-18 9:14 ` [PATCH v16 08/11] net/sxe2: support queue setup and control liujie5
2026-05-18 9:14 ` [PATCH v16 09/11] drivers: add data path for Rx and Tx liujie5
2026-05-18 9:14 ` [PATCH v16 10/11] net/sxe2: add vectorized " liujie5
2026-05-18 9:14 ` [PATCH v16 11/11] net/sxe2: implement Tx done cleanup liujie5
2026-05-19 3:01 ` [PATCH v17 00/11] net/sxe2: fix logic errors and address feedback liujie5
2026-05-19 3:01 ` [PATCH v17 01/11] mailmap: add Jie Liu liujie5
2026-05-19 3:01 ` [PATCH v17 02/11] doc: add sxe2 guide and release notes liujie5
2026-05-19 3:01 ` [PATCH v17 03/11] common/sxe2: add sxe2 basic structures liujie5
2026-05-19 3:01 ` [PATCH v17 04/11] drivers: add base driver skeleton liujie5
2026-05-19 3:01 ` [PATCH v17 05/11] drivers: add base driver probe skeleton liujie5
2026-05-19 3:01 ` [PATCH v17 06/11] drivers: support PCI BAR mapping liujie5
2026-05-19 3:01 ` [PATCH v17 07/11] common/sxe2: add ioctl interface for DMA map and unmap liujie5
2026-05-19 17:41 ` Stephen Hemminger
2026-05-19 3:01 ` [PATCH v17 08/11] net/sxe2: support queue setup and control liujie5
2026-05-19 3:01 ` [PATCH v17 09/11] drivers: add data path for Rx and Tx liujie5
2026-05-19 3:01 ` [PATCH v17 10/11] net/sxe2: add vectorized " liujie5
2026-05-19 3:01 ` [PATCH v17 11/11] net/sxe2: implement Tx done cleanup liujie5
2026-05-19 14:47 ` [PATCH v18 00/11] net/sxe2: fix logic errors and address feedback liujie5
2026-05-19 14:48 ` [PATCH v18 01/11] mailmap: add Jie Liu liujie5
2026-05-19 14:48 ` [PATCH v18 02/11] doc: add sxe2 guide and release notes liujie5
2026-05-19 14:48 ` [PATCH v18 03/11] common/sxe2: add sxe2 basic structures liujie5
2026-05-19 14:48 ` [PATCH v18 04/11] drivers: add base driver skeleton liujie5
2026-05-19 14:48 ` [PATCH v18 05/11] drivers: add base driver probe skeleton liujie5
2026-05-19 14:48 ` [PATCH v18 06/11] drivers: support PCI BAR mapping liujie5
2026-05-19 14:48 ` [PATCH v18 07/11] common/sxe2: add ioctl interface for DMA map and unmap liujie5
2026-05-19 14:48 ` [PATCH v18 08/11] net/sxe2: support queue setup and control liujie5
2026-05-19 14:48 ` [PATCH v18 09/11] drivers: add data path for Rx and Tx liujie5
2026-05-19 14:48 ` [PATCH v18 10/11] net/sxe2: add vectorized " liujie5
2026-05-19 14:48 ` [PATCH v18 11/11] net/sxe2: implement Tx done cleanup liujie5
2026-05-20 2:17 ` [PATCH v19 00/11]net/sxe2: fix logic errors and address feedback liujie5
2026-05-20 2:17 ` [PATCH v19 01/11] mailmap: add Jie Liu liujie5
2026-05-20 2:18 ` [PATCH v19 02/11] doc: add sxe2 guide and release notes liujie5
2026-05-20 2:18 ` [PATCH v19 03/11] common/sxe2: add sxe2 basic structures liujie5
2026-05-20 2:18 ` [PATCH v19 04/11] drivers: add base driver skeleton liujie5
2026-05-20 2:18 ` [PATCH v19 05/11] drivers: add base driver probe skeleton liujie5
2026-05-20 2:18 ` [PATCH v19 06/11] drivers: support PCI BAR mapping liujie5
2026-05-20 2:18 ` [PATCH v19 07/11] common/sxe2: add ioctl interface for DMA map and unmap liujie5
2026-05-20 2:18 ` [PATCH v19 08/11] net/sxe2: support queue setup and control liujie5
2026-05-20 2:18 ` [PATCH v19 09/11] drivers: add data path for Rx and Tx liujie5
2026-05-20 2:18 ` [PATCH v19 10/11] net/sxe2: add vectorized " liujie5
2026-05-20 2:18 ` [PATCH v19 11/11] net/sxe2: implement Tx done cleanup liujie5
2026-05-20 15:38 ` [PATCH v19 00/11]net/sxe2: fix logic errors and address feedback Stephen Hemminger
2026-05-20 0:32 ` [PATCH v18 00/11] net/sxe2: " Stephen Hemminger
2026-05-19 14:16 ` [PATCH v17 " Stephen Hemminger
2026-05-18 17:25 ` Stephen Hemminger [this message]
2026-05-18 0:02 ` [PATCH v15 " Stephen Hemminger
2026-05-18 17:13 ` [PATCH v13 0/5] Support add/remove memory region and get-max-slots Stephen Hemminger
2026-05-20 2:36 ` Bathija, Pravin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260518102511.3888a02c@phoenix.local \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=liujie5@linkdatatechnology.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox