|SUCCESS| [v4,1/1] net/hinic3: Fix VXLAN TSO issue

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164426

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-26

I'll review this DPDK patch for correctness, coding style, and guideline compliance.

commit 66F88402AC (based on patch metadata)
Author: Feifei Wang <wangfeifei40 at huawei.com>
Subject: [v4 1/1] net/hinic3: Fix VXLAN TSO issue

This patch adds the missing RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO flag bit
when VXLAN offload is supported, and removes a conditional check that
was preventing tunnel TSO support from being advertised.

> diff --git a/drivers/net/hinic3/hinic3_ethdev.c b/drivers/net/hinic3/hinic3_ethdev.c
> index f4eb788..4776bc1 100644
> --- a/drivers/net/hinic3/hinic3_ethdev.c
> +++ b/drivers/net/hinic3/hinic3_ethdev.c
> @@ -652,8 +652,12 @@ hinic3_dev_configure(struct rte_eth_dev *dev)
>  static void
>  hinic3_dev_tnl_tso_support(struct rte_eth_dev_info *info, struct hinic3_nic_dev *nic_dev)
>  {
> +	if (HINIC3_SUPPORT_VXLAN_OFFLOAD(nic_dev))
> +		info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO;
> +
>  	if (HINIC3_SUPPORT_GENEVE_OFFLOAD(nic_dev))
>  		info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO;
> +
>  	if (HINIC3_SUPPORT_IPXIP_OFFLOAD(nic_dev))
>  		info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPIP_TNL_TSO;
>  }

The new VXLAN support looks correct. However, there's unnecessary
whitespace added after the GENEVE_TNL_TSO line. According to the coding
style guidelines, avoid introducing extraneous blank lines for style
changes that aren't functionally required.

> @@ -698,7 +702,6 @@ hinic3_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
>  		RTE_ETH_TX_OFFLOAD_SCTP_CKSUM |
>  		RTE_ETH_TX_OFFLOAD_OUTER_IPV4_CKSUM |
>  		RTE_ETH_TX_OFFLOAD_TCP_TSO | RTE_ETH_TX_OFFLOAD_MULTI_SEGS;
> -	if (nic_dev->feature_cap & NIC_F_HTN_CMDQ)
>  		hinic3_dev_tnl_tso_support(info, nic_dev);

Does the removal of this "if (nic_dev->feature_cap & NIC_F_HTN_CMDQ)"
check affect hardware that doesn't have this capability? The call to
hinic3_dev_tnl_tso_support() is now unconditional. Can hardware without
NIC_F_HTN_CMDQ safely execute this function, or will it advertise
unsupported offload capabilities?

Looking at the function hinic3_dev_tnl_tso_support()->
HINIC3_SUPPORT_VXLAN_OFFLOAD(), is the NIC_F_HTN_CMDQ check already
performed within those support macros, or does this change break devices
that lack tunnel command queue support?