Commit f09cbb6c authored by Ivan Vecera's avatar Ivan Vecera Committed by Tony Nguyen
Browse files

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: default avatarWojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: default avatarIvan Vecera <ivecera@redhat.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: default avatarSimon Horman <horms@kernel.org>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent 08cdde31
Loading
Loading
Loading
Loading
+0 −1
Original line number Diff line number Diff line
@@ -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 */
+3 −5
Original line number Diff line number Diff line
@@ -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);
+73 −103
Original line number Diff line number Diff line
@@ -9873,7 +9873,6 @@ static void i40e_vsi_link_event(struct i40e_vsi *vsi, bool link_up)
 **/
static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
{
	struct i40e_veb *veb_it;
	struct i40e_vsi *vsi;
	struct i40e_pf *pf;
	int i;
@@ -9882,12 +9881,7 @@ static void i40e_veb_link_event(struct i40e_veb *veb, bool link_up)
		return;
	pf = veb->pf;

	/* depth first... */
	i40e_pf_for_each_veb(pf, i, veb_it)
		if (veb_it->uplink_seid == veb->seid)
			i40e_veb_link_event(veb_it, link_up);

	/* ... now the local VSIs */
	/* Send link event to contained VSIs */
	i40e_pf_for_each_vsi(pf, i, vsi)
		if (vsi->uplink_seid == veb->seid)
			i40e_vsi_link_event(vsi, link_up);
@@ -10356,56 +10350,57 @@ static void i40e_config_bridge_mode(struct i40e_veb *veb)
}

/**
 * i40e_reconstitute_veb - rebuild the VEB and anything connected to it
 * i40e_reconstitute_veb - rebuild the VEB and VSIs connected to it
 * @veb: pointer to the VEB instance
 *
 * This is a recursive function that first builds the attached VSIs then
 * recurses in to build the next layer of VEB.  We track the connections
 * through our own index numbers because the seid's from the HW could
 * change across the reset.
 * This is a function that builds the attached VSIs. We track the connections
 * through our own index numbers because the seid's from the HW could change
 * across the reset.
 **/
static int i40e_reconstitute_veb(struct i40e_veb *veb)
{
	struct i40e_vsi *ctl_vsi = NULL;
	struct i40e_pf *pf = veb->pf;
	struct i40e_veb *veb_it;
	struct i40e_vsi *vsi;
	int v, ret;

	if (veb->uplink_seid) {
		/* Look for VSI that owns this VEB, temporarily attached to base VEB */
		i40e_pf_for_each_vsi(pf, v, vsi)
			if (vsi->veb_idx == veb->idx &&
			    vsi->flags & I40E_VSI_FLAG_VEB_OWNER) {
				ctl_vsi = vsi;
				break;
	/* As we do not maintain PV (port virtualizer) switch element then
	 * there can be only one non-floating VEB that have uplink to MAC SEID
	 * and its control VSI is the main one.
	 */
	if (WARN_ON(veb->uplink_seid && veb->uplink_seid != pf->mac_seid)) {
		dev_err(&pf->pdev->dev,
			"Invalid uplink SEID for VEB %d\n", veb->idx);
		return -ENOENT;
	}

		if (!ctl_vsi) {
			dev_info(&pf->pdev->dev,
				 "missing owner VSI for veb_idx %d\n",
				 veb->idx);
			ret = -ENOENT;
			goto end_reconstitute;
	if (veb->uplink_seid == pf->mac_seid) {
		/* Check that the LAN VSI has VEB owning flag set */
		ctl_vsi = pf->vsi[pf->lan_vsi];

		if (WARN_ON(ctl_vsi->veb_idx != veb->idx ||
			    !(ctl_vsi->flags & I40E_VSI_FLAG_VEB_OWNER))) {
			dev_err(&pf->pdev->dev,
				"Invalid control VSI for VEB %d\n", veb->idx);
			return -ENOENT;
		}
		if (ctl_vsi != pf->vsi[pf->lan_vsi])
			ctl_vsi->uplink_seid =
				pf->vsi[pf->lan_vsi]->uplink_seid;

		/* Add the control VSI to switch */
		ret = i40e_add_vsi(ctl_vsi);
		if (ret) {
			dev_info(&pf->pdev->dev,
				 "rebuild of veb_idx %d owner VSI failed: %d\n",
			dev_err(&pf->pdev->dev,
				"Rebuild of owner VSI for VEB %d failed: %d\n",
				veb->idx, ret);
			goto end_reconstitute;
			return ret;
		}

		i40e_vsi_reset_stats(ctl_vsi);
	}

	/* create the VEB in the switch and move the VSI onto the VEB */
	ret = i40e_add_veb(veb, ctl_vsi);
	if (ret)
		goto end_reconstitute;
		return ret;

	if (veb->uplink_seid) {
		if (test_bit(I40E_FLAG_VEB_MODE_ENA, pf->flags))
@@ -10427,23 +10422,12 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
				dev_info(&pf->pdev->dev,
					 "rebuild of vsi_idx %d failed: %d\n",
					 v, ret);
				goto end_reconstitute;
				return ret;
			}
			i40e_vsi_reset_stats(vsi);
		}
	}

	/* create any VEBs attached to this VEB - RECURSION */
	i40e_pf_for_each_veb(pf, v, veb_it) {
		if (veb_it->veb_idx == veb->idx) {
			veb_it->uplink_seid = veb->seid;
			ret = i40e_reconstitute_veb(veb_it);
			if (ret)
				break;
		}
	}

end_reconstitute:
	return ret;
}

@@ -10984,10 +10968,9 @@ 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;
@@ -11011,7 +10994,6 @@ 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 PF VSI\n");
@@ -14124,9 +14106,9 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
 **/
int i40e_vsi_release(struct i40e_vsi *vsi)
{
	struct i40e_veb *veb, *veb_it;
	struct i40e_mac_filter *f;
	struct hlist_node *h;
	struct i40e_veb *veb;
	struct i40e_pf *pf;
	u16 uplink_seid;
	int i, n, bkt;
@@ -14190,27 +14172,28 @@ int i40e_vsi_release(struct i40e_vsi *vsi)

	/* If this was the last thing on the VEB, except for the
	 * controlling VSI, remove the VEB, which puts the controlling
	 * VSI onto the next level down in the switch.
	 * VSI onto the uplink port.
	 *
	 * Well, okay, there's one more exception here: don't remove
	 * the orphan VEBs yet.  We'll wait for an explicit remove request
	 * the floating VEBs yet.  We'll wait for an explicit remove request
	 * from up the network stack.
	 */
	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++;      /* count the VSIs */
				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 (n == 0 && veb && veb->uplink_seid != 0)
		/* 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);
	}

	return 0;
}
@@ -14724,14 +14707,11 @@ void i40e_veb_release(struct i40e_veb *veb)
		return;
	}

	/* For regular VEB move the owner VSI to uplink VEB */
	/* For regular VEB move the owner VSI to uplink port */
	if (veb->uplink_seid) {
		vsi->flags &= ~I40E_VSI_FLAG_VEB_OWNER;
		vsi->uplink_seid = veb->uplink_seid;
		if (veb->uplink_seid == pf->mac_seid)
		vsi->veb_idx = I40E_NO_VEB;
		else
			vsi->veb_idx = veb->veb_idx;
	}

	i40e_aq_delete_element(&pf->hw, veb->seid, NULL);
@@ -14811,8 +14791,8 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
				u16 uplink_seid, u16 vsi_seid,
				u8 enabled_tc)
{
	struct i40e_veb *veb, *uplink_veb = NULL;
	struct i40e_vsi *vsi = NULL;
	struct i40e_veb *veb;
	int veb_idx;
	int ret;

@@ -14834,14 +14814,6 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
			return NULL;
		}
	}
	if (uplink_seid && uplink_seid != pf->mac_seid) {
		uplink_veb = i40e_pf_get_veb_by_seid(pf, uplink_seid);
		if (!uplink_veb) {
			dev_info(&pf->pdev->dev,
				 "uplink seid %d not found\n", uplink_seid);
			return NULL;
		}
	}

	/* get veb sw struct */
	veb_idx = i40e_veb_mem_alloc(pf);
@@ -14850,7 +14822,6 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags,
	veb = pf->veb[veb_idx];
	veb->flags = flags;
	veb->uplink_seid = uplink_seid;
	veb->veb_idx = (uplink_veb ? uplink_veb->idx : I40E_NO_VEB);
	veb->enabled_tc = (enabled_tc ? enabled_tc : 0x1);

	/* create the VEB in the switch */
@@ -14921,7 +14892,6 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
		pf->veb[pf->lan_veb]->seid = seid;
		pf->veb[pf->lan_veb]->uplink_seid = pf->mac_seid;
		pf->veb[pf->lan_veb]->pf = pf;
		pf->veb[pf->lan_veb]->veb_idx = I40E_NO_VEB;
		break;
	case I40E_SWITCH_ELEMENT_TYPE_VSI:
		if (num_reported != 1)