> -----Original Message----- > From: Richardson, Bruce <[email protected]> > Sent: 05 May 2026 22:35 > To: Mandal, Anurag <[email protected]> > Cc: [email protected]; Medvedkin, Vladimir <[email protected]> > Subject: Re: [PATCH] net/iavf: add support for QinQ insertion > > On Tue, May 05, 2026 at 05:58:27PM +0100, Mandal, Anurag wrote: > > > -----Original Message----- > > > From: Richardson, Bruce <[email protected]> > > > Sent: 05 May 2026 21:26 > > > To: Mandal, Anurag <[email protected]> > > > Cc: [email protected]; Medvedkin, Vladimir <[email protected]> > > > 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 <[email protected]> > > > > > > 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
Hi Bruce, No. It will not break. Let me explain. There are 3 VLAN insertion scenarios:- 1. Single VLAN insertion which is 0x8100 2. QinQ insertion 0x8100 + 0x8100 3. QinQ insertion 0x88a8+ 0x8100. Now for QinQ insertion, needs both RTE_ETH_RX_OFFLOAD_VLAN_EXTEND and RTE_ETH_TX_OFFLOAD_QINQ_INSERT to be enabled. For case 1, in function iavf_config_vlan_insert_v2(), qinq is not set, so the following will come as Single VLAN is considered as outer vlan. if (!qinq && (insertion_caps->outer & VIRTCHNL_VLAN_ETHERTYPE_8100 For case 2 & 3, the function iavf_config_vlan_insert_v2() calls iavf_config_outer_vlan_insert_v2() which does Outer vlan insertion be it 0x88a8 or 0x8100. > > > > + if (qinq && insert_qinq) > > > > + iavf_config_outer_vlan_insert_v2(adapter, enable); Once, outer vlan insertion is done, then call comes back to original iavf_config_vlan_insert_v2() and does the following as 0x8100 is inner vlan here : > > > > + else if (qinq && (insertion_caps->inner & > > > > +VIRTCHNL_VLAN_ETHERTYPE_8100) &&. Basically, iavf_config_vlan_insert_v2() is responsible for either Single VLAN or inner 8100 insertion and iavf_config_outer_vlan_insert_v2() is responsible for QinQ: Outer VLAN 0x88a8 or outer VLAN 0x8100 insertion. These are done by checking the flags RTE_ETH_RX_OFFLOAD_VLAN_EXTEND and RTE_ETH_TX_OFFLOAD_QINQ_INSERT. Thanks, Anurag M
