On Wed, Feb 11, 2026 at 10:29:17AM +0100, Burakov, Anatoly wrote: > On 2/10/2026 7:20 PM, Bruce Richardson wrote: > > On Tue, Feb 10, 2026 at 02:21:49PM +0100, Burakov, Anatoly wrote: > > > On 2/9/2026 5:45 PM, Bruce Richardson wrote: > > > > Make the VLAN tag insertion logic configurable in the common code, as to > > > > where inner/outer tags get placed. > > > > > > > > Signed-off-by: Bruce Richardson <[email protected]> > > > > --- > > > > > > Hi Bruce, > > > I might be missing something obvious, but... > > > > > > > - /* Descriptor based VLAN insertion */ > > > > - if (ol_flags & (RTE_MBUF_F_TX_VLAN | > > > > RTE_MBUF_F_TX_QINQ)) { > > > > + /* Descriptor based VLAN/QinQ insertion */ > > > > + /* for single vlan offload, only insert in data desc > > > > with VLAN_IN_L2TAG1 is set > > > > + * for qinq offload, we always put inner tag in L2Tag1 > > > > + */ > > > > + if (((ol_flags & RTE_MBUF_F_TX_VLAN) && l2tag1_field == > > > > CI_VLAN_IN_L2TAG1) || > > > > + (ol_flags & RTE_MBUF_F_TX_QINQ)) { > > > > td_cmd |= CI_TX_DESC_CMD_IL2TAG1; > > > > td_tag = tx_pkt->vlan_tci; > > > > } > > > > > > I can see that we insert VLAN tag on TX_VLAN and VLAN_IN_TAG1. But then... > > > > > > > > > > @@ -1004,8 +1004,7 @@ get_context_desc(uint64_t ol_flags, const struct > > > > rte_mbuf *tx_pkt, > > > > /* TX context descriptor based double VLAN insert */ > > > > if (ol_flags & RTE_MBUF_F_TX_QINQ) { > > > > cd_l2tag2 = tx_pkt->vlan_tci_outer; > > > > - cd_type_cmd_tso_mss |= > > > > - ((uint64_t)I40E_TX_CTX_DESC_IL2TAG2 << > > > > I40E_TXD_CTX_QW1_CMD_SHIFT); > > > > + cd_type_cmd_tso_mss |= (I40E_TX_CTX_DESC_IL2TAG2 << > > > > I40E_TXD_CTX_QW1_CMD_SHIFT); > > > > > > this logic is only triggered for QinQ. Meaning, there's nowhere we insert > > > VLAN tag on TX_VLAN and VLAN_IN_TAG2? > > > > > For VLAN_IN_L2TAG2 the vlan tag goes in the context descriptor, and > > creating the context descriptor is the responsibility of each individual > > driver. The two drivers using this common path, i40e and ice both put the > > tag in the TAG1 slot, so don't have a path to add it to the TAG2 slot. We > > make it configurable in this patch so that in the next patches we can add > > in support for the iavf driver - which does support putting the VLAN tag in > > the Tag2 slot, in it's context descriptor function. > > > > /Bruce > > Perhaps we need to document that more explicitly then, and double-check that > all driver paths actually do this in all required cases (i.e. not just for > QinQ but also for regular VLAN insertion case). > There's already a comment at the top of the enum definition, but I've also expanded out the documentation on the L2TAG2 value to clarify that drivers need to set the value themselves rather than relying on the common Tx code.
/Bruce
