2023-11-24 15:04:12

by Ivan Vecera

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

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

Changelog:
v1->v2 - small correction in patch 4 description
- changed helper names in patch 3
v2->v3 - correct patch files (v2 was broken)
v3->v4 - added kdoc stuff
- fixed wrong check in i40e_ndo_bridge_getlink()
v4->v5 - fixed VSI/VEB interation macros

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

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

--
2.41.0


2023-11-24 15:04:42

by Ivan Vecera

[permalink] [raw]
Subject: [PATCH v5 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them

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

Reviewed-by: Wojciech Drewek <[email protected]>
Signed-off-by: Ivan Vecera <[email protected]>
---
drivers/net/ethernet/intel/i40e/i40e.h | 36 +++++++++
.../net/ethernet/intel/i40e/i40e_debugfs.c | 38 ++-------
drivers/net/ethernet/intel/i40e/i40e_main.c | 80 +++++++------------
3 files changed, 68 insertions(+), 86 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 08cea99150bc..000fd78cfa31 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1360,4 +1360,40 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)

struct device *i40e_hw_to_dev(struct i40e_hw *hw);

+/**
+ * i40e_pf_get_vsi_by_seid - find VSI by SEID
+ * @pf: pointer to a PF
+ * @seid: SEID of the VSI
+ **/
+static inline struct i40e_vsi *
+i40e_pf_get_vsi_by_seid(struct i40e_pf *pf, u16 seid)
+{
+ struct i40e_vsi *vsi;
+ int i;
+
+ i40e_pf_for_each_vsi(pf, i, vsi)
+ if (vsi->seid == seid)
+ return vsi;
+
+ return NULL;
+}
+
+/**
+ * i40e_pf_get_veb_by_seid - find VEB by SEID
+ * @pf: pointer to a PF
+ * @seid: SEID of the VSI
+ **/
+static inline struct i40e_veb *
+i40e_pf_get_veb_by_seid(struct i40e_pf *pf, u16 seid)
+{
+ struct i40e_veb *veb;
+ int i;
+
+ i40e_pf_for_each_veb(pf, i, veb)
+ if (veb->seid == seid)
+ return veb;
+
+ return NULL;
+}
+
#endif /* _I40E_H_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index b236b0f93202..990a60889eef 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -24,37 +24,13 @@ enum ring_type {
**/
static struct i40e_vsi *i40e_dbg_find_vsi(struct i40e_pf *pf, int seid)
{
- struct i40e_vsi *vsi;
- int i;
-
if (seid < 0) {
dev_info(&pf->pdev->dev, "%d: bad seid\n", seid);

return NULL;
}

- i40e_pf_for_each_vsi(pf, i, vsi)
- if (vsi->seid == seid)
- return vsi;
-
- return NULL;
-}
-
-/**
- * i40e_dbg_find_veb - searches for the veb with the given seid
- * @pf: the PF structure to search for the veb
- * @seid: seid of the veb it is searching for
- **/
-static struct i40e_veb *i40e_dbg_find_veb(struct i40e_pf *pf, int seid)
-{
- struct i40e_veb *veb;
- int i;
-
- i40e_pf_for_each_veb(pf, i, veb)
- if (veb->seid == seid)
- return veb;
-
- return NULL;
+ return i40e_pf_get_vsi_by_seid(pf, seid);
}

/**************************************************************
@@ -701,7 +677,7 @@ static void i40e_dbg_dump_veb_seid(struct i40e_pf *pf, int seid)
{
struct i40e_veb *veb;

- veb = i40e_dbg_find_veb(pf, seid);
+ veb = i40e_pf_get_veb_by_seid(pf, seid);
if (!veb) {
dev_info(&pf->pdev->dev, "can't find veb %d\n", seid);
return;
@@ -853,7 +829,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,

} else if (strncmp(cmd_buf, "add relay", 9) == 0) {
struct i40e_veb *veb;
- int uplink_seid, i;
+ int uplink_seid;

cnt = sscanf(&cmd_buf[9], "%i %i", &uplink_seid, &vsi_seid);
if (cnt != 2) {
@@ -875,12 +851,8 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
goto command_write_done;
}

- i40e_pf_for_each_veb(pf, i, veb)
- if (veb->seid == uplink_seid)
- break;
-
- if (i >= I40E_MAX_VEB && uplink_seid != 0 &&
- uplink_seid != pf->mac_seid) {
+ veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
+ if (!veb && uplink_seid != 0 && uplink_seid != pf->mac_seid) {
dev_info(&pf->pdev->dev,
"add relay: relay uplink %d not found\n",
uplink_seid);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3a973a92779f..245de0d41f45 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_pf_get_veb_by_seid(pf, vsi->uplink_seid);

br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
if (!br_spec)
@@ -13196,18 +13192,15 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
struct i40e_netdev_priv *np = netdev_priv(dev);
struct i40e_vsi *vsi = np->vsi;
struct i40e_pf *pf = vsi->back;
- struct i40e_veb *veb = NULL;
- int i;
+ struct i40e_veb *veb;

/* Only for PF VSI for now */
if (vsi->seid != pf->vsi[pf->lan_vsi]->seid)
return -EOPNOTSUPP;

/* Find the HW bridge for the PF VSI */
- i40e_pf_for_each_veb(pf, i, veb)
- if (veb->seid == vsi->uplink_seid)
- break;
- if (i == I40E_MAX_VEB)
+ veb = i40e_pf_get_veb_by_seid(pf, vsi->uplink_seid);
+ if (!veb)
return 0;

return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
@@ -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_pf_get_veb_by_seid(pf, uplink_seid);
if (!veb && uplink_seid != pf->mac_seid) {
- i40e_pf_for_each_vsi(pf, i, vsi)
- if (vsi->seid == uplink_seid)
- break;
-
- if (i == pf->num_alloc_vsi) {
+ vsi = i40e_pf_get_vsi_by_seid(pf, uplink_seid);
+ if (!vsi) {
dev_info(&pf->pdev->dev, "no such uplink_seid %d\n",
uplink_seid);
return NULL;
@@ -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_pf_get_veb_by_seid(pf, vsi->uplink_seid);
+ if (!veb) {
dev_info(&pf->pdev->dev, "couldn't add VEB\n");
return NULL;
}
@@ -14834,8 +14817,8 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
u8 enabled_tc)
{
struct i40e_veb *veb, *uplink_veb = NULL;
- struct i40e_vsi *vsi;
- int vsi_idx, veb_idx;
+ struct i40e_vsi *vsi = NULL;
+ int veb_idx;
int ret;

/* if one seid is 0, the other must be 0 to create a floating relay */
@@ -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_pf_get_vsi_by_seid(pf, vsi_seid);
+ if (!vsi) {
+ dev_err(&pf->pdev->dev, "vsi seid %d not found\n",
+ vsi_seid);
+ return NULL;
+ }
}
-
if (uplink_seid && uplink_seid != pf->mac_seid) {
- i40e_pf_for_each_veb(pf, veb_idx, veb) {
- if (veb->seid == uplink_seid) {
- uplink_veb = veb;
- break;
- }
- }
+ uplink_veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
if (!uplink_veb) {
dev_info(&pf->pdev->dev,
"uplink seid %d not found\n", uplink_seid);
@@ -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_pf_get_veb_by_seid(pf, seid);
+ if (veb) {
+ pf->lan_veb = veb->idx;
+ } else {
v = i40e_veb_mem_alloc(pf);
if (v < 0)
break;
--
2.41.0

2023-11-24 15:04:46

by Ivan Vecera

[permalink] [raw]
Subject: [PATCH v5 1/5] i40e: Use existing helper to find flow director VSI

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

Reviewed-by: Wojciech Drewek <[email protected]>
Signed-off-by: Ivan Vecera <[email protected]>
---
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

2023-11-24 15:04:57

by Ivan Vecera

[permalink] [raw]
Subject: [PATCH v5 4/5] i40e: Fix broken support for floating VEBs

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

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

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

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

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

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

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

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

Reviewed-by: Wojciech Drewek <[email protected]>
Signed-off-by: Ivan Vecera <[email protected]>
---
.../net/ethernet/intel/i40e/i40e_debugfs.c | 29 ++++---
drivers/net/ethernet/intel/i40e/i40e_main.c | 87 ++++++++++---------
2 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 990a60889eef..921a97d5479e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -829,10 +829,14 @@ static ssize_t i40e_dbg_command_write(struct file *filp,

} else if (strncmp(cmd_buf, "add relay", 9) == 0) {
struct i40e_veb *veb;
+ u8 enabled_tc = 0x1;
int uplink_seid;

cnt = sscanf(&cmd_buf[9], "%i %i", &uplink_seid, &vsi_seid);
- if (cnt != 2) {
+ if (cnt == 0) {
+ uplink_seid = 0;
+ vsi_seid = 0;
+ } else if (cnt != 2) {
dev_info(&pf->pdev->dev,
"add relay: bad command string, cnt=%d\n",
cnt);
@@ -844,23 +848,28 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
goto command_write_done;
}

- vsi = i40e_dbg_find_vsi(pf, vsi_seid);
- if (!vsi) {
- dev_info(&pf->pdev->dev,
- "add relay: VSI %d not found\n", vsi_seid);
- goto command_write_done;
- }
-
veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
if (!veb && uplink_seid != 0 && uplink_seid != pf->mac_seid) {
dev_info(&pf->pdev->dev,
"add relay: relay uplink %d not found\n",
uplink_seid);
goto command_write_done;
+ } else if (uplink_seid) {
+ vsi = i40e_pf_get_vsi_by_seid(pf, vsi_seid);
+ if (!vsi) {
+ dev_info(&pf->pdev->dev,
+ "add relay: VSI %d not found\n",
+ vsi_seid);
+ goto command_write_done;
+ }
+ enabled_tc = vsi->tc_config.enabled_tc;
+ } else if (vsi_seid) {
+ dev_info(&pf->pdev->dev,
+ "add relay: VSI must be 0 for floating relay\n");
+ goto command_write_done;
}

- veb = i40e_veb_setup(pf, 0, uplink_seid, vsi_seid,
- vsi->tc_config.enabled_tc);
+ veb = i40e_veb_setup(pf, 0, uplink_seid, vsi_seid, enabled_tc);
if (veb)
dev_info(&pf->pdev->dev, "added relay %d\n", veb->seid);
else
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 245de0d41f45..05b79f590100 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;
}
--
2.41.0

2023-11-24 15:05:13

by Ivan Vecera

[permalink] [raw]
Subject: [PATCH v5 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs

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

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

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

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

Reviewed-by: Wojciech Drewek <[email protected]>
Signed-off-by: Ivan Vecera <[email protected]>
---
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..08cea99150bc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -686,6 +686,54 @@ struct i40e_pf {
struct list_head ddp_old_prof;
};

+/**
+ * __i40e_pf_next_vsi - get next valid VSI
+ * @pf: pointer to the PF struct
+ * @idx: pointer to start position number
+ *
+ * Find and return next non-NULL VSI pointer in pf->vsi array and
+ * updates idx position. Returns NULL if no VSI is found.
+ **/
+static __always_inline struct i40e_vsi *
+__i40e_pf_next_vsi(struct i40e_pf *pf, int *idx)
+{
+ while (*idx < pf->num_alloc_vsi) {
+ if (pf->vsi[*idx])
+ return pf->vsi[*idx];
+ (*idx)++;
+ }
+ return NULL;
+}
+
+#define i40e_pf_for_each_vsi(_pf, _i, _vsi) \
+ for (_i = 0, _vsi = __i40e_pf_next_vsi(_pf, &_i); \
+ _vsi; \
+ _i++, _vsi = __i40e_pf_next_vsi(_pf, &_i))
+
+/**
+ * __i40e_pf_next_veb - get next valid VEB
+ * @pf: pointer to the PF struct
+ * @idx: pointer to start position number
+ *
+ * Find and return next non-NULL VEB pointer in pf->veb array and
+ * updates idx position. Returns NULL if no VEB is found.
+ **/
+static __always_inline struct i40e_veb *
+__i40e_pf_next_veb(struct i40e_pf *pf, int *idx)
+{
+ while (*idx < I40E_MAX_VEB) {
+ if (pf->veb[*idx])
+ return pf->veb[*idx];
+ (*idx)++;
+ }
+ return NULL;
+}
+
+#define i40e_pf_for_each_veb(_pf, _i, _veb) \
+ for (_i = 0, _veb = __i40e_pf_next_veb(_pf, &_i); \
+ _veb; \
+ _i++, _veb = __i40e_pf_next_veb(_pf, &_i))
+
/**
* i40e_mac_to_hkey - Convert a 6-byte MAC Address to a u64 hash key
* @macaddr: the MAC Address as the base key
@@ -1120,14 +1168,12 @@ struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id);
static inline struct i40e_vsi *
i40e_find_vsi_by_type(struct i40e_pf *pf, u16 type)
{
+ struct i40e_vsi *vsi;
int i;

- for (i = 0; i < pf->num_alloc_vsi; i++) {
- struct i40e_vsi *vsi = pf->vsi[i];
-
- if (vsi && vsi->type == type)
+ i40e_pf_for_each_vsi(pf, i, vsi)
+ if (vsi->type == type)
return vsi;
- }

return NULL;
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c b/drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c
index 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..3a973a92779f 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(vsi);
+ 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

2023-11-24 15:05:18

by Ivan Vecera

[permalink] [raw]
Subject: [PATCH v5 5/5] i40e: Remove VEB recursion

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

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

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

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

Remove this logic from the driver code this way:

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

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

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

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

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

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

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

Reviewed-by: Wojciech Drewek <[email protected]>
Signed-off-by: Ivan Vecera <[email protected]>
---
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 000fd78cfa31..6f5ff6285c7f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -783,7 +783,6 @@ struct i40e_new_mac_filter {
struct i40e_veb {
struct i40e_pf *pf;
u16 idx;
- u16 veb_idx; /* index of VEB parent */
u16 seid;
u16 uplink_seid;
u16 stats_idx; /* index of VEB parent */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 921a97d5479e..f9ba45f596c9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -683,9 +683,8 @@ static void i40e_dbg_dump_veb_seid(struct i40e_pf *pf, int seid)
return;
}
dev_info(&pf->pdev->dev,
- "veb idx=%d,%d stats_ic=%d seid=%d uplink=%d mode=%s\n",
- veb->idx, veb->veb_idx, veb->stats_idx, veb->seid,
- veb->uplink_seid,
+ "veb idx=%d stats_ic=%d seid=%d uplink=%d mode=%s\n",
+ veb->idx, veb->stats_idx, veb->seid, veb->uplink_seid,
veb->bridge_mode == BRIDGE_MODE_VEPA ? "VEPA" : "VEB");
i40e_dbg_dump_eth_stats(pf, &veb->stats);
}
@@ -848,8 +847,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
goto command_write_done;
}

- veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
- if (!veb && uplink_seid != 0 && uplink_seid != pf->mac_seid) {
+ if (uplink_seid != 0 && uplink_seid != pf->mac_seid) {
dev_info(&pf->pdev->dev,
"add relay: relay uplink %d not found\n",
uplink_seid);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 05b79f590100..7ee5cf5e893d 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_pf_get_veb_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_pf_get_veb_by_seid(pf, uplink_seid);
- if (!uplink_veb) {
- dev_info(&pf->pdev->dev,
- "uplink seid %d not found\n", uplink_seid);
- return NULL;
- }
- }

/* get veb sw struct */
veb_idx = i40e_veb_mem_alloc(pf);
@@ -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

2023-11-27 08:43:49

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] i40e: Fix broken support for floating VEBs

Hi Ivan,

kernel test robot noticed the following build warnings:

url: https://github.com/intel-lab-lkp/linux/commits/Ivan-Vecera/i40e-Use-existing-helper-to-find-flow-director-VSI/20231124-230616
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link: https://lore.kernel.org/r/20231124150343.81520-5-ivecera%40redhat.com
patch subject: [PATCH v5 4/5] i40e: Fix broken support for floating VEBs
config: x86_64-randconfig-161-20231127 (https://download.01.org/0day-ci/archive/20231127/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce: (https://download.01.org/0day-ci/archive/20231127/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

New smatch warnings:
drivers/net/ethernet/intel/i40e/i40e_main.c:14743 i40e_veb_release() error: potentially dereferencing uninitialized 'vsi'.

Old smatch warnings:
[ low confidence ]
drivers/net/ethernet/intel/i40e/i40e_main.c:5966 i40e_set_bw_limit() warn: error code type promoted to positive: 'speed'
drivers/net/ethernet/intel/i40e/i40e_main.c:13476 i40e_queue_pair_toggle_rings() warn: missing error code? 'ret'
drivers/net/ethernet/intel/i40e/i40e_main.c:14272 i40e_vsi_setup_vectors() warn: missing error code? 'ret'
drivers/net/ethernet/intel/i40e/i40e_main.c:15566 i40e_init_recovery_mode() warn: 'vsi->netdev' from register_netdev() not released on lines: 15566.

vim +/vsi +14743 drivers/net/ethernet/intel/i40e/i40e_main.c

41c445ff0f482bb Jesse Brandeburg 2013-09-11 14715 void i40e_veb_release(struct i40e_veb *veb)
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14716 {
0aab77d67d37d09 Ivan Vecera 2023-11-24 14717 struct i40e_vsi *vsi, *vsi_it;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14718 struct i40e_pf *pf;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14719 int i, n = 0;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14720
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14721 pf = veb->pf;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14722
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14723 /* find the remaining VSI and check for extras */
0aab77d67d37d09 Ivan Vecera 2023-11-24 14724 i40e_pf_for_each_vsi(pf, i, vsi_it)
0aab77d67d37d09 Ivan Vecera 2023-11-24 14725 if (vsi_it->uplink_seid == veb->seid) {
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14726 if (vsi_it->flags & I40E_VSI_FLAG_VEB_OWNER)
0aab77d67d37d09 Ivan Vecera 2023-11-24 14727 vsi = vsi_it;

Do we always find a vsi? Presumably, yes, but it's not obvious just
from reading this function.

41c445ff0f482bb Jesse Brandeburg 2013-09-11 14728 n++;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14729 }
0aab77d67d37d09 Ivan Vecera 2023-11-24 14730
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14731 /* Floating VEB has to be empty and regular one must have
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14732 * single owner VSI.
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14733 */
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14734 if ((veb->uplink_seid && n != 1) || (!veb->uplink_seid && n != 0)) {
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14735 dev_info(&pf->pdev->dev,
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14736 "can't remove VEB %d with %d VSIs left\n",
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14737 veb->seid, n);
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14738 return;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14739 }
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14740
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14741 /* For regular VEB move the owner VSI to uplink VEB */
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14742 if (veb->uplink_seid) {
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 @14743 vsi->flags &= ~I40E_VSI_FLAG_VEB_OWNER;
^^^^^^^^^^

41c445ff0f482bb Jesse Brandeburg 2013-09-11 14744 vsi->uplink_seid = veb->uplink_seid;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14745 if (veb->uplink_seid == pf->mac_seid)
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14746 vsi->veb_idx = I40E_NO_VEB;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14747 else
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14748 vsi->veb_idx = veb->veb_idx;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14749 }
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14750
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14751 i40e_aq_delete_element(&pf->hw, veb->seid, NULL);
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14752 i40e_veb_clear(veb);
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14753 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-29 08:39:38

by Ivan Vecera

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] i40e: Fix broken support for floating VEBs

On 27. 11. 23 9:43, Dan Carpenter wrote:
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14720
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14721 pf = veb->pf;
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14722
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14723 /* find the
> remaining VSI and check for extras */
> 0aab77d67d37d09 Ivan Vecera 2023-11-24 14724
> i40e_pf_for_each_vsi(pf, i, vsi_it)
> 0aab77d67d37d09 Ivan Vecera 2023-11-24 14725 if
> (vsi_it->uplink_seid == veb->seid) {
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14726 if
> (vsi_it->flags & I40E_VSI_FLAG_VEB_OWNER)
> 0aab77d67d37d09 Ivan Vecera 2023-11-24 14727 vsi = vsi_it;
>
> Do we always find a vsi? Presumably, yes, but it's not obvious just
> from reading this function.

Yes, if the VEB has uplink (veb->uplink_seid != 0) then it has to have a
downlink VSI that owns it (vsi->flags has I40E_VSI_FLAG_VEB_OWNER set)

Ivan

> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14728 n++;
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14729 }
> 0aab77d67d37d09 Ivan Vecera 2023-11-24 14730
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14731 /* Floating VEB has
> to be empty and regular one must have
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14732 * single owner
> VSI.
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14733 */
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14734 if
> ((veb->uplink_seid && n != 1) || (!veb->uplink_seid
> && n != 0)) {
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14735
> dev_info(&pf->pdev->dev,
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14736 "can't remove
> VEB %d with %d VSIs left\n",
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14737 veb->seid,
> n);
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14738 return;
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14739 }
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14740
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14741 /* For regular VEB
> move the owner VSI to uplink VEB */
> 41c445ff0f482bb Jesse Brandeburg 2013-09-11 14742 if
> (veb->uplink_seid) {
> 93a1bc91a1ccc5a Ivan Vecera 2023-11-24 @14743 vsi->flags
> &= ~I40E_VSI_FLAG_VEB_OWNER;

2023-11-30 05:23:43

by Pucha, HimasekharX Reddy

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v5 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Ivan Vecera
> Sent: Friday, November 24, 2023 8:34 PM
> To: [email protected]
> Cc: Drewek, Wojciech <[email protected]>; [email protected]; Brandeburg, Jesse <[email protected]>; [email protected]; Eric Dumazet <[email protected]>; Nguyen, Anthony L <[email protected]>; Simon Horman <[email protected]>; Keller, Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; David S . Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v5 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs
>
> Introduce i40e_for_each_vsi() and i40e_for_each_veb() helper
> macros and use them to iterate relevant arrays.
>
> Replace pattern:
> for (i = 0; i < pf->num_alloc_vsi; i++)
> by:
> i40e_for_each_vsi(pf, i, vsi)
>
> and pattern:
> for (i = 0; i < I40E_MAX_VEB; i++)
> by
> i40e_for_each_veb(pf, i, veb)
>
> These macros also check if array item pf->vsi[i] or pf->veb[i]
> are not NULL and skip such items so we can remove redundant
> checks from loop bodies.
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> 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(-)
>

Tested-by: Pucha Himasekhar Reddy <[email protected]> (A Contingent worker at Intel)

2023-11-30 05:23:44

by Pucha, HimasekharX Reddy

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v5 1/5] i40e: Use existing helper to find flow director VSI

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Ivan Vecera
> Sent: Friday, November 24, 2023 8:34 PM
> To: [email protected]
> Cc: Drewek, Wojciech <[email protected]>; [email protected]; Brandeburg, Jesse <[email protected]>; [email protected]; Eric Dumazet <[email protected]>; Nguyen, Anthony L <[email protected]>; Simon Horman <[email protected]>; Keller, Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; David S . Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v5 1/5] i40e: Use existing helper to find flow director VSI
>
> Use existing i40e_find_vsi_by_type() to find a VSI
> associated with flow director.
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <[email protected]> (A Contingent worker at Intel)

2023-11-30 05:24:16

by Pucha, HimasekharX Reddy

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v5 4/5] i40e: Fix broken support for floating VEBs

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Ivan Vecera
> Sent: Friday, November 24, 2023 8:34 PM
> To: [email protected]
> Cc: Drewek, Wojciech <[email protected]>; [email protected]; Brandeburg, Jesse <[email protected]>; [email protected]; Eric Dumazet <[email protected]>; Nguyen, Anthony L <[email protected]>; Simon Horman <[email protected]>; Keller, Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; David S . Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v5 4/5] i40e: Fix broken support for floating VEBs
>
> Although the i40e supports so-called floating VEB (VEB without
> an uplink connection to external network), this support is
> broken. This functionality is currently unused (except debugfs)
> but it will be used by subsequent series for switchdev mode
> slow-path. Fix this by following:
>
> 1) Handle correctly floating VEB (VEB with uplink_seid == 0)
> in i40e_reconstitute_veb() and look for owner VSI and
> create it only for non-floating VEBs and also set bridge
> mode only for such VEBs as the floating ones are using
> always VEB mode.
> 2) Handle correctly floating VEB in i40e_veb_release() and
> disallow its release when there are some VSIs. This is
> different from regular VEB that have owner VSI that is
> connected to VEB's uplink after VEB deletion by FW.
> 3) Fix i40e_add_veb() to handle 'vsi' that is NULL for floating
> VEBs. For floating VEB use 0 for downlink SEID and 'true'
> for 'default_port' parameters as per datasheet.
> 4) Fix 'add relay' command in i40e_dbg_command_write() to allow
> to create floating VEB by 'add relay 0 0' or 'add relay'
>
> Tested using debugfs:
> 1) Initial state
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 173.701286] i40e 0000:02:00.0: header: 3 reported 3 total
> [ 173.706701] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 173.713241] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 173.719507] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 2) Add floating VEB
> [root@host net-next]# CMD="/sys/kernel/debug/i40e/0000:02:00.0/command"
> [root@host net-next]# echo add relay > $CMD
> [root@host net-next]# dmesg -c
> [ 245.551720] i40e 0000:02:00.0: added relay 162
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 276.984371] i40e 0000:02:00.0: header: 4 reported 4 total
> [ 276.989779] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 276.996302] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 277.002569] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
> [ 277.009091] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
>
> 3) Add VMDQ2 VSI to this new VEB
> [root@host net-next]# echo add vsi 162 > $CMD
> [root@host net-next]# dmesg -c
> [ 332.314030] i40e 0000:02:00.0: added VSI 394 to relay 162
> [ 332.337486] enp2s0f0np0v0: NIC Link is Up, 40 Gbps Full Duplex, Flow Control: None
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 387.284490] i40e 0000:02:00.0: header: 5 reported 5 total
> [ 387.289904] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
> [ 387.296446] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
> [ 387.302708] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 387.309234] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 387.315500] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 4) Try to delete the VEB
> [root@host net-next]# echo del relay 162 > $CMD
> [root@host net-next]# dmesg -c
> [ 428.749297] i40e 0000:02:00.0: deleting relay 162
> [ 428.754011] i40e 0000:02:00.0: can't remove VEB 162 with 1 VSIs left
>
> 5) Do PF reset and check switch status after rebuild
> [root@host net-next]# echo pfr > $CMD
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 738.056172] i40e 0000:02:00.0: header: 5 reported 5 total
> [ 738.061577] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
> [ 738.068104] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
> [ 738.074367] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 738.080892] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 738.087160] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 6) Delete VSI and delete VEB
> [root@host net-next]# echo del vsi 394 > $CMD
> [root@host net-next]# echo del relay 162 > $CMD
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 1233.081126] i40e 0000:02:00.0: deleting VSI 394
> [ 1239.345139] i40e 0000:02:00.0: deleting relay 162
> [ 1244.886920] i40e 0000:02:00.0: header: 3 reported 3 total
> [ 1244.892328] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 1244.898853] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 1244.905119] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> .../net/ethernet/intel/i40e/i40e_debugfs.c | 29 ++++---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 87 ++++++++++---------
> 2 files changed, 67 insertions(+), 49 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <[email protected]> (A Contingent worker at Intel)

2023-11-30 05:24:20

by Pucha, HimasekharX Reddy

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v5 5/5] i40e: Remove VEB recursion

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Ivan Vecera
> Sent: Friday, November 24, 2023 8:34 PM
> To: [email protected]
> Cc: Drewek, Wojciech <[email protected]>; [email protected]; Brandeburg, Jesse <[email protected]>; [email protected]; Eric Dumazet <[email protected]>; Nguyen, Anthony L <[email protected]>; Simon Horman <[email protected]>; Keller, Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; David S . Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v5 5/5] i40e: Remove VEB recursion
>
> The VEB (virtual embedded switch) as a switch element can be
> connected according datasheet though its uplink to:
> - Physical port
> - Port Virtualizer (not used directly by i40e driver but can
> be present in MFP mode where the physical port is shared
> between PFs)
> - No uplink (aka floating VEB)
>
> But VEB uplink cannot be connected to another VEB and any attempt
> to do so results in:
>
> "i40e 0000:02:00.0: couldn't add VEB, err -EIO aq_err I40E_AQ_RC_ENOENT"
>
> that indicates "the uplink SEID does not point to valid element".
>
> Remove this logic from the driver code this way:
>
> 1) For debugfs only allow to build floating VEB (uplink_seid == 0)
> or main VEB (uplink_seid == mac_seid)
> 2) Do not recurse in i40e_veb_link_event() as no VEB cannot have
> sub-VEBs
> 3) Ditto for i40e_veb_rebuild() + simplify the function as we know
> that the VEB for rebuild can be only the main LAN VEB or some
> of the floating VEBs
> 4) In i40e_rebuild() there is no need to check veb->uplink_seid
> as the possible ones are 0 and MAC SEID
> 5) In i40e_vsi_release() do not take into account VEBs whose
> uplink is another VEB as this is not possible
> 6) Remove veb_idx field from i40e_veb as a VEB cannot have
> sub-VEBs
>
> Tested using i40e debugfs interface:
> 1) Initial state
> [root@cnb-03 net-next]# CMD="/sys/kernel/debug/i40e/0000:02:00.0/command"
> [root@cnb-03 net-next]# echo dump switch > $CMD
> [root@cnb-03 net-next]# dmesg -c
> [ 98.440641] i40e 0000:02:00.0: header: 3 reported 3 total
> [ 98.446053] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 98.452593] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 98.458856] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 2) Add floating VEB
> [root@cnb-03 net-next]# echo add relay > $CMD
> [root@cnb-03 net-next]# dmesg -c
> [ 122.745630] i40e 0000:02:00.0: added relay 162
> [root@cnb-03 net-next]# echo dump switch > $CMD
> [root@cnb-03 net-next]# dmesg -c
> [ 136.650049] i40e 0000:02:00.0: header: 4 reported 4 total
> [ 136.655466] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 136.661994] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 136.668264] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
> [ 136.674787] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
>
> 3) Add VMDQ2 VSI to this new VEB
> [root@cnb-03 net-next]# dmesg -c
> [ 168.351763] i40e 0000:02:00.0: added VSI 394 to relay 162
> [ 168.374652] enp2s0f0np0v0: NIC Link is Up, 40 Gbps Full Duplex, Flow Control: None
> [root@cnb-03 net-next]# echo dump switch > $CMD
> [root@cnb-03 net-next]# dmesg -c
> [ 195.683204] i40e 0000:02:00.0: header: 5 reported 5 total
> [ 195.688611] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
> [ 195.695143] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
> [ 195.701410] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 195.707935] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 195.714201] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 4) Try to delete the VEB
> [root@cnb-03 net-next]# echo del relay 162 > $CMD
> [root@cnb-03 net-next]# dmesg -c
> [ 239.260901] i40e 0000:02:00.0: deleting relay 162
> [ 239.265621] i40e 0000:02:00.0: can't remove VEB 162 with 1 VSIs left
>
> 5) Do PF reset and check switch status after rebuild
> [root@cnb-03 net-next]# echo pfr > $CMD
> [root@cnb-03 net-next]# echo dump switch > $CMD
> [root@cnb-03 net-next]# dmesg -c
> ...
> [ 272.333655] i40e 0000:02:00.0: header: 5 reported 5 total
> [ 272.339066] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
> [ 272.345599] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
> [ 272.351862] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 272.358387] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 272.364654] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 6) Delete VSI and delete VEB
> [ 297.199116] i40e 0000:02:00.0: deleting VSI 394
> [ 299.807580] i40e 0000:02:00.0: deleting relay 162
> [ 309.767905] i40e 0000:02:00.0: header: 3 reported 3 total
> [ 309.773318] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 309.779845] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 309.786111] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> 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(-)
>

Tested-by: Pucha Himasekhar Reddy <[email protected]> (A Contingent worker at Intel)

2023-11-30 05:25:27

by Pucha, HimasekharX Reddy

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v5 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Ivan Vecera
> Sent: Friday, November 24, 2023 8:34 PM
> To: [email protected]
> Cc: Drewek, Wojciech <[email protected]>; [email protected]; Brandeburg, Jesse <[email protected]>; [email protected]; Eric Dumazet <[email protected]>; Nguyen, Anthony L <[email protected]>; Simon Horman <[email protected]>; Keller, Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; David S . Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v5 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them
>
> Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
> VEB or VSI by their SEID value and use these helpers to replace
> existing open-coded loops.
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 36 +++++++++
> .../net/ethernet/intel/i40e/i40e_debugfs.c | 38 ++-------
> drivers/net/ethernet/intel/i40e/i40e_main.c | 80 +++++++------------
> 3 files changed, 68 insertions(+), 86 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <[email protected]> (A Contingent worker at Intel)

2023-12-03 11:42:28

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] i40e: Introduce and use macros for iterating VSIs and VEBs

On Fri, Nov 24, 2023 at 04:03:40PM +0100, 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.
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>

Reviewed-by: Simon Horman <[email protected]>

2023-12-03 11:42:29

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them

On Fri, Nov 24, 2023 at 04:03:41PM +0100, 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.
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>

Reviewed-by: Simon Horman <[email protected]>

2023-12-03 11:42:43

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] i40e: Use existing helper to find flow director VSI

On Fri, Nov 24, 2023 at 04:03:39PM +0100, Ivan Vecera wrote:
> Use existing i40e_find_vsi_by_type() to find a VSI
> associated with flow director.
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>

Reviewed-by: Simon Horman <[email protected]>

2023-12-03 16:27:32

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] i40e: Fix broken support for floating VEBs

On Fri, Nov 24, 2023 at 04:03:42PM +0100, 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 only for such VEBs as the floating ones are using
> always VEB mode.
> 2) Handle correctly floating VEB in i40e_veb_release() and
> disallow its release when there are some VSIs. This is
> different from regular VEB that have owner VSI that is
> connected to VEB's uplink after VEB deletion by FW.
> 3) Fix i40e_add_veb() to handle 'vsi' that is NULL for floating
> VEBs. For floating VEB use 0 for downlink SEID and 'true'
> for 'default_port' parameters as per datasheet.
> 4) Fix 'add relay' command in i40e_dbg_command_write() to allow
> to create floating VEB by 'add relay 0 0' or 'add relay'
>
> Tested using debugfs:
> 1) Initial state
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 173.701286] i40e 0000:02:00.0: header: 3 reported 3 total
> [ 173.706701] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 173.713241] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 173.719507] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 2) Add floating VEB
> [root@host net-next]# CMD="/sys/kernel/debug/i40e/0000:02:00.0/command"
> [root@host net-next]# echo add relay > $CMD
> [root@host net-next]# dmesg -c
> [ 245.551720] i40e 0000:02:00.0: added relay 162
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 276.984371] i40e 0000:02:00.0: header: 4 reported 4 total
> [ 276.989779] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 276.996302] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 277.002569] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
> [ 277.009091] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
>
> 3) Add VMDQ2 VSI to this new VEB
> [root@host net-next]# echo add vsi 162 > $CMD
> [root@host net-next]# dmesg -c
> [ 332.314030] i40e 0000:02:00.0: added VSI 394 to relay 162
> [ 332.337486] enp2s0f0np0v0: NIC Link is Up, 40 Gbps Full Duplex, Flow Control: None
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 387.284490] i40e 0000:02:00.0: header: 5 reported 5 total
> [ 387.289904] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
> [ 387.296446] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
> [ 387.302708] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 387.309234] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 387.315500] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 4) Try to delete the VEB
> [root@host net-next]# echo del relay 162 > $CMD
> [root@host net-next]# dmesg -c
> [ 428.749297] i40e 0000:02:00.0: deleting relay 162
> [ 428.754011] i40e 0000:02:00.0: can't remove VEB 162 with 1 VSIs left
>
> 5) Do PF reset and check switch status after rebuild
> [root@host net-next]# echo pfr > $CMD
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 738.056172] i40e 0000:02:00.0: header: 5 reported 5 total
> [ 738.061577] i40e 0000:02:00.0: type=19 seid=394 uplink=162 downlink=16
> [ 738.068104] i40e 0000:02:00.0: type=17 seid=162 uplink=0 downlink=0
> [ 738.074367] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 738.080892] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 738.087160] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> 6) Delete VSI and delete VEB
> [root@host net-next]# echo del vsi 394 > $CMD
> [root@host net-next]# echo del relay 162 > $CMD
> [root@host net-next]# echo dump switch > $CMD
> [root@host net-next]# dmesg -c
> [ 1233.081126] i40e 0000:02:00.0: deleting VSI 394
> [ 1239.345139] i40e 0000:02:00.0: deleting relay 162
> [ 1244.886920] i40e 0000:02:00.0: header: 3 reported 3 total
> [ 1244.892328] i40e 0000:02:00.0: type=19 seid=392 uplink=160 downlink=16
> [ 1244.898853] i40e 0000:02:00.0: type=17 seid=160 uplink=2 downlink=0
> [ 1244.905119] i40e 0000:02:00.0: type=19 seid=390 uplink=160 downlink=16
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>

Reviewed-by: Simon Horman <[email protected]>

2023-12-03 16:27:32

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] i40e: Remove VEB recursion

On Fri, Nov 24, 2023 at 04:03:43PM +0100, 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
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Signed-off-by: Ivan Vecera <[email protected]>

Reviewed-by: Simon Horman <[email protected]>