Re: [PATCH 1/3] net/iavf: support qinq insertion offload for scalar path

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

On Thu, Jun 19, 2025 at 01:36:56PM +0000, Ciara Loftus wrote:
> Enable Tx QINQ offload if the VF reports support for inserting both an
> outer and inner VLAN tag. The VF capabilities report the locations for
> placing each of the tags - either L2TAG1 in the tx descriptor or L2TAG2
> in the context descriptor. Use this information to configure the
> descriptors correctly.
> This offload was previously incorrectly reported as always supported in
> the device configuration, so this is corrected.
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  doc/guides/nics/features/iavf.ini    |  1 +
>  drivers/net/intel/iavf/iavf_ethdev.c |  8 +++-
>  drivers/net/intel/iavf/iavf_rxtx.c   | 55 +++++++++++++++++++++-------
>  3 files changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/doc/guides/nics/features/iavf.ini b/doc/guides/nics/features/iavf.ini
> index ce9860e963..61c4742197 100644
> --- a/doc/guides/nics/features/iavf.ini
> +++ b/doc/guides/nics/features/iavf.ini
> @@ -29,6 +29,7 @@ Traffic manager      = Y
>  Inline crypto        = Y
>  CRC offload          = Y
>  VLAN offload         = P
> +QinQ offload         = P
>  L3 checksum offload  = Y
>  L4 checksum offload  = Y
>  Timestamp offload    = Y
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
> index b3dacbef84..d058b87d54 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -622,7 +622,7 @@ iavf_dev_vlan_insert_set(struct rte_eth_dev *dev)
>  		return 0;
>  
>  	enable = !!(dev->data->dev_conf.txmode.offloads &
> -		    RTE_ETH_TX_OFFLOAD_VLAN_INSERT);
> +		    (RTE_ETH_TX_OFFLOAD_VLAN_INSERT | RTE_ETH_TX_OFFLOAD_QINQ_INSERT));
>  	iavf_config_vlan_insert_v2(adapter, enable);
>  
>  	return 0;
> @@ -1158,7 +1158,6 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  
>  	dev_info->tx_offload_capa =
>  		RTE_ETH_TX_OFFLOAD_VLAN_INSERT |
> -		RTE_ETH_TX_OFFLOAD_QINQ_INSERT |
>  		RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |
>  		RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
>  		RTE_ETH_TX_OFFLOAD_TCP_CKSUM |
> @@ -1182,6 +1181,11 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_PTP)
>  		dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TIMESTAMP;
>  
> +	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2 &&
> +			vf->vlan_v2_caps.offloads.insertion_support.inner &&
> +			vf->vlan_v2_caps.offloads.insertion_support.outer)
> +		dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_QINQ_INSERT;
> +
>  	if (iavf_ipsec_crypto_supported(adapter)) {
>  		dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_SECURITY;
>  		dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_SECURITY;
> diff --git a/drivers/net/intel/iavf/iavf_rxtx.c b/drivers/net/intel/iavf/iavf_rxtx.c
> index 5411eb6897..1ce9de0699 100644
> --- a/drivers/net/intel/iavf/iavf_rxtx.c
> +++ b/drivers/net/intel/iavf/iavf_rxtx.c
> @@ -797,17 +797,32 @@ iavf_dev_tx_queue_setup(struct rte_eth_dev *dev,
>  			&adapter->vf.vlan_v2_caps.offloads.insertion_support;
>  		uint32_t insertion_cap;
>  
> -		if (insertion_support->outer)
> -			insertion_cap = insertion_support->outer;
> -		else
> -			insertion_cap = insertion_support->inner;
> -
> -		if (insertion_cap & VIRTCHNL_VLAN_TAG_LOCATION_L2TAG1) {
> -			txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG1;
> -			PMD_INIT_LOG(DEBUG, "VLAN insertion_cap: L2TAG1");
> -		} else if (insertion_cap & VIRTCHNL_VLAN_TAG_LOCATION_L2TAG2) {
> -			txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2;
> -			PMD_INIT_LOG(DEBUG, "VLAN insertion_cap: L2TAG2");
> +		if (insertion_support->outer == VIRTCHNL_VLAN_UNSUPPORTED ||
> +				insertion_support->inner == VIRTCHNL_VLAN_UNSUPPORTED) {
> +			/* Only one insertion is supported. */
> +			if (insertion_support->outer)
> +				insertion_cap = insertion_support->outer;
> +			else
> +				insertion_cap = insertion_support->inner;
> +
> +			if (insertion_cap & VIRTCHNL_VLAN_TAG_LOCATION_L2TAG1) {
> +				txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG1;
> +				PMD_INIT_LOG(DEBUG, "VLAN insertion_cap: L2TAG1");
> +			} else if (insertion_cap & VIRTCHNL_VLAN_TAG_LOCATION_L2TAG2) {
> +				txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2;
> +				PMD_INIT_LOG(DEBUG, "VLAN insertion_cap: L2TAG2");

Is this scenario possible - we only support insertion of a single vlan tag,
and it's to go in the context descriptor. In this case, do we not need to
add in the VLAN offload flag to the list of offloads needing a context
descriptor below?

> +			}
> +		} else {
> +			 /* Both outer and inner insertion supported. */
> +			if (insertion_support->inner & VIRTCHNL_VLAN_TAG_LOCATION_L2TAG1) {
> +				txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG1;
> +				PMD_INIT_LOG(DEBUG, "Inner VLAN insertion_cap: L2TAG1");
> +				PMD_INIT_LOG(DEBUG, "Outer VLAN insertion_cap: L2TAG2");
> +			} else {
> +				txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2;
> +				PMD_INIT_LOG(DEBUG, "Inner VLAN insertion_cap: L2TAG2");
> +				PMD_INIT_LOG(DEBUG, "Outer VLAN insertion_cap: L2TAG1");
> +			}
>  		}
>  	} else {
>  		txq->vlan_flag = IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG1;
> @@ -2391,7 +2406,7 @@ iavf_calc_context_desc(struct rte_mbuf *mb, uint8_t vlan_flag)
>  	uint64_t flags = mb->ol_flags;
>  	if (flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG |
>  	    RTE_MBUF_F_TX_TUNNEL_MASK | RTE_MBUF_F_TX_OUTER_IP_CKSUM |
> -	    RTE_MBUF_F_TX_OUTER_UDP_CKSUM))
> +	    RTE_MBUF_F_TX_OUTER_UDP_CKSUM | RTE_MBUF_F_TX_QINQ))
>  		return 1;

In this check, do we need to take into account possible case for only a
single vlan offload, and requiring an L2TAG2 field for it?

/Bruce