Re: [PATCH] net/iavf: add support for QinQ insertion

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

On Tue, May 05, 2026 at 05:58:27PM +0100, Mandal, Anurag wrote:
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: 05 May 2026 21:26
> > To: Mandal, Anurag <anurag.mandal@intel.com>
> > Cc: dev@dpdk.org; Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> > Subject: Re: [PATCH] net/iavf: add support for QinQ insertion
> > 
> > On Thu, Apr 30, 2026 at 08:58:13AM +0000, Anurag Mandal wrote:
> > > QinQ insertion with VLAN TPID 0x88a8 support is absent.
> > >
> > > This patch adds support for QinQ insertion for both Outer VLAN TPIDs
> > > 0x88a8 (802.1ad) & 0x8100 (802.1Q).
> > > VLAN Extend should be enabled for the same and Outer VLAN TPID should
> > > be set properly for TPID 0x88a8.
> > >
> > > Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> > 
> > In general, looks good. One query below.
> > 
> > > ---
> > >  doc/guides/nics/intel_vf.rst        | 19 ++++++++++
> > >  drivers/net/intel/iavf/iavf.h       |  1 +
> > >  drivers/net/intel/iavf/iavf_vchnl.c | 56
> > > +++++++++++++++++++++++++++--
> > >  3 files changed, 74 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/doc/guides/nics/intel_vf.rst
> > > b/doc/guides/nics/intel_vf.rst index dfff69b982..ce11ea6363 100644
> > > --- a/doc/guides/nics/intel_vf.rst
> > > +++ b/doc/guides/nics/intel_vf.rst
> > > @@ -802,3 +802,22 @@ To start ``testpmd``, and enable QinQ strip for
> > default TPID on port 0:
> > >
> > >     testpmd> vlan set extend on 0
> > >     testpmd> vlan set qinq_strip on 0
> > > +
> > > +QinQ Configuration
> > > +~~~~~~~~~~~~~~~~~~
> > > +
> > > +When using QinQ Tx offload (``RTE_ETH_TX_OFFLOAD_QINQ_INSERT``),
> > > +you must also enable ``RTE_ETH_RX_OFFLOAD_VLAN_EXTEND``
> > > +to configure the hardware for double VLAN mode.
> > > +Without this, only the inner VLAN tag will be inserted.
> > > +
> > > +Example::
> > > +
> > > +  struct rte_eth_conf port_conf = {
> > > +      .rxmode = {
> > > +          .offloads = RTE_ETH_RX_OFFLOAD_VLAN_EXTEND,
> > > +      },
> > > +      .txmode = {
> > > +          .offloads = RTE_ETH_TX_OFFLOAD_QINQ_INSERT,
> > > +      },
> > > +  };
> > > diff --git a/drivers/net/intel/iavf/iavf.h
> > > b/drivers/net/intel/iavf/iavf.h index 6a77dacf59..0b6ed221d0 100644
> > > --- a/drivers/net/intel/iavf/iavf.h
> > > +++ b/drivers/net/intel/iavf/iavf.h
> > > @@ -459,6 +459,7 @@ int iavf_get_supported_rxdid(struct iavf_adapter
> > > *adapter);  int iavf_config_vlan_strip_v2(struct iavf_adapter
> > > *adapter, bool enable);  int iavf_config_outer_vlan_strip_v2(struct
> > > iavf_adapter *adapter, bool enable);  int
> > > iavf_config_vlan_insert_v2(struct iavf_adapter *adapter, bool enable);
> > > +int iavf_config_outer_vlan_insert_v2(struct iavf_adapter *adapter,
> > > +bool enable);
> > >  int iavf_add_del_vlan_v2(struct iavf_adapter *adapter, uint16_t vlanid,
> > >  			 bool add);
> > >  int iavf_get_vlan_offload_caps_v2(struct iavf_adapter *adapter); diff
> > > --git a/drivers/net/intel/iavf/iavf_vchnl.c
> > > b/drivers/net/intel/iavf/iavf_vchnl.c
> > > index c2f340db81..6dd7d455ba 100644
> > > --- a/drivers/net/intel/iavf/iavf_vchnl.c
> > > +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> > > @@ -920,6 +920,51 @@ iavf_config_vlan_strip_v2(struct iavf_adapter
> > *adapter, bool enable)
> > >  	return ret;
> > >  }
> > >
> > > +int
> > > +iavf_config_outer_vlan_insert_v2(struct iavf_adapter *adapter, bool
> > > +enable) {
> > > +	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> > > +	struct virtchnl_vlan_supported_caps *insertion_caps;
> > > +	struct virtchnl_vlan_setting vlan_insert;
> > > +	uint8_t msg_buf[IAVF_AQ_BUF_SZ] = {0};
> > > +	struct iavf_cmd_info args;
> > > +	uint32_t *ethertype;
> > > +	int ret;
> > > +
> > > +	memset(&vlan_insert, 0, sizeof(vlan_insert));
> > > +	insertion_caps = &vf->vlan_v2_caps.offloads.insertion_support;
> > > +
> > > +	if ((insertion_caps->outer & VIRTCHNL_VLAN_ETHERTYPE_88A8) &&
> > > +	    (insertion_caps->outer & VIRTCHNL_VLAN_TOGGLE) &&
> > > +	    adapter->tpid == RTE_ETHER_TYPE_QINQ) {
> > > +		ethertype = &vlan_insert.outer_ethertype_setting;
> > > +		*ethertype = VIRTCHNL_VLAN_ETHERTYPE_88A8;
> > > +	} else if ((insertion_caps->outer & VIRTCHNL_VLAN_ETHERTYPE_8100)
> > &&
> > > +		   (insertion_caps->outer & VIRTCHNL_VLAN_TOGGLE) &&
> > > +		   adapter->tpid == RTE_ETHER_TYPE_VLAN) {
> > > +		ethertype = &vlan_insert.outer_ethertype_setting;
> > > +		*ethertype = VIRTCHNL_VLAN_ETHERTYPE_8100;
> > > +	} else {
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	vlan_insert.vport_id = vf->vsi_res->vsi_id;
> > > +
> > > +	args.ops = enable ? VIRTCHNL_OP_ENABLE_VLAN_INSERTION_V2 :
> > > +			    VIRTCHNL_OP_DISABLE_VLAN_INSERTION_V2;
> > > +	args.in_args = (uint8_t *)&vlan_insert;
> > > +	args.in_args_size = sizeof(vlan_insert);
> > > +	args.out_buffer = msg_buf;
> > > +	args.out_size = IAVF_AQ_BUF_SZ;
> > > +	ret = iavf_execute_vf_cmd_safe(adapter, &args);
> > > +	if (ret)
> > > +		PMD_DRV_LOG(ERR, "fail to execute command %s",
> > > +			    enable ?
> > "VIRTCHNL_OP_ENABLE_VLAN_INSERTION_V2" :
> > > +
> > "VIRTCHNL_OP_DISABLE_VLAN_INSERTION_V2");
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  int
> > >  iavf_config_vlan_insert_v2(struct iavf_adapter *adapter, bool enable)
> > > { @@ -929,14 +974,21 @@ iavf_config_vlan_insert_v2(struct iavf_adapter
> > > *adapter, bool enable)
> > >  	uint8_t msg_buf[IAVF_AQ_BUF_SZ] = {0};
> > >  	struct iavf_cmd_info args;
> > >  	uint32_t *ethertype;
> > > +	bool qinq = adapter->dev_data->dev_conf.rxmode.offloads &
> > > +		    RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > > +	bool insert_qinq = adapter->dev_data->dev_conf.txmode.offloads &
> > > +			   RTE_ETH_TX_OFFLOAD_QINQ_INSERT;
> > >  	int ret;
> > >
> > > +	if (qinq && insert_qinq)
> > > +		iavf_config_outer_vlan_insert_v2(adapter, enable);
> > > +
> > >  	insertion_caps = &vf->vlan_v2_caps.offloads.insertion_support;
> > >
> > > -	if ((insertion_caps->outer & VIRTCHNL_VLAN_ETHERTYPE_8100) &&
> > > +	if (!qinq && (insertion_caps->outer &
> > VIRTCHNL_VLAN_ETHERTYPE_8100)
> > > +&&
> > >  	    (insertion_caps->outer & VIRTCHNL_VLAN_TOGGLE))
> > >  		ethertype = &vlan_insert.outer_ethertype_setting;
> > > -	else if ((insertion_caps->inner & VIRTCHNL_VLAN_ETHERTYPE_8100)
> > &&
> > > +	else if (qinq && (insertion_caps->inner &
> > > +VIRTCHNL_VLAN_ETHERTYPE_8100) &&
> > >  		 (insertion_caps->inner & VIRTCHNL_VLAN_TOGGLE))
> > >  		ethertype = &vlan_insert.inner_ethertype_setting;
> > 
> > Is this not a functional change for existing code/apps? Is it possible that there
> > are apps which are depending on the current behaviour with no QinQ support?
> > 
> > /Bruce
> 
> Hi Bruce, 
> 
> Thank you for the review.
> 
> QinQ insertion support is already present in IAVF PMD for TPID 0x8100 (802.1Q).
> Meaning double VLAN insertion resulting in packets having 0x8100 (802.1Q) + 0x8100 (802.1Q), is already supported.
> 
> QinQ insertion with VLAN TPID 0x88a8 support is absent.
> This patch adds support for QinQ insertion for both Outer VLAN TPIDs 0x88a8 (802.1ad).
> So, double VLAN insertion resulting in packets having 0x88a8 (802.1ad) + + 0x8100 (802.1Q) is achieved by this patch.

Right. However, the change above adds an additional condition to the check:

	else if ((insertion_caps->inner & VIRTCHNL_VLAN_ETHERTYPE_8100)

meaning that while previously this check passed if the
insertion_caps->inner value matched, now it only matches if the inner value
matches AND qinq flag is set. Will that not break apps that rely upon the
old behaviour?

/Bruce