|SUCCESS| [v2] net/hns3: fix L4 checksum incorrect for tunnel packets

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

Reviewing patch: net/hns3: fix L4 checksum incorrect for tunnel packets

Commit: 6393fc0b823c ("net/hns3: simplify hardware checksum offloading")
Author: Xingui Yang <yangxingui at huawei.com>
Subject: [PATCH v2] net/hns3: fix L4 checksum incorrect for tunnel packets

This patch fixes incorrect L4 checksum calculation for tunnel packets in
HIP09 simple BD mode by including outer header lengths when calculating
the L4_START position.

> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> index 573604b0cd..4d48cbdc11 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -3982,6 +3982,7 @@ hns3_handle_simple_bd(struct hns3_tx_queue *txq, struct hns3_desc *desc,
>  {
>  #define HNS3_TCP_CSUM_OFFSET	16
>  #define HNS3_UDP_CSUM_OFFSET	6
> +	uint32_t l4_start;
>  
>  	/*
>  	 * In HIP09, NIC HW support Tx simple BD mode that the HW will

[ ... ]

> @@ -4003,9 +4004,13 @@ hns3_handle_simple_bd(struct hns3_tx_queue *txq, struct hns3_desc *desc,
>  	    ((m->ol_flags & RTE_MBUF_F_TX_L4_MASK) == RTE_MBUF_F_TX_TCP_CKSUM ||
>  	     (m->ol_flags & RTE_MBUF_F_TX_L4_MASK) == RTE_MBUF_F_TX_UDP_CKSUM)) {
>  		/* set checksum start and offset, defined in 2 Bytes */
> +		l4_start = m->l2_len + m->l3_len;
> +		if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
> +			l4_start += m->outer_l2_len + m->outer_l3_len;

Does this code handle uint8_t overflow? The mbuf fields l2_len, l3_len,
outer_l2_len and outer_l3_len are all uint8_t types, and their sum could
exceed 255 bytes for deeply nested tunnel packets. Does assigning their
sum to uint32_t l4_start prevent overflow during the addition itself?

Should l4_start be cast to wider types during intermediate calculations to
prevent truncation before assignment?

> +
>  		hns3_set_field(desc->tx.type_cs_vlan_tso_len,
>  			       HNS3_TXD_L4_START_M, HNS3_TXD_L4_START_S,
> -			       (m->l2_len + m->l3_len) >> HNS3_SIMPLE_BD_UNIT);
> +			       l4_start >> HNS3_SIMPLE_BD_UNIT);
>  		hns3_set_field(desc->tx.ol_type_vlan_len_msec,
>  			   HNS3_TXD_L4_CKS_OFFSET_M, HNS3_TXD_L4_CKS_OFFSET_S,
>  			   (m->ol_flags & RTE_MBUF_F_TX_L4_MASK) ==

The patch correctly addresses the tunnel packet checksum calculation issue
by accounting for outer header lengths. The logic is sound, but there is a
potential arithmetic overflow concern with uint8_t fields.