<div data-ntes="ntes_mail_body_root" style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div id="spnEditorContent"><div style="margin: 0;">Hi Stephen,</div><div style="margin: 0;"><br></div><div style="margin: 0;">Thanks for the review. All points will be addressed in v2: the real</div><div style="margin: 0;">bug fixes have been split out and moved ahead with Fixes tags, the doc</div><div style="margin: 0;">gaps are filled, and the tunables that made the burst functions spin are</div><div style="margin: 0;">dropped.</div><div style="margin: 0;"><br></div><div style="margin: 0;"> At 2026-10-11 02:30:00, "Stephen Hemminger" <stephen@networkplumber.org> wrote:</div><div style="margin: 0;"> >On Sat, 10 Oct 2026 22:44:51 +0800</div><div style="margin: 0;">>Jiacheng Ye <yejiacheng4@h-partners.com> wrote:</div><div style="margin: 0;">></div><div style="margin: 0;">>> This series fixes offload capability advertisement and cmdq/compact CQE</div><div style="margin: 0;">>> configuration, and adds FEC, CIR drop statistics, VF LAG/link state, COS</div><div style="margin: 0;">>> mask/map, mbox recording and telemetry, RSS hash configuration, compact CQE</div><div style="margin: 0;">>> Rx/Tx paths, TSO segment expansion, pseudo-header checksum, per-tile cmdq</div><div style="margin: 0;">>> ops, SP560 NIC support, and device kvargs tunables.</div><div style="margin: 0;">>></div><div style="margin: 0;">>> Signed-off-by: Jiacheng Ye <yejiacheng4@h-partners.com></div><div style="margin: 0;">></div><div style="margin: 0;">></div><div style="margin: 0;">>Large patch series, the partial and not complete list of problems identified</div><div style="margin: 0;">>by AI review are:</div><div style="margin: 0;">></div><div style="margin: 0;">>Review: [PATCH v1 00/17] net/hinic3 updates (Jiacheng Ye)</div><div style="margin: 0;">></div><div style="margin: 0;">>Applied to main (d129414) with git am --3way, all 17 apply cleanly.</div><div style="margin: 0;">>Per-commit build with -Dwerror=true passes for every patch.</div><div style="margin: 0;">></div><div style="margin: 0;">>Series summary</div><div style="margin: 0;">>--------------</div><div style="margin: 0;">></div><div style="margin: 0;">>This mixes bug fixes, new hardware (SP560), new features, and a</div><div style="margin: 0;">>rewrite of both data paths. Several real fixes are buried inside</div><div style="margin: 0;">>feature patches and need to move to the front with Fixes tags:</div><div style="margin: 0;">></div><div style="margin: 0;">>  - duplicate TAILQ_REMOVE in hinic3_flow_destroy() (09)</div><div style="margin: 0;">>  - VXLAN VNI spec written into key_mask (09)</div><div style="margin: 0;">>  - NULL deref of hwdev->dev_handle in set_vport_enable (06)</div><div style="margin: 0;">>  - secondary process never got dev_ops (14)</div><div style="margin: 0;">>  - HINIC3_DEV_ID_VF_SP230 changed 0x3750 -> 0x022a (10)</div><div style="margin: 0;"><br></div><div style="margin: 0;">[Jiacheng] Accepted. These five fixes will be split into standalone patches</div><div style="margin: 0;">and moved to the front, each with a Fixes tag.</div><div style="margin: 0;">></div><div style="margin: 0;">>No documentation beyond release notes. hinic3.rst needs SP560 and</div><div style="margin: 0;">>the new devargs; features/hinic3.ini needs FEC, Burst mode info and</div><div style="margin: 0;">>the new rte_flow items (geneve, vxlan_gpe).</div><div style="margin: 0;"><br></div><div style="margin: 0;">[Jiacheng] Accepted. hinic3.rst now lists SP560 and documents the new</div><div style="margin: 0;">devargs, and hinic3.ini adds FEC, Burst mode info, geneve and vxlan_gpe.</div><div style="margin: 0;">></div><div style="margin: 0;">>Patch 09 uses nic_type (HINIC3_IS_SP230_NIC) but nic_type is only</div><div style="margin: 0;">>set in patch 10. Put the NIC type detection first.</div><div style="margin: 0;"><br></div><div style="margin: 0;">[Jiacheng] Accepted. NIC type detection is moved ahead of the flow</div><div style="margin: 0;">refactor.</div><div style="margin: 0;">></div><div style="margin: 0;">>Several new tunables (rx_empty_loop, tx_free_loop) make the burst</div><div style="margin: 0;">>functions spin. rx_burst/tx_burst must return immediately; spinning</div><div style="margin: 0;">>on empty CQ or full SQ is the application's choice, not the PMD's.</div><div style="margin: 0;"><br></div><div style="margin: 0;">[Jiacheng] Accepted. Both tunables and the spinning behavior are removed;</div><div style="margin: 0;">the Tx path reclaims once and returns the short count.</div><div style="margin: 0;">></div><div style="margin: 0;">>Errors</div><div style="margin: 0;">>------</div><div style="margin: 0;">></div><div style="margin: 0;">>[01/17] net/hinic3: fix offload and cmdq/compact CQE config</div><div style="margin: 0;">></div><div style="margin: 0;">>QINQ_STRIP and QINQ_INSERT are advertised but not implemented.</div><div style="margin: 0;">>Nothing in hinic3_tx.c reads vlan_tci_outer and nothing in Rx sets</div><div style="margin: 0;">>RTE_MBUF_F_RX_QINQ_STRIPPED. Adding HINIC3_PKT_TX_QINQ_PKT to</div><div style="margin: 0;">>HINIC3_TX_OFFLOAD_MASK only routes the packet down the offload path;</div><div style="margin: 0;">>the outer tag is never inserted. Drop both capabilities.</div><div style="margin: 0;">></div><div style="margin: 0;">>VXLAN_TNL_TSO is now advertised unconditionally, but</div><div style="margin: 0;">>hinic3_tx_offload_pkt_prepare() returns -EINVAL for VXLAN packets</div><div style="margin: 0;">>when !HINIC3_SUPPORT_VXLAN_OFFLOAD(). Gate it like the others:</div><div style="margin: 0;">></div><div style="margin: 0;">>  if (HINIC3_SUPPORT_VXLAN_OFFLOAD(nic_dev))</div><div style="margin: 0;">>      info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO;</div><div style="margin: 0;">></div><div style="margin: 0;">>in hinic3_dev_tnl_tso_support().</div><div style="margin: 0;">[Jiacheng] Accepted. Both QINQ capabilities are dropped. VXLAN_TNL_TSO is</div><div style="margin: 0;">now gated by HINIC3_SUPPORT_VXLAN_OFFLOAD().</div><div style="margin: 0;">></div><div style="margin: 0;">>[02/17] net/hinic3: add CIR drop statistics and VF LAG support</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_cmd_vf_lag() returns uint8_t but computes</div><div style="margin: 0;">>  bitmap[func_id / 32] & (1ULL << (func_id % 32))</div><div style="margin: 0;">>Bits 8..31 are truncated to 0, and the caller compares "== 1", so</div><div style="margin: 0;">>only func_id % 32 == 0 ever matches. Return a bool:</div><div style="margin: 0;">></div><div style="margin: 0;">>  return !!(vf_lag_info.vf_lag_bitmap.vf_bit_map[func_id / 32] &</div><div style="margin: 0;">>            RTE_BIT32(func_id % 32));</div><div style="margin: 0;">></div><div style="margin: 0;">>and test the result as a boolean in hinic3_tx_queue_setup().</div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">></div><div style="margin: 0;">>get_port_cir_drop() never sets xstats[i].id.</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_xstats_calc_num() now adds HINIC3_PHYPORT_XSTATS_NUM for VF,</div><div style="margin: 0;">>but hinic3_dev_xstats_get() and _get_names() still return before the</div><div style="margin: 0;">>phy port stats for VF. Count and returned entries disagree. Remove</div><div style="margin: 0;">>HINIC3_PHYPORT_XSTATS_NUM from the VF branch.</div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">></div><div style="margin: 0;">>[09/17] net/hinic3: refactor flow rule parsing</div><div style="margin: 0;">></div><div style="margin: 0;">>On SP620, HINIC3_FLOW_KIND_IPINIP is parsed by</div><div style="margin: 0;">>hinic3_flow_parse_fdir_filter(). That parser has no tunnel state:</div><div style="margin: 0;">>the inner IPV4/IPV6 item overwrites the outer key and tunnel_type</div><div style="margin: 0;">>stays NORMAL. The installed rule matches the inner addresses against</div><div style="margin: 0;">>the outer header of non-tunnel traffic. Reject IPIP on SP620 until</div><div style="margin: 0;">>it is parsed by the tunnel parser.</div><div style="margin: 0;">[Jiacheng] Accepted. On SP620, an IPIP rule would degrade into</div><div style="margin: 0;">a plain IP match, so rejecting it (returning NULL) is more correct.</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_flow_parse_action() now uses the last non-VOID action and</div><div style="margin: 0;">>ignores the rest. MARK/QUEUE, COUNT/QUEUE or DROP/QUEUE are accepted</div><div style="margin: 0;">>and silently become QUEUE. Require exactly one action:</div><div style="margin: 0;">></div><div style="margin: 0;">>  for (act = actions; act->type != END; act++) {</div><div style="margin: 0;">>      if (act->type == VOID)</div><div style="margin: 0;">>          continue;</div><div style="margin: 0;">>      if (found) return rte_flow_error_set(..., "Only one action");</div><div style="margin: 0;">>      found = act;</div><div style="margin: 0;">>  }</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. Exactly one non-VOID action is now required.</div><div style="margin: 0;">>[10/17] net/hinic3: add SP560 NIC type identification and handling</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_func_init() reads rq_wqe_type from</div><div style="margin: 0;">>/sys/module/hinic{3,5}/parameters and fails probe with -EINVAL if</div><div style="margin: 0;">>it is not there. With the device bound to vfio-pci the kernel driver</div><div style="margin: 0;">>is normally not loaded, so probe fails. A PMD must not take device</div><div style="margin: 0;">>configuration from another driver's module parameter. Use firmware</div><div style="margin: 0;">>feature bits (RX_HW/SW_COMPACT_CQE are already there) and the</div><div style="margin: 0;">>rx_cqe_compact_en devarg from patch 11; drop hinic3_read_module_param.</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_cmdq_get_ops() caches the result in a function static. The</div><div style="margin: 0;">>first probed device decides the ops for every device in the process;</div><div style="margin: 0;">>SP230 (HTN) plus SP620 in one system gets the wrong command format.</div><div style="margin: 0;">>Remove the cache:</div><div style="margin: 0;">></div><div style="margin: 0;">>  return cmdq_ops[nic_dev->nic_type];</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">>[11/17] net/hinic3: add device kvargs parameters support</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_nic_common_config_get() runs after hinic3_func_init(). On a</div><div style="margin: 0;">>parse error hinic3_dev_init() returns without undoing func_init, so</div><div style="margin: 0;">>hwdev, interrupts and queues leak. Parse devargs first, before any</div><div style="margin: 0;">>allocation.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>[12/17] net/hinic3: add compact CQE Rx path and ptype table</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_init_rx_ptype_table() is called from hinic3_func_init(),</div><div style="margin: 0;">>before hinic3_dev_init() parses devargs and calls</div><div style="margin: 0;">>hinic3_nic_feature_init(). config.rx_cqe_compact_en is still 0, so</div><div style="margin: 0;">>the normal-CQE table is always built and</div><div style="margin: 0;">>hinic3_recv_pkts_compact_cqe() indexes it with compact ptype values.</div><div style="margin: 0;">>Build the table after hinic3_nic_feature_init().</div><div style="margin: 0;">></div><div style="margin: 0;">>A table allocation failure is only logged; both Rx paths then</div><div style="margin: 0;">>dereference a NULL ptype_tbl. Fail probe instead.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">>[13/17] net/hinic3: expand TSO from 127 to 255 segments</div><div style="margin: 0;">></div><div style="margin: 0;">>wqe_info->last_cpy_mbuf_usable is set only in the overflow branch of</div><div style="margin: 0;">>hinic3_is_tso_sge_valid() and never cleared. wqe_info is declared</div><div style="margin: 0;">>once per burst, so every later packet that goes through the copy path</div><div style="margin: 0;">>has its last copy mbuf truncated. Clear it with sge_cnt and</div><div style="margin: 0;">>cpy_mbuf_cnt in hinic3_get_tx_offload():</div><div style="margin: 0;">></div><div style="margin: 0;">>  wqe_info->last_cpy_mbuf_usable = 0;</div><div style="margin: 0;">></div><div style="margin: 0;">>When the payload after the first 28 segments exceeds 227 * 4K, the</div><div style="margin: 0;">>copy path cuts the last copy mbuf to header + N * MSS and reports the</div><div style="margin: 0;">>packet as sent. The tail of the TCP payload is lost without any</div><div style="margin: 0;">>error. Treat this as invalid (return false) and count it.</div><div style="margin: 0;">[Jiacheng] Accepted. The oversized payload will be rejected and counted.</div><div style="margin: 0;">></div><div style="margin: 0;">>[16/17] net/hinic3: add per-tile prefetch/drop/CEQ cmdq ops</div><div style="margin: 0;">></div><div style="margin: 0;">>STN prepare_rq_ctxt_ceq_and_prefetch() sets pi_paddr_hi/lo to</div><div style="margin: 0;">>rq_ci_paddr for compact RQs, then hinic3_rq_prepare_ctxt()</div><div style="margin: 0;">>unconditionally overwrites both with pi_dma_addr after the call.</div><div style="margin: 0;">>Either the assignment is needed and compact RQ context is wrong, or</div><div style="margin: 0;">>it is dead code. Skip the pi_dma_addr assignment for</div><div style="margin: 0;">>HINIC3_COMPACT_RQ_WQE, or call the op last.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>[17/17] net/hinic3: add mbox count telemetry support</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_telemetry_info() walks every valid ethdev port and casts its</div><div style="margin: 0;">>dev_private to struct hinic3_nic_dev. Any other PMD in the process</div><div style="margin: 0;">>gets its private data dereferenced as hinic3. Check the driver</div><div style="margin: 0;">>before touching dev_private.</div><div style="margin: 0;">></div><div style="margin: 0;">>struct mbox_cnt_info is (32 + 16) * 4112, about 197KB, on the stack</div><div style="margin: 0;">>of the telemetry thread. Do not build an array at all; add each</div><div style="margin: 0;">>port to the dict as it is visited.</div><div style="margin: 0;">></div><div style="margin: 0;">>Better: drop the private telemetry endpoint and report the mbox</div><div style="margin: 0;">>counters as xstats, or implement eth_dev_ops.eth_dev_priv_dump</div><div style="margin: 0;">>(rte_eth_dev_priv_dump) which exists for this; the mbox history dump</div><div style="margin: 0;">>from patch 04 belongs there too.</div><div style="margin: 0;">[Jiacheng] Accepted. Implemented eth_dev_priv_dump() and moved the</div><div style="margin: 0;">mbox/AEQ history dump there.</div><div style="margin: 0;">></div><div style="margin: 0;">>Warnings</div><div style="margin: 0;">>--------</div><div style="margin: 0;">></div><div style="margin: 0;">>[03/17] "Fec param is valid, failed to set fec param." should say</div><div style="margin: 0;">>invalid. nic_dev->fec_mode is written and never read.</div><div style="margin: 0;">>hwdev->speed is only updated on link up, so fec_get_capability</div><div style="margin: 0;">>reports a stale speed after link down.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">>[04/17] "mbox_header = *((uint64_t *)data)" is an unaligned,</div><div style="margin: 0;">>aliasing read of a uint8_t array. Use memcpy().</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">>[05/17] cos_mask comes from firmware (uint8_t) and indexes</div><div style="margin: 0;">>cos_map[HINIC3_COS_NUM_MAX]. Bound it:</div><div style="margin: 0;">>  txq->cos = nic_dev->cos_map[txq->cos & (HINIC3_COS_NUM_MAX - 1)</div><div style="margin: 0;">>                              & nic_dev->cos_mask];</div><div style="margin: 0;">>The removal of the HTN cos limit and of "% HINIC3_COS_NUM_MAX_HTN"</div><div style="margin: 0;">>for VF is not explained in the commit message.</div><div style="margin: 0;">[Jiacheng] Accepted. The cos_mask index is now bounded with</div><div style="margin: 0;">(HINIC3_COS_NUM_MAX - 1), and the commit message explains the removed</div><div style="margin: 0;">HTN limit and VF modulo.</div><div style="margin: 0;">></div><div style="margin: 0;">>[06/17] link_status and vf_valid_status are now bitfields in one</div><div style="margin: 0;">>byte. link_status is written by the LSC interrupt thread and</div><div style="margin: 0;">>vf_valid_status by the control thread; the read-modify-write loses</div><div style="margin: 0;">>updates. Use two plain uint8_t.</div><div style="margin: 0;">></div><div style="margin: 0;">>hinic3_dev_set_link_up() on VF returns -EAGAIN when the PF carrier</div><div style="margin: 0;">>is down. set_link_up is the admin state; carrier down is reported</div><div style="margin: 0;">>through link_update, not as an error.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>[09/17] Patch 09 depends on nic_type from patch 10; at this commit</div><div style="margin: 0;">>SP230 gets the STN TCAM limit (2048). Reorder.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. </div><div style="margin: 0;">>GENEVE and VXLAN_GPE specs are cast to rte_flow_item_vxlan. The VNI</div><div style="margin: 0;">>offset happens to match, but masks on the other fields (geneve</div><div style="margin: 0;">>protocol and option length, gpe protocol) are silently ignored.</div><div style="margin: 0;">>Use the proper item types and reject unsupported mask fields.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. GENEVE and VXLAN_GPE use their proper item types and</div><div style="margin: 0;">unsupported mask fields are rejected.</div><div style="margin: 0;">>hinic3_flow_query() returns 0 without filling hits/bytes and does</div><div style="margin: 0;">>not check the action is COUNT; it only logs the rule at INFO. Remove</div><div style="margin: 0;">>.query until COUNT is supported.</div><div style="margin: 0;">[Jiacheng] .query is removed for now and will be added back once COUNT is supported.</div><div style="margin: 0;">></div><div style="margin: 0;">>[10/17] The SP230 VF device ID change (0x3750 -> 0x022a) drops</div><div style="margin: 0;">>support for the old ID. Separate fix patch with an explanation.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted. The VF device ID change is now a standalone fix</div><div style="margin: 0;">patch with an explanation.</div><div style="margin: 0;">>[11/17] No RTE_PMD_REGISTER_PARAM_STRING() for the new devargs and</div><div style="margin: 0;">>no documentation. strtol() result is not checked for trailing</div><div style="margin: 0;">>garbage and negative values go into unsigned fields.</div><div style="margin: 0;">[Jiacheng] Accepted. RTE_PMD_REGISTER_PARAM_STRING() and docs are added,</div><div style="margin: 0;">and strtol() now rejects trailing garbage and negative values.</div><div style="margin: 0;">></div><div style="margin: 0;">>Defaults change without mention: Rx CQE coalesce 63 -> 7, timer</div><div style="margin: 0;">>15 -> 8, Tx CI pending limit 3 -> 2, coalescing time 16 -> 2.</div><div style="margin: 0;">[Jiacheng] Documented. The default parameters are adjusted to reduce latency.</div><div style="margin: 0;">></div><div style="margin: 0;">>In a secondary process hinic3_dev_init() re-parses devargs into the</div><div style="margin: 0;">>shared nic_dev->config, overwriting the primary's settings. Skip it</div><div style="margin: 0;">>when not primary.</div><div style="margin: 0;">[Jiacheng] Accepted. Devargs are parsed only in the primary process so</div><div style="margin: 0;">secondary processes do not overwrite the primary's settings.</div><div style="margin: 0;">></div><div style="margin: 0;">>At this commit rx_cqe_compact_en defaults to 1 with no capability</div><div style="margin: 0;">>check (added in patch 12), so vport enable requests compact CQE on</div><div style="margin: 0;">>hardware without support.</div><div style="margin: 0;">[Jiacheng] Accepted. rx_cqe_compact_en is now only enabled when the</div><div style="margin: 0;">hardware advertises the compact CQE capability.</div><div style="margin: 0;">></div><div style="margin: 0;">>[12/17] rx_empty_loop makes hinic3_recv_pkts() spin re-reading the</div><div style="margin: 0;">>same CQE. Drop the devarg.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>Non-ASCII characters in C comments ("VXLAN ¡¢NVGRE ..."). Use ASCII.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Replaced with ASCII.</div><div style="margin: 0;">>[14/17] Many unrelated changes: secondary process dev_ops, SP230 VF</div><div style="margin: 0;">>software stats, CIR drop SP620 gating, cos_map SP230 gating,</div><div style="margin: 0;">>DEFAULT_DRV_FEATURE bit 27 (RX_SW_COMPACT_CQE), queue state reset in</div><div style="margin: 0;">>dev_stop, txq depth check, tx_burst_mode_get, new xstats. Split.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>tx_free_loop (default 1000) makes both xmit functions spin on a full</div><div style="margin: 0;">>SQ. Reclaim once and return the short count.</div><div style="margin: 0;">[Jiacheng] Accepted. Removed; the Tx path reclaims once and returns.</div><div style="margin: 0;">></div><div style="margin: 0;">>Both xmit functions break out of the burst when</div><div style="margin: 0;">>hinic3_get_tx_offload() fails. A bad packet then blocks the queue</div><div style="margin: 0;">>forever since the application retries it. Free it, count it, and</div><div style="margin: 0;">>continue.</div><div style="margin: 0;">[Jiacheng] Accepted. The offending packet is now freed, counted, and</div><div style="margin: 0;">the burst continues instead of breaking out.</div><div style="margin: 0;">></div><div style="margin: 0;">>[15/17] Pseudo-header checksums are computed inside tx_burst by</div><div style="margin: 0;">>writing into packet headers. That is what .tx_pkt_prepare is for</div><div style="margin: 0;">>(rte_net_intel_cksum_prepare() plus the IPv6 extension walk). As</div><div style="margin: 0;">>written it modifies data of shared/indirect mbufs and assumes all</div><div style="margin: 0;">>headers are in the first segment.</div><div style="margin: 0;">[Jiacheng] Accepted. Pseudo-header checksums moved to tx_pkt_prepare, and</div><div style="margin: 0;">shared/indirect mbufs skip the offload to avoid modifying shared data.</div><div style="margin: 0;">></div><div style="margin: 0;">>Outer IPv4 UDP checksum is only written if the application left it</div><div style="margin: 0;">>nonzero. RTE_MBUF_F_TX_OUTER_UDP_CKSUM means compute it.</div><div style="margin: 0;">[Jiacheng] Accepted. The outer UDP checksum is now always computed.</div><div style="margin: 0;">></div><div style="margin: 0;">>Info</div><div style="margin: 0;">>----</div><div style="margin: 0;">></div><div style="margin: 0;">>[03/17] HINIC3_FEC_MODE_* equal RTE_ETH_FEC_MODE_CAPA_MASK() values;</div><div style="margin: 0;">>use the ethdev macros. Pass NULL, not 0, for the unused pointer.</div><div style="margin: 0;">>Typo "covert".</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>[04/17] Storing the constant PCI address in every mbox record is</div><div style="margin: 0;">>redundant. Dumping all AEQ entries at ERR on each timeout floods</div><div style="margin: 0;">>the log.</div><div style="margin: 0;">[Jiacheng] Accepted. The redundant fields are removed and the full AEQ</div><div style="margin: 0;">dump only runs from eth_dev_priv_dump.</div><div style="margin: 0;">></div><div style="margin: 0;">>[08/17] rss_type is saved even when hinic3_set_rss_type() fails.</div><div style="margin: 0;">>Compact CQE sets the RSS hash flag on every packet whenever any RSS</div><div style="margin: 0;">>type is configured.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">>[09/17] vlan_flag is no longer set in the TCAM key, so rules now</div><div style="margin: 0;">>match tagged traffic as well. Mention it in the commit message.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">>[12/17] On SP230 the Rx interrupt vectors are set up even when</div><div style="margin: 0;">>intr_conf.rxq is 0; not explained. dev_supported_ptypes_get misses</div><div style="margin: 0;">>L2_ETHER, L4_FRAG, L4_ICMP that the table produces.</div><div style="margin: 0;">></div><div style="margin: 0;">[Jiacheng] On SP230 the device asserts the Rx MSI-X vector during</div><div style="margin: 0;">aggregation, so the vectors are set up with coalescing/auto-mask to have</div><div style="margin: 0;">those assertions masked instead of preempting a core. The missing</div><div style="margin: 0;">L2_ETHER, L4_FRAG and L4_ICMP ptypes are now returned.</div><div style="margin: 0;">>[13/17] Comment says "SP230 is 9600, SP560/620 is 4K"; the code</div><div style="margin: 0;">>uses 65536 for SP230 and 9600 otherwise.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>[14/17] tx_burst_mode_get stores txmode.offloads in uint16_t and does</div><div style="margin: 0;">>not say which Tx function (compact or normal) is active, which is</div><div style="margin: 0;">>the useful part. rte_eth_dev_attach_secondary() in func_init is</div><div style="margin: 0;">>redundant; probe already attached. "rte_prefetch0(++tx_pkts)"</div><div style="margin: 0;">>prefetches the array slot, not the mbuf.</div><div style="margin: 0;">[Jiacheng] Accepted.</div><div style="margin: 0;">></div><div style="margin: 0;">>Leftovers:</div><div style="margin: 0;">>  is_sp230_pci_dev() declared, never defined</div><div style="margin: 0;">>  HINIC3_MAX_QUEUE_DEPTH defined in both hinic3_ethdev.h and .c</div><div style="margin: 0;">>  HINIC3_QUEUE_STAT_CNTRS, HINIC3_RSS_QUEUE_BUF, IPV4_VERSION,</div><div style="margin: 0;">>  IPV6_VERSION unused</div><div style="margin: 0;">>  struct hinic3_tx_offload_info unused</div><div style="margin: 0;">>  txq->tx_free_loop set, never read</div><div style="margin: 0;">>  service_cap.port_cos_valid_bitmap unused</div><div style="margin: 0;">>  hinic3_ethertype_filter.flags never set</div><div style="margin: 0;">[Jiacheng] Accepted. All of these leftovers are cleaned up.</div><div style="margin: 0;"><br></div><div style="margin: 0;">Thanks again for the review.</div><div style="margin: 0;"><br></div><div style="margin: 0;">Regards,</div><div style="margin: 0;">Jiacheng</div></div></div>