>
> 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.lof...@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?
This scenario is possible. It's the case I hit when I test with a VF from my
Fortville card.
This code block is the same as what's in the existing code, it's wrapped around
a new if condition checking if only one insertion capability is supported
(inner or outer). The new code is in the else {}.
No update is needed to iavf_calc_context_desc because what you are describing
is already in place.
>
> > + }
> > + } 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?
This already exists. Here's what's already there:
if (flags & RTE_MBUF_F_TX_VLAN &&
vlan_flag & IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2)
return 1;
Thanks,
Ciara
>
> /Bruce
