[PATCH 3/8] net/txgbe: fix packet type to parse from offload flags

[Ferruh Yigit]

On 1/18/2023 6:00 AM, Jiawen Wu wrote:
> In some external applications, developers may fill in wrong packet_type
> in rte_mbuf for transmission. It will result in Tx ring hang when Tx
> checksum offload is on. So change it to parse from ol_flags.
> 

Can you please give more information on what packet_type value is
causing problem in Tx path?

> Fixes: ca46fcd753b1 ("net/txgbe: support Tx with hardware offload")
> Cc: stable at dpdk.org
> 
> Signed-off-by: Jiawen Wu <jiawenwu at trustnetic.com>
> ---
>  drivers/net/txgbe/txgbe_rxtx.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/txgbe/txgbe_rxtx.c b/drivers/net/txgbe/txgbe_rxtx.c
> index ae70ca3beb..e91aaf60ef 100644
> --- a/drivers/net/txgbe/txgbe_rxtx.c
> +++ b/drivers/net/txgbe/txgbe_rxtx.c
> @@ -516,20 +516,21 @@ tx_desc_ol_flags_to_cmdtype(uint64_t ol_flags)
>  	return cmdtype;
>  }
>  
> -static inline uint8_t
> -tx_desc_ol_flags_to_ptid(uint64_t oflags, uint32_t ptype)
> +static inline uint32_t
> +tx_desc_ol_flags_to_ptype(uint64_t oflags)
>  {
> +	uint32_t ptype;
>  	bool tun;
>  
> -	if (ptype)
> -		return txgbe_encode_ptype(ptype);
> -
>  	/* Only support flags in TXGBE_TX_OFFLOAD_MASK */
>  	tun = !!(oflags & RTE_MBUF_F_TX_TUNNEL_MASK);
>  
>  	/* L2 level */
>  	ptype = RTE_PTYPE_L2_ETHER;
>  	if (oflags & RTE_MBUF_F_TX_VLAN)
> +		ptype |= (tun ? RTE_PTYPE_INNER_L2_ETHER_VLAN : RTE_PTYPE_L2_ETHER_VLAN);
> +
> +	if (oflags & RTE_MBUF_F_TX_QINQ) //tun + qinq is not supported

checkpatch is complaining about c99 comment syntax ('//').

>  		ptype |= RTE_PTYPE_L2_ETHER_VLAN;
>  
>  	/* L3 level */
> @@ -587,6 +588,14 @@ tx_desc_ol_flags_to_ptid(uint64_t oflags, uint32_t ptype)
>  		break;
>  	}
>  
> +	return ptype;
> +}
> +
> +static inline uint8_t
> +tx_desc_ol_flags_to_ptid(uint64_t oflags, uint32_t ptype)
> +{
> +	ptype = tx_desc_ol_flags_to_ptype(oflags);
> +

This function get 'ptype' as parameter and immediately overwrites is
with calculated value, what is the point of getting 'ptype' as argument.

>  	return txgbe_encode_ptype(ptype);
>  }
>  

Overall why 'ptype' is calculated for Tx path, I see this value is used
to see if it is tunneled packet or not, is there any other usage of
ptype in this path? If not why parse all packet types?