Re: [PATCH 2/4] net/ice: add vector tunnel context encoding

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Aug 24, 2026 at 10:21:52AM +0000, Anurag Mandal wrote:
> Added helpers to encode tunnel context descriptors,
> and checksum offsets.
> 
> Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> ---

Some comments inline below.

/Bruce

>  drivers/net/intel/ice/ice_rxtx_vec_common.h | 53 ++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/intel/ice/ice_rxtx_vec_common.h b/drivers/net/intel/ice/ice_rxtx_vec_common.h
> index 1d83a087cc..b84456357f 100644
> --- a/drivers/net/intel/ice/ice_rxtx_vec_common.h
> +++ b/drivers/net/intel/ice/ice_rxtx_vec_common.h
> @@ -123,8 +123,12 @@ ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
>  
>  	/* Tx Checksum Offload */
>  	/* SET MACLEN */
> -	td_offset |= (tx_pkt->l2_len >> 1) <<
> -		CI_TX_DESC_LEN_MACLEN_S;
> +	if (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
> +		td_offset |= (tx_pkt->outer_l2_len >> 1) <<
> +			CI_TX_DESC_LEN_MACLEN_S;
> +	else
> +		td_offset |= (tx_pkt->l2_len >> 1) <<
> +			CI_TX_DESC_LEN_MACLEN_S;

I don't think these lines need to be split. It's copy-paste from iavf, but I'd
still adjust to be single-line.

>  
>  	/* Enable L3 checksum offload */
>  	if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
> @@ -172,4 +176,49 @@ ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
>  
>  	*txd_hi |= ((uint64_t)td_cmd) << CI_TXD_QW1_CMD_S;
>  }
> +
> +static inline uint64_t
> +ice_txd_tunneling_ctx(const struct rte_mbuf *tx_pkt)
> +{
> +	const uint64_t ol_flags = tx_pkt->ol_flags;
> +	uint64_t ctx = 0;
> +
> +	if (!(ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK))
> +		return 0;
> +
> +	if (ol_flags & RTE_MBUF_F_TX_OUTER_IP_CKSUM)
> +		ctx |= ICE_TX_CTX_EIPT_IPV4;
> +	else if (ol_flags & RTE_MBUF_F_TX_OUTER_IPV4)
> +		ctx |= ICE_TX_CTX_EIPT_IPV4_NO_CSUM;
> +	else if (ol_flags & RTE_MBUF_F_TX_OUTER_IPV6)
> +		ctx |= ICE_TX_CTX_EIPT_IPV6;
> +
> +	ctx |= (uint64_t)(tx_pkt->outer_l3_len >> 2) << ICE_TXD_CTX_QW0_EIPLEN_S;

Is this meant to be unconditionally encoded in the context?
Same with the l2_len below? Are we relying on the HW to ignore these values
if the flags for them are not set? Comparing against iavf, these are set in
the switch block per protocol.

> +
> +	switch (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) {
> +	case RTE_MBUF_F_TX_TUNNEL_IPIP:
> +		break;
> +	case RTE_MBUF_F_TX_TUNNEL_VXLAN:
> +	case RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE:
> +	case RTE_MBUF_F_TX_TUNNEL_GTP:
> +	case RTE_MBUF_F_TX_TUNNEL_GENEVE:
> +		ctx |= ICE_TXD_CTX_UDP_TUNNELING;
> +		break;
> +	case RTE_MBUF_F_TX_TUNNEL_GRE:
> +		ctx |= ICE_TXD_CTX_GRE_TUNNELING;
> +		break;
> +	default:
> +		PMD_TX_LOG(ERR, "Tunnel type not supported");
> +		return ctx;

Should this not return 0, rather than a half-completed ctx?

> +	}
> +
> +	ctx |= (uint64_t)(tx_pkt->l2_len >> 1) << ICE_TXD_CTX_QW0_NATLEN_S;
> +
> +	if ((ctx & ICE_TXD_CTX_QW0_EIPT_M) &&
> +			(ctx & ICE_TXD_CTX_UDP_TUNNELING) &&
> +			(ol_flags & RTE_MBUF_F_TX_OUTER_UDP_CKSUM))
> +		ctx |= ICE_TXD_CTX_QW0_L4T_CS_M;
> +
> +	return ctx;
> +}
>  #endif
> -- 
> 2.34.1
>