Re: [V2 0/7] hinic3 change for support new SPx NIC

Stephen Hemminger
2026-03-16T15:46:00Z

On Mon, 16 Mar 2026 21:43:22 +0800
Feifei Wang <wff_light@vip.163.com> wrote:

> From: chenyi221 <chenyi221@huawei.com>
>
> Change hinic3 driver to support Huawei new SPx series NIC.
>
> v2:
> --fix build issues
>
> Feifei Wang (7):
> net/hinic3: add support for new SPx series NIC
> net/hinic3: add enhance cmdq support for new SPx series NIC
> net/hinic3: use different callback func to split new/old cmdq
> operations
> net/hinic3: add fun init ops to support Compact CQE
> net/hinic3: add rx ops to support Compact CQE
> net/hinic3: add tx ops to support Compact CQE
> net/hinic3: use different callback func to support htn fdir
>
> drivers/net/hinic3/base/hinic3_cmd.h | 80 ++-
> drivers/net/hinic3/base/hinic3_cmdq.c | 370 ++++------
> drivers/net/hinic3/base/hinic3_cmdq.h | 112 ++-
> drivers/net/hinic3/base/hinic3_cmdq_enhance.c | 111 +++
> drivers/net/hinic3/base/hinic3_cmdq_enhance.h | 125 ++++
> drivers/net/hinic3/base/hinic3_csr.h | 18 +-
> drivers/net/hinic3/base/hinic3_hw_comm.c | 15 +-
> drivers/net/hinic3/base/hinic3_hw_comm.h | 31 +-
> drivers/net/hinic3/base/hinic3_hwdev.c | 13 +-
> drivers/net/hinic3/base/hinic3_hwdev.h | 18 +
> drivers/net/hinic3/base/hinic3_hwif.c | 10 +-
> drivers/net/hinic3/base/hinic3_mgmt.c | 5 +-
> drivers/net/hinic3/base/hinic3_mgmt.h | 2 +
> drivers/net/hinic3/base/hinic3_nic_cfg.c | 182 ++---
> drivers/net/hinic3/base/hinic3_nic_cfg.h | 98 ++-
> drivers/net/hinic3/base/meson.build | 1 +
> drivers/net/hinic3/hinic3_ethdev.c | 280 ++++++--
> drivers/net/hinic3/hinic3_ethdev.h | 120 ++--
> drivers/net/hinic3/hinic3_fdir.c | 657 +++++++++++++-----
> drivers/net/hinic3/hinic3_fdir.h | 361 ++++++++--
> drivers/net/hinic3/hinic3_nic_io.c | 525 ++++++--------
> drivers/net/hinic3/hinic3_nic_io.h | 163 ++++-
> drivers/net/hinic3/hinic3_rx.c | 265 +++++--
> drivers/net/hinic3/hinic3_rx.h | 182 ++++-
> drivers/net/hinic3/hinic3_tx.c | 458 ++++++------
> drivers/net/hinic3/hinic3_tx.h | 154 +++-
> .../net/hinic3/htn_adapt/hinic3_htn_cmdq.c | 167 +++++
> .../net/hinic3/htn_adapt/hinic3_htn_cmdq.h | 63 ++
> drivers/net/hinic3/htn_adapt/meson.build | 7 +
> drivers/net/hinic3/meson.build | 8 +-
> .../net/hinic3/stn_adapt/hinic3_stn_cmdq.c | 151 ++++
> .../net/hinic3/stn_adapt/hinic3_stn_cmdq.h | 46 ++
> drivers/net/hinic3/stn_adapt/meson.build | 7 +
> 33 files changed, 3362 insertions(+), 1443 deletions(-)
> create mode 100644 drivers/net/hinic3/base/hinic3_cmdq_enhance.c
> create mode 100644 drivers/net/hinic3/base/hinic3_cmdq_enhance.h
> create mode 100644 drivers/net/hinic3/htn_adapt/hinic3_htn_cmdq.c
> create mode 100644 drivers/net/hinic3/htn_adapt/hinic3_htn_cmdq.h
> create mode 100644 drivers/net/hinic3/htn_adapt/meson.build
> create mode 100644 drivers/net/hinic3/stn_adapt/hinic3_stn_cmdq.c
> create mode 100644 drivers/net/hinic3/stn_adapt/hinic3_stn_cmdq.h
> create mode 100644 drivers/net/hinic3/stn_adapt/meson.build
>
> --
> 2.45.1.windows.1
>

Still lots of things that need to be addressed here.
See the following AI patch review.

Patch 1/7: net/hinic3: add support for new SPx series NIC

HINIC3_DEV_ID_SP230 is defined as 0X0229 with an uppercase X.
Every other hex constant in the file uses lowercase 0x. Should be
0x0229.

HINIC3_DEV_ID_SP920 has no VF counterpart and no entry in
hinic3_is_vf_dev(). Is that intentional?

Patch 2/7: net/hinic3: add enhance cmdq support

The unified cmdq_sync_cmd() retains rte_smp_rmb() which is a
deprecated barrier. Since this function is being rewritten, please
convert to rte_atomic_thread_fence(rte_memory_order_acquire).

HINIC3_CMDQ_BUF_SIZE changed from 2048 to 1024 without any
explanation in the commit message. If existing normal-cmdq commands
relied on the larger buffer this could silently truncate them.

Patch 3/7: net/hinic3: use different callback func to split new/old cmdq operations

The call sites in hinic3_ethdev.c use hinic3_cmdq_get_stn_ops()
and hinic3_cmdq_get_htn_ops() but the actual definitions are
hinic3_nic_cmdq_get_stn_ops() and hinic3_nic_cmdq_get_htn_ops().
This will not link. Patch 4 fixes the call site but each commit
must compile independently.

In htn_adapt/hinic3_htn_cmdq.c the static function
prepare_rss_indir_table_cmd_header() is called before it is
defined, with no forward declaration. This will fail with
-Werror=implicit-function-declaration.

Patch 4/7: net/hinic3: add fun init ops to support Compact CQE

In hinic3_rx_queue_setup(), when ci_mz allocation fails the code
calls hinic3_memzone_free(ci_mz) on a NULL pointer before jumping
to the error label. This call is either a NULL dereference or dead
code and should be removed.

The error-path goto labels in hinic3_func_init() are misordered.
The allocation order is mac_addrs, cmdq_ops, rx_ops, tx_ops,
mc_list but the cleanup labels do not reverse this properly. For
example if cmdq_ops allocation fails, mac_addrs is never freed.
If tx_ops fails, neither cmdq_ops nor mac_addrs are freed. This
leaks memory on every init failure path.

The ternary in hinic3_pf_get_default_cos() assigns
HINIC3_COS_NUM_MAX (8) when NIC_F_HTN_CMDQ is set and
HINIC3_COS_NUM_MAX_HTN (4) when it is not. The naming suggests
this is backwards. Please verify.

Patch 5/7: net/hinic3: add rx ops to support Compact CQE

hinic3_poll_integrated_cqe_rq_empty() uses __atomic_load_n() with
__ATOMIC_ACQUIRE. New code must use rte_atomic_load_explicit()
with rte_memory_order_acquire instead of the GCC built-in.

Patch 7/7: net/hinic3: use different callback func to support htn fdir

Copy-paste bug in hinic3_rss_hash_update() and hinic3_rss_conf_get().
In both functions the else branch has:

rss_type.ipv6_ext = 0;
rss_type.ipv6_ext = 0;

The second line should be rss_type.tcp_ipv6_ext = 0. As written
tcp_ipv6_ext is never cleared on non-HTN hardware which will
produce incorrect RSS configuration.