[PATCH iwl-net] ice: do not report success for a VF setting that was not applied

[Linkui Xiao <xiaolinkui@126.com>]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

__ice_set_vf_mac(), ice_set_vf_trust() and ice_set_vf_port_vlan() all
check that the VF is ready for configuration, take vf->cfg_lock and then
call ice_reset_vf() to push the new setting into hardware. The result of
ice_reset_vf() is not looked at, and it could not be: it returns 0 both
when the reset ran and when it bailed out because the VF is disabled.

The check runs before the lock is taken, so a reset can disable the VF
in between. ice_reset_all_vfs() raises ICE_VF_DIS in pf->state before it
walks the VF table, rebuilds every VSI and clears the bit again. An
ndo_set_vf_mac() that passed ice_check_vf_ready_for_cfg() just before
that then waits for cfg_lock, stores the new address in vf->dev_lan_addr
and vf->hw_lan_addr and calls ice_reset_vf(), which sees the VF
disabled, stops its queues and returns 0 without reprogramming anything.
__ice_set_vf_mac() reports success, so ip link and the devlink port
function now show a MAC address the hardware never got, and the switch
keeps enforcing the previous one until something else resets the VF.

ice_set_vf_trust() and ice_set_vf_port_vlan() have the same shape: the
port VLAN lands in vf->port_vlan_info and the trust flag in vf->trusted
before the reset is skipped.

Re-check ice_is_vf_disabled() once cfg_lock is held and fail with
-EINVAL, the same code ice_check_vf_ready_for_cfg() returns for a
disabled VF. The VF is being reset or removed at that point, so the
caller should retry rather than be told the setting is in place.
Changing what ice_reset_vf() returns on that path is not an option, too
many callers rely on it meaning "nothing to do".

The race was found by code inspection of the ndo VF handlers, it was not
reproduced at runtime and no error message was observed.

Fixes: e6ba5273d4ed ("ice: Fix race conditions between virtchnl handling and VF ndo ops")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 drivers/net/ethernet/intel/ice/ice_sriov.c | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index e04de0215596..de0631673149 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -1327,6 +1327,15 @@ int __ice_set_vf_mac(struct ice_pf *pf, u16 vf_id, const u8 *mac)
 
 	mutex_lock(&vf->cfg_lock);
 
+	/* The VF can have been disabled by a reset after the check above, and
+	 * ice_reset_vf() then skips the reprogram and still returns 0. Fail
+	 * here instead of reporting a MAC address that hardware never got.
+	 */
+	if (ice_is_vf_disabled(vf)) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	/* VF is notified of its new MAC via the PF's response to the
 	 * VIRTCHNL_OP_GET_VF_RESOURCES message after the VF has been reset
 	 */
@@ -1345,6 +1354,8 @@ int __ice_set_vf_mac(struct ice_pf *pf, u16 vf_id, const u8 *mac)
 	}
 
 	ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
+
+out_unlock:
 	mutex_unlock(&vf->cfg_lock);
 
 out_put_vf:
@@ -1401,6 +1412,14 @@ int ice_set_vf_trust(struct net_device *netdev, int vf_id, bool trusted)
 
 	mutex_lock(&vf->cfg_lock);
 
+	/* See __ice_set_vf_mac(), the VF can have been disabled after the
+	 * check above and ice_reset_vf() would skip the reprogram.
+	 */
+	if (ice_is_vf_disabled(vf)) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	while (!trusted && vf->num_mac_lldp)
 		ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false);
 
@@ -1409,6 +1428,7 @@ int ice_set_vf_trust(struct net_device *netdev, int vf_id, bool trusted)
 	dev_info(ice_pf_to_dev(pf), "VF %u is now %strusted\n",
 		 vf_id, trusted ? "" : "un");
 
+out_unlock:
 	mutex_unlock(&vf->cfg_lock);
 
 out_put_vf:
@@ -1721,6 +1741,14 @@ ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos,
 
 	mutex_lock(&vf->cfg_lock);
 
+	/* See __ice_set_vf_mac(), the VF can have been disabled after the
+	 * check above and ice_reset_vf() would skip the reprogram.
+	 */
+	if (ice_is_vf_disabled(vf)) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	vf->port_vlan_info = ICE_VLAN(local_vlan_proto, vlan_id, qos);
 	if (ice_vf_is_port_vlan_ena(vf))
 		dev_info(dev, "Setting VLAN %u, QoS %u, TPID 0x%04x on VF %d\n",
@@ -1729,6 +1757,8 @@ ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos,
 		dev_info(dev, "Clearing port VLAN on VF %d\n", vf_id);
 
 	ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
+
+out_unlock:
 	mutex_unlock(&vf->cfg_lock);
 
 out_put_vf:
-- 
2.25.1