Re: [PATCH v3 00/27] net/hinic3: add PMD features and fixes

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 19 Oct 2026 23:19:02 +0800
Jiacheng Ye <yejiacheng4@h-partners.com> wrote:

> This series fixes offload capability advertisement, cmdq/compact CQE
> configuration, and several defects found during review, and adds NIC
> type identification, FEC, CIR drop statistics, VF LAG/link state, COS
> mask/map, mbox recording, RSS hash configuration, compact CQE Rx/Tx
> paths, TSO segment expansion, Tx burst mode reporting, queue statistics,
> pseudo-header checksum, per-tile cmdq ops, SP560 NIC support, and device
> kvargs tunables.
> 
> v3:
> - Move the ethertype-flow TCAM index leak fix out of the flow-refactor
>   patch into its own patch with a Fixes tag and a stable Cc tag.
> - Set burst function pointers and tx_pkt_prepare for both primary and
>   secondary processes, and select the Tx path on its own feature bit.
> - Zero the RQ/SQ context buffers and honour dp_intr_en for the STN CEQ
>   attribute in compact CQE mode.
> - Return the number of Tx packets consumed instead of the doorbell
>   count, and guard the Tx prefetch against overreading the array.
> - Validate device kvargs and reject values above the hardware field
>   width; document the real defaults and units.
> - Reject mismatched queue depths and reset saved depths in
>   dev_configure(), and size the TSO copy pool per port.
> - Report FEC capabilities per supported speed regardless of link state.
> - Route the private dump through fprintf() on the provided FILE*.
> - Derive the pseudo-header L4 protocol from the offload flags and fail
>   shared or indirect mbufs; add Fixes and stable Cc tags.
> - Drop stable Cc tags where the Fixes target is not in an LTS release
>   and remove dead code and unused definitions.

Still lots of AI feedback, biggest issue is the bisect break.
You should run semi-automated incremental builds on any large
patch series like this. CI does that.


Subject: Re: [PATCH 00/27] net/hinic3: fixes and new features

This is v3 of the series; please mark it as such in the subject, and
put a changelog in the cover letter so reviewers can see what changed.

Applies to main (26.11-rc1). Tip builds with -Dwerror=true. Most of
the v2 comments are addressed: Tx burst return value, prefetch, kvargs
error and bounds, secondary process burst setup, RQ/SQ context memset,
queue depth checks, per-port copy pool, priv_dump to FILE, TCAM leak
split out, Fixes/Cc tags.


Errors
======

Series: bisect broken at 20 through 26.

[PATCH 20/27] sets tx_pkt_prepare = hinic3_prep_pkts, which is not
defined until 27/27, and uses hinic3_xmit_pkts_compact_cqe, which
arrives in 22/27.
[PATCH 21/27] uses nic_dev->txq_depth, which is added in 23/27.

Every commit must build. Move 27/27 ahead of 20 (it is a Fixes: patch
anyway), select only hinic3_xmit_pkts in 20 and add the compact
choice in 22, and add txq_depth in 21 or move the pool sizing to 23.


[PATCH 08/27] net/hinic3: add SP230 Rx queue interrupt handling

On SP230 hinic3_init_rxq_intr() now runs the full Rx interrupt setup
even when intr_conf.rxq is 0. That path requires
rte_intr_cap_multiple(), so on igb_uio or uio_pci_generic dev_start
fails with -ENOTSUP for an application that never asked for
interrupts. It also creates eventfds and sets rxq->dp_intr_en = 1,
so the RQ context is programmed with the CEQ interrupt enabled.

Split the SP230 masking out of the interrupt setup:

	if (!dev->data->dev_conf.intr_conf.rxq) {
		if (HINIC3_IS_SP230_NIC(nic_dev))
			return hinic3_sp230_mask_rxq_msix(dev);
		return 0;
	}

where the helper only programs coalescing/auto-mask on vectors
i + HINIC3_RX_VEC_START, with no efd, no intr_vec and no dp_intr_en.


[PATCH 21/27] net/hinic3: expand TSO from 127 to 255 segments

hinic3_copy_tx_mbuf_multi() always copies from the start of the source
segment:

	rte_memcpy((uint8_t *)cur_mbuf->buf_addr + copy_offset,
		   (uint8_t *)mbuf->buf_addr + mbuf->data_off,
		   copy_size);

When a source segment spans two copy mbufs, the second copy repeats
the first bytes of that segment, and the packet goes out corrupted.
This was also in v2; I missed it there.

	uint32_t src_off = mbuf->data_len - remaining_len;

	rte_memcpy((uint8_t *)cur_mbuf->buf_addr + copy_offset,
		   rte_pktmbuf_mtod_offset(mbuf, uint8_t *, src_off),
		   copy_size);


Warnings
========

[PATCH 18/27] net/hinic3: add SP560 NIC type identification and
  handling

The STN op still forces EN=1 (interrupt disabled) with SW compact CQE,
and the commit marks Rx interrupt as "P". An application that sets
intr_conf.rxq still gets success from configure and start, and then
never gets an interrupt. Reject it:

	if (dev->data->dev_conf.intr_conf.rxq &&
	    nic_dev->config.rx_cqe_compact_en &&
	    HINIC3_SUPPORT_RX_SW_COMPACT_CQE(nic_dev))
		return -ENOTSUP;


[PATCH 21/27] net/hinic3: expand TSO from 127 to 255 segments

The copy pool is now num_sqs * txq_depth 4 KB mbufs, created at every
dev_start. With 16 queues of 4096 descriptors that is about 280 MB of
hugepage memory per port, for a slow path that only fires on packets
with too many segments. Cap it, for example:

	depth = nic_dev->num_sqs * RTE_MIN(nic_dev->txq_depth, 256u);


[PATCH 27/27] net/hinic3: add pseudo-header checksum calculation

For IPv6 the L4 offset still comes from walking extension headers,
while the descriptor (hinic3_get_tx_offload) uses mbuf->l3_len. When
the two disagree, software writes the checksum in one place and the
hardware computes it at another. mbuf->l3_len is the application's
contract and includes extension headers; use it, and drop
hinic3_get_ipv6_len_proto().


Info
====

[PATCH 27/27] "l4_proto = (uint32_t)proto << 24" is only correct on
little endian. The old rte_cpu_to_be_16(proto) was fine, since the
ones' complement sum ignores word position. Use
rte_cpu_to_be_32(proto).

[PATCH 27/27] Has Cc: stable, but it depends on NIC type
identification from 07/27. Put it right after 07, so the stable
backport is the pair.