DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/i40e: add outer VLAN processing
@ 2021-11-18  9:39 Robin Zhang
  2022-05-19  6:18 ` [PATCH v2] " Robin Zhang
  0 siblings, 1 reply; 18+ messages in thread
From: Robin Zhang @ 2021-11-18  9:39 UTC (permalink / raw)
  To: dev; +Cc: beilei.xing, qi.z.zhang, junfeng.guo, stevex.yang, Robin Zhang

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, add support for outer VLAN processing in DPDK. This will
not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 58 +++++++++++++++++++++++++++++++---
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 344cbd25d3..6e6c0d51ac 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2591,11 +2591,31 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	int ret;
 	uint8_t aq_fail = 0;
 	int retries = 0;
+	int mask = 0;
+	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 
 	PMD_INIT_FUNC_TRACE();
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	/*
+	 * To avoid global register conflict with kernel driver, need set
+	 * switch configuration back to default, disable double vlan and
+	 * clear the VLAN filters when dev close.
+	 */
+	if (pf->is_outer_vlan_processing &&
+	    (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
+		mask = RTE_ETH_VLAN_EXTEND_MASK;
+		rxmode->offloads &= ~RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+
+		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_FILTER) {
+			mask |= RTE_ETH_VLAN_FILTER_MASK;
+			rxmode->offloads &= ~RTE_ETH_RX_OFFLOAD_VLAN_FILTER;
+		}
+
+		i40e_vlan_offload_set(dev, mask);
+	}
+
 	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
 	if (ret)
 		PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);
@@ -3918,6 +3938,7 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
 	int qinq = dev->data->dev_conf.rxmode.offloads &
 		   RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
 	int ret = 0;
+	u16 sw_flags = 0, valid_flags = 0;
 
 	if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER &&
 	     vlan_type != RTE_ETH_VLAN_TYPE_OUTER) ||
@@ -3935,15 +3956,28 @@ 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->is_outer_vlan_processing) {
+				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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			if (pf->is_outer_vlan_processing) {
+				sw_flags = 0;
+				valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
+			}
+			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER) {
+				if (pf->is_outer_vlan_processing)
+					hw->first_tag = rte_cpu_to_le_16(tpid);
+				else
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -4022,9 +4056,12 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->is_outer_vlan_processing)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			i40e_vsi_config_double_vlan(vsi, FALSE);
+		}
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4854,6 +4891,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->is_outer_vlan_processing = true;
+	} else {
+		pf->is_outer_vlan_processing = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index d8042abbd9..e62e2fba4f 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1190,6 +1190,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* The enable flag for outer VLAN processing */
+	bool is_outer_vlan_processing;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5] net/i40e: add outer VLAN processing
  2022-06-09 14:43     ` [PATCH v4] " Kevin Liu
@ 2022-04-07 20:01       ` Kevin Liu
  2022-06-10 15:52         ` [PATCH v6] " Kevin Liu
  2022-06-10  1:01       ` [PATCH v4] " Zhang, Qi Z
  1 sibling, 1 reply; 18+ messages in thread
From: Kevin Liu @ 2022-04-07 20:01 UTC (permalink / raw)
  To: dev; +Cc: stevex.yang, beilei.xing, Yuying.Zhang, Robin Zhang, Kevin Liu

From: Robin Zhang <robinx.zhang@intel.com>

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, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..03b584a2bb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2513,11 +2513,24 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	struct i40e_vsi *main_vsi = pf->main_vsi;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	int i;
 
 	if (hw->adapter_stopped == 1)
 		return 0;
 
+	/*
+	 * It is a workaround, when firmware > v8.3, if the double VLAN
+	 * is disabled when the program exits, an abnormal error will occur
+	 * on the NIC. Need to enable double VLAN when dev is closed.
+	 */
+	if (pf->is_outer_vlan_processing) {
+		if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
+			rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+			i40e_vlan_offload_set(dev, RTE_ETH_VLAN_EXTEND_MASK);
+		}
+	}
+
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
 		rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
 		rte_intr_enable(intr_handle);
@@ -3909,6 +3922,7 @@ 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 &&
@@ -3927,15 +3941,32 @@ 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->is_outer_vlan_processing) {
+				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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			/*
+			 * When firmware > 8.3, if tpid is equal to 0x88A8,
+			 * indicates that the disable double VLAN operation is in progress.
+			 * Need set switch configuration back to default.
+			 */
+			if (pf->is_outer_vlan_processing && tpid == RTE_ETHER_TYPE_QINQ) {
+				sw_flags = 0;
+				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_OUTER)
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -3987,8 +4018,13 @@ 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 *mac_filter;
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
+	struct i40e_mac_filter *f;
+	int i, num;
+	void *temp;
+	int ret;
 
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4043,33 @@ 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++;
+		}
 		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
 			i40e_vsi_config_double_vlan(vsi, TRUE);
 			/* Set global registers with default ethertype. */
@@ -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->is_outer_vlan_processing)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			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.");
+		}
+		rte_free(mac_filter);
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->is_outer_vlan_processing = true;
+	} else {
+		pf->is_outer_vlan_processing = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..531ada447d 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* The enable flag for outer VLAN processing */
+	bool is_outer_vlan_processing;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] net/i40e: add outer VLAN processing
  2021-11-18  9:39 [PATCH] net/i40e: add outer VLAN processing Robin Zhang
@ 2022-05-19  6:18 ` Robin Zhang
  2022-06-09 11:26   ` [PATCH v3] " Kevin Liu
  0 siblings, 1 reply; 18+ messages in thread
From: Robin Zhang @ 2022-05-19  6:18 UTC (permalink / raw)
  To: dev; +Cc: beilei.xing, yuying.zhang, stevex.yang, kevinx.liu, Robin Zhang

Outer VLAN processing is supported after firmware v8.4, kernel driver
also changes the default behavior to support this feature. To align with
kernel driver, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 79 +++++++++++++++++++++++++++++++---
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..6650a8ded9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3910,6 +3910,7 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
 	int qinq = dev->data->dev_conf.rxmode.offloads &
 		   RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
 	int ret = 0;
+	u16 sw_flags = 0, valid_flags = 0;
 
 	if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER &&
 	     vlan_type != RTE_ETH_VLAN_TYPE_OUTER) ||
@@ -3927,15 +3928,28 @@ 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->is_outer_vlan_processing) {
+				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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			if (pf->is_outer_vlan_processing) {
+				sw_flags = 0;
+				valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
+			}
+			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER) {
+				if (pf->is_outer_vlan_processing)
+					hw->first_tag = rte_cpu_to_le_16(tpid);
+				else
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -3987,8 +4001,12 @@ 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 *mac_filter;
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
+	struct i40e_mac_filter *f;
+	int i, num, ret;
+	void *temp;
 
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4025,35 @@ 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 changes 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 at least.
+		**/
+		/* 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 != I40E_SUCCESS) {
+				PMD_DRV_LOG(ERR, "Failed to delete mac filter");
+				return -EIO;
+			}
+			i++;
+		}
 		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
 			i40e_vsi_config_double_vlan(vsi, TRUE);
 			/* Set global registers with default ethertype. */
@@ -4014,9 +4061,20 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->is_outer_vlan_processing)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			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 != I40E_SUCCESS) {
+				PMD_DRV_LOG(ERR, "Failed to add mac filter");
+				return -EIO;
+			}
+		}
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4904,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->is_outer_vlan_processing = true;
+	} else {
+		pf->is_outer_vlan_processing = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..531ada447d 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* The enable flag for outer VLAN processing */
+	bool is_outer_vlan_processing;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3] net/i40e: add outer VLAN processing
  2022-05-19  6:18 ` [PATCH v2] " Robin Zhang
@ 2022-06-09 11:26   ` Kevin Liu
  2022-06-09 14:43     ` [PATCH v4] " Kevin Liu
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Liu @ 2022-06-09 11:26 UTC (permalink / raw)
  To: dev; +Cc: Yuying.Zhang, beilei.xing, stevex.yang, Robin Zhang, Kevin Liu

From: Robin Zhang <robinx.zhang@intel.com>

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, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 76 +++++++++++++++++++++++++++++++---
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..12c5af7919 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3909,6 +3909,7 @@ 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 &&
@@ -3927,15 +3928,34 @@ 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->is_outer_vlan_processing) {
+				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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			/*When FW > 8.3, the tpid must be set to QinQ to close extend*/
+			if (tpid == RTE_ETHER_TYPE_QINQ) {
+				if (pf->is_outer_vlan_processing) {
+					sw_flags = 0;
+					valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
+				}
+				if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER) {
+					if (pf->is_outer_vlan_processing)
+						hw->first_tag = rte_cpu_to_le_16(tpid);
+					else
+						hw->second_tag = rte_cpu_to_le_16(tpid);
+				}
+			} else {
+				if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -3987,8 +4007,12 @@ 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 *mac_filter;
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
+	struct i40e_mac_filter *f;
+	int i, num;
+	void *temp;
 
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4031,30 @@ 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;
+			i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
+		}
 		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
 			i40e_vsi_config_double_vlan(vsi, TRUE);
 			/* Set global registers with default ethertype. */
@@ -4014,9 +4062,16 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->is_outer_vlan_processing)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			i40e_vsi_config_double_vlan(vsi, FALSE);
+		}
+		/* Restore all mac */
+		for (i = 0; i < num; i++)
+			i40e_vsi_add_mac(vsi, &mac_filter[i]);
+		rte_free(mac_filter);
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4901,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->is_outer_vlan_processing = true;
+	} else {
+		pf->is_outer_vlan_processing = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..531ada447d 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* The enable flag for outer VLAN processing */
+	bool is_outer_vlan_processing;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4] net/i40e: add outer VLAN processing
  2022-06-09 11:26   ` [PATCH v3] " Kevin Liu
@ 2022-06-09 14:43     ` Kevin Liu
  2022-04-07 20:01       ` [PATCH v5] " Kevin Liu
  2022-06-10  1:01       ` [PATCH v4] " Zhang, Qi Z
  0 siblings, 2 replies; 18+ messages in thread
From: Kevin Liu @ 2022-06-09 14:43 UTC (permalink / raw)
  To: dev; +Cc: Yuying.Zhang, beilei.xing, stevex.yang, Robin Zhang, Kevin Liu

From: Robin Zhang <robinx.zhang@intel.com>

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, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 83 ++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..736afb268b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3909,6 +3909,7 @@ 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 &&
@@ -3927,15 +3928,34 @@ 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->is_outer_vlan_processing) {
+				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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			/*When FW > 8.3, the tpid must be set to QinQ to close extend*/
+			if (tpid == RTE_ETHER_TYPE_QINQ) {
+				if (pf->is_outer_vlan_processing) {
+					sw_flags = 0;
+					valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
+				}
+				if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER) {
+					if (pf->is_outer_vlan_processing)
+						hw->first_tag = rte_cpu_to_le_16(tpid);
+					else
+						hw->second_tag = rte_cpu_to_le_16(tpid);
+				}
+			} else {
+				if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -3987,8 +4007,13 @@ 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 *mac_filter;
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
+	struct i40e_mac_filter *f;
+	int i, num;
+	void *temp;
+	int ret;
 
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4032,33 @@ 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++;
+		}
 		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
 			i40e_vsi_config_double_vlan(vsi, TRUE);
 			/* Set global registers with default ethertype. */
@@ -4014,9 +4066,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->is_outer_vlan_processing)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			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.");
+		}
+		rte_free(mac_filter);
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4908,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->is_outer_vlan_processing = true;
+	} else {
+		pf->is_outer_vlan_processing = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..531ada447d 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* The enable flag for outer VLAN processing */
+	bool is_outer_vlan_processing;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v4] net/i40e: add outer VLAN processing
  2022-06-09 14:43     ` [PATCH v4] " Kevin Liu
  2022-04-07 20:01       ` [PATCH v5] " Kevin Liu
@ 2022-06-10  1:01       ` Zhang, Qi Z
  1 sibling, 0 replies; 18+ messages in thread
From: Zhang, Qi Z @ 2022-06-10  1:01 UTC (permalink / raw)
  To: Liu, KevinX, dev
  Cc: Zhang, Yuying, Xing, Beilei, Yang, SteveX, Zhang, RobinX, Liu, KevinX



> -----Original Message-----
> From: Kevin Liu <kevinx.liu@intel.com>
> Sent: Thursday, June 9, 2022 10:43 PM
> To: dev@dpdk.org
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> RobinX <robinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v4] net/i40e: add outer VLAN processing
> 
> From: Robin Zhang <robinx.zhang@intel.com>
> 
> 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,
> add support for outer VLAN processing in DPDK.
> 
> 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 will not impact on an old firmware.
> 
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 83 ++++++++++++++++++++++++++++++++--
>  drivers/net/i40e/i40e_ethdev.h |  3 ++
>  2 files changed, 81 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 755786dc10..736afb268b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -3909,6 +3909,7 @@ 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 && @@ -3927,15
> +3928,34 @@ 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->is_outer_vlan_processing) {
> +				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)
>  				hw->second_tag = rte_cpu_to_le_16(tpid);
>  		} else {
> -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> -				hw->second_tag = rte_cpu_to_le_16(tpid);
> +			/*When FW > 8.3, the tpid must be set to QinQ to
> close extend*/

The comment is confusing, there is no firmware version related check in below code
And what is "set to QinQ to close extend" means?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v6] net/i40e: add outer VLAN processing
  2022-06-10 15:52         ` [PATCH v6] " Kevin Liu
@ 2022-06-10  8:06           ` Zhang, Qi Z
  2022-06-10  8:14             ` Liu, KevinX
  2022-06-10 16:29           ` [PATCH v7] " Kevin Liu
  1 sibling, 1 reply; 18+ messages in thread
From: Zhang, Qi Z @ 2022-06-10  8:06 UTC (permalink / raw)
  To: Liu, KevinX, dev
  Cc: Zhang, Yuying, Xing, Beilei, Yang, SteveX, Zhang, RobinX, Liu, KevinX



> -----Original Message-----
> From: Kevin Liu <kevinx.liu@intel.com>
> Sent: Friday, June 10, 2022 11:52 PM
> To: dev@dpdk.org
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> RobinX <robinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v6] net/i40e: add outer VLAN processing
> 
> From: Robin Zhang <robinx.zhang@intel.com>
> 
> 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,
> add support for outer VLAN processing in DPDK.
> 
> 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 will not impact on an old firmware.
> 
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
>  drivers/net/i40e/i40e_ethdev.h |  3 ++
>  2 files changed, 92 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 755786dc10..c708bdb0d4 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
>  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>  	struct i40e_filter_control_settings settings;
>  	struct rte_flow *p_flow;
>  	uint32_t reg;
> @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>  		return 0;
> 
> +	/*
> +	 * It is a workaround, when firmware > v8.3, if the double VLAN
> +	 * is disabled when the program exits, an abnormal error will occur
> +	 * on the NIC. Need to enable double VLAN when dev is closed.
> +	 */
> +	if (pf->is_outer_vlan_processing) {
> +		if (!(rxmode->offloads &
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> +			rxmode->offloads |=
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> +			i40e_vlan_offload_set(dev,
> RTE_ETH_VLAN_EXTEND_MASK);
> +		}
> +	}
> +
>  	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
>  	if (ret)
>  		PMD_INIT_LOG(WARNING, "failed to free switch domain: %d",
> ret); @@ -3909,6 +3922,7 @@ 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 && @@ -3927,15
> +3941,32 @@ 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->is_outer_vlan_processing) {
> +				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)
>  				hw->second_tag = rte_cpu_to_le_16(tpid);
>  		} else {
> -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> -				hw->second_tag = rte_cpu_to_le_16(tpid);
> +			/*
> +			 * When firmware > 8.3, if tpid is equal to 0x88A8,
> +			 * indicates that the disable double VLAN operation is
> in progress.
> +			 * Need set switch configuration back to default.
> +			 */
> +			if (pf->is_outer_vlan_processing && tpid ==
> RTE_ETHER_TYPE_QINQ) {
> +				sw_flags = 0;
> +				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_OUTER)
> +					hw->second_tag =
> rte_cpu_to_le_16(tpid);
> +			}
>  		}
> -		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> +		ret = i40e_aq_set_switch_config(hw, sw_flags,
> +						valid_flags, 0, NULL);
>  		if (ret != I40E_SUCCESS) {
>  			PMD_DRV_LOG(ERR,
>  				    "Set switch config failed aq_err: %d", @@ -
> 3987,8 +4018,13 @@ 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 *mac_filter;
>  	struct i40e_vsi *vsi = pf->main_vsi;
>  	struct rte_eth_rxmode *rxmode;
> +	struct i40e_mac_filter *f;
> +	int i, num;
> +	void *temp;
> +	int ret;
> 
>  	rxmode = &dev->data->dev_conf.rxmode;
>  	if (mask & RTE_ETH_VLAN_FILTER_MASK) { @@ -4007,6 +4043,33
> @@ 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++;
> +		}
>  		if (rxmode->offloads &
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
>  			i40e_vsi_config_double_vlan(vsi, TRUE);
>  			/* Set global registers with default ethertype. */ @@
> -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> mask)
>  					   RTE_ETHER_TYPE_VLAN);
>  			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
>  					   RTE_ETHER_TYPE_VLAN);
> -		}
> -		else
> +		} else {
> +			if (pf->is_outer_vlan_processing)
> +				i40e_vlan_tpid_set(dev,
> RTE_ETH_VLAN_TYPE_OUTER,
> +					   RTE_ETHER_TYPE_QINQ);
>  			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.");
> +		}
> +		rte_free(mac_filter);
>  	}
> 
>  	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
> @@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
>  		return -EINVAL;
>  	}
> 
> +	/**
> +	 * Enable outer VLAN processing if firmware version is greater
> +	 * than v8.3
> +	 */
> +	if (hw->aq.fw_maj_ver > 8 ||
> +	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> +		pf->is_outer_vlan_processing = true;

Why not just name the flag as "fw8_3gt?

Then you don't need to repeat explain "when firmware > 8.3" above the is_outer_vlan_processing flag.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v6] net/i40e: add outer VLAN processing
  2022-06-10  8:06           ` Zhang, Qi Z
@ 2022-06-10  8:14             ` Liu, KevinX
  0 siblings, 0 replies; 18+ messages in thread
From: Liu, KevinX @ 2022-06-10  8:14 UTC (permalink / raw)
  To: Zhang, Qi Z, dev; +Cc: Zhang, Yuying, Xing, Beilei, Yang, SteveX, Zhang, RobinX



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2022年6月10日 16:07
> To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> RobinX <robinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH v6] net/i40e: add outer VLAN processing
> 
> 
> 
> > -----Original Message-----
> > From: Kevin Liu <kevinx.liu@intel.com>
> > Sent: Friday, June 10, 2022 11:52 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> > RobinX <robinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH v6] net/i40e: add outer VLAN processing
> >
> > From: Robin Zhang <robinx.zhang@intel.com>
> >
> > 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, add support for outer VLAN processing in DPDK.
> >
> > 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 will not impact on an old firmware.
> >
> > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 94
> > ++++++++++++++++++++++++++++++++--
> >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> >  2 files changed, 92 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..c708bdb0d4 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> >  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> >  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> >  	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> >  	struct i40e_filter_control_settings settings;
> >  	struct rte_flow *p_flow;
> >  	uint32_t reg;
> > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> >  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >  		return 0;
> >
> > +	/*
> > +	 * It is a workaround, when firmware > v8.3, if the double VLAN
> > +	 * is disabled when the program exits, an abnormal error will occur
> > +	 * on the NIC. Need to enable double VLAN when dev is closed.
> > +	 */
> > +	if (pf->is_outer_vlan_processing) {
> > +		if (!(rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > +			rxmode->offloads |=
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > +			i40e_vlan_offload_set(dev,
> > RTE_ETH_VLAN_EXTEND_MASK);
> > +		}
> > +	}
> > +
> >  	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
> >  	if (ret)
> >  		PMD_INIT_LOG(WARNING, "failed to free switch
> domain: %d", ret); @@
> > -3909,6 +3922,7 @@ 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 && @@ -3927,15
> > +3941,32 @@ 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->is_outer_vlan_processing) {
> > +				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)
> >  				hw->second_tag = rte_cpu_to_le_16(tpid);
> >  		} else {
> > -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > -				hw->second_tag = rte_cpu_to_le_16(tpid);
> > +			/*
> > +			 * When firmware > 8.3, if tpid is equal to 0x88A8,
> > +			 * indicates that the disable double VLAN operation is
> > in progress.
> > +			 * Need set switch configuration back to default.
> > +			 */
> > +			if (pf->is_outer_vlan_processing && tpid ==
> > RTE_ETHER_TYPE_QINQ) {
> > +				sw_flags = 0;
> > +				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_OUTER)
> > +					hw->second_tag =
> > rte_cpu_to_le_16(tpid);
> > +			}
> >  		}
> > -		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> > +		ret = i40e_aq_set_switch_config(hw, sw_flags,
> > +						valid_flags, 0, NULL);
> >  		if (ret != I40E_SUCCESS) {
> >  			PMD_DRV_LOG(ERR,
> >  				    "Set switch config failed aq_err: %d", @@ -
> > 3987,8 +4018,13 @@ 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 *mac_filter;
> >  	struct i40e_vsi *vsi = pf->main_vsi;
> >  	struct rte_eth_rxmode *rxmode;
> > +	struct i40e_mac_filter *f;
> > +	int i, num;
> > +	void *temp;
> > +	int ret;
> >
> >  	rxmode = &dev->data->dev_conf.rxmode;
> >  	if (mask & RTE_ETH_VLAN_FILTER_MASK) { @@ -4007,6 +4043,33
> @@
> > 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++;
> > +		}
> >  		if (rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
> >  			i40e_vsi_config_double_vlan(vsi, TRUE);
> >  			/* Set global registers with default ethertype. */ @@
> > -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> > mask)
> >  					   RTE_ETHER_TYPE_VLAN);
> >  			i40e_vlan_tpid_set(dev,
> RTE_ETH_VLAN_TYPE_INNER,
> >  					   RTE_ETHER_TYPE_VLAN);
> > -		}
> > -		else
> > +		} else {
> > +			if (pf->is_outer_vlan_processing)
> > +				i40e_vlan_tpid_set(dev,
> > RTE_ETH_VLAN_TYPE_OUTER,
> > +					   RTE_ETHER_TYPE_QINQ);
> >  			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.");
> > +		}
> > +		rte_free(mac_filter);
> >  	}
> >
> >  	if (mask & RTE_ETH_QINQ_STRIP_MASK) { @@ -4846,6 +4919,17 @@
> > i40e_pf_parameter_init(struct rte_eth_dev *dev)
> >  		return -EINVAL;
> >  	}
> >
> > +	/**
> > +	 * Enable outer VLAN processing if firmware version is greater
> > +	 * than v8.3
> > +	 */
> > +	if (hw->aq.fw_maj_ver > 8 ||
> > +	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> > +		pf->is_outer_vlan_processing = true;
> 
> Why not just name the flag as "fw8_3gt?
> 
> Then you don't need to repeat explain "when firmware > 8.3" above the
> is_outer_vlan_processing flag.
> 
That's a good idea, I will send v7, thank you!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-10 16:29           ` [PATCH v7] " Kevin Liu
@ 2022-06-10 14:27             ` Ben Magistro
  2022-06-13  2:14               ` Liu, KevinX
  2022-06-14  2:43             ` Zhang, Yuying
  1 sibling, 1 reply; 18+ messages in thread
From: Ben Magistro @ 2022-06-10 14:27 UTC (permalink / raw)
  To: Kevin Liu; +Cc: dev, Yuying.Zhang, beilei.xing, stevex.yang, Robin Zhang

[-- Attachment #1: Type: text/plain, Size: 10026 bytes --]

I'm trying to understand if this change is at all related to an issue we
are experiencing with newer firmwares (
https://mails.dpdk.org/archives/dev/2022-April/238621.html) that happened
to start with 8.4 and affected qinq offload processing.  I can say loading
this patch and running testpmd does not seem to have any effect on the
issue we are experiencing.

Side note, there is also a minor typo in the comment block
"i40e_vlan_tpie_set" vs "i40e_vlan_tpid_set".

Thanks

On Fri, Jun 10, 2022 at 4:30 AM Kevin Liu <kevinx.liu@intel.com> wrote:

> From: Robin Zhang <robinx.zhang@intel.com>
>
> 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, add support for outer VLAN processing in DPDK.
>
> 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 will not impact on an old firmware.
>
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
>  drivers/net/i40e/i40e_ethdev.h |  3 ++
>  2 files changed, 92 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c
> b/drivers/net/i40e/i40e_ethdev.c
> index 755786dc10..4cae163cb9 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
>         struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>         struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>         struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> +       struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>         struct i40e_filter_control_settings settings;
>         struct rte_flow *p_flow;
>         uint32_t reg;
> @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>                 return 0;
>
> +       /*
> +        * It is a workaround, if the double VLAN is disabled when
> +        * the program exits, an abnormal error will occur on the
> +        * NIC. Need to enable double VLAN when dev is closed.
> +        */
> +       if (pf->fw8_3gt) {
> +               if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> +                       rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> +                       i40e_vlan_offload_set(dev,
> RTE_ETH_VLAN_EXTEND_MASK);
> +               }
> +       }
> +
>         ret = rte_eth_switch_domain_free(pf->switch_domain_id);
>         if (ret)
>                 PMD_INIT_LOG(WARNING, "failed to free switch domain: %d",
> ret);
> @@ -3909,6 +3922,7 @@ 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 &&
> @@ -3927,15 +3941,32 @@ 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)
>                                 hw->second_tag = rte_cpu_to_le_16(tpid);
>                 } else {
> -                       if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> -                               hw->second_tag = rte_cpu_to_le_16(tpid);
> +                       /*
> +                        * If tpid is equal to 0x88A8, indicates that the
> +                        * disable double VLAN operation is in progress.
> +                        * Need set switch configuration back to default.
> +                        */
> +                       if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
> +                               sw_flags = 0;
> +                               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_OUTER)
> +                                       hw->second_tag =
> rte_cpu_to_le_16(tpid);
> +                       }
>                 }
> -               ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> +               ret = i40e_aq_set_switch_config(hw, sw_flags,
> +                                               valid_flags, 0, NULL);
>                 if (ret != I40E_SUCCESS) {
>                         PMD_DRV_LOG(ERR,
>                                     "Set switch config failed aq_err: %d",
> @@ -3987,8 +4018,13 @@ 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 *mac_filter;
>         struct i40e_vsi *vsi = pf->main_vsi;
>         struct rte_eth_rxmode *rxmode;
> +       struct i40e_mac_filter *f;
> +       int i, num;
> +       void *temp;
> +       int ret;
>
>         rxmode = &dev->data->dev_conf.rxmode;
>         if (mask & RTE_ETH_VLAN_FILTER_MASK) {
> @@ -4007,6 +4043,33 @@ 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++;
> +               }
>                 if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
>                         i40e_vsi_config_double_vlan(vsi, TRUE);
>                         /* Set global registers with default ethertype. */
> @@ -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> mask)
>                                            RTE_ETHER_TYPE_VLAN);
>                         i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
>                                            RTE_ETHER_TYPE_VLAN);
> -               }
> -               else
> +               } else {
> +                       if (pf->fw8_3gt)
> +                               i40e_vlan_tpid_set(dev,
> RTE_ETH_VLAN_TYPE_OUTER,
> +                                          RTE_ETHER_TYPE_QINQ);
>                         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.");
> +               }
> +               rte_free(mac_filter);
>         }
>
>         if (mask & RTE_ETH_QINQ_STRIP_MASK) {
> @@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
>                 return -EINVAL;
>         }
>
> +       /**
> +        * Enable outer VLAN processing if firmware version is greater
> +        * than v8.3
> +        */
> +       if (hw->aq.fw_maj_ver > 8 ||
> +           (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> +               pf->fw8_3gt = true;
> +       } else {
> +               pf->fw8_3gt = false;
> +       }
> +
>         return 0;
>  }
>
> diff --git a/drivers/net/i40e/i40e_ethdev.h
> b/drivers/net/i40e/i40e_ethdev.h
> index a1ebdc093c..fe943a45ff 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -1188,6 +1188,9 @@ struct i40e_pf {
>         /* Switch Domain Id */
>         uint16_t switch_domain_id;
>
> +       /* When firmware > 8.3, the enable flag for outer VLAN processing
> */
> +       bool fw8_3gt;
> +
>         struct i40e_vf_msg_cfg vf_msg_cfg;
>         uint64_t prev_rx_bytes;
>         uint64_t prev_tx_bytes;
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 12768 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v6] net/i40e: add outer VLAN processing
  2022-04-07 20:01       ` [PATCH v5] " Kevin Liu
@ 2022-06-10 15:52         ` Kevin Liu
  2022-06-10  8:06           ` Zhang, Qi Z
  2022-06-10 16:29           ` [PATCH v7] " Kevin Liu
  0 siblings, 2 replies; 18+ messages in thread
From: Kevin Liu @ 2022-06-10 15:52 UTC (permalink / raw)
  To: dev; +Cc: Yuying.Zhang, beilei.xing, stevex.yang, Robin Zhang, Kevin Liu

From: Robin Zhang <robinx.zhang@intel.com>

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, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..c708bdb0d4 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct i40e_filter_control_settings settings;
 	struct rte_flow *p_flow;
 	uint32_t reg;
@@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	/*
+	 * It is a workaround, when firmware > v8.3, if the double VLAN
+	 * is disabled when the program exits, an abnormal error will occur
+	 * on the NIC. Need to enable double VLAN when dev is closed.
+	 */
+	if (pf->is_outer_vlan_processing) {
+		if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
+			rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+			i40e_vlan_offload_set(dev, RTE_ETH_VLAN_EXTEND_MASK);
+		}
+	}
+
 	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
 	if (ret)
 		PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);
@@ -3909,6 +3922,7 @@ 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 &&
@@ -3927,15 +3941,32 @@ 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->is_outer_vlan_processing) {
+				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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			/*
+			 * When firmware > 8.3, if tpid is equal to 0x88A8,
+			 * indicates that the disable double VLAN operation is in progress.
+			 * Need set switch configuration back to default.
+			 */
+			if (pf->is_outer_vlan_processing && tpid == RTE_ETHER_TYPE_QINQ) {
+				sw_flags = 0;
+				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_OUTER)
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -3987,8 +4018,13 @@ 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 *mac_filter;
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
+	struct i40e_mac_filter *f;
+	int i, num;
+	void *temp;
+	int ret;
 
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4043,33 @@ 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++;
+		}
 		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
 			i40e_vsi_config_double_vlan(vsi, TRUE);
 			/* Set global registers with default ethertype. */
@@ -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->is_outer_vlan_processing)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			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.");
+		}
+		rte_free(mac_filter);
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->is_outer_vlan_processing = true;
+	} else {
+		pf->is_outer_vlan_processing = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..531ada447d 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* The enable flag for outer VLAN processing */
+	bool is_outer_vlan_processing;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-10 15:52         ` [PATCH v6] " Kevin Liu
  2022-06-10  8:06           ` Zhang, Qi Z
@ 2022-06-10 16:29           ` Kevin Liu
  2022-06-10 14:27             ` Ben Magistro
  2022-06-14  2:43             ` Zhang, Yuying
  1 sibling, 2 replies; 18+ messages in thread
From: Kevin Liu @ 2022-06-10 16:29 UTC (permalink / raw)
  To: dev; +Cc: Yuying.Zhang, beilei.xing, stevex.yang, Robin Zhang, Kevin Liu

From: Robin Zhang <robinx.zhang@intel.com>

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, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..4cae163cb9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct i40e_filter_control_settings settings;
 	struct rte_flow *p_flow;
 	uint32_t reg;
@@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	/*
+	 * It is a workaround, if the double VLAN is disabled when
+	 * the program exits, an abnormal error will occur on the
+	 * NIC. Need to enable double VLAN when dev is closed.
+	 */
+	if (pf->fw8_3gt) {
+		if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
+			rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+			i40e_vlan_offload_set(dev, RTE_ETH_VLAN_EXTEND_MASK);
+		}
+	}
+
 	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
 	if (ret)
 		PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);
@@ -3909,6 +3922,7 @@ 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 &&
@@ -3927,15 +3941,32 @@ 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)
 				hw->second_tag = rte_cpu_to_le_16(tpid);
 		} else {
-			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-				hw->second_tag = rte_cpu_to_le_16(tpid);
+			/*
+			 * If tpid is equal to 0x88A8, indicates that the
+			 * disable double VLAN operation is in progress.
+			 * Need set switch configuration back to default.
+			 */
+			if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
+				sw_flags = 0;
+				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_OUTER)
+					hw->second_tag = rte_cpu_to_le_16(tpid);
+			}
 		}
-		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+		ret = i40e_aq_set_switch_config(hw, sw_flags,
+						valid_flags, 0, NULL);
 		if (ret != I40E_SUCCESS) {
 			PMD_DRV_LOG(ERR,
 				    "Set switch config failed aq_err: %d",
@@ -3987,8 +4018,13 @@ 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 *mac_filter;
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
+	struct i40e_mac_filter *f;
+	int i, num;
+	void *temp;
+	int ret;
 
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4043,33 @@ 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++;
+		}
 		if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
 			i40e_vsi_config_double_vlan(vsi, TRUE);
 			/* Set global registers with default ethertype. */
@@ -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 					   RTE_ETHER_TYPE_VLAN);
 			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
 					   RTE_ETHER_TYPE_VLAN);
-		}
-		else
+		} else {
+			if (pf->fw8_3gt)
+				i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+					   RTE_ETHER_TYPE_QINQ);
 			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.");
+		}
+		rte_free(mac_filter);
 	}
 
 	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/**
+	 * Enable outer VLAN processing if firmware version is greater
+	 * than v8.3
+	 */
+	if (hw->aq.fw_maj_ver > 8 ||
+	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+		pf->fw8_3gt = true;
+	} else {
+		pf->fw8_3gt = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..fe943a45ff 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
 
+	/* When firmware > 8.3, the enable flag for outer VLAN processing */
+	bool fw8_3gt;
+
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
 	uint64_t prev_tx_bytes;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-10 14:27             ` Ben Magistro
@ 2022-06-13  2:14               ` Liu, KevinX
  2022-06-13 15:01                 ` Ben Magistro
  0 siblings, 1 reply; 18+ messages in thread
From: Liu, KevinX @ 2022-06-13  2:14 UTC (permalink / raw)
  To: Ben Magistro
  Cc: dev, Zhang, Yuying, Xing, Beilei, Yang, SteveX, Zhang, RobinX

[-- Attachment #1: Type: text/plain, Size: 10582 bytes --]

Hi, Ben
This patch can only take effect after firmware v8.6. It cannot be used on firmware v8.4 and v8.5. The reason is that the firmware team gave a clear reply that there are many related bugs in firmware v8.4 and v8.5, and they were fixed in firmware v8.6. The firmware team recommends using v8.6.

Regards
Kevin

From: Ben Magistro <koncept1@gmail.com>
Sent: 2022年6月10日 22:27
To: Liu, KevinX <kevinx.liu@intel.com>
Cc: dev@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
Subject: Re: [PATCH v7] net/i40e: add outer VLAN processing

I'm trying to understand if this change is at all related to an issue we are experiencing with newer firmwares (https://mails.dpdk.org/archives/dev/2022-April/238621.html) that happened to start with 8.4 and affected qinq offload processing.  I can say loading this patch and running testpmd does not seem to have any effect on the issue we are experiencing.

Side note, there is also a minor typo in the comment block "i40e_vlan_tpie_set" vs "i40e_vlan_tpid_set".

Thanks

On Fri, Jun 10, 2022 at 4:30 AM Kevin Liu <kevinx.liu@intel.com<mailto:kevinx.liu@intel.com>> wrote:
From: Robin Zhang <robinx.zhang@intel.com<mailto:robinx.zhang@intel.com>>

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, add support for outer VLAN processing in DPDK.

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 will not impact on an old firmware.

Signed-off-by: Robin Zhang <robinx.zhang@intel.com<mailto:robinx.zhang@intel.com>>
Signed-off-by: Kevin Liu <kevinx.liu@intel.com<mailto:kevinx.liu@intel.com>>
---
 drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..4cae163cb9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
        struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
        struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
        struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+       struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
        struct i40e_filter_control_settings settings;
        struct rte_flow *p_flow;
        uint32_t reg;
@@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
        if (rte_eal_process_type() != RTE_PROC_PRIMARY)
                return 0;

+       /*
+        * It is a workaround, if the double VLAN is disabled when
+        * the program exits, an abnormal error will occur on the
+        * NIC. Need to enable double VLAN when dev is closed.
+        */
+       if (pf->fw8_3gt) {
+               if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
+                       rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+                       i40e_vlan_offload_set(dev, RTE_ETH_VLAN_EXTEND_MASK);
+               }
+       }
+
        ret = rte_eth_switch_domain_free(pf->switch_domain_id);
        if (ret)
                PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);
@@ -3909,6 +3922,7 @@ 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 &&
@@ -3927,15 +3941,32 @@ 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)
                                hw->second_tag = rte_cpu_to_le_16(tpid);
                } else {
-                       if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-                               hw->second_tag = rte_cpu_to_le_16(tpid);
+                       /*
+                        * If tpid is equal to 0x88A8, indicates that the
+                        * disable double VLAN operation is in progress.
+                        * Need set switch configuration back to default.
+                        */
+                       if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
+                               sw_flags = 0;
+                               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_OUTER)
+                                       hw->second_tag = rte_cpu_to_le_16(tpid);
+                       }
                }
-               ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
+               ret = i40e_aq_set_switch_config(hw, sw_flags,
+                                               valid_flags, 0, NULL);
                if (ret != I40E_SUCCESS) {
                        PMD_DRV_LOG(ERR,
                                    "Set switch config failed aq_err: %d",
@@ -3987,8 +4018,13 @@ 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 *mac_filter;
        struct i40e_vsi *vsi = pf->main_vsi;
        struct rte_eth_rxmode *rxmode;
+       struct i40e_mac_filter *f;
+       int i, num;
+       void *temp;
+       int ret;

        rxmode = &dev->data->dev_conf.rxmode;
        if (mask & RTE_ETH_VLAN_FILTER_MASK) {
@@ -4007,6 +4043,33 @@ 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++;
+               }
                if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
                        i40e_vsi_config_double_vlan(vsi, TRUE);
                        /* Set global registers with default ethertype. */
@@ -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
                                           RTE_ETHER_TYPE_VLAN);
                        i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
                                           RTE_ETHER_TYPE_VLAN);
-               }
-               else
+               } else {
+                       if (pf->fw8_3gt)
+                               i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_OUTER,
+                                          RTE_ETHER_TYPE_QINQ);
                        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.");
+               }
+               rte_free(mac_filter);
        }

        if (mask & RTE_ETH_QINQ_STRIP_MASK) {
@@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
                return -EINVAL;
        }

+       /**
+        * Enable outer VLAN processing if firmware version is greater
+        * than v8.3
+        */
+       if (hw->aq.fw_maj_ver > 8 ||
+           (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
+               pf->fw8_3gt = true;
+       } else {
+               pf->fw8_3gt = false;
+       }
+
        return 0;
 }

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index a1ebdc093c..fe943a45ff 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1188,6 +1188,9 @@ struct i40e_pf {
        /* Switch Domain Id */
        uint16_t switch_domain_id;

+       /* When firmware > 8.3, the enable flag for outer VLAN processing */
+       bool fw8_3gt;
+
        struct i40e_vf_msg_cfg vf_msg_cfg;
        uint64_t prev_rx_bytes;
        uint64_t prev_tx_bytes;
--
2.34.1

[-- Attachment #2: Type: text/html, Size: 21406 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-13  2:14               ` Liu, KevinX
@ 2022-06-13 15:01                 ` Ben Magistro
  0 siblings, 0 replies; 18+ messages in thread
From: Ben Magistro @ 2022-06-13 15:01 UTC (permalink / raw)
  To: Liu, KevinX; +Cc: dev, Zhang, Yuying, Xing, Beilei, Yang, SteveX, Zhang, RobinX

[-- Attachment #1: Type: text/plain, Size: 11538 bytes --]

Thanks for the information on 8.4.  Unfortunately most of NICs are from
Dell so will need to push them to produce another update package and would
prefer to wait till we know what is required to fix our issue.

I loaded 8.6 and 8.7 on one of our test hosts and our issue still exists so
will continue pushing the open case we have with Intel as that is where we
still think this lies.

Cheers,
Ben

On Sun, Jun 12, 2022 at 10:14 PM Liu, KevinX <kevinx.liu@intel.com> wrote:

> Hi, Ben
>
> This patch can only take effect after firmware v8.6. It cannot be used on
> firmware v8.4 and v8.5. The reason is that the firmware team gave a clear
> reply that there are many related bugs in firmware v8.4 and v8.5, and they
> were fixed in firmware v8.6. The firmware team recommends using v8.6.
>
>
>
> Regards
>
> Kevin
>
>
>
> *From:* Ben Magistro <koncept1@gmail.com>
> *Sent:* 2022年6月10日 22:27
> *To:* Liu, KevinX <kevinx.liu@intel.com>
> *Cc:* dev@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei <
> beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> RobinX <robinx.zhang@intel.com>
> *Subject:* Re: [PATCH v7] net/i40e: add outer VLAN processing
>
>
>
> I'm trying to understand if this change is at all related to an issue we
> are experiencing with newer firmwares (
> https://mails.dpdk.org/archives/dev/2022-April/238621.html) that happened
> to start with 8.4 and affected qinq offload processing.  I can say loading
> this patch and running testpmd does not seem to have any effect on the
> issue we are experiencing.
>
>
>
> Side note, there is also a minor typo in the comment block
> "i40e_vlan_tpie_set" vs "i40e_vlan_tpid_set".
>
>
>
> Thanks
>
>
>
> On Fri, Jun 10, 2022 at 4:30 AM Kevin Liu <kevinx.liu@intel.com> wrote:
>
> From: Robin Zhang <robinx.zhang@intel.com>
>
> 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, add support for outer VLAN processing in DPDK.
>
> 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 will not impact on an old firmware.
>
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
>  drivers/net/i40e/i40e_ethdev.h |  3 ++
>  2 files changed, 92 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c
> b/drivers/net/i40e/i40e_ethdev.c
> index 755786dc10..4cae163cb9 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
>         struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>         struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>         struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> +       struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>         struct i40e_filter_control_settings settings;
>         struct rte_flow *p_flow;
>         uint32_t reg;
> @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>                 return 0;
>
> +       /*
> +        * It is a workaround, if the double VLAN is disabled when
> +        * the program exits, an abnormal error will occur on the
> +        * NIC. Need to enable double VLAN when dev is closed.
> +        */
> +       if (pf->fw8_3gt) {
> +               if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> +                       rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> +                       i40e_vlan_offload_set(dev,
> RTE_ETH_VLAN_EXTEND_MASK);
> +               }
> +       }
> +
>         ret = rte_eth_switch_domain_free(pf->switch_domain_id);
>         if (ret)
>                 PMD_INIT_LOG(WARNING, "failed to free switch domain: %d",
> ret);
> @@ -3909,6 +3922,7 @@ 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 &&
> @@ -3927,15 +3941,32 @@ 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)
>                                 hw->second_tag = rte_cpu_to_le_16(tpid);
>                 } else {
> -                       if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> -                               hw->second_tag = rte_cpu_to_le_16(tpid);
> +                       /*
> +                        * If tpid is equal to 0x88A8, indicates that the
> +                        * disable double VLAN operation is in progress.
> +                        * Need set switch configuration back to default.
> +                        */
> +                       if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
> +                               sw_flags = 0;
> +                               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_OUTER)
> +                                       hw->second_tag =
> rte_cpu_to_le_16(tpid);
> +                       }
>                 }
> -               ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> +               ret = i40e_aq_set_switch_config(hw, sw_flags,
> +                                               valid_flags, 0, NULL);
>                 if (ret != I40E_SUCCESS) {
>                         PMD_DRV_LOG(ERR,
>                                     "Set switch config failed aq_err: %d",
> @@ -3987,8 +4018,13 @@ 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 *mac_filter;
>         struct i40e_vsi *vsi = pf->main_vsi;
>         struct rte_eth_rxmode *rxmode;
> +       struct i40e_mac_filter *f;
> +       int i, num;
> +       void *temp;
> +       int ret;
>
>         rxmode = &dev->data->dev_conf.rxmode;
>         if (mask & RTE_ETH_VLAN_FILTER_MASK) {
> @@ -4007,6 +4043,33 @@ 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++;
> +               }
>                 if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
>                         i40e_vsi_config_double_vlan(vsi, TRUE);
>                         /* Set global registers with default ethertype. */
> @@ -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> mask)
>                                            RTE_ETHER_TYPE_VLAN);
>                         i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
>                                            RTE_ETHER_TYPE_VLAN);
> -               }
> -               else
> +               } else {
> +                       if (pf->fw8_3gt)
> +                               i40e_vlan_tpid_set(dev,
> RTE_ETH_VLAN_TYPE_OUTER,
> +                                          RTE_ETHER_TYPE_QINQ);
>                         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.");
> +               }
> +               rte_free(mac_filter);
>         }
>
>         if (mask & RTE_ETH_QINQ_STRIP_MASK) {
> @@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
>                 return -EINVAL;
>         }
>
> +       /**
> +        * Enable outer VLAN processing if firmware version is greater
> +        * than v8.3
> +        */
> +       if (hw->aq.fw_maj_ver > 8 ||
> +           (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> +               pf->fw8_3gt = true;
> +       } else {
> +               pf->fw8_3gt = false;
> +       }
> +
>         return 0;
>  }
>
> diff --git a/drivers/net/i40e/i40e_ethdev.h
> b/drivers/net/i40e/i40e_ethdev.h
> index a1ebdc093c..fe943a45ff 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -1188,6 +1188,9 @@ struct i40e_pf {
>         /* Switch Domain Id */
>         uint16_t switch_domain_id;
>
> +       /* When firmware > 8.3, the enable flag for outer VLAN processing
> */
> +       bool fw8_3gt;
> +
>         struct i40e_vf_msg_cfg vf_msg_cfg;
>         uint64_t prev_rx_bytes;
>         uint64_t prev_tx_bytes;
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 16026 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-10 16:29           ` [PATCH v7] " Kevin Liu
  2022-06-10 14:27             ` Ben Magistro
@ 2022-06-14  2:43             ` Zhang, Yuying
  2022-06-14  3:06               ` Liu, KevinX
  1 sibling, 1 reply; 18+ messages in thread
From: Zhang, Yuying @ 2022-06-14  2:43 UTC (permalink / raw)
  To: Liu, KevinX, dev; +Cc: Xing, Beilei, Yang, SteveX, Zhang, RobinX

Hi Kevin,

> -----Original Message-----
> From: Liu, KevinX <kevinx.liu@intel.com>
> Sent: Saturday, June 11, 2022 12:30 AM
> To: dev@dpdk.org
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> RobinX <robinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v7] net/i40e: add outer VLAN processing
> 
> From: Robin Zhang <robinx.zhang@intel.com>
> 
> Outer VLAN processing is supported after firmware v8.4, kernel driver also

Since this patch can only be enabled with firmware v8.6, should you sync with dpdk here?

> change the default behavior to support this feature. To align with kernel
> driver, add support for outer VLAN processing in DPDK.
> 
> 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 will not impact on an old firmware.
> 
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 94 ++++++++++++++++++++++++++++++++--
>  drivers/net/i40e/i40e_ethdev.h |  3 ++
>  2 files changed, 92 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 755786dc10..4cae163cb9 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
>  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>  	struct i40e_filter_control_settings settings;
>  	struct rte_flow *p_flow;
>  	uint32_t reg;
> @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>  		return 0;
> 
> +	/*
> +	 * It is a workaround, if the double VLAN is disabled when
> +	 * the program exits, an abnormal error will occur on the
> +	 * NIC. Need to enable double VLAN when dev is closed.
> +	 */

What is the root cause of this error, I suggest finding a true fix instead of adding
additonal process here.

> +	if (pf->fw8_3gt) {
> +		if (!(rxmode->offloads &
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> +			rxmode->offloads |=
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> +			i40e_vlan_offload_set(dev,
> RTE_ETH_VLAN_EXTEND_MASK);
> +		}
> +	}
> +
>  	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
>  	if (ret)
>  		PMD_INIT_LOG(WARNING, "failed to free switch
> domain: %d", ret); @@ -3909,6 +3922,7 @@ 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 && @@ -3927,15
> +3941,32 @@ 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)
>  				hw->second_tag = rte_cpu_to_le_16(tpid);
>  		} else {
> -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> -				hw->second_tag = rte_cpu_to_le_16(tpid);
> +			/*
> +			 * If tpid is equal to 0x88A8, indicates that the
> +			 * disable double VLAN operation is in progress.
> +			 * Need set switch configuration back to default.
> +			 */

I don't suppose we need to set qinq tpid in vlan case. Please explain this situation in details.

> +			if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
> +				sw_flags = 0;
> +				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_OUTER)
> +					hw->second_tag =
> rte_cpu_to_le_16(tpid);
> +			}
>  		}
> -		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> +		ret = i40e_aq_set_switch_config(hw, sw_flags,
> +						valid_flags, 0, NULL);
>  		if (ret != I40E_SUCCESS) {
>  			PMD_DRV_LOG(ERR,
>  				    "Set switch config failed aq_err: %d", @@ -
> 3987,8 +4018,13 @@ 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 *mac_filter;
>  	struct i40e_vsi *vsi = pf->main_vsi;
>  	struct rte_eth_rxmode *rxmode;
> +	struct i40e_mac_filter *f;
> +	int i, num;
> +	void *temp;
> +	int ret;
> 
>  	rxmode = &dev->data->dev_conf.rxmode;
>  	if (mask & RTE_ETH_VLAN_FILTER_MASK) { @@ -4007,6 +4043,33
> @@ 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++;
> +		}
>  		if (rxmode->offloads &
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
>  			i40e_vsi_config_double_vlan(vsi, TRUE);
>  			/* Set global registers with default ethertype. */ @@
> -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> mask)
>  					   RTE_ETHER_TYPE_VLAN);
>  			i40e_vlan_tpid_set(dev, RTE_ETH_VLAN_TYPE_INNER,
>  					   RTE_ETHER_TYPE_VLAN);
> -		}
> -		else
> +		} else {
> +			if (pf->fw8_3gt)
> +				i40e_vlan_tpid_set(dev,
> RTE_ETH_VLAN_TYPE_OUTER,
> +					   RTE_ETHER_TYPE_QINQ);
>  			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.");
> +		}
> +		rte_free(mac_filter);
>  	}
> 
>  	if (mask & RTE_ETH_QINQ_STRIP_MASK) {
> @@ -4846,6 +4919,17 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
>  		return -EINVAL;
>  	}
> 
> +	/**
> +	 * Enable outer VLAN processing if firmware version is greater
> +	 * than v8.3
> +	 */
> +	if (hw->aq.fw_maj_ver > 8 ||
> +	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> +		pf->fw8_3gt = true;
> +	} else {
> +		pf->fw8_3gt = false;
> +	}
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index a1ebdc093c..fe943a45ff 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -1188,6 +1188,9 @@ struct i40e_pf {
>  	/* Switch Domain Id */
>  	uint16_t switch_domain_id;
> 
> +	/* When firmware > 8.3, the enable flag for outer VLAN processing */
> +	bool fw8_3gt;
> +
>  	struct i40e_vf_msg_cfg vf_msg_cfg;
>  	uint64_t prev_rx_bytes;
>  	uint64_t prev_tx_bytes;
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-14  2:43             ` Zhang, Yuying
@ 2022-06-14  3:06               ` Liu, KevinX
  2022-06-14  8:39                 ` Zhang, Yuying
  0 siblings, 1 reply; 18+ messages in thread
From: Liu, KevinX @ 2022-06-14  3:06 UTC (permalink / raw)
  To: Zhang, Yuying, dev; +Cc: Xing, Beilei, Yang, SteveX, Zhang, RobinX

Hi, Yuying

> -----Original Message-----
> From: Zhang, Yuying <yuying.zhang@intel.com>
> Sent: 2022年6月14日 10:44
> To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> 
> Hi Kevin,
> 
> > -----Original Message-----
> > From: Liu, KevinX <kevinx.liu@intel.com>
> > Sent: Saturday, June 11, 2022 12:30 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Zhang,
> > RobinX <robinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH v7] net/i40e: add outer VLAN processing
> >
> > From: Robin Zhang <robinx.zhang@intel.com>
> >
> > Outer VLAN processing is supported after firmware v8.4, kernel driver
> > also
> 
> Since this patch can only be enabled with firmware v8.6, should you sync with
> dpdk here?
OK, I'll revise it here.
> 
> > change the default behavior to support this feature. To align with
> > kernel driver, add support for outer VLAN processing in DPDK.
> >
> > 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 will not impact on an old firmware.
> >
> > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 94
> > ++++++++++++++++++++++++++++++++--
> >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> >  2 files changed, 92 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..4cae163cb9 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> >  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> >  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> >  	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> >  	struct i40e_filter_control_settings settings;
> >  	struct rte_flow *p_flow;
> >  	uint32_t reg;
> > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> >  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >  		return 0;
> >
> > +	/*
> > +	 * It is a workaround, if the double VLAN is disabled when
> > +	 * the program exits, an abnormal error will occur on the
> > +	 * NIC. Need to enable double VLAN when dev is closed.
> > +	 */
> 
> What is the root cause of this error, I suggest finding a true fix instead of
> adding additonal process here.
About this error, dpdk has reported a known issue. Because it doesn't know the root cause of the problem, it adds a workaround here to temporarily avoid some problems.
> 
> > +	if (pf->fw8_3gt) {
> > +		if (!(rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > +			rxmode->offloads |=
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > +			i40e_vlan_offload_set(dev,
> > RTE_ETH_VLAN_EXTEND_MASK);
> > +		}
> > +	}
> > +
> >  	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
> >  	if (ret)
> >  		PMD_INIT_LOG(WARNING, "failed to free switch
> > domain: %d", ret); @@ -3909,6 +3922,7 @@ 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 && @@ -3927,15
> > +3941,32 @@ 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)
> >  				hw->second_tag = rte_cpu_to_le_16(tpid);
> >  		} else {
> > -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > -				hw->second_tag = rte_cpu_to_le_16(tpid);
> > +			/*
> > +			 * If tpid is equal to 0x88A8, indicates that the
> > +			 * disable double VLAN operation is in progress.
> > +			 * Need set switch configuration back to default.
> > +			 */
> 
> I don't suppose we need to set qinq tpid in vlan case. Please explain this
> situation in details.
I'll think about how to explain this place. Thank you.
> 
> > +			if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
> > +				sw_flags = 0;
> > +				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_OUTER)
> > +					hw->second_tag =
> > rte_cpu_to_le_16(tpid);
> > +			}
> >  		}
> > -		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> > +		ret = i40e_aq_set_switch_config(hw, sw_flags,
> > +						valid_flags, 0, NULL);
> >  		if (ret != I40E_SUCCESS) {
> >  			PMD_DRV_LOG(ERR,
> >  				    "Set switch config failed aq_err: %d", @@ -
> > 3987,8 +4018,13 @@ 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 *mac_filter;
> >  	struct i40e_vsi *vsi = pf->main_vsi;
> >  	struct rte_eth_rxmode *rxmode;
> > +	struct i40e_mac_filter *f;
> > +	int i, num;
> > +	void *temp;
> > +	int ret;
> >
> >  	rxmode = &dev->data->dev_conf.rxmode;
> >  	if (mask & RTE_ETH_VLAN_FILTER_MASK) { @@ -4007,6 +4043,33
> @@
> > 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++;
> > +		}
> >  		if (rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
> >  			i40e_vsi_config_double_vlan(vsi, TRUE);
> >  			/* Set global registers with default ethertype. */ @@
> > -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> > mask)
> >  					   RTE_ETHER_TYPE_VLAN);
> >  			i40e_vlan_tpid_set(dev,
> RTE_ETH_VLAN_TYPE_INNER,
> >  					   RTE_ETHER_TYPE_VLAN);
> > -		}
> > -		else
> > +		} else {
> > +			if (pf->fw8_3gt)
> > +				i40e_vlan_tpid_set(dev,
> > RTE_ETH_VLAN_TYPE_OUTER,
> > +					   RTE_ETHER_TYPE_QINQ);
> >  			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.");
> > +		}
> > +		rte_free(mac_filter);
> >  	}
> >
> >  	if (mask & RTE_ETH_QINQ_STRIP_MASK) { @@ -4846,6 +4919,17 @@
> > i40e_pf_parameter_init(struct rte_eth_dev *dev)
> >  		return -EINVAL;
> >  	}
> >
> > +	/**
> > +	 * Enable outer VLAN processing if firmware version is greater
> > +	 * than v8.3
> > +	 */
> > +	if (hw->aq.fw_maj_ver > 8 ||
> > +	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> > +		pf->fw8_3gt = true;
> > +	} else {
> > +		pf->fw8_3gt = false;
> > +	}
> > +
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.h
> > b/drivers/net/i40e/i40e_ethdev.h index a1ebdc093c..fe943a45ff 100644
> > --- a/drivers/net/i40e/i40e_ethdev.h
> > +++ b/drivers/net/i40e/i40e_ethdev.h
> > @@ -1188,6 +1188,9 @@ struct i40e_pf {
> >  	/* Switch Domain Id */
> >  	uint16_t switch_domain_id;
> >
> > +	/* When firmware > 8.3, the enable flag for outer VLAN processing
> */
> > +	bool fw8_3gt;
> > +
> >  	struct i40e_vf_msg_cfg vf_msg_cfg;
> >  	uint64_t prev_rx_bytes;
> >  	uint64_t prev_tx_bytes;
> > --
> > 2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-14  3:06               ` Liu, KevinX
@ 2022-06-14  8:39                 ` Zhang, Yuying
  2022-06-14  8:43                   ` Liu, KevinX
  2022-06-15 12:12                   ` Zhang, Qi Z
  0 siblings, 2 replies; 18+ messages in thread
From: Zhang, Yuying @ 2022-06-14  8:39 UTC (permalink / raw)
  To: Liu, KevinX, dev; +Cc: Xing, Beilei, Yang, SteveX, Zhang, RobinX

Hi Kevin,

Workaround should be replaced when root cause be found.

Best regards,
Yuying

> -----Original Message-----
> From: Liu, KevinX <kevinx.liu@intel.com>
> Sent: Tuesday, June 14, 2022 11:07 AM
> To: Zhang, Yuying <yuying.zhang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> 
> Hi, Yuying
> 
> > -----Original Message-----
> > From: Zhang, Yuying <yuying.zhang@intel.com>
> > Sent: 2022年6月14日 10:44
> > To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> > <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> >
> > Hi Kevin,
> >
> > > -----Original Message-----
> > > From: Liu, KevinX <kevinx.liu@intel.com>
> > > Sent: Saturday, June 11, 2022 12:30 AM
> > > To: dev@dpdk.org
> > > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> > > Zhang, RobinX <robinx.zhang@intel.com>; Liu, KevinX
> > > <kevinx.liu@intel.com>
> > > Subject: [PATCH v7] net/i40e: add outer VLAN processing
> > >
> > > From: Robin Zhang <robinx.zhang@intel.com>
> > >
> > > Outer VLAN processing is supported after firmware v8.4, kernel
> > > driver also
> >
> > Since this patch can only be enabled with firmware v8.6, should you
> > sync with dpdk here?
> OK, I'll revise it here.
> >
> > > change the default behavior to support this feature. To align with
> > > kernel driver, add support for outer VLAN processing in DPDK.
> > >
> > > 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 will not impact on an old firmware.
> > >
> > > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>

Acked-by: Yuying Zhang <yuying.zhang@intel.com>

> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c | 94
> > > ++++++++++++++++++++++++++++++++--
> > >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> > >  2 files changed, 92 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..4cae163cb9 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > >  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > > >dev_private);
> > >  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > >  	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > > +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > >  	struct i40e_filter_control_settings settings;
> > >  	struct rte_flow *p_flow;
> > >  	uint32_t reg;
> > > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > >  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > >  		return 0;
> > >
> > > +	/*
> > > +	 * It is a workaround, if the double VLAN is disabled when
> > > +	 * the program exits, an abnormal error will occur on the
> > > +	 * NIC. Need to enable double VLAN when dev is closed.
> > > +	 */
> >
> > What is the root cause of this error, I suggest finding a true fix
> > instead of adding additonal process here.
> About this error, dpdk has reported a known issue. Because it doesn't know
> the root cause of the problem, it adds a workaround here to temporarily
> avoid some problems.
> >
> > > +	if (pf->fw8_3gt) {
> > > +		if (!(rxmode->offloads &
> > > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > > +			rxmode->offloads |=
> > > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > > +			i40e_vlan_offload_set(dev,
> > > RTE_ETH_VLAN_EXTEND_MASK);
> > > +		}
> > > +	}
> > > +
> > >  	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
> > >  	if (ret)
> > >  		PMD_INIT_LOG(WARNING, "failed to free switch
> > > domain: %d", ret); @@ -3909,6 +3922,7 @@ 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 && @@ -3927,15
> > > +3941,32 @@ 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)
> > >  				hw->second_tag = rte_cpu_to_le_16(tpid);
> > >  		} else {
> > > -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > > -				hw->second_tag = rte_cpu_to_le_16(tpid);
> > > +			/*
> > > +			 * If tpid is equal to 0x88A8, indicates that the
> > > +			 * disable double VLAN operation is in progress.
> > > +			 * Need set switch configuration back to default.
> > > +			 */
> >
> > I don't suppose we need to set qinq tpid in vlan case. Please explain
> > this situation in details.
> I'll think about how to explain this place. Thank you.
> >
> > > +			if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
> > > +				sw_flags = 0;
> > > +				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_OUTER)
> > > +					hw->second_tag =
> > > rte_cpu_to_le_16(tpid);
> > > +			}
> > >  		}
> > > -		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> > > +		ret = i40e_aq_set_switch_config(hw, sw_flags,
> > > +						valid_flags, 0, NULL);
> > >  		if (ret != I40E_SUCCESS) {
> > >  			PMD_DRV_LOG(ERR,
> > >  				    "Set switch config failed aq_err: %d", @@ -
> > > 3987,8 +4018,13 @@ 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 *mac_filter;
> > >  	struct i40e_vsi *vsi = pf->main_vsi;
> > >  	struct rte_eth_rxmode *rxmode;
> > > +	struct i40e_mac_filter *f;
> > > +	int i, num;
> > > +	void *temp;
> > > +	int ret;
> > >
> > >  	rxmode = &dev->data->dev_conf.rxmode;
> > >  	if (mask & RTE_ETH_VLAN_FILTER_MASK) { @@ -4007,6 +4043,33
> > @@
> > > 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++;
> > > +		}
> > >  		if (rxmode->offloads &
> > > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
> > >  			i40e_vsi_config_double_vlan(vsi, TRUE);
> > >  			/* Set global registers with default ethertype. */ @@
> > > -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> > > int
> > > mask)
> > >  					   RTE_ETHER_TYPE_VLAN);
> > >  			i40e_vlan_tpid_set(dev,
> > RTE_ETH_VLAN_TYPE_INNER,
> > >  					   RTE_ETHER_TYPE_VLAN);
> > > -		}
> > > -		else
> > > +		} else {
> > > +			if (pf->fw8_3gt)
> > > +				i40e_vlan_tpid_set(dev,
> > > RTE_ETH_VLAN_TYPE_OUTER,
> > > +					   RTE_ETHER_TYPE_QINQ);
> > >  			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.");
> > > +		}
> > > +		rte_free(mac_filter);
> > >  	}
> > >
> > >  	if (mask & RTE_ETH_QINQ_STRIP_MASK) { @@ -4846,6 +4919,17
> @@
> > > i40e_pf_parameter_init(struct rte_eth_dev *dev)
> > >  		return -EINVAL;
> > >  	}
> > >
> > > +	/**
> > > +	 * Enable outer VLAN processing if firmware version is greater
> > > +	 * than v8.3
> > > +	 */
> > > +	if (hw->aq.fw_maj_ver > 8 ||
> > > +	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> > > +		pf->fw8_3gt = true;
> > > +	} else {
> > > +		pf->fw8_3gt = false;
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.h
> > > b/drivers/net/i40e/i40e_ethdev.h index a1ebdc093c..fe943a45ff 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.h
> > > +++ b/drivers/net/i40e/i40e_ethdev.h
> > > @@ -1188,6 +1188,9 @@ struct i40e_pf {
> > >  	/* Switch Domain Id */
> > >  	uint16_t switch_domain_id;
> > >
> > > +	/* When firmware > 8.3, the enable flag for outer VLAN processing
> > */
> > > +	bool fw8_3gt;
> > > +
> > >  	struct i40e_vf_msg_cfg vf_msg_cfg;
> > >  	uint64_t prev_rx_bytes;
> > >  	uint64_t prev_tx_bytes;
> > > --
> > > 2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-14  8:39                 ` Zhang, Yuying
@ 2022-06-14  8:43                   ` Liu, KevinX
  2022-06-15 12:12                   ` Zhang, Qi Z
  1 sibling, 0 replies; 18+ messages in thread
From: Liu, KevinX @ 2022-06-14  8:43 UTC (permalink / raw)
  To: Zhang, Yuying, dev; +Cc: Xing, Beilei, Yang, SteveX, Zhang, RobinX

Yes, that is necessary. Thank you very much for your review!

> -----Original Message-----
> From: Zhang, Yuying <yuying.zhang@intel.com>
> Sent: 2022年6月14日 16:40
> To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> 
> Hi Kevin,
> 
> Workaround should be replaced when root cause be found.
> 
> Best regards,
> Yuying
> 
> > -----Original Message-----
> > From: Liu, KevinX <kevinx.liu@intel.com>
> > Sent: Tuesday, June 14, 2022 11:07 AM
> > To: Zhang, Yuying <yuying.zhang@intel.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> > <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> >
> > Hi, Yuying
> >
> > > -----Original Message-----
> > > From: Zhang, Yuying <yuying.zhang@intel.com>
> > > Sent: 2022年6月14日 10:44
> > > To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> > > <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> > > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> > >
> > > Hi Kevin,
> > >
> > > > -----Original Message-----
> > > > From: Liu, KevinX <kevinx.liu@intel.com>
> > > > Sent: Saturday, June 11, 2022 12:30 AM
> > > > To: dev@dpdk.org
> > > > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > > <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> > > > Zhang, RobinX <robinx.zhang@intel.com>; Liu, KevinX
> > > > <kevinx.liu@intel.com>
> > > > Subject: [PATCH v7] net/i40e: add outer VLAN processing
> > > >
> > > > From: Robin Zhang <robinx.zhang@intel.com>
> > > >
> > > > Outer VLAN processing is supported after firmware v8.4, kernel
> > > > driver also
> > >
> > > Since this patch can only be enabled with firmware v8.6, should you
> > > sync with dpdk here?
> > OK, I'll revise it here.
> > >
> > > > change the default behavior to support this feature. To align with
> > > > kernel driver, add support for outer VLAN processing in DPDK.
> > > >
> > > > 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 will not impact on an old firmware.
> > > >
> > > > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > > > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> 
> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
> 
> > > > ---
> > > >  drivers/net/i40e/i40e_ethdev.c | 94
> > > > ++++++++++++++++++++++++++++++++--
> > > >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> > > >  2 files changed, 92 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..4cae163cb9
> > > > 100644
> > > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > > >  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > > > >dev_private);
> > > >  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > > >  	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > > > +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > > >  	struct i40e_filter_control_settings settings;
> > > >  	struct rte_flow *p_flow;
> > > >  	uint32_t reg;
> > > > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > > >  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > > >  		return 0;
> > > >
> > > > +	/*
> > > > +	 * It is a workaround, if the double VLAN is disabled when
> > > > +	 * the program exits, an abnormal error will occur on the
> > > > +	 * NIC. Need to enable double VLAN when dev is closed.
> > > > +	 */
> > >
> > > What is the root cause of this error, I suggest finding a true fix
> > > instead of adding additonal process here.
> > About this error, dpdk has reported a known issue. Because it doesn't
> > know the root cause of the problem, it adds a workaround here to
> > temporarily avoid some problems.
> > >
> > > > +	if (pf->fw8_3gt) {
> > > > +		if (!(rxmode->offloads &
> > > > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > > > +			rxmode->offloads |=
> > > > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > > > +			i40e_vlan_offload_set(dev,
> > > > RTE_ETH_VLAN_EXTEND_MASK);
> > > > +		}
> > > > +	}
> > > > +
> > > >  	ret = rte_eth_switch_domain_free(pf->switch_domain_id);
> > > >  	if (ret)
> > > >  		PMD_INIT_LOG(WARNING, "failed to free switch
> > > > domain: %d", ret); @@ -3909,6 +3922,7 @@ 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 && @@ -3927,15
> > > > +3941,32 @@ 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)
> > > >  				hw->second_tag = rte_cpu_to_le_16(tpid);
> > > >  		} else {
> > > > -			if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > > > -				hw->second_tag = rte_cpu_to_le_16(tpid);
> > > > +			/*
> > > > +			 * If tpid is equal to 0x88A8, indicates that the
> > > > +			 * disable double VLAN operation is in progress.
> > > > +			 * Need set switch configuration back to default.
> > > > +			 */
> > >
> > > I don't suppose we need to set qinq tpid in vlan case. Please
> > > explain this situation in details.
> > I'll think about how to explain this place. Thank you.
> > >
> > > > +			if (pf->fw8_3gt && tpid == RTE_ETHER_TYPE_QINQ) {
> > > > +				sw_flags = 0;
> > > > +				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_OUTER)
> > > > +					hw->second_tag =
> > > > rte_cpu_to_le_16(tpid);
> > > > +			}
> > > >  		}
> > > > -		ret = i40e_aq_set_switch_config(hw, 0, 0, 0, NULL);
> > > > +		ret = i40e_aq_set_switch_config(hw, sw_flags,
> > > > +						valid_flags, 0, NULL);
> > > >  		if (ret != I40E_SUCCESS) {
> > > >  			PMD_DRV_LOG(ERR,
> > > >  				    "Set switch config failed aq_err: %d", @@ -
> > > > 3987,8 +4018,13 @@ 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 *mac_filter;
> > > >  	struct i40e_vsi *vsi = pf->main_vsi;
> > > >  	struct rte_eth_rxmode *rxmode;
> > > > +	struct i40e_mac_filter *f;
> > > > +	int i, num;
> > > > +	void *temp;
> > > > +	int ret;
> > > >
> > > >  	rxmode = &dev->data->dev_conf.rxmode;
> > > >  	if (mask & RTE_ETH_VLAN_FILTER_MASK) { @@ -4007,6 +4043,33
> > > @@
> > > > 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++;
> > > > +		}
> > > >  		if (rxmode->offloads &
> > > > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
> > > >  			i40e_vsi_config_double_vlan(vsi, TRUE);
> > > >  			/* Set global registers with default ethertype. */ @@
> > > > -4014,9 +4077,19 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> > > > int
> > > > mask)
> > > >  					   RTE_ETHER_TYPE_VLAN);
> > > >  			i40e_vlan_tpid_set(dev,
> > > RTE_ETH_VLAN_TYPE_INNER,
> > > >  					   RTE_ETHER_TYPE_VLAN);
> > > > -		}
> > > > -		else
> > > > +		} else {
> > > > +			if (pf->fw8_3gt)
> > > > +				i40e_vlan_tpid_set(dev,
> > > > RTE_ETH_VLAN_TYPE_OUTER,
> > > > +					   RTE_ETHER_TYPE_QINQ);
> > > >  			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.");
> > > > +		}
> > > > +		rte_free(mac_filter);
> > > >  	}
> > > >
> > > >  	if (mask & RTE_ETH_QINQ_STRIP_MASK) { @@ -4846,6 +4919,17
> > @@
> > > > i40e_pf_parameter_init(struct rte_eth_dev *dev)
> > > >  		return -EINVAL;
> > > >  	}
> > > >
> > > > +	/**
> > > > +	 * Enable outer VLAN processing if firmware version is greater
> > > > +	 * than v8.3
> > > > +	 */
> > > > +	if (hw->aq.fw_maj_ver > 8 ||
> > > > +	    (hw->aq.fw_maj_ver == 8 && hw->aq.fw_min_ver > 3)) {
> > > > +		pf->fw8_3gt = true;
> > > > +	} else {
> > > > +		pf->fw8_3gt = false;
> > > > +	}
> > > > +
> > > >  	return 0;
> > > >  }
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_ethdev.h
> > > > b/drivers/net/i40e/i40e_ethdev.h index a1ebdc093c..fe943a45ff
> > > > 100644
> > > > --- a/drivers/net/i40e/i40e_ethdev.h
> > > > +++ b/drivers/net/i40e/i40e_ethdev.h
> > > > @@ -1188,6 +1188,9 @@ struct i40e_pf {
> > > >  	/* Switch Domain Id */
> > > >  	uint16_t switch_domain_id;
> > > >
> > > > +	/* When firmware > 8.3, the enable flag for outer VLAN
> > > > +processing
> > > */
> > > > +	bool fw8_3gt;
> > > > +
> > > >  	struct i40e_vf_msg_cfg vf_msg_cfg;
> > > >  	uint64_t prev_rx_bytes;
> > > >  	uint64_t prev_tx_bytes;
> > > > --
> > > > 2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v7] net/i40e: add outer VLAN processing
  2022-06-14  8:39                 ` Zhang, Yuying
  2022-06-14  8:43                   ` Liu, KevinX
@ 2022-06-15 12:12                   ` Zhang, Qi Z
  1 sibling, 0 replies; 18+ messages in thread
From: Zhang, Qi Z @ 2022-06-15 12:12 UTC (permalink / raw)
  To: Zhang, Yuying, Liu, KevinX, dev; +Cc: Xing, Beilei, Yang, SteveX, Zhang, RobinX



> -----Original Message-----
> From: Zhang, Yuying <yuying.zhang@intel.com>
> Sent: Tuesday, June 14, 2022 4:40 PM
> To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> 
> Hi Kevin,
> 
> Workaround should be replaced when root cause be found.
> 
> Best regards,
> Yuying
> 
> > -----Original Message-----
> > From: Liu, KevinX <kevinx.liu@intel.com>
> > Sent: Tuesday, June 14, 2022 11:07 AM
> > To: Zhang, Yuying <yuying.zhang@intel.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> > <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> >
> > Hi, Yuying
> >
> > > -----Original Message-----
> > > From: Zhang, Yuying <yuying.zhang@intel.com>
> > > Sent: 2022年6月14日 10:44
> > > To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, SteveX
> > > <stevex.yang@intel.com>; Zhang, RobinX <robinx.zhang@intel.com>
> > > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> > >
> > > Hi Kevin,
> > >
> > > > -----Original Message-----
> > > > From: Liu, KevinX <kevinx.liu@intel.com>
> > > > Sent: Saturday, June 11, 2022 12:30 AM
> > > > To: dev@dpdk.org
> > > > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > > <beilei.xing@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> > > > Zhang, RobinX <robinx.zhang@intel.com>; Liu, KevinX
> > > > <kevinx.liu@intel.com>
> > > > Subject: [PATCH v7] net/i40e: add outer VLAN processing
> > > >
> > > > From: Robin Zhang <robinx.zhang@intel.com>
> > > >
> > > > Outer VLAN processing is supported after firmware v8.4, kernel
> > > > driver also
> > >
> > > Since this patch can only be enabled with firmware v8.6, should you
> > > sync with dpdk here?
> > OK, I'll revise it here.
> > >
> > > > change the default behavior to support this feature. To align with
> > > > kernel driver, add support for outer VLAN processing in DPDK.
> > > >
> > > > 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 will not impact on an old firmware.
> > > >
> > > > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > > > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> 
> Acked-by: Yuying Zhang <yuying.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-06-15 12:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18  9:39 [PATCH] net/i40e: add outer VLAN processing Robin Zhang
2022-05-19  6:18 ` [PATCH v2] " Robin Zhang
2022-06-09 11:26   ` [PATCH v3] " Kevin Liu
2022-06-09 14:43     ` [PATCH v4] " Kevin Liu
2022-04-07 20:01       ` [PATCH v5] " Kevin Liu
2022-06-10 15:52         ` [PATCH v6] " Kevin Liu
2022-06-10  8:06           ` Zhang, Qi Z
2022-06-10  8:14             ` Liu, KevinX
2022-06-10 16:29           ` [PATCH v7] " Kevin Liu
2022-06-10 14:27             ` Ben Magistro
2022-06-13  2:14               ` Liu, KevinX
2022-06-13 15:01                 ` Ben Magistro
2022-06-14  2:43             ` Zhang, Yuying
2022-06-14  3:06               ` Liu, KevinX
2022-06-14  8:39                 ` Zhang, Yuying
2022-06-14  8:43                   ` Liu, KevinX
2022-06-15 12:12                   ` Zhang, Qi Z
2022-06-10  1:01       ` [PATCH v4] " Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).