> -----Original Message-----
> From: Zhang, Yuying <yuying.zh...@intel.com>
> Sent: Tuesday, September 6, 2022 10:16 AM
> To: Liu, KevinX <kevinx....@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.x...@intel.com>; Yang, SteveX <stevex.y...@intel.com>
> Subject: RE: [PATCH] net/i40e: fix single VLAN cannot work normal
>
> Hi,
>
> > -----Original Message-----
> > From: Liu, KevinX <kevinx....@intel.com>
> > Sent: Friday, August 19, 2022 12:04 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying <yuying.zh...@intel.com>; Xing, Beilei
> > <beilei.x...@intel.com>; Yang, SteveX <stevex.y...@intel.com>; Liu,
> > KevinX <kevinx....@intel.com>
> > Subject: [PATCH] net/i40e: fix single VLAN cannot work normal
> >
> > After disable QinQ, single VLAN can not work normal.
> > The reason is that QinQ is not disabled correctly.
> >
> > Before configuring QinQ, need to back up and clean MAC/VLAN filters of
> > all ports. After configuring QinQ, restore MAC/VLAN filters of all
> > ports. When disable QinQ, need to set valid_flags to 0x0008 and set
> > first_tag
> to 0x88a8.
> >
> > Signed-off-by: Kevin Liu <kevinx....@intel.com>
> > ---
> > doc/guides/nics/i40e.rst | 1 -
> > drivers/net/i40e/i40e_ethdev.c | 159
> > +++++++++++++++++++++++----------
> > 2 files changed, 111 insertions(+), 49 deletions(-)
> >
> > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index
> > abb99406b3..15b796e67a 100644
> > --- a/doc/guides/nics/i40e.rst
> > +++ b/doc/guides/nics/i40e.rst
> > @@ -983,7 +983,6 @@ If FW version >= 8.4, there'll be some Vlan related
> issues:
> >
> > #. TCI input set for QinQ is invalid.
> > #. Fail to configure TPID for QinQ.
> > -#. Need to enable QinQ before enabling Vlan filter.
> > #. Fail to strip outer Vlan.
> >
> > Example of getting best performance with l3fwd example diff --git
> > a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> > index
> > 67d79de08d..27cfda6ff8 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -325,6 +325,7 @@ static int i40e_veb_release(struct i40e_veb *veb);
> > static struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf,
> > struct i40e_vsi *vsi);
> > static int i40e_vsi_config_double_vlan(struct i40e_vsi *vsi, int on);
> > +static int i40e_vsi_config_qinq(struct i40e_vsi *vsi, int on);
> > static inline int i40e_find_all_mac_for_vlan(struct i40e_vsi *vsi,
> > struct i40e_macvlan_filter *mv_f,
> > int num,
> > @@ -3909,7 +3910,6 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > int qinq = dev->data->dev_conf.rxmode.offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > - u16 sw_flags = 0, valid_flags = 0;
> > int ret = 0;
> >
> > if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER && @@ -3928,10
> > +3928,6 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > /* 802.1ad frames ability is added in NVM API 1.7*/
> > if (hw->flags & I40E_HW_FLAG_802_1AD_CAPABLE) {
> > if (qinq) {
> > - if (pf->fw8_3gt) {
> > - sw_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > - valid_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > - }
> > if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > hw->first_tag = rte_cpu_to_le_16(tpid);
> > else if (vlan_type == RTE_ETH_VLAN_TYPE_INNER) @@
> > -3940,8 +3936,8 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > hw->second_tag = rte_cpu_to_le_16(tpid);
> > }
> > - ret = i40e_aq_set_switch_config(hw, sw_flags,
> > - valid_flags, 0, NULL);
> > + ret = i40e_aq_set_switch_config(hw, 0,
> > + 0, 0, NULL);
> > if (ret != I40E_SUCCESS) {
> > PMD_DRV_LOG(ERR,
> > "Set switch config failed aq_err: %d", @@ -
> > 3993,11 +3989,15 @@ static int i40e_vlan_offload_set(struct
> > rte_eth_dev *dev, int mask) {
> > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > + struct i40e_mac_filter_info *vmac_filter[RTE_MAX_ETHPORTS];
> > + struct i40e_vsi *vvsi[RTE_MAX_ETHPORTS];
> > struct i40e_mac_filter_info *mac_filter;
> > struct i40e_vsi *vsi = pf->main_vsi;
> > struct rte_eth_rxmode *rxmode;
> > + int vnum[RTE_MAX_ETHPORTS];
> > struct i40e_mac_filter *f;
> > - int i, num;
> > + int port_num = 0;
> > + int i, num, j;
> > void *temp;
> > int ret;
> >
> > @@ -4018,50 +4018,80 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> > int
> > mask)
> > }
> >
> > if (mask & RTE_ETH_VLAN_EXTEND_MASK) {
> > - i = 0;
> > - num = vsi->mac_num;
> > - mac_filter = rte_zmalloc("mac_filter_info_data",
> > - num * sizeof(*mac_filter), 0);
> > - if (mac_filter == NULL) {
> > - PMD_DRV_LOG(ERR, "failed to allocate memory");
> > - return I40E_ERR_NO_MEMORY;
> > - }
> > -
> > - /*
> > - * Outer VLAN processing is supported after firmware v8.4,
> > kernel driver
> > - * also change the default behavior to support this feature. To
> > align with
> > - * kernel driver, set switch config in 'i40e_vlan_tpie_set' to
> > support for
> > - * outer VLAN processing. But it is forbidden for firmware to
> > change the
> > - * Inner/Outer VLAN configuration while there are MAC/VLAN
> > filters in the
> > - * switch table. Therefore, we need to clear the MAC table
> > before setting
> > - * config, and then restore the MAC table after setting. This
> > feature is
> > - * recommended to be used in firmware v8.6.
> > - */
> > - /* Remove all existing mac */
> > - RTE_TAILQ_FOREACH_SAFE(f, &vsi->mac_list, next, temp) {
> > - mac_filter[i] = f->mac_info;
> > - ret = i40e_vsi_delete_mac(vsi, &f-
> > >mac_info.mac_addr);
> > - if (ret)
> > - PMD_DRV_LOG(ERR, "i40e vsi delete mac fail.");
> > - i++;
> > + /*back up and clean mac/vlan filters of all ports*/
> > + for (j = 0; j < RTE_MAX_ETHPORTS; j++) {
> > + /*
> > + * It is impossible to confirm whether the port is pf
> > + * only through the state field, so it is also necessary
> > + * to verify the intr_handle field.
> > + */
> > + if (rte_eth_devices[j].state != RTE_ETH_DEV_UNUSED
> > &&
> > +
> > rte_eth_devices[j].intr_handle) {
> > + struct rte_eth_dev *tmp_dev =
> > &rte_eth_devices[j];
> > + struct i40e_pf *tmp_pf =
> > + I40E_DEV_PRIVATE_TO_PF(tmp_dev-
> > >data->dev_private);
> > + struct i40e_vsi *tmp_vsi = tmp_pf->main_vsi;
> > + i = 0;
> > + num = tmp_vsi->mac_num;
> > + mac_filter =
> > rte_zmalloc("mac_filter_info_data",
> > + num * sizeof(*mac_filter), 0);
> > + if (mac_filter == NULL) {
> > + PMD_DRV_LOG(ERR, "failed to
> > allocate memory");
> > + return I40E_ERR_NO_MEMORY;
> > + }
> > + /*
> > + * Outer VLAN processing is supported after
> > firmware v8.4,
> > + * kernel driver also change the default
> > behavior to support
> > + * this feature. To align with kernel driver,
> > set
> > switch
> > + * config in 'i40e_vlan_tpid_set' to support for
> > outer VLAN
> > + * processing. But it is forbidden for firmware
> > to change the
> > + * Inner/Outer VLAN configuration while there
> > are MAC/VLAN
> > + * filters in the switch table. Therefore, we
> > need to clean
> > + * MAC/VLAN filters of all ports before setting
> > config, and
> > + * then restore the MAC table after setting.
> > This
> > feature is
> > + * recommended to be used in firmware v8.6.
> > + */
> > + /* Remove all existing mac */
> > + RTE_TAILQ_FOREACH_SAFE(f, &tmp_vsi-
> > >mac_list, next, temp) {
> > + mac_filter[i] = f->mac_info;
> > + ret = i40e_vsi_delete_mac(tmp_vsi, &f-
> > >mac_info.mac_addr);
> > + if (ret)
> > + PMD_DRV_LOG(ERR, "i40e vsi
> > delete mac fail.");
> > + i++;
> > + }
> > + vmac_filter[j] = mac_filter;
> > + vvsi[j] = tmp_vsi;
> > + vnum[j] = num;
> > + port_num++;
> > + }
> > }
> > if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)
> {
> > - i40e_vsi_config_double_vlan(vsi, TRUE);
> > - /* Set global registers with default ethertype. */
> > - i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
> > - RTE_ETHER_TYPE_VLAN);
> > - i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
> > - RTE_ETHER_TYPE_VLAN);
> > + if (pf->fw8_3gt) {
> > + i40e_vsi_config_qinq(vsi, TRUE);
> > + } else {
> > + i40e_vsi_config_double_vlan(vsi, TRUE);
> > + /* Set global registers with default ethertype.
> > */
> > + i40e_vlan_tpid_set(dev,
> > RTE_ETH_VLAN_TYPE_OUTER,
> > +
> > RTE_ETHER_TYPE_VLAN);
> > + i40e_vlan_tpid_set(dev,
> > RTE_ETH_VLAN_TYPE_INNER,
> > +
> > RTE_ETHER_TYPE_VLAN);
> > + }
> > } else {
> > - i40e_vsi_config_double_vlan(vsi, FALSE);
> > - }
> > - /* Restore all mac */
> > - for (i = 0; i < num; i++) {
> > - ret = i40e_vsi_add_mac(vsi, &mac_filter[i]);
> > - if (ret)
> > - PMD_DRV_LOG(ERR, "i40e vsi add mac fail.");
> > + if (pf->fw8_3gt)
> > + i40e_vsi_config_qinq(vsi, FALSE);
> > + else
> > + i40e_vsi_config_double_vlan(vsi, FALSE);
> > + }
> > + /*restore mac/vlan filters of all ports*/
> > + for (j = 0; j < port_num; j++) {
>
> The index here is incorrect.
Thanks, I will send v2.
>
> > + mac_filter = vmac_filter[j];
> > + for (i = 0; i < vnum[j]; i++) {
> > + ret = i40e_vsi_add_mac(vvsi[j], &mac_filter[i]);
> > + if (ret)
> > + PMD_DRV_LOG(ERR, "i40e vsi add mac
> > fail.");
> > + }
> > + rte_free(mac_filter);
> > }
> > - rte_free(mac_filter);
> > }
> >
> > if (mask & RTE_ETH_QINQ_STRIP_MASK) { @@ -6181,6 +6211,39 @@
> > i40e_vsi_config_double_vlan(struct i40e_vsi *vsi, int
> > on)
> > return i40e_aq_set_port_parameters(hw, vsi->seid, 0, 1, on, NULL);
> > }
> >
> > +static int
> > +i40e_vsi_config_qinq(struct i40e_vsi *vsi, int on) {
> > + struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
> > + u16 sw_flags = 0, valid_flags = 0;
> > + int ret = 0;
> > +
> > + ret = i40e_aq_set_port_parameters(hw, vsi->seid, 0, 1, on, NULL);
> > + if (ret != I40E_SUCCESS) {
> > + PMD_DRV_LOG(ERR, "Failed to set port params");
> > + return -1;
> > + }
> > +
> > + if (on) {
> > + sw_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > + valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > + hw->first_tag = rte_cpu_to_le_16(RTE_ETHER_TYPE_VLAN);
> > + } else {
> > + valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > + hw->first_tag = rte_cpu_to_le_16(RTE_ETHER_TYPE_QINQ);
> > + }
> > +
> > + ret = i40e_aq_set_switch_config(hw, sw_flags, valid_flags, 0, NULL);
> > + if (ret) {
> > + PMD_DRV_LOG(ERR,
> > + "Set switch config failed aq_err: %d",
> > + hw->aq.asq_last_status);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
>
> Qinq is double vlan, please use i40e_vsi_config_double_vlan() function for bug
> fixing.
Ok, I will fix it.
>
> > +
> > static int
> > i40e_update_flow_control(struct i40e_hw *hw) {
> > --
> > 2.34.1
