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: 90+ 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-21 15:16 ` Thomas Monjalon
2026-05-26 14:13 ` Thomas Monjalon
2026-06-01 9:51 ` Thomas Monjalon
2026-06-01 10:14 ` liujie5
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.