[PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling
@ 2023-11-13 12:58 Ivan Vecera
  2023-11-13 12:58 ` [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI Ivan Vecera
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Ivan Vecera @ 2023-11-13 12:58 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Wojciech Drewek, Simon Horman, mschmidt

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

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        |  91 ++-
 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, 371 insertions(+), 390 deletions(-)

-- 
2.41.0


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

* [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI
  2023-11-13 12:58 [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling Ivan Vecera
@ 2023-11-13 12:58 ` Ivan Vecera
  2023-11-13 13:09   ` Wojciech Drewek
  2023-11-13 12:58 ` [PATCH iwl-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs Ivan Vecera
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-13 12:58 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Wojciech Drewek, Simon Horman, mschmidt

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

Signed-off-by: Ivan Vecera <ivecera@redhat.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 51ee870ffa36..90966878333c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -15645,6 +15645,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;
@@ -15655,7 +15656,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)
@@ -16005,12 +16005,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


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

* Re: [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI
  2023-11-13 12:58 ` [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI Ivan Vecera
@ 2023-11-13 13:09   ` Wojciech Drewek
  0 siblings, 0 replies; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-13 13:09 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 13.11.2023 13:58, Ivan Vecera wrote:
> Use existing i40e_find_vsi_by_type() to find a VSI
> associated with flow director.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---

Thanks Ivan, nice cleanup
Reviewed-by: Wojciech Drewek <wojciech.drewek@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 51ee870ffa36..90966878333c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -15645,6 +15645,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;
> @@ -15655,7 +15656,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)
> @@ -16005,12 +16005,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.

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

* [PATCH iwl-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs
  2023-11-13 12:58 [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling Ivan Vecera
  2023-11-13 12:58 ` [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI Ivan Vecera
@ 2023-11-13 12:58 ` Ivan Vecera
  2023-11-13 13:21   ` Wojciech Drewek
  2023-11-13 12:58 ` [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them Ivan Vecera
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-13 12:58 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Wojciech Drewek, Simon Horman, mschmidt

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.

Signed-off-by: Ivan Vecera <ivecera@redhat.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 f1627ab211cd..1e9266de270b 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;						\
+	     _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;						\
+	     _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 4721845fda6e..57020effd2c7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
@@ -948,16 +948,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 90966878333c..c410fc31a051 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -304,11 +304,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;
 }
@@ -546,24 +547,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;
 }
@@ -2875,6 +2871,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)
@@ -2886,11 +2883,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 */
@@ -5162,6 +5158,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))
@@ -5171,9 +5168,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);
 }
 
@@ -5270,12 +5268,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);
 }
 
 /**
@@ -5284,12 +5281,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);
 }
 
 /**
@@ -5350,14 +5346,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(pf->vsi[v]);
+		if (ret)
+			break;
 	}
 
 	return ret;
@@ -6774,32 +6769,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
 		 */
@@ -6808,17 +6800,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);
 		}
 	}
 }
@@ -9253,7 +9245,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)) {
@@ -9309,29 +9303,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);
@@ -9886,6 +9871,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;
 
@@ -9894,14 +9881,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 */
@@ -10016,15 +10005,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);
@@ -10387,18 +10375,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);
@@ -10428,13 +10416,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) {
@@ -10448,10 +10434,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;
 		}
@@ -10725,6 +10711,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;
 
@@ -10738,11 +10725,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);
@@ -10856,6 +10841,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 = EBADE;
 	u32 val;
 	int v;
@@ -10998,14 +10984,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;
 
@@ -11015,13 +10997,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);
@@ -12112,6 +12094,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
@@ -12128,13 +12111,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);
@@ -13136,8 +13118,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 */
@@ -13145,10 +13127,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)
@@ -13221,12 +13204,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,
@@ -14157,9 +14138,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;
@@ -14229,20 +14210,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);
@@ -14419,22 +14398,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;
@@ -14462,11 +14437,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;
 		}
@@ -14695,29 +14669,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.
@@ -14755,19 +14724,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",
@@ -14865,6 +14834,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;
 
@@ -14878,9 +14848,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);
@@ -14888,10 +14859,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;
 			}
 		}
@@ -14913,7 +14883,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)
@@ -14944,6 +14914,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,
@@ -14962,12 +14933,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)
@@ -16253,6 +16224,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;
 
@@ -16310,24 +16283,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);
 
@@ -16364,18 +16332,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


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

* Re: [PATCH iwl-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs
  2023-11-13 12:58 ` [PATCH iwl-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs Ivan Vecera
@ 2023-11-13 13:21   ` Wojciech Drewek
  0 siblings, 0 replies; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-13 13:21 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 13.11.2023 13:58, Ivan Vecera wrote:
> 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.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---

Again, nice cleanup.
Reviewed-by: Wojciech Drewek <wojciech.drewek@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 f1627ab211cd..1e9266de270b 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;						\
> +	     _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;						\
> +	     _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 4721845fda6e..57020effd2c7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
> @@ -948,16 +948,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 90966878333c..c410fc31a051 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -304,11 +304,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;
>  }
> @@ -546,24 +547,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;
>  }
> @@ -2875,6 +2871,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)
> @@ -2886,11 +2883,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 */
> @@ -5162,6 +5158,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))
> @@ -5171,9 +5168,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);
>  }
>  
> @@ -5270,12 +5268,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);
>  }
>  
>  /**
> @@ -5284,12 +5281,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);
>  }
>  
>  /**
> @@ -5350,14 +5346,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(pf->vsi[v]);
> +		if (ret)
> +			break;
>  	}
>  
>  	return ret;
> @@ -6774,32 +6769,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
>  		 */
> @@ -6808,17 +6800,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);
>  		}
>  	}
>  }
> @@ -9253,7 +9245,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)) {
> @@ -9309,29 +9303,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);
> @@ -9886,6 +9871,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;
>  
> @@ -9894,14 +9881,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 */
> @@ -10016,15 +10005,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);
> @@ -10387,18 +10375,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);
> @@ -10428,13 +10416,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) {
> @@ -10448,10 +10434,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;
>  		}
> @@ -10725,6 +10711,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;
>  
> @@ -10738,11 +10725,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);
> @@ -10856,6 +10841,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 = EBADE;
>  	u32 val;
>  	int v;
> @@ -10998,14 +10984,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;
>  
> @@ -11015,13 +10997,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);
> @@ -12112,6 +12094,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
> @@ -12128,13 +12111,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);
> @@ -13136,8 +13118,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 */
> @@ -13145,10 +13127,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)
> @@ -13221,12 +13204,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,
> @@ -14157,9 +14138,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;
> @@ -14229,20 +14210,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);
> @@ -14419,22 +14398,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;
> @@ -14462,11 +14437,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;
>  		}
> @@ -14695,29 +14669,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.
> @@ -14755,19 +14724,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",
> @@ -14865,6 +14834,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;
>  
> @@ -14878,9 +14848,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);
> @@ -14888,10 +14859,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;
>  			}
>  		}
> @@ -14913,7 +14883,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)
> @@ -14944,6 +14914,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,
> @@ -14962,12 +14933,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)
> @@ -16253,6 +16224,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;
>  
> @@ -16310,24 +16283,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);
>  
> @@ -16364,18 +16332,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;
>  	}
>  

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

* [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-13 12:58 [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling Ivan Vecera
  2023-11-13 12:58 ` [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI Ivan Vecera
  2023-11-13 12:58 ` [PATCH iwl-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs Ivan Vecera
@ 2023-11-13 12:58 ` Ivan Vecera
  2023-11-13 13:27   ` Wojciech Drewek
  2023-11-13 12:58 ` [PATCH iwl-next 4/5] i40e: Fix broken support for floating VEBs Ivan Vecera
  2023-11-13 12:58 ` [PATCH iwl-next 5/5] i40e: Remove VEB recursion Ivan Vecera
  4 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-13 12:58 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Wojciech Drewek, Simon Horman, mschmidt

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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h        | 34 +++++++++
 .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++--------
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 76 ++++++-------------
 3 files changed, 64 insertions(+), 84 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 1e9266de270b..220b5ce31519 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
 
 struct device *i40e_hw_to_dev(struct i40e_hw *hw);
 
+/**
+ * i40e_vsi_get_by_seid - find VSI by SEID
+ * @pf: pointer to a PF
+ **/
+static inline struct i40e_vsi *
+i40e_vsi_get_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_veb_get_by_seid - find VEB by SEID
+ * @pf: pointer to a PF
+ **/
+static inline struct i40e_veb *
+i40e_veb_get_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..d26348db125f 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_vsi_get_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_veb_get_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_veb_get_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 c410fc31a051..e7be145f298a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -13120,18 +13120,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_veb_get_by_seid(pf, vsi->uplink_seid);
 
 	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
 	if (!br_spec)
@@ -13197,17 +13193,14 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
 	struct i40e_veb *veb = NULL;
-	int i;
 
 	/* 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_veb_get_by_seid(pf, vsi->uplink_seid);
+	if (!vsi)
 		return 0;
 
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
@@ -14382,8 +14375,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
@@ -14398,18 +14391,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_veb_get_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_vsi_get_by_seid(pf, uplink_seid);
+		if (!vsi) {
 			dev_info(&pf->pdev->dev, "no such uplink_seid %d\n",
 				 uplink_seid);
 			return NULL;
@@ -14437,10 +14422,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_veb_get_by_seid(pf, vsi->uplink_seid);
+		if (!veb) {
 			dev_info(&pf->pdev->dev, "couldn't add VEB\n");
 			return NULL;
 		}
@@ -14835,7 +14818,7 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
 {
 	struct i40e_veb *veb, *uplink_veb = NULL;
 	struct i40e_vsi *vsi;
-	int vsi_idx, veb_idx;
+	int veb_idx;
 	int ret;
 
 	/* if one seid is 0, the other must be 0 to create a floating relay */
@@ -14848,23 +14831,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_vsi_get_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_veb_get_by_seid(pf, uplink_seid);
 		if (!uplink_veb) {
 			dev_info(&pf->pdev->dev,
 				 "uplink seid %d not found\n", uplink_seid);
@@ -14886,7 +14862,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;
@@ -14933,13 +14910,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_veb_get_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


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

* Re: [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-13 12:58 ` [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them Ivan Vecera
@ 2023-11-13 13:27   ` Wojciech Drewek
  2023-11-15 17:01     ` Ivan Vecera
  0 siblings, 1 reply; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-13 13:27 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 13.11.2023 13:58, Ivan Vecera wrote:
> 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.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---

Only one nit
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/intel/i40e/i40e.h        | 34 +++++++++
>  .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++--------
>  drivers/net/ethernet/intel/i40e/i40e_main.c   | 76 ++++++-------------
>  3 files changed, 64 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 1e9266de270b..220b5ce31519 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>  
>  struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>  
> +/**
> + * i40e_vsi_get_by_seid - find VSI by SEID
> + * @pf: pointer to a PF
> + **/
> +static inline struct i40e_vsi *
> +i40e_vsi_get_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_veb_get_by_seid - find VEB by SEID
> + * @pf: pointer to a PF
> + **/
> +static inline struct i40e_veb *
> +i40e_veb_get_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;
> +}

I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.

> +
>  #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..d26348db125f 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_vsi_get_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_veb_get_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_veb_get_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 c410fc31a051..e7be145f298a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -13120,18 +13120,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_veb_get_by_seid(pf, vsi->uplink_seid);
>  
>  	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
>  	if (!br_spec)
> @@ -13197,17 +13193,14 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>  	struct i40e_vsi *vsi = np->vsi;
>  	struct i40e_pf *pf = vsi->back;
>  	struct i40e_veb *veb = NULL;
> -	int i;
>  
>  	/* 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_veb_get_by_seid(pf, vsi->uplink_seid);
> +	if (!vsi)
>  		return 0;
>  
>  	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
> @@ -14382,8 +14375,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
> @@ -14398,18 +14391,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_veb_get_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_vsi_get_by_seid(pf, uplink_seid);
> +		if (!vsi) {
>  			dev_info(&pf->pdev->dev, "no such uplink_seid %d\n",
>  				 uplink_seid);
>  			return NULL;
> @@ -14437,10 +14422,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_veb_get_by_seid(pf, vsi->uplink_seid);
> +		if (!veb) {
>  			dev_info(&pf->pdev->dev, "couldn't add VEB\n");
>  			return NULL;
>  		}
> @@ -14835,7 +14818,7 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
>  {
>  	struct i40e_veb *veb, *uplink_veb = NULL;
>  	struct i40e_vsi *vsi;
> -	int vsi_idx, veb_idx;
> +	int veb_idx;
>  	int ret;
>  
>  	/* if one seid is 0, the other must be 0 to create a floating relay */
> @@ -14848,23 +14831,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_vsi_get_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_veb_get_by_seid(pf, uplink_seid);
>  		if (!uplink_veb) {
>  			dev_info(&pf->pdev->dev,
>  				 "uplink seid %d not found\n", uplink_seid);
> @@ -14886,7 +14862,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;
> @@ -14933,13 +14910,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_veb_get_by_seid(pf, seid);
> +			if (veb) {
> +				pf->lan_veb = veb->idx;
> +			} else {
>  				v = i40e_veb_mem_alloc(pf);
>  				if (v < 0)
>  					break;

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

* Re: [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-13 13:27   ` Wojciech Drewek
@ 2023-11-15 17:01     ` Ivan Vecera
  2023-11-16 12:37       ` Wojciech Drewek
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-15 17:01 UTC (permalink / raw)
  To: Wojciech Drewek, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt


On 13. 11. 23 14:27, Wojciech Drewek wrote:
> 
> On 13.11.2023 13:58, Ivan Vecera wrote:
>> 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.
>>
>> Signed-off-by: Ivan Vecera<ivecera@redhat.com>
>> ---
> Only one nit
> Reviewed-by: Wojciech Drewek<wojciech.drewek@intel.com>
> 
>>   drivers/net/ethernet/intel/i40e/i40e.h        | 34 +++++++++
>>   .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++--------
>>   drivers/net/ethernet/intel/i40e/i40e_main.c   | 76 ++++++-------------
>>   3 files changed, 64 insertions(+), 84 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index 1e9266de270b..220b5ce31519 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>   
>>   struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>   
>> +/**
>> + * i40e_vsi_get_by_seid - find VSI by SEID
>> + * @pf: pointer to a PF
>> + **/
>> +static inline struct i40e_vsi *
>> +i40e_vsi_get_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_veb_get_by_seid - find VEB by SEID
>> + * @pf: pointer to a PF
>> + **/
>> +static inline struct i40e_veb *
>> +i40e_veb_get_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;
>> +}
> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.

I'd rather use i40e_pf_ prefix...

What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?

Ivan


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

* Re: [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-15 17:01     ` Ivan Vecera
@ 2023-11-16 12:37       ` Wojciech Drewek
  2023-11-16 13:59         ` Ivan Vecera
  0 siblings, 1 reply; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-16 12:37 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 15.11.2023 18:01, Ivan Vecera wrote:
> 
> On 13. 11. 23 14:27, Wojciech Drewek wrote:
>>
>> On 13.11.2023 13:58, Ivan Vecera wrote:
>>> 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.
>>>
>>> Signed-off-by: Ivan Vecera<ivecera@redhat.com>
>>> ---
>> Only one nit
>> Reviewed-by: Wojciech Drewek<wojciech.drewek@intel.com>
>>
>>>   drivers/net/ethernet/intel/i40e/i40e.h        | 34 +++++++++
>>>   .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++--------
>>>   drivers/net/ethernet/intel/i40e/i40e_main.c   | 76 ++++++-------------
>>>   3 files changed, 64 insertions(+), 84 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>>> index 1e9266de270b..220b5ce31519 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>>     struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>>   +/**
>>> + * i40e_vsi_get_by_seid - find VSI by SEID
>>> + * @pf: pointer to a PF
>>> + **/
>>> +static inline struct i40e_vsi *
>>> +i40e_vsi_get_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_veb_get_by_seid - find VEB by SEID
>>> + * @pf: pointer to a PF
>>> + **/
>>> +static inline struct i40e_veb *
>>> +i40e_veb_get_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;
>>> +}
>> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
> 
> I'd rather use i40e_pf_ prefix...
> 
> What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?

Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"

> 
> Ivan
> 

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

* Re: [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-16 12:37       ` Wojciech Drewek
@ 2023-11-16 13:59         ` Ivan Vecera
  2023-11-16 14:21           ` Wojciech Drewek
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-16 13:59 UTC (permalink / raw)
  To: Wojciech Drewek, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt


On 16. 11. 23 13:37, Wojciech Drewek wrote:
> 
> 
> On 15.11.2023 18:01, Ivan Vecera wrote:
>>
>> On 13. 11. 23 14:27, Wojciech Drewek wrote:
>>>
>>> On 13.11.2023 13:58, Ivan Vecera wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Ivan Vecera<ivecera@redhat.com>
>>>> ---
>>> Only one nit
>>> Reviewed-by: Wojciech Drewek<wojciech.drewek@intel.com>
>>>
>>>>    drivers/net/ethernet/intel/i40e/i40e.h        | 34 +++++++++
>>>>    .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++--------
>>>>    drivers/net/ethernet/intel/i40e/i40e_main.c   | 76 ++++++-------------
>>>>    3 files changed, 64 insertions(+), 84 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>>>> index 1e9266de270b..220b5ce31519 100644
>>>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>>>      struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>>>    +/**
>>>> + * i40e_vsi_get_by_seid - find VSI by SEID
>>>> + * @pf: pointer to a PF
>>>> + **/
>>>> +static inline struct i40e_vsi *
>>>> +i40e_vsi_get_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_veb_get_by_seid - find VEB by SEID
>>>> + * @pf: pointer to a PF
>>>> + **/
>>>> +static inline struct i40e_veb *
>>>> +i40e_veb_get_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;
>>>> +}
>>> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
>>
>> I'd rather use i40e_pf_ prefix...
>>
>> What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?
> 
> Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"

OK, got it... Will repost v2 with this change + "too many also..." issue ;-)

Btw. what about the last patch?

Ivan


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

* Re: [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-16 13:59         ` Ivan Vecera
@ 2023-11-16 14:21           ` Wojciech Drewek
  2023-11-16 14:46             ` Ivan Vecera
  0 siblings, 1 reply; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-16 14:21 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 16.11.2023 14:59, Ivan Vecera wrote:
> 
> On 16. 11. 23 13:37, Wojciech Drewek wrote:
>>
>>
>> On 15.11.2023 18:01, Ivan Vecera wrote:
>>>
>>> On 13. 11. 23 14:27, Wojciech Drewek wrote:
>>>>
>>>> On 13.11.2023 13:58, Ivan Vecera wrote:
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Ivan Vecera<ivecera@redhat.com>
>>>>> ---
>>>> Only one nit
>>>> Reviewed-by: Wojciech Drewek<wojciech.drewek@intel.com>
>>>>
>>>>>    drivers/net/ethernet/intel/i40e/i40e.h        | 34 +++++++++
>>>>>    .../net/ethernet/intel/i40e/i40e_debugfs.c    | 38 ++--------
>>>>>    drivers/net/ethernet/intel/i40e/i40e_main.c   | 76 ++++++-------------
>>>>>    3 files changed, 64 insertions(+), 84 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>>>>> index 1e9266de270b..220b5ce31519 100644
>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>>>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>>>>      struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>>>>    +/**
>>>>> + * i40e_vsi_get_by_seid - find VSI by SEID
>>>>> + * @pf: pointer to a PF
>>>>> + **/
>>>>> +static inline struct i40e_vsi *
>>>>> +i40e_vsi_get_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_veb_get_by_seid - find VEB by SEID
>>>>> + * @pf: pointer to a PF
>>>>> + **/
>>>>> +static inline struct i40e_veb *
>>>>> +i40e_veb_get_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;
>>>>> +}
>>>> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
>>>
>>> I'd rather use i40e_pf_ prefix...
>>>
>>> What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?
>>
>> Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"
> 
> OK, got it... Will repost v2 with this change + "too many also..." issue ;-)

Thanks

> 
> Btw. what about the last patch?

Reviewed :)

> 
> Ivan
> 

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

* Re: [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
  2023-11-16 14:21           ` Wojciech Drewek
@ 2023-11-16 14:46             ` Ivan Vecera
  0 siblings, 0 replies; 16+ messages in thread
From: Ivan Vecera @ 2023-11-16 14:46 UTC (permalink / raw)
  To: Wojciech Drewek, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt


On 16. 11. 23 15:21, Wojciech Drewek wrote:
>>> Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"
>> OK, got it... Will repost v2 with this change + "too many also..." issue 😉
> Thanks
> 
>> Btw. what about the last patch?
> Reviewed 🙂

Thanks, Wojciech! But I already submitted v2 of the series...but without 
your 'Reviewed-by:' tag in patch 5. Could you please "review" the v2 of 
patch 5 again?

Thanks,
Ivan


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

* [PATCH iwl-next 4/5] i40e: Fix broken support for floating VEBs
  2023-11-13 12:58 [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling Ivan Vecera
                   ` (2 preceding siblings ...)
  2023-11-13 12:58 ` [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them Ivan Vecera
@ 2023-11-13 12:58 ` Ivan Vecera
  2023-11-14 13:21   ` Wojciech Drewek
  2023-11-13 12:58 ` [PATCH iwl-next 5/5] i40e: Remove VEB recursion Ivan Vecera
  4 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-13 12:58 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Wojciech Drewek, Simon Horman, mschmidt

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 also 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

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../net/ethernet/intel/i40e/i40e_debugfs.c    | 29 +++---
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 89 ++++++++++---------
 2 files changed, 68 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index d26348db125f..b2e3bde8ae1d 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_veb_get_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_vsi_get_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 e7be145f298a..6ae1206e1c1c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -10379,41 +10379,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) {
@@ -14716,29 +14723,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);
@@ -14756,8 +14763,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 */
@@ -14789,9 +14796,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;
 }
@@ -14817,7 +14826,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;
+	struct i40e_vsi *vsi = NULL;
 	int veb_idx;
 	int ret;
 
-- 
2.41.0


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

* Re: [PATCH iwl-next 4/5] i40e: Fix broken support for floating VEBs
  2023-11-13 12:58 ` [PATCH iwl-next 4/5] i40e: Fix broken support for floating VEBs Ivan Vecera
@ 2023-11-14 13:21   ` Wojciech Drewek
  0 siblings, 0 replies; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-14 13:21 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 13.11.2023 13:58, Ivan Vecera wrote:
> 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 also only for such VEBs as the floating ones are using

nit: Too many "also"

>    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
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

> ---
>  .../net/ethernet/intel/i40e/i40e_debugfs.c    | 29 +++---
>  drivers/net/ethernet/intel/i40e/i40e_main.c   | 89 ++++++++++---------
>  2 files changed, 68 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> index d26348db125f..b2e3bde8ae1d 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_veb_get_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_vsi_get_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 e7be145f298a..6ae1206e1c1c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -10379,41 +10379,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) {
> @@ -14716,29 +14723,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);
> @@ -14756,8 +14763,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 */
> @@ -14789,9 +14796,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;
>  }
> @@ -14817,7 +14826,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;
> +	struct i40e_vsi *vsi = NULL;
>  	int veb_idx;
>  	int ret;
>  

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

* [PATCH iwl-next 5/5] i40e: Remove VEB recursion
  2023-11-13 12:58 [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling Ivan Vecera
                   ` (3 preceding siblings ...)
  2023-11-13 12:58 ` [PATCH iwl-next 4/5] i40e: Fix broken support for floating VEBs Ivan Vecera
@ 2023-11-13 12:58 ` Ivan Vecera
  2023-11-16 14:20   ` Wojciech Drewek
  4 siblings, 1 reply; 16+ messages in thread
From: Ivan Vecera @ 2023-11-13 12:58 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Wojciech Drewek, Simon Horman, mschmidt

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

Signed-off-by: Ivan Vecera <ivecera@redhat.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 220b5ce31519..ae955d4f7bca 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 b2e3bde8ae1d..9dc52b2f0715 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_veb_get_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 6ae1206e1c1c..8bcca758e589 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9871,7 +9871,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;
@@ -9880,12 +9879,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);
@@ -10363,56 +10357,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))
@@ -10434,23 +10429,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;
 }
 
@@ -10990,31 +10974,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);
 			}
 		}
 	}
@@ -14138,9 +14120,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;
@@ -14204,27 +14186,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_veb_get_by_seid(pf, uplink_seid);
+	if (veb && veb->uplink_seid) {
+		n = 0;
+
+		/* 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++;
 
-	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;
+		/* 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;
 }
@@ -14738,14 +14721,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);
@@ -14825,8 +14805,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;
 
@@ -14848,14 +14828,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_veb_get_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);
@@ -14864,7 +14836,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 */
@@ -14935,7 +14906,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


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

* Re: [PATCH iwl-next 5/5] i40e: Remove VEB recursion
  2023-11-13 12:58 ` [PATCH iwl-next 5/5] i40e: Remove VEB recursion Ivan Vecera
@ 2023-11-16 14:20   ` Wojciech Drewek
  0 siblings, 0 replies; 16+ messages in thread
From: Wojciech Drewek @ 2023-11-16 14:20 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, intel-wired-lan, linux-kernel,
	Jacob Keller, Simon Horman, mschmidt



On 13.11.2023 13:58, Ivan Vecera wrote:
> 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
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@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 220b5ce31519..ae955d4f7bca 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 b2e3bde8ae1d..9dc52b2f0715 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_veb_get_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 6ae1206e1c1c..8bcca758e589 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -9871,7 +9871,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;
> @@ -9880,12 +9879,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);
> @@ -10363,56 +10357,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))
> @@ -10434,23 +10429,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;
>  }
>  
> @@ -10990,31 +10974,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);
>  			}
>  		}
>  	}
> @@ -14138,9 +14120,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;
> @@ -14204,27 +14186,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_veb_get_by_seid(pf, uplink_seid);
> +	if (veb && veb->uplink_seid) {
> +		n = 0;
> +
> +		/* 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++;
>  
> -	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;
> +		/* 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;
>  }
> @@ -14738,14 +14721,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);
> @@ -14825,8 +14805,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;
>  
> @@ -14848,14 +14828,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_veb_get_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);
> @@ -14864,7 +14836,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 */
> @@ -14935,7 +14906,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)

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

end of thread, other threads:[~2023-11-16 14:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-13 12:58 [PATCH iwl-next 0/5] i40e: Simplify VSI and VEB handling Ivan Vecera
2023-11-13 12:58 ` [PATCH iwl-next 1/5] i40e: Use existing helper to find flow director VSI Ivan Vecera
2023-11-13 13:09   ` Wojciech Drewek
2023-11-13 12:58 ` [PATCH iwl-next 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs Ivan Vecera
2023-11-13 13:21   ` Wojciech Drewek
2023-11-13 12:58 ` [PATCH iwl-next 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them Ivan Vecera
2023-11-13 13:27   ` Wojciech Drewek
2023-11-15 17:01     ` Ivan Vecera
2023-11-16 12:37       ` Wojciech Drewek
2023-11-16 13:59         ` Ivan Vecera
2023-11-16 14:21           ` Wojciech Drewek
2023-11-16 14:46             ` Ivan Vecera
2023-11-13 12:58 ` [PATCH iwl-next 4/5] i40e: Fix broken support for floating VEBs Ivan Vecera
2023-11-14 13:21   ` Wojciech Drewek
2023-11-13 12:58 ` [PATCH iwl-next 5/5] i40e: Remove VEB recursion Ivan Vecera
2023-11-16 14:20   ` Wojciech Drewek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome