[PATCH net-next 0/5][pull request] i40e: Simplify VSI and VEB handling

[PATCH net-next 0/5][pull request] i40e: Simplify VSI and VEB handling

[Tony Nguyen]

Ivan Vecera says:

The series simplifies handling of VSIs and VEBs by introducing for-each
iterating macros, 'find' helper functions. Also removes the VEB
recursion because the VEBs cannot have sub-VEBs according datasheet and
fixes the support for floating VEBs.

The series content:
Patch 1 - Uses existing helper function for find FDIR VSI instead of loop
Patch 2 - Adds and uses macros to iterate VSI and VEB arrays
Patch 3 - Adds 2 helper functions to find VSIs and VEBs by their SEID
Patch 4 - Fixes broken support for floating VEBs
Patch 5 - Removes VEB recursion and simplifies VEB handling

The following are changes since commit 71b605d32017e5b8d257db7344bc2f8e8fcc973e:
  net: phy: aquantia: add AQR113 PHY ID
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 40GbE

Ivan Vecera (5):
  i40e: Use existing helper to find flow director VSI
  i40e: Introduce and use macros for iterating VSIs and VEBs
  i40e: Add helpers to find VSI and VEB by SEID and use them
  i40e: Fix broken support for floating VEBs
  i40e: Remove VEB recursion

 drivers/net/ethernet/intel/i40e/i40e.h        |  93 ++-
 drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c |  10 +-
 .../net/ethernet/intel/i40e/i40e_debugfs.c    |  97 ++-
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 563 ++++++++----------
 4 files changed, 373 insertions(+), 390 deletions(-)

-- 
2.41.0

[PATCH net-next 1/5] i40e: Use existing helper to find flow director VSI

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

Use existing i40e_find_vsi_by_type() to find a VSI
associated with flow director.

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 54eb55464e31..6e59a9509868 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -15630,6 +15630,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 #ifdef CONFIG_I40E_DCB
 	enum i40e_get_fw_lldp_status_resp lldp_status;
 #endif /* CONFIG_I40E_DCB */
+	struct i40e_vsi *vsi;
 	struct i40e_pf *pf;
 	struct i40e_hw *hw;
 	u16 wol_nvm_bits;
@@ -15640,7 +15641,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 #endif /* CONFIG_I40E_DCB */
 	int err;
 	u32 val;
-	u32 i;
 
 	err = pci_enable_device_mem(pdev);
 	if (err)
@@ -15990,12 +15990,9 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	INIT_LIST_HEAD(&pf->vsi[pf->lan_vsi]->ch_list);
 
 	/* if FDIR VSI was set up, start it now */
-	for (i = 0; i < pf->num_alloc_vsi; i++) {
-		if (pf->vsi[i] && pf->vsi[i]->type == I40E_VSI_FDIR) {
-			i40e_vsi_open(pf->vsi[i]);
-			break;
-		}
-	}
+	vsi = i40e_find_vsi_by_type(pf, I40E_VSI_FDIR);
+	if (vsi)
+		i40e_vsi_open(vsi);
 
 	/* The driver only wants link up/down and module qualification
 	 * reports from firmware.  Note the negative logic.
-- 
2.41.0

Re: [PATCH net-next 1/5] i40e: Use existing helper to find flow director VSI

[Jacob Keller]

On 2/16/2024 1:42 PM, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> Use existing i40e_find_vsi_by_type() to find a VSI
> associated with flow director.
> 
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[PATCH net-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

Introduce i40e_for_each_vsi() and i40e_for_each_veb() helper
macros and use them to iterate relevant arrays.

Replace pattern:
for (i = 0; i < pf->num_alloc_vsi; i++)
by:
i40e_for_each_vsi(pf, i, vsi)

and pattern:
for (i = 0; i < I40E_MAX_VEB; i++)
by
i40e_for_each_veb(pf, i, veb)

These macros also check if array item pf->vsi[i] or pf->veb[i]
are not NULL and skip such items so we can remove redundant
checks from loop bodies.

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h        |  56 ++-
 drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c |  10 +-
 .../net/ethernet/intel/i40e/i40e_debugfs.c    |  54 +--
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 389 ++++++++----------
 4 files changed, 264 insertions(+), 245 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 9b701615c7c6..5acb26644be7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -686,6 +686,54 @@ struct i40e_pf {
 	struct list_head ddp_old_prof;
 };
 
+/**
+ * __i40e_pf_next_vsi - get next valid VSI
+ * @pf: pointer to the PF struct
+ * @idx: pointer to start position number
+ *
+ * Find and return next non-NULL VSI pointer in pf->vsi array and
+ * updates idx position. Returns NULL if no VSI is found.
+ **/
+static __always_inline struct i40e_vsi *
+__i40e_pf_next_vsi(struct i40e_pf *pf, int *idx)
+{
+	while (*idx < pf->num_alloc_vsi) {
+		if (pf->vsi[*idx])
+			return pf->vsi[*idx];
+		(*idx)++;
+	}
+	return NULL;
+}
+
+#define i40e_pf_for_each_vsi(_pf, _i, _vsi)			\
+	for (_i = 0, _vsi = __i40e_pf_next_vsi(_pf, &_i);	\
+	     _vsi;						\
+	     _i++, _vsi = __i40e_pf_next_vsi(_pf, &_i))
+
+/**
+ * __i40e_pf_next_veb - get next valid VEB
+ * @pf: pointer to the PF struct
+ * @idx: pointer to start position number
+ *
+ * Find and return next non-NULL VEB pointer in pf->veb array and
+ * updates idx position. Returns NULL if no VEB is found.
+ **/
+static __always_inline struct i40e_veb *
+__i40e_pf_next_veb(struct i40e_pf *pf, int *idx)
+{
+	while (*idx < I40E_MAX_VEB) {
+		if (pf->veb[*idx])
+			return pf->veb[*idx];
+		(*idx)++;
+	}
+	return NULL;
+}
+
+#define i40e_pf_for_each_veb(_pf, _i, _veb)			\
+	for (_i = 0, _veb = __i40e_pf_next_veb(_pf, &_i);	\
+	     _veb;						\
+	     _i++, _veb = __i40e_pf_next_veb(_pf, &_i))
+
 /**
  * i40e_mac_to_hkey - Convert a 6-byte MAC Address to a u64 hash key
  * @macaddr: the MAC Address as the base key
@@ -1120,14 +1168,12 @@ struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id);
 static inline struct i40e_vsi *
 i40e_find_vsi_by_type(struct i40e_pf *pf, u16 type)
 {
+	struct i40e_vsi *vsi;
 	int i;
 
-	for (i = 0; i < pf->num_alloc_vsi; i++) {
-		struct i40e_vsi *vsi = pf->vsi[i];
-
-		if (vsi && vsi->type == type)
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->type == type)
 			return vsi;
-	}
 
 	return NULL;
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
index b96a92187ab3..8aa43aefe84c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
@@ -947,16 +947,16 @@ static int i40e_dcbnl_vsi_del_app(struct i40e_vsi *vsi,
 static void i40e_dcbnl_del_app(struct i40e_pf *pf,
 			       struct i40e_dcb_app_priority_table *app)
 {
+	struct i40e_vsi *vsi;
 	int v, err;
 
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (pf->vsi[v] && pf->vsi[v]->netdev) {
-			err = i40e_dcbnl_vsi_del_app(pf->vsi[v], app);
+	i40e_pf_for_each_vsi(pf, v, vsi)
+		if (vsi->netdev) {
+			err = i40e_dcbnl_vsi_del_app(vsi, app);
 			dev_dbg(&pf->pdev->dev, "Deleting app for VSI seid=%d err=%d sel=%d proto=0x%x prio=%d\n",
-				pf->vsi[v]->seid, err, app->selector,
+				vsi->seid, err, app->selector,
 				app->protocolid, app->priority);
 		}
-	}
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index ef70ddbe9c2f..b236b0f93202 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -24,14 +24,18 @@ enum ring_type {
  **/
 static struct i40e_vsi *i40e_dbg_find_vsi(struct i40e_pf *pf, int seid)
 {
+	struct i40e_vsi *vsi;
 	int i;
 
-	if (seid < 0)
+	if (seid < 0) {
 		dev_info(&pf->pdev->dev, "%d: bad seid\n", seid);
-	else
-		for (i = 0; i < pf->num_alloc_vsi; i++)
-			if (pf->vsi[i] && (pf->vsi[i]->seid == seid))
-				return pf->vsi[i];
+
+		return NULL;
+	}
+
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->seid == seid)
+			return vsi;
 
 	return NULL;
 }
@@ -43,11 +47,13 @@ static struct i40e_vsi *i40e_dbg_find_vsi(struct i40e_pf *pf, int seid)
  **/
 static struct i40e_veb *i40e_dbg_find_veb(struct i40e_pf *pf, int seid)
 {
+	struct i40e_veb *veb;
 	int i;
 
-	for (i = 0; i < I40E_MAX_VEB; i++)
-		if (pf->veb[i] && pf->veb[i]->seid == seid)
-			return pf->veb[i];
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->seid == seid)
+			return veb;
+
 	return NULL;
 }
 
@@ -653,12 +659,11 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
  **/
 static void i40e_dbg_dump_vsi_no_seid(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int i;
 
-	for (i = 0; i < pf->num_alloc_vsi; i++)
-		if (pf->vsi[i])
-			dev_info(&pf->pdev->dev, "dump vsi[%d]: %d\n",
-				 i, pf->vsi[i]->seid);
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		dev_info(&pf->pdev->dev, "dump vsi[%d]: %d\n", i, vsi->seid);
 }
 
 /**
@@ -718,11 +723,8 @@ static void i40e_dbg_dump_veb_all(struct i40e_pf *pf)
 	struct i40e_veb *veb;
 	int i;
 
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		veb = pf->veb[i];
-		if (veb)
-			i40e_dbg_dump_veb_seid(pf, veb->seid);
-	}
+	i40e_pf_for_each_veb(pf, i, veb)
+		i40e_dbg_dump_veb_seid(pf, veb->seid);
 }
 
 /**
@@ -873,9 +875,10 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			goto command_write_done;
 		}
 
-		for (i = 0; i < I40E_MAX_VEB; i++)
-			if (pf->veb[i] && pf->veb[i]->seid == uplink_seid)
+		i40e_pf_for_each_veb(pf, i, veb)
+			if (veb->seid == uplink_seid)
 				break;
+
 		if (i >= I40E_MAX_VEB && uplink_seid != 0 &&
 		    uplink_seid != pf->mac_seid) {
 			dev_info(&pf->pdev->dev,
@@ -892,7 +895,9 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			dev_info(&pf->pdev->dev, "add relay failed\n");
 
 	} else if (strncmp(cmd_buf, "del relay", 9) == 0) {
+		struct i40e_veb *veb;
 		int i;
+
 		cnt = sscanf(&cmd_buf[9], "%i", &veb_seid);
 		if (cnt != 1) {
 			dev_info(&pf->pdev->dev,
@@ -906,9 +911,10 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 		}
 
 		/* find the veb */
-		for (i = 0; i < I40E_MAX_VEB; i++)
-			if (pf->veb[i] && pf->veb[i]->seid == veb_seid)
+		i40e_pf_for_each_veb(pf, i, veb)
+			if (veb->seid == veb_seid)
 				break;
+
 		if (i >= I40E_MAX_VEB) {
 			dev_info(&pf->pdev->dev,
 				 "del relay: relay %d not found\n", veb_seid);
@@ -916,7 +922,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 		}
 
 		dev_info(&pf->pdev->dev, "deleting relay %d\n", veb_seid);
-		i40e_veb_release(pf->veb[i]);
+		i40e_veb_release(veb);
 	} else if (strncmp(cmd_buf, "add pvid", 8) == 0) {
 		unsigned int v;
 		int ret;
@@ -1251,8 +1257,8 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			if (cnt == 0) {
 				int i;
 
-				for (i = 0; i < pf->num_alloc_vsi; i++)
-					i40e_vsi_reset_stats(pf->vsi[i]);
+				i40e_pf_for_each_vsi(pf, i, vsi)
+					i40e_vsi_reset_stats(vsi);
 				dev_info(&pf->pdev->dev, "vsi clear stats called for all vsi's\n");
 			} else if (cnt == 1) {
 				vsi = i40e_dbg_find_vsi(pf, vsi_seid);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 6e59a9509868..43e1b4f7f9dc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -310,11 +310,12 @@ static int i40e_put_lump(struct i40e_lump_tracking *pile, u16 index, u16 id)
  **/
 struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id)
 {
+	struct i40e_vsi *vsi;
 	int i;
 
-	for (i = 0; i < pf->num_alloc_vsi; i++)
-		if (pf->vsi[i] && (pf->vsi[i]->id == id))
-			return pf->vsi[i];
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->id == id)
+			return vsi;
 
 	return NULL;
 }
@@ -552,24 +553,19 @@ void i40e_vsi_reset_stats(struct i40e_vsi *vsi)
  **/
 void i40e_pf_reset_stats(struct i40e_pf *pf)
 {
+	struct i40e_veb *veb;
 	int i;
 
 	memset(&pf->stats, 0, sizeof(pf->stats));
 	memset(&pf->stats_offsets, 0, sizeof(pf->stats_offsets));
 	pf->stat_offsets_loaded = false;
 
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		if (pf->veb[i]) {
-			memset(&pf->veb[i]->stats, 0,
-			       sizeof(pf->veb[i]->stats));
-			memset(&pf->veb[i]->stats_offsets, 0,
-			       sizeof(pf->veb[i]->stats_offsets));
-			memset(&pf->veb[i]->tc_stats, 0,
-			       sizeof(pf->veb[i]->tc_stats));
-			memset(&pf->veb[i]->tc_stats_offsets, 0,
-			       sizeof(pf->veb[i]->tc_stats_offsets));
-			pf->veb[i]->stat_offsets_loaded = false;
-		}
+	i40e_pf_for_each_veb(pf, i, veb) {
+		memset(&veb->stats, 0, sizeof(veb->stats));
+		memset(&veb->stats_offsets, 0, sizeof(veb->stats_offsets));
+		memset(&veb->tc_stats, 0, sizeof(veb->tc_stats));
+		memset(&veb->tc_stats_offsets, 0, sizeof(veb->tc_stats_offsets));
+		veb->stat_offsets_loaded = false;
 	}
 	pf->hw_csum_rx_error = 0;
 }
@@ -2879,6 +2875,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
  **/
 static void i40e_sync_filters_subtask(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int v;
 
 	if (!pf)
@@ -2890,11 +2887,10 @@ static void i40e_sync_filters_subtask(struct i40e_pf *pf)
 		return;
 	}
 
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (pf->vsi[v] &&
-		    (pf->vsi[v]->flags & I40E_VSI_FLAG_FILTER_CHANGED) &&
-		    !test_bit(__I40E_VSI_RELEASING, pf->vsi[v]->state)) {
-			int ret = i40e_sync_vsi_filters(pf->vsi[v]);
+	i40e_pf_for_each_vsi(pf, v, vsi) {
+		if ((vsi->flags & I40E_VSI_FLAG_FILTER_CHANGED) &&
+		    !test_bit(__I40E_VSI_RELEASING, vsi->state)) {
+			int ret = i40e_sync_vsi_filters(vsi);
 
 			if (ret) {
 				/* come back and try again later */
@@ -5166,6 +5162,7 @@ static void i40e_reset_interrupt_capability(struct i40e_pf *pf)
  **/
 static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int i;
 
 	if (test_bit(__I40E_MISC_IRQ_REQUESTED, pf->state))
@@ -5175,9 +5172,10 @@ static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)
 		      I40E_IWARP_IRQ_PILE_ID);
 
 	i40e_put_lump(pf->irq_pile, 0, I40E_PILE_VALID_BIT-1);
-	for (i = 0; i < pf->num_alloc_vsi; i++)
-		if (pf->vsi[i])
-			i40e_vsi_free_q_vectors(pf->vsi[i]);
+
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		i40e_vsi_free_q_vectors(vsi);
+
 	i40e_reset_interrupt_capability(pf);
 }
 
@@ -5274,12 +5272,11 @@ static void i40e_unquiesce_vsi(struct i40e_vsi *vsi)
  **/
 static void i40e_pf_quiesce_all_vsi(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int v;
 
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (pf->vsi[v])
-			i40e_quiesce_vsi(pf->vsi[v]);
-	}
+	i40e_pf_for_each_vsi(pf, v, vsi)
+		i40e_quiesce_vsi(vsi);
 }
 
 /**
@@ -5288,12 +5285,11 @@ static void i40e_pf_quiesce_all_vsi(struct i40e_pf *pf)
  **/
 static void i40e_pf_unquiesce_all_vsi(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int v;
 
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (pf->vsi[v])
-			i40e_unquiesce_vsi(pf->vsi[v]);
-	}
+	i40e_pf_for_each_vsi(pf, v, vsi)
+		i40e_unquiesce_vsi(vsi);
 }
 
 /**
@@ -5354,14 +5350,13 @@ int i40e_vsi_wait_queues_disabled(struct i40e_vsi *vsi)
  **/
 static int i40e_pf_wait_queues_disabled(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int v, ret = 0;
 
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (pf->vsi[v]) {
-			ret = i40e_vsi_wait_queues_disabled(pf->vsi[v]);
-			if (ret)
-				break;
-		}
+	i40e_pf_for_each_vsi(pf, v, vsi) {
+		ret = i40e_vsi_wait_queues_disabled(vsi);
+		if (ret)
+			break;
 	}
 
 	return ret;
@@ -6778,32 +6773,29 @@ int i40e_veb_config_tc(struct i40e_veb *veb, u8 enabled_tc)
  **/
 static void i40e_dcb_reconfigure(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
+	struct i40e_veb *veb;
 	u8 tc_map = 0;
 	int ret;
-	u8 v;
+	int v;
 
 	/* Enable the TCs available on PF to all VEBs */
 	tc_map = i40e_pf_get_tc_map(pf);
 	if (tc_map == I40E_DEFAULT_TRAFFIC_CLASS)
 		return;
 
-	for (v = 0; v < I40E_MAX_VEB; v++) {
-		if (!pf->veb[v])
-			continue;
-		ret = i40e_veb_config_tc(pf->veb[v], tc_map);
+	i40e_pf_for_each_veb(pf, v, veb) {
+		ret = i40e_veb_config_tc(veb, tc_map);
 		if (ret) {
 			dev_info(&pf->pdev->dev,
 				 "Failed configuring TC for VEB seid=%d\n",
-				 pf->veb[v]->seid);
+				 veb->seid);
 			/* Will try to configure as many components */
 		}
 	}
 
 	/* Update each VSI */
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (!pf->vsi[v])
-			continue;
-
+	i40e_pf_for_each_vsi(pf, v, vsi) {
 		/* - Enable all TCs for the LAN VSI
 		 * - For all others keep them at TC0 for now
 		 */
@@ -6812,17 +6804,17 @@ static void i40e_dcb_reconfigure(struct i40e_pf *pf)
 		else
 			tc_map = I40E_DEFAULT_TRAFFIC_CLASS;
 
-		ret = i40e_vsi_config_tc(pf->vsi[v], tc_map);
+		ret = i40e_vsi_config_tc(vsi, tc_map);
 		if (ret) {
 			dev_info(&pf->pdev->dev,
 				 "Failed configuring TC for VSI seid=%d\n",
-				 pf->vsi[v]->seid);
+				 vsi->seid);
 			/* Will try to configure as many components */
 		} else {
 			/* Re-configure VSI vectors based on updated TC map */
-			i40e_vsi_map_rings_to_vectors(pf->vsi[v]);
-			if (pf->vsi[v]->netdev)
-				i40e_dcbnl_set_all(pf->vsi[v]);
+			i40e_vsi_map_rings_to_vectors(vsi);
+			if (vsi->netdev)
+				i40e_dcbnl_set_all(vsi);
 		}
 	}
 }
@@ -9257,7 +9249,9 @@ int i40e_close(struct net_device *netdev)
  **/
 void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags, bool lock_acquired)
 {
+	struct i40e_vsi *vsi;
 	u32 val;
+	int i;
 
 	/* do the biggest reset indicated */
 	if (reset_flags & BIT_ULL(__I40E_GLOBAL_RESET_REQUESTED)) {
@@ -9313,29 +9307,20 @@ void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags, bool lock_acquired)
 			 "FW LLDP is enabled\n");
 
 	} else if (reset_flags & BIT_ULL(__I40E_REINIT_REQUESTED)) {
-		int v;
-
 		/* Find the VSI(s) that requested a re-init */
-		dev_info(&pf->pdev->dev,
-			 "VSI reinit requested\n");
-		for (v = 0; v < pf->num_alloc_vsi; v++) {
-			struct i40e_vsi *vsi = pf->vsi[v];
+		dev_info(&pf->pdev->dev, "VSI reinit requested\n");
 
-			if (vsi != NULL &&
-			    test_and_clear_bit(__I40E_VSI_REINIT_REQUESTED,
+		i40e_pf_for_each_vsi(pf, i, vsi) {
+			if (test_and_clear_bit(__I40E_VSI_REINIT_REQUESTED,
 					       vsi->state))
-				i40e_vsi_reinit_locked(pf->vsi[v]);
+				i40e_vsi_reinit_locked(vsi);
 		}
 	} else if (reset_flags & BIT_ULL(__I40E_DOWN_REQUESTED)) {
-		int v;
-
 		/* Find the VSI(s) that needs to be brought down */
 		dev_info(&pf->pdev->dev, "VSI down requested\n");
-		for (v = 0; v < pf->num_alloc_vsi; v++) {
-			struct i40e_vsi *vsi = pf->vsi[v];
 
-			if (vsi != NULL &&
-			    test_and_clear_bit(__I40E_VSI_DOWN_REQUESTED,
+		i40e_pf_for_each_vsi(pf, i, vsi) {
+			if (test_and_clear_bit(__I40E_VSI_DOWN_REQUESTED,
 					       vsi->state)) {
 				set_bit(__I40E_VSI_DOWN, vsi->state);
 				i40e_down(vsi);
@@ -9888,6 +9873,8 @@ static void i40e_vsi_link_event(struct i40e_vsi *vsi, bool link_up)
  **/
 static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
 {
+	struct i40e_veb *veb_it;
+	struct i40e_vsi *vsi;
 	struct i40e_pf *pf;
 	int i;
 
@@ -9896,14 +9883,14 @@ static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
 	pf = veb->pf;
 
 	/* depth first... */
-	for (i = 0; i < I40E_MAX_VEB; i++)
-		if (pf->veb[i] && (pf->veb[i]->uplink_seid == veb->seid))
-			i40e_veb_link_event(pf->veb[i], link_up);
+	i40e_pf_for_each_veb(pf, i, veb_it)
+		if (veb_it->uplink_seid == veb->seid)
+			i40e_veb_link_event(veb_it, link_up);
 
 	/* ... now the local VSIs */
-	for (i = 0; i < pf->num_alloc_vsi; i++)
-		if (pf->vsi[i] && (pf->vsi[i]->uplink_seid == veb->seid))
-			i40e_vsi_link_event(pf->vsi[i], link_up);
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->uplink_seid == veb->seid)
+			i40e_vsi_link_event(vsi, link_up);
 }
 
 /**
@@ -9995,6 +9982,8 @@ static void i40e_link_event(struct i40e_pf *pf)
  **/
 static void i40e_watchdog_subtask(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
+	struct i40e_veb *veb;
 	int i;
 
 	/* if interface is down do nothing */
@@ -10015,15 +10004,14 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
 	/* Update the stats for active netdevs so the network stack
 	 * can look at updated numbers whenever it cares to
 	 */
-	for (i = 0; i < pf->num_alloc_vsi; i++)
-		if (pf->vsi[i] && pf->vsi[i]->netdev)
-			i40e_update_stats(pf->vsi[i]);
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->netdev)
+			i40e_update_stats(vsi);
 
 	if (test_bit(I40E_FLAG_VEB_STATS_ENA, pf->flags)) {
 		/* Update the stats for the active switching components */
-		for (i = 0; i < I40E_MAX_VEB; i++)
-			if (pf->veb[i])
-				i40e_update_veb_stats(pf->veb[i]);
+		i40e_pf_for_each_veb(pf, i, veb)
+			i40e_update_veb_stats(veb);
 	}
 
 	i40e_ptp_rx_hang(pf);
@@ -10380,18 +10368,18 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
 {
 	struct i40e_vsi *ctl_vsi = NULL;
 	struct i40e_pf *pf = veb->pf;
-	int v, veb_idx;
-	int ret;
+	struct i40e_veb *veb_it;
+	struct i40e_vsi *vsi;
+	int v, ret;
 
 	/* build VSI that owns this VEB, temporarily attached to base VEB */
-	for (v = 0; v < pf->num_alloc_vsi && !ctl_vsi; v++) {
-		if (pf->vsi[v] &&
-		    pf->vsi[v]->veb_idx == veb->idx &&
-		    pf->vsi[v]->flags & I40E_VSI_FLAG_VEB_OWNER) {
-			ctl_vsi = pf->vsi[v];
+	i40e_pf_for_each_vsi(pf, v, vsi)
+		if (vsi->veb_idx == veb->idx &&
+		    vsi->flags & I40E_VSI_FLAG_VEB_OWNER) {
+			ctl_vsi = vsi;
 			break;
 		}
-	}
+
 	if (!ctl_vsi) {
 		dev_info(&pf->pdev->dev,
 			 "missing owner VSI for veb_idx %d\n", veb->idx);
@@ -10421,13 +10409,11 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
 	i40e_config_bridge_mode(veb);
 
 	/* create the remaining VSIs attached to this VEB */
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (!pf->vsi[v] || pf->vsi[v] == ctl_vsi)
+	i40e_pf_for_each_vsi(pf, v, vsi) {
+		if (vsi == ctl_vsi)
 			continue;
 
-		if (pf->vsi[v]->veb_idx == veb->idx) {
-			struct i40e_vsi *vsi = pf->vsi[v];
-
+		if (vsi->veb_idx == veb->idx) {
 			vsi->uplink_seid = veb->seid;
 			ret = i40e_add_vsi(vsi);
 			if (ret) {
@@ -10441,10 +10427,10 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
 	}
 
 	/* create any VEBs attached to this VEB - RECURSION */
-	for (veb_idx = 0; veb_idx < I40E_MAX_VEB; veb_idx++) {
-		if (pf->veb[veb_idx] && pf->veb[veb_idx]->veb_idx == veb->idx) {
-			pf->veb[veb_idx]->uplink_seid = veb->seid;
-			ret = i40e_reconstitute_veb(pf->veb[veb_idx]);
+	i40e_pf_for_each_veb(pf, v, veb_it) {
+		if (veb_it->veb_idx == veb->idx) {
+			veb_it->uplink_seid = veb->seid;
+			ret = i40e_reconstitute_veb(veb_it);
 			if (ret)
 				break;
 		}
@@ -10718,6 +10704,7 @@ static void i40e_clean_xps_state(struct i40e_vsi *vsi)
 static void i40e_prep_for_reset(struct i40e_pf *pf)
 {
 	struct i40e_hw *hw = &pf->hw;
+	struct i40e_vsi *vsi;
 	int ret = 0;
 	u32 v;
 
@@ -10732,11 +10719,9 @@ static void i40e_prep_for_reset(struct i40e_pf *pf)
 	/* quiesce the VSIs and their queues that are not already DOWN */
 	i40e_pf_quiesce_all_vsi(pf);
 
-	for (v = 0; v < pf->num_alloc_vsi; v++) {
-		if (pf->vsi[v]) {
-			i40e_clean_xps_state(pf->vsi[v]);
-			pf->vsi[v]->seid = 0;
-		}
+	i40e_pf_for_each_vsi(pf, v, vsi) {
+		i40e_clean_xps_state(vsi);
+		vsi->seid = 0;
 	}
 
 	i40e_shutdown_adminq(&pf->hw);
@@ -10850,6 +10835,7 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 	const bool is_recovery_mode_reported = i40e_check_recovery_mode(pf);
 	struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi];
 	struct i40e_hw *hw = &pf->hw;
+	struct i40e_veb *veb;
 	int ret;
 	u32 val;
 	int v;
@@ -10992,14 +10978,10 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 	if (vsi->uplink_seid != pf->mac_seid) {
 		dev_dbg(&pf->pdev->dev, "attempting to rebuild switch\n");
 		/* find the one VEB connected to the MAC, and find orphans */
-		for (v = 0; v < I40E_MAX_VEB; v++) {
-			if (!pf->veb[v])
-				continue;
-
-			if (pf->veb[v]->uplink_seid == pf->mac_seid ||
-			    pf->veb[v]->uplink_seid == 0) {
-				ret = i40e_reconstitute_veb(pf->veb[v]);
-
+		i40e_pf_for_each_veb(pf, v, veb) {
+			if (veb->uplink_seid == pf->mac_seid ||
+			    veb->uplink_seid == 0) {
+				ret = i40e_reconstitute_veb(veb);
 				if (!ret)
 					continue;
 
@@ -11009,13 +10991,13 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 				 * If orphan failed, we'll report the error
 				 * but try to keep going.
 				 */
-				if (pf->veb[v]->uplink_seid == pf->mac_seid) {
+				if (veb->uplink_seid == pf->mac_seid) {
 					dev_info(&pf->pdev->dev,
 						 "rebuild of switch failed: %d, will try to set up simple PF connection\n",
 						 ret);
 					vsi->uplink_seid = pf->mac_seid;
 					break;
-				} else if (pf->veb[v]->uplink_seid == 0) {
+				} else if (veb->uplink_seid == 0) {
 					dev_info(&pf->pdev->dev,
 						 "rebuild of orphan VEB failed: %d\n",
 						 ret);
@@ -12098,6 +12080,7 @@ static int i40e_init_interrupt_scheme(struct i40e_pf *pf)
  */
 static int i40e_restore_interrupt_scheme(struct i40e_pf *pf)
 {
+	struct i40e_vsi *vsi;
 	int err, i;
 
 	/* We cleared the MSI and MSI-X flags when disabling the old interrupt
@@ -12114,13 +12097,12 @@ static int i40e_restore_interrupt_scheme(struct i40e_pf *pf)
 	/* Now that we've re-acquired IRQs, we need to remap the vectors and
 	 * rings together again.
 	 */
-	for (i = 0; i < pf->num_alloc_vsi; i++) {
-		if (pf->vsi[i]) {
-			err = i40e_vsi_alloc_q_vectors(pf->vsi[i]);
-			if (err)
-				goto err_unwind;
-			i40e_vsi_map_rings_to_vectors(pf->vsi[i]);
-		}
+	i40e_pf_for_each_vsi(pf, i, vsi) {
+		err = i40e_vsi_alloc_q_vectors(vsi);
+		if (err)
+			goto err_unwind;
+
+		i40e_vsi_map_rings_to_vectors(vsi);
 	}
 
 	err = i40e_setup_misc_vector(pf);
@@ -13122,8 +13104,8 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
 	struct i40e_netdev_priv *np = netdev_priv(dev);
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
-	struct i40e_veb *veb = NULL;
 	struct nlattr *attr, *br_spec;
+	struct i40e_veb *veb;
 	int i, rem;
 
 	/* Only for PF VSI for now */
@@ -13131,10 +13113,11 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
 		return -EOPNOTSUPP;
 
 	/* Find the HW bridge for PF VSI */
-	for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
-		if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
-			veb = pf->veb[i];
-	}
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->seid == vsi->uplink_seid)
+			break;
+	if (i == I40E_MAX_VEB)
+		veb = NULL; /* No VEB found */
 
 	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
 	if (!br_spec)
@@ -13207,12 +13190,10 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 		return -EOPNOTSUPP;
 
 	/* Find the HW bridge for the PF VSI */
-	for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
-		if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
-			veb = pf->veb[i];
-	}
-
-	if (!veb)
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->seid == vsi->uplink_seid)
+			break;
+	if (i == I40E_MAX_VEB)
 		return 0;
 
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
@@ -14143,9 +14124,9 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
  **/
 int i40e_vsi_release(struct i40e_vsi *vsi)
 {
+	struct i40e_veb *veb, *veb_it;
 	struct i40e_mac_filter *f;
 	struct hlist_node *h;
-	struct i40e_veb *veb = NULL;
 	struct i40e_pf *pf;
 	u16 uplink_seid;
 	int i, n, bkt;
@@ -14215,20 +14196,18 @@ int i40e_vsi_release(struct i40e_vsi *vsi)
 	 * the orphan VEBs yet.  We'll wait for an explicit remove request
 	 * from up the network stack.
 	 */
-	for (n = 0, i = 0; i < pf->num_alloc_vsi; i++) {
-		if (pf->vsi[i] &&
-		    pf->vsi[i]->uplink_seid == uplink_seid &&
-		    (pf->vsi[i]->flags & I40E_VSI_FLAG_VEB_OWNER) == 0) {
+	n = 0;
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->uplink_seid == uplink_seid &&
+		    (vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
 			n++;      /* count the VSIs */
-		}
-	}
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		if (!pf->veb[i])
-			continue;
-		if (pf->veb[i]->uplink_seid == uplink_seid)
+
+	veb = NULL;
+	i40e_pf_for_each_veb(pf, i, veb_it) {
+		if (veb_it->uplink_seid == uplink_seid)
 			n++;     /* count the VEBs */
-		if (pf->veb[i]->seid == uplink_seid)
-			veb = pf->veb[i];
+		if (veb_it->seid == uplink_seid)
+			veb = veb_it;
 	}
 	if (n == 0 && veb && veb->uplink_seid != 0)
 		i40e_veb_release(veb);
@@ -14405,22 +14384,18 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 	 *
 	 * Find which uplink_seid we were given and create a new VEB if needed
 	 */
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		if (pf->veb[i] && pf->veb[i]->seid == uplink_seid) {
-			veb = pf->veb[i];
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->seid == uplink_seid)
 			break;
-		}
-	}
+	if (i == I40E_MAX_VEB)
+		veb = NULL;
 
 	if (!veb && uplink_seid != pf->mac_seid) {
-
-		for (i = 0; i < pf->num_alloc_vsi; i++) {
-			if (pf->vsi[i] && pf->vsi[i]->seid == uplink_seid) {
-				vsi = pf->vsi[i];
+		i40e_pf_for_each_vsi(pf, i, vsi)
+			if (vsi->seid == uplink_seid)
 				break;
-			}
-		}
-		if (!vsi) {
+
+		if (i == pf->num_alloc_vsi) {
 			dev_info(&pf->pdev->dev, "no such uplink_seid %d\n",
 				 uplink_seid);
 			return NULL;
@@ -14448,11 +14423,10 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 			}
 			i40e_config_bridge_mode(veb);
 		}
-		for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
-			if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
-				veb = pf->veb[i];
-		}
-		if (!veb) {
+		i40e_pf_for_each_veb(pf, i, veb)
+			if (veb->seid == vsi->uplink_seid)
+				break;
+		if (i == I40E_MAX_VEB) {
 			dev_info(&pf->pdev->dev, "couldn't add VEB\n");
 			return NULL;
 		}
@@ -14681,29 +14655,24 @@ static void i40e_switch_branch_release(struct i40e_veb *branch)
 	struct i40e_pf *pf = branch->pf;
 	u16 branch_seid = branch->seid;
 	u16 veb_idx = branch->idx;
+	struct i40e_vsi *vsi;
+	struct i40e_veb *veb;
 	int i;
 
 	/* release any VEBs on this VEB - RECURSION */
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		if (!pf->veb[i])
-			continue;
-		if (pf->veb[i]->uplink_seid == branch->seid)
-			i40e_switch_branch_release(pf->veb[i]);
-	}
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->uplink_seid == branch->seid)
+			i40e_switch_branch_release(veb);
 
 	/* Release the VSIs on this VEB, but not the owner VSI.
 	 *
 	 * NOTE: Removing the last VSI on a VEB has the SIDE EFFECT of removing
 	 *       the VEB itself, so don't use (*branch) after this loop.
 	 */
-	for (i = 0; i < pf->num_alloc_vsi; i++) {
-		if (!pf->vsi[i])
-			continue;
-		if (pf->vsi[i]->uplink_seid == branch_seid &&
-		   (pf->vsi[i]->flags & I40E_VSI_FLAG_VEB_OWNER) == 0) {
-			i40e_vsi_release(pf->vsi[i]);
-		}
-	}
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->uplink_seid == branch_seid &&
+		    (vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
+			i40e_vsi_release(vsi);
 
 	/* There's one corner case where the VEB might not have been
 	 * removed, so double check it here and remove it if needed.
@@ -14741,19 +14710,19 @@ static void i40e_veb_clear(struct i40e_veb *veb)
  **/
 void i40e_veb_release(struct i40e_veb *veb)
 {
-	struct i40e_vsi *vsi = NULL;
+	struct i40e_vsi *vsi, *vsi_it;
 	struct i40e_pf *pf;
 	int i, n = 0;
 
 	pf = veb->pf;
 
 	/* find the remaining VSI and check for extras */
-	for (i = 0; i < pf->num_alloc_vsi; i++) {
-		if (pf->vsi[i] && pf->vsi[i]->uplink_seid == veb->seid) {
+	i40e_pf_for_each_vsi(pf, i, vsi_it)
+		if (vsi_it->uplink_seid == veb->seid) {
+			vsi = vsi_it;
 			n++;
-			vsi = pf->vsi[i];
 		}
-	}
+
 	if (n != 1) {
 		dev_info(&pf->pdev->dev,
 			 "can't remove VEB %d with %d VSIs left\n",
@@ -14851,6 +14820,7 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 				u8 enabled_tc)
 {
 	struct i40e_veb *veb, *uplink_veb = NULL;
+	struct i40e_vsi *vsi;
 	int vsi_idx, veb_idx;
 	int ret;
 
@@ -14864,9 +14834,10 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 	}
 
 	/* make sure there is such a vsi and uplink */
-	for (vsi_idx = 0; vsi_idx < pf->num_alloc_vsi; vsi_idx++)
-		if (pf->vsi[vsi_idx] && pf->vsi[vsi_idx]->seid == vsi_seid)
+	i40e_pf_for_each_vsi(pf, vsi_idx, vsi)
+		if (vsi->seid == vsi_seid)
 			break;
+
 	if (vsi_idx == pf->num_alloc_vsi && vsi_seid != 0) {
 		dev_info(&pf->pdev->dev, "vsi seid %d not found\n",
 			 vsi_seid);
@@ -14874,10 +14845,9 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 	}
 
 	if (uplink_seid && uplink_seid != pf->mac_seid) {
-		for (veb_idx = 0; veb_idx < I40E_MAX_VEB; veb_idx++) {
-			if (pf->veb[veb_idx] &&
-			    pf->veb[veb_idx]->seid == uplink_seid) {
-				uplink_veb = pf->veb[veb_idx];
+		i40e_pf_for_each_veb(pf, veb_idx, veb) {
+			if (veb->seid == uplink_seid) {
+				uplink_veb = veb;
 				break;
 			}
 		}
@@ -14899,7 +14869,7 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 	veb->enabled_tc = (enabled_tc ? enabled_tc : 0x1);
 
 	/* create the VEB in the switch */
-	ret = i40e_add_veb(veb, pf->vsi[vsi_idx]);
+	ret = i40e_add_veb(veb, vsi);
 	if (ret)
 		goto err_veb;
 	if (vsi_idx == pf->lan_vsi)
@@ -14930,6 +14900,7 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
 	u16 uplink_seid = le16_to_cpu(ele->uplink_seid);
 	u8 element_type = ele->element_type;
 	u16 seid = le16_to_cpu(ele->seid);
+	struct i40e_veb *veb;
 
 	if (printconfig)
 		dev_info(&pf->pdev->dev,
@@ -14948,12 +14919,12 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
 			int v;
 
 			/* find existing or else empty VEB */
-			for (v = 0; v < I40E_MAX_VEB; v++) {
-				if (pf->veb[v] && (pf->veb[v]->seid == seid)) {
+			i40e_pf_for_each_veb(pf, v, veb)
+				if (veb->seid == seid) {
 					pf->lan_veb = v;
 					break;
 				}
-			}
+
 			if (pf->lan_veb >= I40E_MAX_VEB) {
 				v = i40e_veb_mem_alloc(pf);
 				if (v < 0)
@@ -16238,6 +16209,8 @@ static void i40e_remove(struct pci_dev *pdev)
 {
 	struct i40e_pf *pf = pci_get_drvdata(pdev);
 	struct i40e_hw *hw = &pf->hw;
+	struct i40e_vsi *vsi;
+	struct i40e_veb *veb;
 	int ret_code;
 	int i;
 
@@ -16295,24 +16268,19 @@ static void i40e_remove(struct pci_dev *pdev)
 	/* If there is a switch structure or any orphans, remove them.
 	 * This will leave only the PF's VSI remaining.
 	 */
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		if (!pf->veb[i])
-			continue;
-
-		if (pf->veb[i]->uplink_seid == pf->mac_seid ||
-		    pf->veb[i]->uplink_seid == 0)
-			i40e_switch_branch_release(pf->veb[i]);
-	}
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->uplink_seid == pf->mac_seid ||
+		    veb->uplink_seid == 0)
+			i40e_switch_branch_release(veb);
 
 	/* Now we can shutdown the PF's VSIs, just before we kill
 	 * adminq and hmc.
 	 */
-	for (i = pf->num_alloc_vsi; i--;)
-		if (pf->vsi[i]) {
-			i40e_vsi_close(pf->vsi[i]);
-			i40e_vsi_release(pf->vsi[i]);
-			pf->vsi[i] = NULL;
-		}
+	i40e_pf_for_each_vsi(pf, i, vsi) {
+		i40e_vsi_close(vsi);
+		i40e_vsi_release(vsi);
+		pf->vsi[i] = NULL;
+	}
 
 	i40e_cloud_filter_exit(pf);
 
@@ -16349,18 +16317,17 @@ static void i40e_remove(struct pci_dev *pdev)
 	/* Clear all dynamic memory lists of rings, q_vectors, and VSIs */
 	rtnl_lock();
 	i40e_clear_interrupt_scheme(pf);
-	for (i = 0; i < pf->num_alloc_vsi; i++) {
-		if (pf->vsi[i]) {
-			if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
-				i40e_vsi_clear_rings(pf->vsi[i]);
-			i40e_vsi_clear(pf->vsi[i]);
-			pf->vsi[i] = NULL;
-		}
+	i40e_pf_for_each_vsi(pf, i, vsi) {
+		if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
+			i40e_vsi_clear_rings(vsi);
+
+		i40e_vsi_clear(vsi);
+		pf->vsi[i] = NULL;
 	}
 	rtnl_unlock();
 
-	for (i = 0; i < I40E_MAX_VEB; i++) {
-		kfree(pf->veb[i]);
+	i40e_pf_for_each_veb(pf, i, veb) {
+		kfree(veb);
 		pf->veb[i] = NULL;
 	}
 
-- 
2.41.0

Re: [PATCH net-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs

[Jacob Keller]

On 2/16/2024 1:42 PM, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> Introduce i40e_for_each_vsi() and i40e_for_each_veb() helper
> macros and use them to iterate relevant arrays.
> 
> Replace pattern:
> for (i = 0; i < pf->num_alloc_vsi; i++)
> by:
> i40e_for_each_vsi(pf, i, vsi)
> 
> and pattern:
> for (i = 0; i < I40E_MAX_VEB; i++)
> by
> i40e_for_each_veb(pf, i, veb)
> 
> These macros also check if array item pf->vsi[i] or pf->veb[i]
> are not NULL and skip such items so we can remove redundant
> checks from loop bodies.
> 

Much cleaner result in the loop bodies!

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks,
Jake

> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e.h        |  56 ++-
>  drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c |  10 +-
>  .../net/ethernet/intel/i40e/i40e_debugfs.c    |  54 +--
>  drivers/net/ethernet/intel/i40e/i40e_main.c   | 389 ++++++++----------
>  4 files changed, 264 insertions(+), 245 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 9b701615c7c6..5acb26644be7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -686,6 +686,54 @@ struct i40e_pf {
>  	struct list_head ddp_old_prof;
>  };
>  
> +/**
> + * __i40e_pf_next_vsi - get next valid VSI
> + * @pf: pointer to the PF struct
> + * @idx: pointer to start position number
> + *
> + * Find and return next non-NULL VSI pointer in pf->vsi array and
> + * updates idx position. Returns NULL if no VSI is found.
> + **/
> +static __always_inline struct i40e_vsi *
> +__i40e_pf_next_vsi(struct i40e_pf *pf, int *idx)
> +{
> +	while (*idx < pf->num_alloc_vsi) {
> +		if (pf->vsi[*idx])
> +			return pf->vsi[*idx];
> +		(*idx)++;
> +	}
> +	return NULL;
> +}
> +
> +#define i40e_pf_for_each_vsi(_pf, _i, _vsi)			\
> +	for (_i = 0, _vsi = __i40e_pf_next_vsi(_pf, &_i);	\
> +	     _vsi;						\
> +	     _i++, _vsi = __i40e_pf_next_vsi(_pf, &_i))
> +
> +/**
> + * __i40e_pf_next_veb - get next valid VEB
> + * @pf: pointer to the PF struct
> + * @idx: pointer to start position number
> + *
> + * Find and return next non-NULL VEB pointer in pf->veb array and
> + * updates idx position. Returns NULL if no VEB is found.
> + **/
> +static __always_inline struct i40e_veb *
> +__i40e_pf_next_veb(struct i40e_pf *pf, int *idx)
> +{
> +	while (*idx < I40E_MAX_VEB) {
> +		if (pf->veb[*idx])
> +			return pf->veb[*idx];
> +		(*idx)++;
> +	}
> +	return NULL;
> +}
> +
> +#define i40e_pf_for_each_veb(_pf, _i, _veb)			\
> +	for (_i = 0, _veb = __i40e_pf_next_veb(_pf, &_i);	\
> +	     _veb;						\
> +	     _i++, _veb = __i40e_pf_next_veb(_pf, &_i))
> +
>  /**
>   * i40e_mac_to_hkey - Convert a 6-byte MAC Address to a u64 hash key
>   * @macaddr: the MAC Address as the base key
> @@ -1120,14 +1168,12 @@ struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id);
>  static inline struct i40e_vsi *
>  i40e_find_vsi_by_type(struct i40e_pf *pf, u16 type)
>  {
> +	struct i40e_vsi *vsi;
>  	int i;
>  
> -	for (i = 0; i < pf->num_alloc_vsi; i++) {
> -		struct i40e_vsi *vsi = pf->vsi[i];
> -
> -		if (vsi && vsi->type == type)
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->type == type)
>  			return vsi;
> -	}
>  
>  	return NULL;
>  }
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
> index b96a92187ab3..8aa43aefe84c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
> @@ -947,16 +947,16 @@ static int i40e_dcbnl_vsi_del_app(struct i40e_vsi *vsi,
>  static void i40e_dcbnl_del_app(struct i40e_pf *pf,
>  			       struct i40e_dcb_app_priority_table *app)
>  {
> +	struct i40e_vsi *vsi;
>  	int v, err;
>  
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (pf->vsi[v] && pf->vsi[v]->netdev) {
> -			err = i40e_dcbnl_vsi_del_app(pf->vsi[v], app);
> +	i40e_pf_for_each_vsi(pf, v, vsi)
> +		if (vsi->netdev) {
> +			err = i40e_dcbnl_vsi_del_app(vsi, app);
>  			dev_dbg(&pf->pdev->dev, "Deleting app for VSI seid=%d err=%d sel=%d proto=0x%x prio=%d\n",
> -				pf->vsi[v]->seid, err, app->selector,
> +				vsi->seid, err, app->selector,
>  				app->protocolid, app->priority);
>  		}
> -	}
>  }
>  
>  /**
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> index ef70ddbe9c2f..b236b0f93202 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> @@ -24,14 +24,18 @@ enum ring_type {
>   **/
>  static struct i40e_vsi *i40e_dbg_find_vsi(struct i40e_pf *pf, int seid)
>  {
> +	struct i40e_vsi *vsi;
>  	int i;
>  
> -	if (seid < 0)
> +	if (seid < 0) {
>  		dev_info(&pf->pdev->dev, "%d: bad seid\n", seid);
> -	else
> -		for (i = 0; i < pf->num_alloc_vsi; i++)
> -			if (pf->vsi[i] && (pf->vsi[i]->seid == seid))
> -				return pf->vsi[i];
> +
> +		return NULL;
> +	}
> +
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->seid == seid)
> +			return vsi;
>  
>  	return NULL;
>  }
> @@ -43,11 +47,13 @@ static struct i40e_vsi *i40e_dbg_find_vsi(struct i40e_pf *pf, int seid)
>   **/
>  static struct i40e_veb *i40e_dbg_find_veb(struct i40e_pf *pf, int seid)
>  {
> +	struct i40e_veb *veb;
>  	int i;
>  
> -	for (i = 0; i < I40E_MAX_VEB; i++)
> -		if (pf->veb[i] && pf->veb[i]->seid == seid)
> -			return pf->veb[i];
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		if (veb->seid == seid)
> +			return veb;
> +
>  	return NULL;
>  }
>  
> @@ -653,12 +659,11 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
>   **/
>  static void i40e_dbg_dump_vsi_no_seid(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int i;
>  
> -	for (i = 0; i < pf->num_alloc_vsi; i++)
> -		if (pf->vsi[i])
> -			dev_info(&pf->pdev->dev, "dump vsi[%d]: %d\n",
> -				 i, pf->vsi[i]->seid);
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		dev_info(&pf->pdev->dev, "dump vsi[%d]: %d\n", i, vsi->seid);
>  }
>  
>  /**
> @@ -718,11 +723,8 @@ static void i40e_dbg_dump_veb_all(struct i40e_pf *pf)
>  	struct i40e_veb *veb;
>  	int i;
>  
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		veb = pf->veb[i];
> -		if (veb)
> -			i40e_dbg_dump_veb_seid(pf, veb->seid);
> -	}
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		i40e_dbg_dump_veb_seid(pf, veb->seid);
>  }
>  
>  /**
> @@ -873,9 +875,10 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
>  			goto command_write_done;
>  		}
>  
> -		for (i = 0; i < I40E_MAX_VEB; i++)
> -			if (pf->veb[i] && pf->veb[i]->seid == uplink_seid)
> +		i40e_pf_for_each_veb(pf, i, veb)
> +			if (veb->seid == uplink_seid)
>  				break;
> +
>  		if (i >= I40E_MAX_VEB && uplink_seid != 0 &&
>  		    uplink_seid != pf->mac_seid) {
>  			dev_info(&pf->pdev->dev,
> @@ -892,7 +895,9 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
>  			dev_info(&pf->pdev->dev, "add relay failed\n");
>  
>  	} else if (strncmp(cmd_buf, "del relay", 9) == 0) {
> +		struct i40e_veb *veb;
>  		int i;
> +
>  		cnt = sscanf(&cmd_buf[9], "%i", &veb_seid);
>  		if (cnt != 1) {
>  			dev_info(&pf->pdev->dev,
> @@ -906,9 +911,10 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
>  		}
>  
>  		/* find the veb */
> -		for (i = 0; i < I40E_MAX_VEB; i++)
> -			if (pf->veb[i] && pf->veb[i]->seid == veb_seid)
> +		i40e_pf_for_each_veb(pf, i, veb)
> +			if (veb->seid == veb_seid)
>  				break;
> +
>  		if (i >= I40E_MAX_VEB) {
>  			dev_info(&pf->pdev->dev,
>  				 "del relay: relay %d not found\n", veb_seid);
> @@ -916,7 +922,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
>  		}
>  
>  		dev_info(&pf->pdev->dev, "deleting relay %d\n", veb_seid);
> -		i40e_veb_release(pf->veb[i]);
> +		i40e_veb_release(veb);
>  	} else if (strncmp(cmd_buf, "add pvid", 8) == 0) {
>  		unsigned int v;
>  		int ret;
> @@ -1251,8 +1257,8 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
>  			if (cnt == 0) {
>  				int i;
>  
> -				for (i = 0; i < pf->num_alloc_vsi; i++)
> -					i40e_vsi_reset_stats(pf->vsi[i]);
> +				i40e_pf_for_each_vsi(pf, i, vsi)
> +					i40e_vsi_reset_stats(vsi);
>  				dev_info(&pf->pdev->dev, "vsi clear stats called for all vsi's\n");
>  			} else if (cnt == 1) {
>  				vsi = i40e_dbg_find_vsi(pf, vsi_seid);
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 6e59a9509868..43e1b4f7f9dc 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -310,11 +310,12 @@ static int i40e_put_lump(struct i40e_lump_tracking *pile, u16 index, u16 id)
>   **/
>  struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id)
>  {
> +	struct i40e_vsi *vsi;
>  	int i;
>  
> -	for (i = 0; i < pf->num_alloc_vsi; i++)
> -		if (pf->vsi[i] && (pf->vsi[i]->id == id))
> -			return pf->vsi[i];
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->id == id)
> +			return vsi;
>  
>  	return NULL;
>  }
> @@ -552,24 +553,19 @@ void i40e_vsi_reset_stats(struct i40e_vsi *vsi)
>   **/
>  void i40e_pf_reset_stats(struct i40e_pf *pf)
>  {
> +	struct i40e_veb *veb;
>  	int i;
>  
>  	memset(&pf->stats, 0, sizeof(pf->stats));
>  	memset(&pf->stats_offsets, 0, sizeof(pf->stats_offsets));
>  	pf->stat_offsets_loaded = false;
>  
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		if (pf->veb[i]) {
> -			memset(&pf->veb[i]->stats, 0,
> -			       sizeof(pf->veb[i]->stats));
> -			memset(&pf->veb[i]->stats_offsets, 0,
> -			       sizeof(pf->veb[i]->stats_offsets));
> -			memset(&pf->veb[i]->tc_stats, 0,
> -			       sizeof(pf->veb[i]->tc_stats));
> -			memset(&pf->veb[i]->tc_stats_offsets, 0,
> -			       sizeof(pf->veb[i]->tc_stats_offsets));
> -			pf->veb[i]->stat_offsets_loaded = false;
> -		}
> +	i40e_pf_for_each_veb(pf, i, veb) {
> +		memset(&veb->stats, 0, sizeof(veb->stats));
> +		memset(&veb->stats_offsets, 0, sizeof(veb->stats_offsets));
> +		memset(&veb->tc_stats, 0, sizeof(veb->tc_stats));
> +		memset(&veb->tc_stats_offsets, 0, sizeof(veb->tc_stats_offsets));
> +		veb->stat_offsets_loaded = false;
>  	}
>  	pf->hw_csum_rx_error = 0;
>  }
> @@ -2879,6 +2875,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
>   **/
>  static void i40e_sync_filters_subtask(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int v;
>  
>  	if (!pf)
> @@ -2890,11 +2887,10 @@ static void i40e_sync_filters_subtask(struct i40e_pf *pf)
>  		return;
>  	}
>  
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (pf->vsi[v] &&
> -		    (pf->vsi[v]->flags & I40E_VSI_FLAG_FILTER_CHANGED) &&
> -		    !test_bit(__I40E_VSI_RELEASING, pf->vsi[v]->state)) {
> -			int ret = i40e_sync_vsi_filters(pf->vsi[v]);
> +	i40e_pf_for_each_vsi(pf, v, vsi) {
> +		if ((vsi->flags & I40E_VSI_FLAG_FILTER_CHANGED) &&
> +		    !test_bit(__I40E_VSI_RELEASING, vsi->state)) {
> +			int ret = i40e_sync_vsi_filters(vsi);
>  
>  			if (ret) {
>  				/* come back and try again later */
> @@ -5166,6 +5162,7 @@ static void i40e_reset_interrupt_capability(struct i40e_pf *pf)
>   **/
>  static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int i;
>  
>  	if (test_bit(__I40E_MISC_IRQ_REQUESTED, pf->state))
> @@ -5175,9 +5172,10 @@ static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)
>  		      I40E_IWARP_IRQ_PILE_ID);
>  
>  	i40e_put_lump(pf->irq_pile, 0, I40E_PILE_VALID_BIT-1);
> -	for (i = 0; i < pf->num_alloc_vsi; i++)
> -		if (pf->vsi[i])
> -			i40e_vsi_free_q_vectors(pf->vsi[i]);
> +
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		i40e_vsi_free_q_vectors(vsi);
> +
>  	i40e_reset_interrupt_capability(pf);
>  }
>  
> @@ -5274,12 +5272,11 @@ static void i40e_unquiesce_vsi(struct i40e_vsi *vsi)
>   **/
>  static void i40e_pf_quiesce_all_vsi(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int v;
>  
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (pf->vsi[v])
> -			i40e_quiesce_vsi(pf->vsi[v]);
> -	}
> +	i40e_pf_for_each_vsi(pf, v, vsi)
> +		i40e_quiesce_vsi(vsi);
>  }
>  
>  /**
> @@ -5288,12 +5285,11 @@ static void i40e_pf_quiesce_all_vsi(struct i40e_pf *pf)
>   **/
>  static void i40e_pf_unquiesce_all_vsi(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int v;
>  
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (pf->vsi[v])
> -			i40e_unquiesce_vsi(pf->vsi[v]);
> -	}
> +	i40e_pf_for_each_vsi(pf, v, vsi)
> +		i40e_unquiesce_vsi(vsi);
>  }
>  
>  /**
> @@ -5354,14 +5350,13 @@ int i40e_vsi_wait_queues_disabled(struct i40e_vsi *vsi)
>   **/
>  static int i40e_pf_wait_queues_disabled(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int v, ret = 0;
>  
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (pf->vsi[v]) {
> -			ret = i40e_vsi_wait_queues_disabled(pf->vsi[v]);
> -			if (ret)
> -				break;
> -		}
> +	i40e_pf_for_each_vsi(pf, v, vsi) {
> +		ret = i40e_vsi_wait_queues_disabled(vsi);
> +		if (ret)
> +			break;
>  	}
>  
>  	return ret;
> @@ -6778,32 +6773,29 @@ int i40e_veb_config_tc(struct i40e_veb *veb, u8 enabled_tc)
>   **/
>  static void i40e_dcb_reconfigure(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
> +	struct i40e_veb *veb;
>  	u8 tc_map = 0;
>  	int ret;
> -	u8 v;
> +	int v;
>  
>  	/* Enable the TCs available on PF to all VEBs */
>  	tc_map = i40e_pf_get_tc_map(pf);
>  	if (tc_map == I40E_DEFAULT_TRAFFIC_CLASS)
>  		return;
>  
> -	for (v = 0; v < I40E_MAX_VEB; v++) {
> -		if (!pf->veb[v])
> -			continue;
> -		ret = i40e_veb_config_tc(pf->veb[v], tc_map);
> +	i40e_pf_for_each_veb(pf, v, veb) {
> +		ret = i40e_veb_config_tc(veb, tc_map);
>  		if (ret) {
>  			dev_info(&pf->pdev->dev,
>  				 "Failed configuring TC for VEB seid=%d\n",
> -				 pf->veb[v]->seid);
> +				 veb->seid);
>  			/* Will try to configure as many components */
>  		}
>  	}
>  
>  	/* Update each VSI */
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (!pf->vsi[v])
> -			continue;
> -
> +	i40e_pf_for_each_vsi(pf, v, vsi) {
>  		/* - Enable all TCs for the LAN VSI
>  		 * - For all others keep them at TC0 for now
>  		 */
> @@ -6812,17 +6804,17 @@ static void i40e_dcb_reconfigure(struct i40e_pf *pf)
>  		else
>  			tc_map = I40E_DEFAULT_TRAFFIC_CLASS;
>  
> -		ret = i40e_vsi_config_tc(pf->vsi[v], tc_map);
> +		ret = i40e_vsi_config_tc(vsi, tc_map);
>  		if (ret) {
>  			dev_info(&pf->pdev->dev,
>  				 "Failed configuring TC for VSI seid=%d\n",
> -				 pf->vsi[v]->seid);
> +				 vsi->seid);
>  			/* Will try to configure as many components */
>  		} else {
>  			/* Re-configure VSI vectors based on updated TC map */
> -			i40e_vsi_map_rings_to_vectors(pf->vsi[v]);
> -			if (pf->vsi[v]->netdev)
> -				i40e_dcbnl_set_all(pf->vsi[v]);
> +			i40e_vsi_map_rings_to_vectors(vsi);
> +			if (vsi->netdev)
> +				i40e_dcbnl_set_all(vsi);
>  		}
>  	}
>  }
> @@ -9257,7 +9249,9 @@ int i40e_close(struct net_device *netdev)
>   **/
>  void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags, bool lock_acquired)
>  {
> +	struct i40e_vsi *vsi;
>  	u32 val;
> +	int i;
>  
>  	/* do the biggest reset indicated */
>  	if (reset_flags & BIT_ULL(__I40E_GLOBAL_RESET_REQUESTED)) {
> @@ -9313,29 +9307,20 @@ void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags, bool lock_acquired)
>  			 "FW LLDP is enabled\n");
>  
>  	} else if (reset_flags & BIT_ULL(__I40E_REINIT_REQUESTED)) {
> -		int v;
> -
>  		/* Find the VSI(s) that requested a re-init */
> -		dev_info(&pf->pdev->dev,
> -			 "VSI reinit requested\n");
> -		for (v = 0; v < pf->num_alloc_vsi; v++) {
> -			struct i40e_vsi *vsi = pf->vsi[v];
> +		dev_info(&pf->pdev->dev, "VSI reinit requested\n");
>  
> -			if (vsi != NULL &&
> -			    test_and_clear_bit(__I40E_VSI_REINIT_REQUESTED,
> +		i40e_pf_for_each_vsi(pf, i, vsi) {
> +			if (test_and_clear_bit(__I40E_VSI_REINIT_REQUESTED,
>  					       vsi->state))
> -				i40e_vsi_reinit_locked(pf->vsi[v]);
> +				i40e_vsi_reinit_locked(vsi);
>  		}
>  	} else if (reset_flags & BIT_ULL(__I40E_DOWN_REQUESTED)) {
> -		int v;
> -
>  		/* Find the VSI(s) that needs to be brought down */
>  		dev_info(&pf->pdev->dev, "VSI down requested\n");
> -		for (v = 0; v < pf->num_alloc_vsi; v++) {
> -			struct i40e_vsi *vsi = pf->vsi[v];
>  
> -			if (vsi != NULL &&
> -			    test_and_clear_bit(__I40E_VSI_DOWN_REQUESTED,
> +		i40e_pf_for_each_vsi(pf, i, vsi) {
> +			if (test_and_clear_bit(__I40E_VSI_DOWN_REQUESTED,
>  					       vsi->state)) {
>  				set_bit(__I40E_VSI_DOWN, vsi->state);
>  				i40e_down(vsi);
> @@ -9888,6 +9873,8 @@ static void i40e_vsi_link_event(struct i40e_vsi *vsi, bool link_up)
>   **/
>  static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
>  {
> +	struct i40e_veb *veb_it;
> +	struct i40e_vsi *vsi;
>  	struct i40e_pf *pf;
>  	int i;
>  
> @@ -9896,14 +9883,14 @@ static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
>  	pf = veb->pf;
>  
>  	/* depth first... */
> -	for (i = 0; i < I40E_MAX_VEB; i++)
> -		if (pf->veb[i] && (pf->veb[i]->uplink_seid == veb->seid))
> -			i40e_veb_link_event(pf->veb[i], link_up);
> +	i40e_pf_for_each_veb(pf, i, veb_it)
> +		if (veb_it->uplink_seid == veb->seid)
> +			i40e_veb_link_event(veb_it, link_up);
>  
>  	/* ... now the local VSIs */
> -	for (i = 0; i < pf->num_alloc_vsi; i++)
> -		if (pf->vsi[i] && (pf->vsi[i]->uplink_seid == veb->seid))
> -			i40e_vsi_link_event(pf->vsi[i], link_up);
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->uplink_seid == veb->seid)
> +			i40e_vsi_link_event(vsi, link_up);
>  }
>  
>  /**
> @@ -9995,6 +9982,8 @@ static void i40e_link_event(struct i40e_pf *pf)
>   **/
>  static void i40e_watchdog_subtask(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
> +	struct i40e_veb *veb;
>  	int i;
>  
>  	/* if interface is down do nothing */
> @@ -10015,15 +10004,14 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
>  	/* Update the stats for active netdevs so the network stack
>  	 * can look at updated numbers whenever it cares to
>  	 */
> -	for (i = 0; i < pf->num_alloc_vsi; i++)
> -		if (pf->vsi[i] && pf->vsi[i]->netdev)
> -			i40e_update_stats(pf->vsi[i]);
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->netdev)
> +			i40e_update_stats(vsi);
>  
>  	if (test_bit(I40E_FLAG_VEB_STATS_ENA, pf->flags)) {
>  		/* Update the stats for the active switching components */
> -		for (i = 0; i < I40E_MAX_VEB; i++)
> -			if (pf->veb[i])
> -				i40e_update_veb_stats(pf->veb[i]);
> +		i40e_pf_for_each_veb(pf, i, veb)
> +			i40e_update_veb_stats(veb);
>  	}
>  
>  	i40e_ptp_rx_hang(pf);
> @@ -10380,18 +10368,18 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
>  {
>  	struct i40e_vsi *ctl_vsi = NULL;
>  	struct i40e_pf *pf = veb->pf;
> -	int v, veb_idx;
> -	int ret;
> +	struct i40e_veb *veb_it;
> +	struct i40e_vsi *vsi;
> +	int v, ret;
>  
>  	/* build VSI that owns this VEB, temporarily attached to base VEB */
> -	for (v = 0; v < pf->num_alloc_vsi && !ctl_vsi; v++) {
> -		if (pf->vsi[v] &&
> -		    pf->vsi[v]->veb_idx == veb->idx &&
> -		    pf->vsi[v]->flags & I40E_VSI_FLAG_VEB_OWNER) {
> -			ctl_vsi = pf->vsi[v];
> +	i40e_pf_for_each_vsi(pf, v, vsi)
> +		if (vsi->veb_idx == veb->idx &&
> +		    vsi->flags & I40E_VSI_FLAG_VEB_OWNER) {
> +			ctl_vsi = vsi;
>  			break;
>  		}
> -	}
> +
>  	if (!ctl_vsi) {
>  		dev_info(&pf->pdev->dev,
>  			 "missing owner VSI for veb_idx %d\n", veb->idx);
> @@ -10421,13 +10409,11 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
>  	i40e_config_bridge_mode(veb);
>  
>  	/* create the remaining VSIs attached to this VEB */
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (!pf->vsi[v] || pf->vsi[v] == ctl_vsi)
> +	i40e_pf_for_each_vsi(pf, v, vsi) {
> +		if (vsi == ctl_vsi)
>  			continue;
>  
> -		if (pf->vsi[v]->veb_idx == veb->idx) {
> -			struct i40e_vsi *vsi = pf->vsi[v];
> -
> +		if (vsi->veb_idx == veb->idx) {
>  			vsi->uplink_seid = veb->seid;
>  			ret = i40e_add_vsi(vsi);
>  			if (ret) {
> @@ -10441,10 +10427,10 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
>  	}
>  
>  	/* create any VEBs attached to this VEB - RECURSION */
> -	for (veb_idx = 0; veb_idx < I40E_MAX_VEB; veb_idx++) {
> -		if (pf->veb[veb_idx] && pf->veb[veb_idx]->veb_idx == veb->idx) {
> -			pf->veb[veb_idx]->uplink_seid = veb->seid;
> -			ret = i40e_reconstitute_veb(pf->veb[veb_idx]);
> +	i40e_pf_for_each_veb(pf, v, veb_it) {
> +		if (veb_it->veb_idx == veb->idx) {
> +			veb_it->uplink_seid = veb->seid;
> +			ret = i40e_reconstitute_veb(veb_it);
>  			if (ret)
>  				break;
>  		}
> @@ -10718,6 +10704,7 @@ static void i40e_clean_xps_state(struct i40e_vsi *vsi)
>  static void i40e_prep_for_reset(struct i40e_pf *pf)
>  {
>  	struct i40e_hw *hw = &pf->hw;
> +	struct i40e_vsi *vsi;
>  	int ret = 0;
>  	u32 v;
>  
> @@ -10732,11 +10719,9 @@ static void i40e_prep_for_reset(struct i40e_pf *pf)
>  	/* quiesce the VSIs and their queues that are not already DOWN */
>  	i40e_pf_quiesce_all_vsi(pf);
>  
> -	for (v = 0; v < pf->num_alloc_vsi; v++) {
> -		if (pf->vsi[v]) {
> -			i40e_clean_xps_state(pf->vsi[v]);
> -			pf->vsi[v]->seid = 0;
> -		}
> +	i40e_pf_for_each_vsi(pf, v, vsi) {
> +		i40e_clean_xps_state(vsi);
> +		vsi->seid = 0;
>  	}
>  
>  	i40e_shutdown_adminq(&pf->hw);
> @@ -10850,6 +10835,7 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
>  	const bool is_recovery_mode_reported = i40e_check_recovery_mode(pf);
>  	struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi];
>  	struct i40e_hw *hw = &pf->hw;
> +	struct i40e_veb *veb;
>  	int ret;
>  	u32 val;
>  	int v;
> @@ -10992,14 +10978,10 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
>  	if (vsi->uplink_seid != pf->mac_seid) {
>  		dev_dbg(&pf->pdev->dev, "attempting to rebuild switch\n");
>  		/* find the one VEB connected to the MAC, and find orphans */
> -		for (v = 0; v < I40E_MAX_VEB; v++) {
> -			if (!pf->veb[v])
> -				continue;
> -
> -			if (pf->veb[v]->uplink_seid == pf->mac_seid ||
> -			    pf->veb[v]->uplink_seid == 0) {
> -				ret = i40e_reconstitute_veb(pf->veb[v]);
> -
> +		i40e_pf_for_each_veb(pf, v, veb) {
> +			if (veb->uplink_seid == pf->mac_seid ||
> +			    veb->uplink_seid == 0) {
> +				ret = i40e_reconstitute_veb(veb);
>  				if (!ret)
>  					continue;
>  
> @@ -11009,13 +10991,13 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
>  				 * If orphan failed, we'll report the error
>  				 * but try to keep going.
>  				 */
> -				if (pf->veb[v]->uplink_seid == pf->mac_seid) {
> +				if (veb->uplink_seid == pf->mac_seid) {
>  					dev_info(&pf->pdev->dev,
>  						 "rebuild of switch failed: %d, will try to set up simple PF connection\n",
>  						 ret);
>  					vsi->uplink_seid = pf->mac_seid;
>  					break;
> -				} else if (pf->veb[v]->uplink_seid == 0) {
> +				} else if (veb->uplink_seid == 0) {
>  					dev_info(&pf->pdev->dev,
>  						 "rebuild of orphan VEB failed: %d\n",
>  						 ret);
> @@ -12098,6 +12080,7 @@ static int i40e_init_interrupt_scheme(struct i40e_pf *pf)
>   */
>  static int i40e_restore_interrupt_scheme(struct i40e_pf *pf)
>  {
> +	struct i40e_vsi *vsi;
>  	int err, i;
>  
>  	/* We cleared the MSI and MSI-X flags when disabling the old interrupt
> @@ -12114,13 +12097,12 @@ static int i40e_restore_interrupt_scheme(struct i40e_pf *pf)
>  	/* Now that we've re-acquired IRQs, we need to remap the vectors and
>  	 * rings together again.
>  	 */
> -	for (i = 0; i < pf->num_alloc_vsi; i++) {
> -		if (pf->vsi[i]) {
> -			err = i40e_vsi_alloc_q_vectors(pf->vsi[i]);
> -			if (err)
> -				goto err_unwind;
> -			i40e_vsi_map_rings_to_vectors(pf->vsi[i]);
> -		}
> +	i40e_pf_for_each_vsi(pf, i, vsi) {
> +		err = i40e_vsi_alloc_q_vectors(vsi);
> +		if (err)
> +			goto err_unwind;
> +
> +		i40e_vsi_map_rings_to_vectors(vsi);
>  	}
>  
>  	err = i40e_setup_misc_vector(pf);
> @@ -13122,8 +13104,8 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
>  	struct i40e_netdev_priv *np = netdev_priv(dev);
>  	struct i40e_vsi *vsi = np->vsi;
>  	struct i40e_pf *pf = vsi->back;
> -	struct i40e_veb *veb = NULL;
>  	struct nlattr *attr, *br_spec;
> +	struct i40e_veb *veb;
>  	int i, rem;
>  
>  	/* Only for PF VSI for now */
> @@ -13131,10 +13113,11 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
>  		return -EOPNOTSUPP;
>  
>  	/* Find the HW bridge for PF VSI */
> -	for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
> -		if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
> -			veb = pf->veb[i];
> -	}
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		if (veb->seid == vsi->uplink_seid)
> +			break;
> +	if (i == I40E_MAX_VEB)
> +		veb = NULL; /* No VEB found */
>  
>  	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
>  	if (!br_spec)
> @@ -13207,12 +13190,10 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>  		return -EOPNOTSUPP;
>  
>  	/* Find the HW bridge for the PF VSI */
> -	for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
> -		if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
> -			veb = pf->veb[i];
> -	}
> -
> -	if (!veb)
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		if (veb->seid == vsi->uplink_seid)
> +			break;
> +	if (i == I40E_MAX_VEB)
>  		return 0;
>  
>  	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
> @@ -14143,9 +14124,9 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
>   **/
>  int i40e_vsi_release(struct i40e_vsi *vsi)
>  {
> +	struct i40e_veb *veb, *veb_it;
>  	struct i40e_mac_filter *f;
>  	struct hlist_node *h;
> -	struct i40e_veb *veb = NULL;
>  	struct i40e_pf *pf;
>  	u16 uplink_seid;
>  	int i, n, bkt;
> @@ -14215,20 +14196,18 @@ int i40e_vsi_release(struct i40e_vsi *vsi)
>  	 * the orphan VEBs yet.  We'll wait for an explicit remove request
>  	 * from up the network stack.
>  	 */
> -	for (n = 0, i = 0; i < pf->num_alloc_vsi; i++) {
> -		if (pf->vsi[i] &&
> -		    pf->vsi[i]->uplink_seid == uplink_seid &&
> -		    (pf->vsi[i]->flags & I40E_VSI_FLAG_VEB_OWNER) == 0) {
> +	n = 0;
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->uplink_seid == uplink_seid &&
> +		    (vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
>  			n++;      /* count the VSIs */
> -		}
> -	}
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		if (!pf->veb[i])
> -			continue;
> -		if (pf->veb[i]->uplink_seid == uplink_seid)
> +
> +	veb = NULL;
> +	i40e_pf_for_each_veb(pf, i, veb_it) {
> +		if (veb_it->uplink_seid == uplink_seid)
>  			n++;     /* count the VEBs */
> -		if (pf->veb[i]->seid == uplink_seid)
> -			veb = pf->veb[i];
> +		if (veb_it->seid == uplink_seid)
> +			veb = veb_it;
>  	}
>  	if (n == 0 && veb && veb->uplink_seid != 0)
>  		i40e_veb_release(veb);
> @@ -14405,22 +14384,18 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
>  	 *
>  	 * Find which uplink_seid we were given and create a new VEB if needed
>  	 */
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		if (pf->veb[i] && pf->veb[i]->seid == uplink_seid) {
> -			veb = pf->veb[i];
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		if (veb->seid == uplink_seid)
>  			break;
> -		}
> -	}
> +	if (i == I40E_MAX_VEB)
> +		veb = NULL;
>  
>  	if (!veb && uplink_seid != pf->mac_seid) {
> -
> -		for (i = 0; i < pf->num_alloc_vsi; i++) {
> -			if (pf->vsi[i] && pf->vsi[i]->seid == uplink_seid) {
> -				vsi = pf->vsi[i];
> +		i40e_pf_for_each_vsi(pf, i, vsi)
> +			if (vsi->seid == uplink_seid)
>  				break;
> -			}
> -		}
> -		if (!vsi) {
> +
> +		if (i == pf->num_alloc_vsi) {
>  			dev_info(&pf->pdev->dev, "no such uplink_seid %d\n",
>  				 uplink_seid);
>  			return NULL;
> @@ -14448,11 +14423,10 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
>  			}
>  			i40e_config_bridge_mode(veb);
>  		}
> -		for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
> -			if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
> -				veb = pf->veb[i];
> -		}
> -		if (!veb) {
> +		i40e_pf_for_each_veb(pf, i, veb)
> +			if (veb->seid == vsi->uplink_seid)
> +				break;
> +		if (i == I40E_MAX_VEB) {
>  			dev_info(&pf->pdev->dev, "couldn't add VEB\n");
>  			return NULL;
>  		}
> @@ -14681,29 +14655,24 @@ static void i40e_switch_branch_release(struct i40e_veb *branch)
>  	struct i40e_pf *pf = branch->pf;
>  	u16 branch_seid = branch->seid;
>  	u16 veb_idx = branch->idx;
> +	struct i40e_vsi *vsi;
> +	struct i40e_veb *veb;
>  	int i;
>  
>  	/* release any VEBs on this VEB - RECURSION */
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		if (!pf->veb[i])
> -			continue;
> -		if (pf->veb[i]->uplink_seid == branch->seid)
> -			i40e_switch_branch_release(pf->veb[i]);
> -	}
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		if (veb->uplink_seid == branch->seid)
> +			i40e_switch_branch_release(veb);
>  
>  	/* Release the VSIs on this VEB, but not the owner VSI.
>  	 *
>  	 * NOTE: Removing the last VSI on a VEB has the SIDE EFFECT of removing
>  	 *       the VEB itself, so don't use (*branch) after this loop.
>  	 */
> -	for (i = 0; i < pf->num_alloc_vsi; i++) {
> -		if (!pf->vsi[i])
> -			continue;
> -		if (pf->vsi[i]->uplink_seid == branch_seid &&
> -		   (pf->vsi[i]->flags & I40E_VSI_FLAG_VEB_OWNER) == 0) {
> -			i40e_vsi_release(pf->vsi[i]);
> -		}
> -	}
> +	i40e_pf_for_each_vsi(pf, i, vsi)
> +		if (vsi->uplink_seid == branch_seid &&
> +		    (vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
> +			i40e_vsi_release(vsi);
>  
>  	/* There's one corner case where the VEB might not have been
>  	 * removed, so double check it here and remove it if needed.
> @@ -14741,19 +14710,19 @@ static void i40e_veb_clear(struct i40e_veb *veb)
>   **/
>  void i40e_veb_release(struct i40e_veb *veb)
>  {
> -	struct i40e_vsi *vsi = NULL;
> +	struct i40e_vsi *vsi, *vsi_it;
>  	struct i40e_pf *pf;
>  	int i, n = 0;
>  
>  	pf = veb->pf;
>  
>  	/* find the remaining VSI and check for extras */
> -	for (i = 0; i < pf->num_alloc_vsi; i++) {
> -		if (pf->vsi[i] && pf->vsi[i]->uplink_seid == veb->seid) {
> +	i40e_pf_for_each_vsi(pf, i, vsi_it)
> +		if (vsi_it->uplink_seid == veb->seid) {
> +			vsi = vsi_it;
>  			n++;
> -			vsi = pf->vsi[i];
>  		}
> -	}
> +
>  	if (n != 1) {
>  		dev_info(&pf->pdev->dev,
>  			 "can't remove VEB %d with %d VSIs left\n",
> @@ -14851,6 +14820,7 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
>  				u8 enabled_tc)
>  {
>  	struct i40e_veb *veb, *uplink_veb = NULL;
> +	struct i40e_vsi *vsi;
>  	int vsi_idx, veb_idx;
>  	int ret;
>  
> @@ -14864,9 +14834,10 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
>  	}
>  
>  	/* make sure there is such a vsi and uplink */
> -	for (vsi_idx = 0; vsi_idx < pf->num_alloc_vsi; vsi_idx++)
> -		if (pf->vsi[vsi_idx] && pf->vsi[vsi_idx]->seid == vsi_seid)
> +	i40e_pf_for_each_vsi(pf, vsi_idx, vsi)
> +		if (vsi->seid == vsi_seid)
>  			break;
> +
>  	if (vsi_idx == pf->num_alloc_vsi && vsi_seid != 0) {
>  		dev_info(&pf->pdev->dev, "vsi seid %d not found\n",
>  			 vsi_seid);
> @@ -14874,10 +14845,9 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
>  	}
>  
>  	if (uplink_seid && uplink_seid != pf->mac_seid) {
> -		for (veb_idx = 0; veb_idx < I40E_MAX_VEB; veb_idx++) {
> -			if (pf->veb[veb_idx] &&
> -			    pf->veb[veb_idx]->seid == uplink_seid) {
> -				uplink_veb = pf->veb[veb_idx];
> +		i40e_pf_for_each_veb(pf, veb_idx, veb) {
> +			if (veb->seid == uplink_seid) {
> +				uplink_veb = veb;
>  				break;
>  			}
>  		}
> @@ -14899,7 +14869,7 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
>  	veb->enabled_tc = (enabled_tc ? enabled_tc : 0x1);
>  
>  	/* create the VEB in the switch */
> -	ret = i40e_add_veb(veb, pf->vsi[vsi_idx]);
> +	ret = i40e_add_veb(veb, vsi);
>  	if (ret)
>  		goto err_veb;
>  	if (vsi_idx == pf->lan_vsi)
> @@ -14930,6 +14900,7 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
>  	u16 uplink_seid = le16_to_cpu(ele->uplink_seid);
>  	u8 element_type = ele->element_type;
>  	u16 seid = le16_to_cpu(ele->seid);
> +	struct i40e_veb *veb;
>  
>  	if (printconfig)
>  		dev_info(&pf->pdev->dev,
> @@ -14948,12 +14919,12 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
>  			int v;
>  
>  			/* find existing or else empty VEB */
> -			for (v = 0; v < I40E_MAX_VEB; v++) {
> -				if (pf->veb[v] && (pf->veb[v]->seid == seid)) {
> +			i40e_pf_for_each_veb(pf, v, veb)
> +				if (veb->seid == seid) {
>  					pf->lan_veb = v;
>  					break;
>  				}
> -			}
> +
>  			if (pf->lan_veb >= I40E_MAX_VEB) {
>  				v = i40e_veb_mem_alloc(pf);
>  				if (v < 0)
> @@ -16238,6 +16209,8 @@ static void i40e_remove(struct pci_dev *pdev)
>  {
>  	struct i40e_pf *pf = pci_get_drvdata(pdev);
>  	struct i40e_hw *hw = &pf->hw;
> +	struct i40e_vsi *vsi;
> +	struct i40e_veb *veb;
>  	int ret_code;
>  	int i;
>  
> @@ -16295,24 +16268,19 @@ static void i40e_remove(struct pci_dev *pdev)
>  	/* If there is a switch structure or any orphans, remove them.
>  	 * This will leave only the PF's VSI remaining.
>  	 */
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		if (!pf->veb[i])
> -			continue;
> -
> -		if (pf->veb[i]->uplink_seid == pf->mac_seid ||
> -		    pf->veb[i]->uplink_seid == 0)
> -			i40e_switch_branch_release(pf->veb[i]);
> -	}
> +	i40e_pf_for_each_veb(pf, i, veb)
> +		if (veb->uplink_seid == pf->mac_seid ||
> +		    veb->uplink_seid == 0)
> +			i40e_switch_branch_release(veb);
>  
>  	/* Now we can shutdown the PF's VSIs, just before we kill
>  	 * adminq and hmc.
>  	 */
> -	for (i = pf->num_alloc_vsi; i--;)
> -		if (pf->vsi[i]) {
> -			i40e_vsi_close(pf->vsi[i]);
> -			i40e_vsi_release(pf->vsi[i]);
> -			pf->vsi[i] = NULL;
> -		}
> +	i40e_pf_for_each_vsi(pf, i, vsi) {
> +		i40e_vsi_close(vsi);
> +		i40e_vsi_release(vsi);
> +		pf->vsi[i] = NULL;
> +	}
>  
>  	i40e_cloud_filter_exit(pf);
>  
> @@ -16349,18 +16317,17 @@ static void i40e_remove(struct pci_dev *pdev)
>  	/* Clear all dynamic memory lists of rings, q_vectors, and VSIs */
>  	rtnl_lock();
>  	i40e_clear_interrupt_scheme(pf);
> -	for (i = 0; i < pf->num_alloc_vsi; i++) {
> -		if (pf->vsi[i]) {
> -			if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
> -				i40e_vsi_clear_rings(pf->vsi[i]);
> -			i40e_vsi_clear(pf->vsi[i]);
> -			pf->vsi[i] = NULL;
> -		}
> +	i40e_pf_for_each_vsi(pf, i, vsi) {
> +		if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
> +			i40e_vsi_clear_rings(vsi);
> +
> +		i40e_vsi_clear(vsi);
> +		pf->vsi[i] = NULL;
>  	}
>  	rtnl_unlock();
>  
> -	for (i = 0; i < I40E_MAX_VEB; i++) {
> -		kfree(pf->veb[i]);
> +	i40e_pf_for_each_veb(pf, i, veb) {
> +		kfree(veb);
>  		pf->veb[i] = NULL;
>  	}
>  

[PATCH net-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
VEB or VSI by their SEID value and use these helpers to replace
existing open-coded loops.

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h        | 36 +++++++++
 .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++-------
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 80 +++++++------------
 3 files changed, 68 insertions(+), 86 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 5acb26644be7..2990ff4f0306 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1355,4 +1355,40 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
 
 struct device *i40e_hw_to_dev(struct i40e_hw *hw);
 
+/**
+ * i40e_pf_get_vsi_by_seid - find VSI by SEID
+ * @pf: pointer to a PF
+ * @seid: SEID of the VSI
+ **/
+static inline struct i40e_vsi *
+i40e_pf_get_vsi_by_seid(struct i40e_pf *pf, u16 seid)
+{
+	struct i40e_vsi *vsi;
+	int i;
+
+	i40e_pf_for_each_vsi(pf, i, vsi)
+		if (vsi->seid == seid)
+			return vsi;
+
+	return NULL;
+}
+
+/**
+ * i40e_pf_get_veb_by_seid - find VEB by SEID
+ * @pf: pointer to a PF
+ * @seid: SEID of the VSI
+ **/
+static inline struct i40e_veb *
+i40e_pf_get_veb_by_seid(struct i40e_pf *pf, u16 seid)
+{
+	struct i40e_veb *veb;
+	int i;
+
+	i40e_pf_for_each_veb(pf, i, veb)
+		if (veb->seid == seid)
+			return veb;
+
+	return NULL;
+}
+
 #endif /* _I40E_H_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index b236b0f93202..990a60889eef 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -24,37 +24,13 @@ enum ring_type {
  **/
 static struct i40e_vsi *i40e_dbg_find_vsi(struct i40e_pf *pf, int seid)
 {
-	struct i40e_vsi *vsi;
-	int i;
-
 	if (seid < 0) {
 		dev_info(&pf->pdev->dev, "%d: bad seid\n", seid);
 
 		return NULL;
 	}
 
-	i40e_pf_for_each_vsi(pf, i, vsi)
-		if (vsi->seid == seid)
-			return vsi;
-
-	return NULL;
-}
-
-/**
- * i40e_dbg_find_veb - searches for the veb with the given seid
- * @pf: the PF structure to search for the veb
- * @seid: seid of the veb it is searching for
- **/
-static struct i40e_veb *i40e_dbg_find_veb(struct i40e_pf *pf, int seid)
-{
-	struct i40e_veb *veb;
-	int i;
-
-	i40e_pf_for_each_veb(pf, i, veb)
-		if (veb->seid == seid)
-			return veb;
-
-	return NULL;
+	return i40e_pf_get_vsi_by_seid(pf, seid);
 }
 
 /**************************************************************
@@ -701,7 +677,7 @@ static void i40e_dbg_dump_veb_seid(struct i40e_pf *pf, int seid)
 {
 	struct i40e_veb *veb;
 
-	veb = i40e_dbg_find_veb(pf, seid);
+	veb = i40e_pf_get_veb_by_seid(pf, seid);
 	if (!veb) {
 		dev_info(&pf->pdev->dev, "can't find veb %d\n", seid);
 		return;
@@ -853,7 +829,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 
 	} else if (strncmp(cmd_buf, "add relay", 9) == 0) {
 		struct i40e_veb *veb;
-		int uplink_seid, i;
+		int uplink_seid;
 
 		cnt = sscanf(&cmd_buf[9], "%i %i", &uplink_seid, &vsi_seid);
 		if (cnt != 2) {
@@ -875,12 +851,8 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			goto command_write_done;
 		}
 
-		i40e_pf_for_each_veb(pf, i, veb)
-			if (veb->seid == uplink_seid)
-				break;
-
-		if (i >= I40E_MAX_VEB && uplink_seid != 0 &&
-		    uplink_seid != pf->mac_seid) {
+		veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
+		if (!veb && uplink_seid != 0 && uplink_seid != pf->mac_seid) {
 			dev_info(&pf->pdev->dev,
 				 "add relay: relay uplink %d not found\n",
 				 uplink_seid);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 43e1b4f7f9dc..e495a212d076 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -13106,18 +13106,14 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
 	struct i40e_pf *pf = vsi->back;
 	struct nlattr *attr, *br_spec;
 	struct i40e_veb *veb;
-	int i, rem;
+	int rem;
 
 	/* Only for PF VSI for now */
 	if (vsi->seid != pf->vsi[pf->lan_vsi]->seid)
 		return -EOPNOTSUPP;
 
 	/* Find the HW bridge for PF VSI */
-	i40e_pf_for_each_veb(pf, i, veb)
-		if (veb->seid == vsi->uplink_seid)
-			break;
-	if (i == I40E_MAX_VEB)
-		veb = NULL; /* No VEB found */
+	veb = i40e_pf_get_veb_by_seid(pf, vsi->uplink_seid);
 
 	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
 	if (!br_spec)
@@ -13182,18 +13178,15 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	struct i40e_netdev_priv *np = netdev_priv(dev);
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
-	struct i40e_veb *veb = NULL;
-	int i;
+	struct i40e_veb *veb;
 
 	/* Only for PF VSI for now */
 	if (vsi->seid != pf->vsi[pf->lan_vsi]->seid)
 		return -EOPNOTSUPP;
 
 	/* Find the HW bridge for the PF VSI */
-	i40e_pf_for_each_veb(pf, i, veb)
-		if (veb->seid == vsi->uplink_seid)
-			break;
-	if (i == I40E_MAX_VEB)
+	veb = i40e_pf_get_veb_by_seid(pf, vsi->uplink_seid);
+	if (!veb)
 		return 0;
 
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
@@ -14368,8 +14361,8 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 	struct i40e_vsi *vsi = NULL;
 	struct i40e_veb *veb = NULL;
 	u16 alloc_queue_pairs;
-	int ret, i;
 	int v_idx;
+	int ret;
 
 	/* The requested uplink_seid must be either
 	 *     - the PF's port seid
@@ -14384,18 +14377,10 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 	 *
 	 * Find which uplink_seid we were given and create a new VEB if needed
 	 */
-	i40e_pf_for_each_veb(pf, i, veb)
-		if (veb->seid == uplink_seid)
-			break;
-	if (i == I40E_MAX_VEB)
-		veb = NULL;
-
+	veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
 	if (!veb && uplink_seid != pf->mac_seid) {
-		i40e_pf_for_each_vsi(pf, i, vsi)
-			if (vsi->seid == uplink_seid)
-				break;
-
-		if (i == pf->num_alloc_vsi) {
+		vsi = i40e_pf_get_vsi_by_seid(pf, uplink_seid);
+		if (!vsi) {
 			dev_info(&pf->pdev->dev, "no such uplink_seid %d\n",
 				 uplink_seid);
 			return NULL;
@@ -14423,10 +14408,8 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 			}
 			i40e_config_bridge_mode(veb);
 		}
-		i40e_pf_for_each_veb(pf, i, veb)
-			if (veb->seid == vsi->uplink_seid)
-				break;
-		if (i == I40E_MAX_VEB) {
+		veb = i40e_pf_get_veb_by_seid(pf, vsi->uplink_seid);
+		if (!veb) {
 			dev_info(&pf->pdev->dev, "couldn't add VEB\n");
 			return NULL;
 		}
@@ -14820,8 +14803,8 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 				u8 enabled_tc)
 {
 	struct i40e_veb *veb, *uplink_veb = NULL;
-	struct i40e_vsi *vsi;
-	int vsi_idx, veb_idx;
+	struct i40e_vsi *vsi = NULL;
+	int veb_idx;
 	int ret;
 
 	/* if one seid is 0, the other must be 0 to create a floating relay */
@@ -14834,23 +14817,16 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 	}
 
 	/* make sure there is such a vsi and uplink */
-	i40e_pf_for_each_vsi(pf, vsi_idx, vsi)
-		if (vsi->seid == vsi_seid)
-			break;
-
-	if (vsi_idx == pf->num_alloc_vsi && vsi_seid != 0) {
-		dev_info(&pf->pdev->dev, "vsi seid %d not found\n",
-			 vsi_seid);
-		return NULL;
+	if (vsi_seid) {
+		vsi = i40e_pf_get_vsi_by_seid(pf, vsi_seid);
+		if (!vsi) {
+			dev_err(&pf->pdev->dev, "vsi seid %d not found\n",
+				vsi_seid);
+			return NULL;
+		}
 	}
-
 	if (uplink_seid && uplink_seid != pf->mac_seid) {
-		i40e_pf_for_each_veb(pf, veb_idx, veb) {
-			if (veb->seid == uplink_seid) {
-				uplink_veb = veb;
-				break;
-			}
-		}
+		uplink_veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
 		if (!uplink_veb) {
 			dev_info(&pf->pdev->dev,
 				 "uplink seid %d not found\n", uplink_seid);
@@ -14872,7 +14848,8 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 	ret = i40e_add_veb(veb, vsi);
 	if (ret)
 		goto err_veb;
-	if (vsi_idx == pf->lan_vsi)
+
+	if (vsi && vsi->idx == pf->lan_vsi)
 		pf->lan_veb = veb->idx;
 
 	return veb;
@@ -14919,13 +14896,10 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
 			int v;
 
 			/* find existing or else empty VEB */
-			i40e_pf_for_each_veb(pf, v, veb)
-				if (veb->seid == seid) {
-					pf->lan_veb = v;
-					break;
-				}
-
-			if (pf->lan_veb >= I40E_MAX_VEB) {
+			veb = i40e_pf_get_veb_by_seid(pf, seid);
+			if (veb) {
+				pf->lan_veb = veb->idx;
+			} else {
 				v = i40e_veb_mem_alloc(pf);
 				if (v < 0)
 					break;
-- 
2.41.0

Re: [PATCH net-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them

[Jacob Keller]

On 2/16/2024 1:42 PM, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
> VEB or VSI by their SEID value and use these helpers to replace
> existing open-coded loops.
> 
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[PATCH net-next 4/5] i40e: Fix broken support for floating VEBs

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

Although the i40e supports so-called floating VEB (VEB without
an uplink connection to external network), this support is
broken. This functionality is currently unused (except debugfs)
but it will be used by subsequent series for switchdev mode
slow-path. Fix this by following:

1) Handle correctly floating VEB (VEB with uplink_seid == 0)
   in i40e_reconstitute_veb() and look for owner VSI and
   create it only for non-floating VEBs and also set bridge
   mode only for such VEBs as the floating ones are using
   always VEB mode.
2) Handle correctly floating VEB in i40e_veb_release() and
   disallow its release when there are some VSIs. This is
   different from regular VEB that have owner VSI that is
   connected to VEB's uplink after VEB deletion by FW.
3) Fix i40e_add_veb() to handle 'vsi' that is NULL for floating
   VEBs. For floating VEB use 0 for downlink SEID and 'true'
   for 'default_port' parameters as per datasheet.
4) Fix 'add relay' command in i40e_dbg_command_write() to allow
   to create floating VEB by 'add relay 0 0' or 'add relay'

Tested using debugfs:
1) Initial state
[root@host net-next]# echo dump switch > $CMD
[root@host net-next]# dmesg -c
[  173.701286] i40e 0000:02:00.0: header: 3 reported 3 total
[  173.706701] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  173.713241] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  173.719507] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

2) Add floating VEB
[root@host net-next]# CMD="/sys/kernel/debug/i40e/0000:02:00.0/command"
[root@host net-next]# echo add relay > $CMD
[root@host net-next]# dmesg -c
[  245.551720] i40e 0000:02:00.0: added relay 162
[root@host net-next]# echo dump switch > $CMD
[root@host net-next]# dmesg -c
[  276.984371] i40e 0000:02:00.0: header: 4 reported 4 total
[  276.989779] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  276.996302] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  277.002569] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
[  277.009091] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0

3) Add VMDQ2 VSI to this new VEB
[root@host net-next]# echo add vsi 162 > $CMD
[root@host net-next]# dmesg -c
[  332.314030] i40e 0000:02:00.0: added VSI 394 to relay 162
[  332.337486] enp2s0f0np0v0: NIC Link is Up, 40 Gbps Full Duplex, Flow Control: None
[root@host net-next]# echo dump switch > $CMD
[root@host net-next]# dmesg -c
[  387.284490] i40e 0000:02:00.0: header: 5 reported 5 total
[  387.289904] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
[  387.296446] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
[  387.302708] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  387.309234] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  387.315500] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

4) Try to delete the VEB
[root@host net-next]# echo del relay 162 > $CMD
[root@host net-next]# dmesg -c
[  428.749297] i40e 0000:02:00.0: deleting relay 162
[  428.754011] i40e 0000:02:00.0: can't remove VEB 162 with 1 VSIs left

5) Do PF reset and check switch status after rebuild
[root@host net-next]# echo pfr > $CMD
[root@host net-next]# echo dump switch > $CMD
[root@host net-next]# dmesg -c
[  738.056172] i40e 0000:02:00.0: header: 5 reported 5 total
[  738.061577] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
[  738.068104] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
[  738.074367] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  738.080892] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  738.087160] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

6) Delete VSI and delete VEB
[root@host net-next]# echo del vsi 394 > $CMD
[root@host net-next]# echo del relay 162 > $CMD
[root@host net-next]# echo dump switch > $CMD
[root@host net-next]# dmesg -c
[ 1233.081126] i40e 0000:02:00.0: deleting VSI 394
[ 1239.345139] i40e 0000:02:00.0: deleting relay 162
[ 1244.886920] i40e 0000:02:00.0: header: 3 reported 3 total
[ 1244.892328] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[ 1244.898853] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[ 1244.905119] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 .../net/ethernet/intel/i40e/i40e_debugfs.c    | 29 ++++---
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 87 ++++++++++---------
 2 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 990a60889eef..921a97d5479e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -829,10 +829,14 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 
 	} else if (strncmp(cmd_buf, "add relay", 9) == 0) {
 		struct i40e_veb *veb;
+		u8 enabled_tc = 0x1;
 		int uplink_seid;
 
 		cnt = sscanf(&cmd_buf[9], "%i %i", &uplink_seid, &vsi_seid);
-		if (cnt != 2) {
+		if (cnt == 0) {
+			uplink_seid = 0;
+			vsi_seid = 0;
+		} else if (cnt != 2) {
 			dev_info(&pf->pdev->dev,
 				 "add relay: bad command string, cnt=%d\n",
 				 cnt);
@@ -844,23 +848,28 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			goto command_write_done;
 		}
 
-		vsi = i40e_dbg_find_vsi(pf, vsi_seid);
-		if (!vsi) {
-			dev_info(&pf->pdev->dev,
-				 "add relay: VSI %d not found\n", vsi_seid);
-			goto command_write_done;
-		}
-
 		veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
 		if (!veb && uplink_seid != 0 && uplink_seid != pf->mac_seid) {
 			dev_info(&pf->pdev->dev,
 				 "add relay: relay uplink %d not found\n",
 				 uplink_seid);
 			goto command_write_done;
+		} else if (uplink_seid) {
+			vsi = i40e_pf_get_vsi_by_seid(pf, vsi_seid);
+			if (!vsi) {
+				dev_info(&pf->pdev->dev,
+					 "add relay: VSI %d not found\n",
+					 vsi_seid);
+				goto command_write_done;
+			}
+			enabled_tc = vsi->tc_config.enabled_tc;
+		} else if (vsi_seid) {
+			dev_info(&pf->pdev->dev,
+				 "add relay: VSI must be 0 for floating relay\n");
+			goto command_write_done;
 		}
 
-		veb = i40e_veb_setup(pf, 0, uplink_seid, vsi_seid,
-				     vsi->tc_config.enabled_tc);
+		veb = i40e_veb_setup(pf, 0, uplink_seid, vsi_seid, enabled_tc);
 		if (veb)
 			dev_info(&pf->pdev->dev, "added relay %d\n", veb->seid);
 		else
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index e495a212d076..31d1b1ff07b0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -10372,41 +10372,48 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
 	struct i40e_vsi *vsi;
 	int v, ret;
 
-	/* build VSI that owns this VEB, temporarily attached to base VEB */
-	i40e_pf_for_each_vsi(pf, v, vsi)
-		if (vsi->veb_idx == veb->idx &&
-		    vsi->flags & I40E_VSI_FLAG_VEB_OWNER) {
-			ctl_vsi = vsi;
-			break;
+	if (veb->uplink_seid) {
+		/* Look for VSI that owns this VEB, temporarily attached to base VEB */
+		i40e_pf_for_each_vsi(pf, v, vsi)
+			if (vsi->veb_idx == veb->idx &&
+			    vsi->flags & I40E_VSI_FLAG_VEB_OWNER) {
+				ctl_vsi = vsi;
+				break;
+			}
+
+		if (!ctl_vsi) {
+			dev_info(&pf->pdev->dev,
+				 "missing owner VSI for veb_idx %d\n",
+				 veb->idx);
+			ret = -ENOENT;
+			goto end_reconstitute;
 		}
+		if (ctl_vsi != pf->vsi[pf->lan_vsi])
+			ctl_vsi->uplink_seid =
+				pf->vsi[pf->lan_vsi]->uplink_seid;
 
-	if (!ctl_vsi) {
-		dev_info(&pf->pdev->dev,
-			 "missing owner VSI for veb_idx %d\n", veb->idx);
-		ret = -ENOENT;
-		goto end_reconstitute;
-	}
-	if (ctl_vsi != pf->vsi[pf->lan_vsi])
-		ctl_vsi->uplink_seid = pf->vsi[pf->lan_vsi]->uplink_seid;
-	ret = i40e_add_vsi(ctl_vsi);
-	if (ret) {
-		dev_info(&pf->pdev->dev,
-			 "rebuild of veb_idx %d owner VSI failed: %d\n",
-			 veb->idx, ret);
-		goto end_reconstitute;
+		ret = i40e_add_vsi(ctl_vsi);
+		if (ret) {
+			dev_info(&pf->pdev->dev,
+				 "rebuild of veb_idx %d owner VSI failed: %d\n",
+				 veb->idx, ret);
+			goto end_reconstitute;
+		}
+		i40e_vsi_reset_stats(ctl_vsi);
 	}
-	i40e_vsi_reset_stats(ctl_vsi);
 
 	/* create the VEB in the switch and move the VSI onto the VEB */
 	ret = i40e_add_veb(veb, ctl_vsi);
 	if (ret)
 		goto end_reconstitute;
 
-	if (test_bit(I40E_FLAG_VEB_MODE_ENA, pf->flags))
-		veb->bridge_mode = BRIDGE_MODE_VEB;
-	else
-		veb->bridge_mode = BRIDGE_MODE_VEPA;
-	i40e_config_bridge_mode(veb);
+	if (veb->uplink_seid) {
+		if (test_bit(I40E_FLAG_VEB_MODE_ENA, pf->flags))
+			veb->bridge_mode = BRIDGE_MODE_VEB;
+		else
+			veb->bridge_mode = BRIDGE_MODE_VEPA;
+		i40e_config_bridge_mode(veb);
+	}
 
 	/* create the remaining VSIs attached to this VEB */
 	i40e_pf_for_each_vsi(pf, v, vsi) {
@@ -14702,29 +14709,29 @@ void i40e_veb_release(struct i40e_veb *veb)
 	/* find the remaining VSI and check for extras */
 	i40e_pf_for_each_vsi(pf, i, vsi_it)
 		if (vsi_it->uplink_seid == veb->seid) {
-			vsi = vsi_it;
+			if (vsi_it->flags & I40E_VSI_FLAG_VEB_OWNER)
+				vsi = vsi_it;
 			n++;
 		}
 
-	if (n != 1) {
+	/* Floating VEB has to be empty and regular one must have
+	 * single owner VSI.
+	 */
+	if ((veb->uplink_seid && n != 1) || (!veb->uplink_seid && n != 0)) {
 		dev_info(&pf->pdev->dev,
 			 "can't remove VEB %d with %d VSIs left\n",
 			 veb->seid, n);
 		return;
 	}
 
-	/* move the remaining VSI to uplink veb */
-	vsi->flags &= ~I40E_VSI_FLAG_VEB_OWNER;
+	/* For regular VEB move the owner VSI to uplink VEB */
 	if (veb->uplink_seid) {
+		vsi->flags &= ~I40E_VSI_FLAG_VEB_OWNER;
 		vsi->uplink_seid = veb->uplink_seid;
 		if (veb->uplink_seid == pf->mac_seid)
 			vsi->veb_idx = I40E_NO_VEB;
 		else
 			vsi->veb_idx = veb->veb_idx;
-	} else {
-		/* floating VEB */
-		vsi->uplink_seid = pf->vsi[pf->lan_vsi]->uplink_seid;
-		vsi->veb_idx = pf->vsi[pf->lan_vsi]->veb_idx;
 	}
 
 	i40e_aq_delete_element(&pf->hw, veb->seid, NULL);
@@ -14742,8 +14749,8 @@ static int i40e_add_veb(struct i40e_veb *veb, struct i40e_vsi *vsi)
 	bool enable_stats = !!test_bit(I40E_FLAG_VEB_STATS_ENA, pf->flags);
 	int ret;
 
-	ret = i40e_aq_add_veb(&pf->hw, veb->uplink_seid, vsi->seid,
-			      veb->enabled_tc, false,
+	ret = i40e_aq_add_veb(&pf->hw, veb->uplink_seid, vsi ? vsi->seid : 0,
+			      veb->enabled_tc, vsi ? false : true,
 			      &veb->seid, enable_stats, NULL);
 
 	/* get a VEB from the hardware */
@@ -14775,9 +14782,11 @@ static int i40e_add_veb(struct i40e_veb *veb, struct i40e_vsi *vsi)
 		return -ENOENT;
 	}
 
-	vsi->uplink_seid = veb->seid;
-	vsi->veb_idx = veb->idx;
-	vsi->flags |= I40E_VSI_FLAG_VEB_OWNER;
+	if (vsi) {
+		vsi->uplink_seid = veb->seid;
+		vsi->veb_idx = veb->idx;
+		vsi->flags |= I40E_VSI_FLAG_VEB_OWNER;
+	}
 
 	return 0;
 }
-- 
2.41.0

Re: [PATCH net-next 4/5] i40e: Fix broken support for floating VEBs

[Jacob Keller]

On 2/16/2024 1:42 PM, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> Although the i40e supports so-called floating VEB (VEB without
> an uplink connection to external network), this support is
> broken. This functionality is currently unused (except debugfs)
> but it will be used by subsequent series for switchdev mode
> slow-path. Fix this by following:
> > 1) Handle correctly floating VEB (VEB with uplink_seid == 0)
>    in i40e_reconstitute_veb() and look for owner VSI and
>    create it only for non-floating VEBs and also set bridge
>    mode only for such VEBs as the floating ones are using
>    always VEB mode.
> 2) Handle correctly floating VEB in i40e_veb_release() and
>    disallow its release when there are some VSIs. This is
>    different from regular VEB that have owner VSI that is
>    connected to VEB's uplink after VEB deletion by FW.
> 3) Fix i40e_add_veb() to handle 'vsi' that is NULL for floating
>    VEBs. For floating VEB use 0 for downlink SEID and 'true'
>    for 'default_port' parameters as per datasheet.
> 4) Fix 'add relay' command in i40e_dbg_command_write() to allow
>    to create floating VEB by 'add relay 0 0' or 'add relay'
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[PATCH net-next 5/5] i40e: Remove VEB recursion

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

The VEB (virtual embedded switch) as a switch element can be
connected according datasheet though its uplink to:
- Physical port
- Port Virtualizer (not used directly by i40e driver but can
  be present in MFP mode where the physical port is shared
  between PFs)
- No uplink (aka floating VEB)

But VEB uplink cannot be connected to another VEB and any attempt
to do so results in:

"i40e 0000:02:00.0: couldn't add VEB, err -EIO aq_err I40E_AQ_RC_ENOENT"

that indicates "the uplink SEID does not point to valid element".

Remove this logic from the driver code this way:

1) For debugfs only allow to build floating VEB (uplink_seid == 0)
   or main VEB (uplink_seid == mac_seid)
2) Do not recurse in i40e_veb_link_event() as no VEB cannot have
   sub-VEBs
3) Ditto for i40e_veb_rebuild() + simplify the function as we know
   that the VEB for rebuild can be only the main LAN VEB or some
   of the floating VEBs
4) In i40e_rebuild() there is no need to check veb->uplink_seid
   as the possible ones are 0 and MAC SEID
5) In i40e_vsi_release() do not take into account VEBs whose
   uplink is another VEB as this is not possible
6) Remove veb_idx field from i40e_veb as a VEB cannot have
   sub-VEBs

Tested using i40e debugfs interface:
1) Initial state
[root@cnb-03 net-next]# CMD="/sys/kernel/debug/i40e/0000:02:00.0/command"
[root@cnb-03 net-next]# echo dump switch > $CMD
[root@cnb-03 net-next]# dmesg -c
[   98.440641] i40e 0000:02:00.0: header: 3 reported 3 total
[   98.446053] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[   98.452593] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[   98.458856] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

2) Add floating VEB
[root@cnb-03 net-next]# echo add relay > $CMD
[root@cnb-03 net-next]# dmesg -c
[  122.745630] i40e 0000:02:00.0: added relay 162
[root@cnb-03 net-next]# echo dump switch > $CMD
[root@cnb-03 net-next]# dmesg -c
[  136.650049] i40e 0000:02:00.0: header: 4 reported 4 total
[  136.655466] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  136.661994] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  136.668264] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
[  136.674787] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0

3) Add VMDQ2 VSI to this new VEB
[root@cnb-03 net-next]# dmesg -c
[  168.351763] i40e 0000:02:00.0: added VSI 394 to relay 162
[  168.374652] enp2s0f0np0v0: NIC Link is Up, 40 Gbps Full Duplex, Flow Control: None
[root@cnb-03 net-next]# echo dump switch > $CMD
[root@cnb-03 net-next]# dmesg -c
[  195.683204] i40e 0000:02:00.0: header: 5 reported 5 total
[  195.688611] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
[  195.695143] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
[  195.701410] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  195.707935] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  195.714201] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

4) Try to delete the VEB
[root@cnb-03 net-next]# echo del relay 162 > $CMD
[root@cnb-03 net-next]# dmesg -c
[  239.260901] i40e 0000:02:00.0: deleting relay 162
[  239.265621] i40e 0000:02:00.0: can't remove VEB 162 with 1 VSIs left

5) Do PF reset and check switch status after rebuild
[root@cnb-03 net-next]# echo pfr > $CMD
[root@cnb-03 net-next]# echo dump switch > $CMD
[root@cnb-03 net-next]# dmesg -c
...
[  272.333655] i40e 0000:02:00.0: header: 5 reported 5 total
[  272.339066] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
[  272.345599] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
[  272.351862] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  272.358387] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  272.364654] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

6) Delete VSI and delete VEB
[  297.199116] i40e 0000:02:00.0: deleting VSI 394
[  299.807580] i40e 0000:02:00.0: deleting relay 162
[  309.767905] i40e 0000:02:00.0: header: 3 reported 3 total
[  309.773318] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
[  309.779845] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
[  309.786111] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h        |   1 -
 .../net/ethernet/intel/i40e/i40e_debugfs.c    |   8 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 176 ++++++++----------
 3 files changed, 76 insertions(+), 109 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 2990ff4f0306..ba24f3fa92c3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -783,7 +783,6 @@ struct i40e_new_mac_filter {
 struct i40e_veb {
 	struct i40e_pf *pf;
 	u16 idx;
-	u16 veb_idx;		/* index of VEB parent */
 	u16 seid;
 	u16 uplink_seid;
 	u16 stats_idx;		/* index of VEB parent */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 921a97d5479e..f9ba45f596c9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -683,9 +683,8 @@ static void i40e_dbg_dump_veb_seid(struct i40e_pf *pf, int seid)
 		return;
 	}
 	dev_info(&pf->pdev->dev,
-		 "veb idx=%d,%d stats_ic=%d  seid=%d uplink=%d mode=%s\n",
-		 veb->idx, veb->veb_idx, veb->stats_idx, veb->seid,
-		 veb->uplink_seid,
+		 "veb idx=%d stats_ic=%d  seid=%d uplink=%d mode=%s\n",
+		 veb->idx, veb->stats_idx, veb->seid, veb->uplink_seid,
 		 veb->bridge_mode == BRIDGE_MODE_VEPA ? "VEPA" : "VEB");
 	i40e_dbg_dump_eth_stats(pf, &veb->stats);
 }
@@ -848,8 +847,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			goto command_write_done;
 		}
 
-		veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
-		if (!veb && uplink_seid != 0 && uplink_seid != pf->mac_seid) {
+		if (uplink_seid != 0 && uplink_seid != pf->mac_seid) {
 			dev_info(&pf->pdev->dev,
 				 "add relay: relay uplink %d not found\n",
 				 uplink_seid);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 31d1b1ff07b0..f12092cdb1f0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9873,7 +9873,6 @@ static void i40e_vsi_link_event(struct i40e_vsi *vsi, bool link_up)
  **/
 static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
 {
-	struct i40e_veb *veb_it;
 	struct i40e_vsi *vsi;
 	struct i40e_pf *pf;
 	int i;
@@ -9882,12 +9881,7 @@ static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
 		return;
 	pf = veb->pf;
 
-	/* depth first... */
-	i40e_pf_for_each_veb(pf, i, veb_it)
-		if (veb_it->uplink_seid == veb->seid)
-			i40e_veb_link_event(veb_it, link_up);
-
-	/* ... now the local VSIs */
+	/* Send link event to contained VSIs */
 	i40e_pf_for_each_vsi(pf, i, vsi)
 		if (vsi->uplink_seid == veb->seid)
 			i40e_vsi_link_event(vsi, link_up);
@@ -10356,56 +10350,57 @@ static void i40e_config_bridge_mode(struct i40e_veb *veb)
 }
 
 /**
- * i40e_reconstitute_veb - rebuild the VEB and anything connected to it
+ * i40e_reconstitute_veb - rebuild the VEB and VSIs connected to it
  * @veb: pointer to the VEB instance
  *
- * This is a recursive function that first builds the attached VSIs then
- * recurses in to build the next layer of VEB.  We track the connections
- * through our own index numbers because the seid's from the HW could
- * change across the reset.
+ * This is a function that builds the attached VSIs. We track the connections
+ * through our own index numbers because the seid's from the HW could change
+ * across the reset.
  **/
 static int i40e_reconstitute_veb(struct i40e_veb *veb)
 {
 	struct i40e_vsi *ctl_vsi = NULL;
 	struct i40e_pf *pf = veb->pf;
-	struct i40e_veb *veb_it;
 	struct i40e_vsi *vsi;
 	int v, ret;
 
-	if (veb->uplink_seid) {
-		/* Look for VSI that owns this VEB, temporarily attached to base VEB */
-		i40e_pf_for_each_vsi(pf, v, vsi)
-			if (vsi->veb_idx == veb->idx &&
-			    vsi->flags & I40E_VSI_FLAG_VEB_OWNER) {
-				ctl_vsi = vsi;
-				break;
-			}
+	/* As we do not maintain PV (port virtualizer) switch element then
+	 * there can be only one non-floating VEB that have uplink to MAC SEID
+	 * and its control VSI is the main one.
+	 */
+	if (WARN_ON(veb->uplink_seid && veb->uplink_seid != pf->mac_seid)) {
+		dev_err(&pf->pdev->dev,
+			"Invalid uplink SEID for VEB %d\n", veb->idx);
+		return -ENOENT;
+	}
 
-		if (!ctl_vsi) {
-			dev_info(&pf->pdev->dev,
-				 "missing owner VSI for veb_idx %d\n",
-				 veb->idx);
-			ret = -ENOENT;
-			goto end_reconstitute;
+	if (veb->uplink_seid == pf->mac_seid) {
+		/* Check that the LAN VSI has VEB owning flag set */
+		ctl_vsi = pf->vsi[pf->lan_vsi];
+
+		if (WARN_ON(ctl_vsi->veb_idx != veb->idx ||
+			    !(ctl_vsi->flags & I40E_VSI_FLAG_VEB_OWNER))) {
+			dev_err(&pf->pdev->dev,
+				"Invalid control VSI for VEB %d\n", veb->idx);
+			return -ENOENT;
 		}
-		if (ctl_vsi != pf->vsi[pf->lan_vsi])
-			ctl_vsi->uplink_seid =
-				pf->vsi[pf->lan_vsi]->uplink_seid;
 
+		/* Add the control VSI to switch */
 		ret = i40e_add_vsi(ctl_vsi);
 		if (ret) {
-			dev_info(&pf->pdev->dev,
-				 "rebuild of veb_idx %d owner VSI failed: %d\n",
-				 veb->idx, ret);
-			goto end_reconstitute;
+			dev_err(&pf->pdev->dev,
+				"Rebuild of owner VSI for VEB %d failed: %d\n",
+				veb->idx, ret);
+			return ret;
 		}
+
 		i40e_vsi_reset_stats(ctl_vsi);
 	}
 
 	/* create the VEB in the switch and move the VSI onto the VEB */
 	ret = i40e_add_veb(veb, ctl_vsi);
 	if (ret)
-		goto end_reconstitute;
+		return ret;
 
 	if (veb->uplink_seid) {
 		if (test_bit(I40E_FLAG_VEB_MODE_ENA, pf->flags))
@@ -10427,23 +10422,12 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
 				dev_info(&pf->pdev->dev,
 					 "rebuild of vsi_idx %d failed: %d\n",
 					 v, ret);
-				goto end_reconstitute;
+				return ret;
 			}
 			i40e_vsi_reset_stats(vsi);
 		}
 	}
 
-	/* create any VEBs attached to this VEB - RECURSION */
-	i40e_pf_for_each_veb(pf, v, veb_it) {
-		if (veb_it->veb_idx == veb->idx) {
-			veb_it->uplink_seid = veb->seid;
-			ret = i40e_reconstitute_veb(veb_it);
-			if (ret)
-				break;
-		}
-	}
-
-end_reconstitute:
 	return ret;
 }
 
@@ -10984,31 +10968,29 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 	 */
 	if (vsi->uplink_seid != pf->mac_seid) {
 		dev_dbg(&pf->pdev->dev, "attempting to rebuild switch\n");
-		/* find the one VEB connected to the MAC, and find orphans */
+
+		/* Rebuild VEBs */
 		i40e_pf_for_each_veb(pf, v, veb) {
-			if (veb->uplink_seid == pf->mac_seid ||
-			    veb->uplink_seid == 0) {
-				ret = i40e_reconstitute_veb(veb);
-				if (!ret)
-					continue;
-
-				/* If Main VEB failed, we're in deep doodoo,
-				 * so give up rebuilding the switch and set up
-				 * for minimal rebuild of PF VSI.
-				 * If orphan failed, we'll report the error
-				 * but try to keep going.
-				 */
-				if (veb->uplink_seid == pf->mac_seid) {
-					dev_info(&pf->pdev->dev,
-						 "rebuild of switch failed: %d, will try to set up simple PF connection\n",
-						 ret);
-					vsi->uplink_seid = pf->mac_seid;
-					break;
-				} else if (veb->uplink_seid == 0) {
-					dev_info(&pf->pdev->dev,
-						 "rebuild of orphan VEB failed: %d\n",
-						 ret);
-				}
+			ret = i40e_reconstitute_veb(veb);
+			if (!ret)
+				continue;
+
+			/* If Main VEB failed, we're in deep doodoo,
+			 * so give up rebuilding the switch and set up
+			 * for minimal rebuild of PF VSI.
+			 * If orphan failed, we'll report the error
+			 * but try to keep going.
+			 */
+			if (veb->uplink_seid == pf->mac_seid) {
+				dev_info(&pf->pdev->dev,
+					 "rebuild of switch failed: %d, will try to set up simple PF connection\n",
+					 ret);
+				vsi->uplink_seid = pf->mac_seid;
+				break;
+			} else if (veb->uplink_seid == 0) {
+				dev_info(&pf->pdev->dev,
+					 "rebuild of orphan VEB failed: %d\n",
+					 ret);
 			}
 		}
 	}
@@ -14124,9 +14106,9 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
  **/
 int i40e_vsi_release(struct i40e_vsi *vsi)
 {
-	struct i40e_veb *veb, *veb_it;
 	struct i40e_mac_filter *f;
 	struct hlist_node *h;
+	struct i40e_veb *veb;
 	struct i40e_pf *pf;
 	u16 uplink_seid;
 	int i, n, bkt;
@@ -14190,27 +14172,28 @@ int i40e_vsi_release(struct i40e_vsi *vsi)
 
 	/* If this was the last thing on the VEB, except for the
 	 * controlling VSI, remove the VEB, which puts the controlling
-	 * VSI onto the next level down in the switch.
+	 * VSI onto the uplink port.
 	 *
 	 * Well, okay, there's one more exception here: don't remove
-	 * the orphan VEBs yet.  We'll wait for an explicit remove request
+	 * the floating VEBs yet.  We'll wait for an explicit remove request
 	 * from up the network stack.
 	 */
-	n = 0;
-	i40e_pf_for_each_vsi(pf, i, vsi)
-		if (vsi->uplink_seid == uplink_seid &&
-		    (vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
-			n++;      /* count the VSIs */
+	veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
+	if (veb && veb->uplink_seid) {
+		n = 0;
 
-	veb = NULL;
-	i40e_pf_for_each_veb(pf, i, veb_it) {
-		if (veb_it->uplink_seid == uplink_seid)
-			n++;     /* count the VEBs */
-		if (veb_it->seid == uplink_seid)
-			veb = veb_it;
+		/* Count non-controlling VSIs present on  the VEB */
+		i40e_pf_for_each_vsi(pf, i, vsi)
+			if (vsi->uplink_seid == uplink_seid &&
+			    (vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
+				n++;
+
+		/* If there is no VSI except the control one then release
+		 * the VEB and put the control VSI onto VEB uplink.
+		 */
+		if (!n)
+			i40e_veb_release(veb);
 	}
-	if (n == 0 && veb && veb->uplink_seid != 0)
-		i40e_veb_release(veb);
 
 	return 0;
 }
@@ -14724,14 +14707,11 @@ void i40e_veb_release(struct i40e_veb *veb)
 		return;
 	}
 
-	/* For regular VEB move the owner VSI to uplink VEB */
+	/* For regular VEB move the owner VSI to uplink port */
 	if (veb->uplink_seid) {
 		vsi->flags &= ~I40E_VSI_FLAG_VEB_OWNER;
 		vsi->uplink_seid = veb->uplink_seid;
-		if (veb->uplink_seid == pf->mac_seid)
-			vsi->veb_idx = I40E_NO_VEB;
-		else
-			vsi->veb_idx = veb->veb_idx;
+		vsi->veb_idx = I40E_NO_VEB;
 	}
 
 	i40e_aq_delete_element(&pf->hw, veb->seid, NULL);
@@ -14811,8 +14791,8 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 				u16 uplink_seid, u16 vsi_seid,
 				u8 enabled_tc)
 {
-	struct i40e_veb *veb, *uplink_veb = NULL;
 	struct i40e_vsi *vsi = NULL;
+	struct i40e_veb *veb;
 	int veb_idx;
 	int ret;
 
@@ -14834,14 +14814,6 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 			return NULL;
 		}
 	}
-	if (uplink_seid && uplink_seid != pf->mac_seid) {
-		uplink_veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
-		if (!uplink_veb) {
-			dev_info(&pf->pdev->dev,
-				 "uplink seid %d not found\n", uplink_seid);
-			return NULL;
-		}
-	}
 
 	/* get veb sw struct */
 	veb_idx = i40e_veb_mem_alloc(pf);
@@ -14850,7 +14822,6 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 	veb = pf->veb[veb_idx];
 	veb->flags = flags;
 	veb->uplink_seid = uplink_seid;
-	veb->veb_idx = (uplink_veb ? uplink_veb->idx : I40E_NO_VEB);
 	veb->enabled_tc = (enabled_tc ? enabled_tc : 0x1);
 
 	/* create the VEB in the switch */
@@ -14921,7 +14892,6 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
 		pf->veb[pf->lan_veb]->seid = seid;
 		pf->veb[pf->lan_veb]->uplink_seid = pf->mac_seid;
 		pf->veb[pf->lan_veb]->pf = pf;
-		pf->veb[pf->lan_veb]->veb_idx = I40E_NO_VEB;
 		break;
 	case I40E_SWITCH_ELEMENT_TYPE_VSI:
 		if (num_reported != 1)
-- 
2.41.0

Re: [PATCH net-next 5/5] i40e: Remove VEB recursion

[Jacob Keller]

On 2/16/2024 1:42 PM, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> The VEB (virtual embedded switch) as a switch element can be
> connected according datasheet though its uplink to:
> - Physical port
> - Port Virtualizer (not used directly by i40e driver but can
>   be present in MFP mode where the physical port is shared
>   between PFs)
> - No uplink (aka floating VEB)
> 
> But VEB uplink cannot be connected to another VEB and any attempt
> to do so results in:
> 
> "i40e 0000:02:00.0: couldn't add VEB, err -EIO aq_err I40E_AQ_RC_ENOENT"
> 
> that indicates "the uplink SEID does not point to valid element".
> 
> Remove this logic from the driver code this way:
> 
> 1) For debugfs only allow to build floating VEB (uplink_seid == 0)
>    or main VEB (uplink_seid == mac_seid)
> 2) Do not recurse in i40e_veb_link_event() as no VEB cannot have
>    sub-VEBs
> 3) Ditto for i40e_veb_rebuild() + simplify the function as we know
>    that the VEB for rebuild can be only the main LAN VEB or some
>    of the floating VEBs
> 4) In i40e_rebuild() there is no need to check veb->uplink_seid
>    as the possible ones are 0 and MAC SEID
> 5) In i40e_vsi_release() do not take into account VEBs whose
>    uplink is another VEB as this is not possible
> 6) Remove veb_idx field from i40e_veb as a VEB cannot have
>    sub-VEBs
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net-next 0/5][pull request] i40e: Simplify VSI and VEB handling

[Jakub Kicinski]

On Fri, 16 Feb 2024 13:42:37 -0800 Tony Nguyen wrote:
> The series simplifies handling of VSIs and VEBs by introducing for-each
> iterating macros, 'find' helper functions. Also removes the VEB
> recursion because the VEBs cannot have sub-VEBs according datasheet and
> fixes the support for floating VEBs.
> 
> The series content:
> Patch 1 - Uses existing helper function for find FDIR VSI instead of loop
> Patch 2 - Adds and uses macros to iterate VSI and VEB arrays
> Patch 3 - Adds 2 helper functions to find VSIs and VEBs by their SEID
> Patch 4 - Fixes broken support for floating VEBs
> Patch 5 - Removes VEB recursion and simplifies VEB handling

FTR looks like DaveM already pulled this.