ice_setup_tc() and dcbnl ops are called with rtnl held and then take
pf->tc_mutex. LLDP MIB change handler and ice_dcb_rebuild() (the latter
only on its error path, via ice_pf_dcb_cfg(..., false)) do the opposite:
take pf->tc_mutex and then rtnl. The MIB change handler runs from the
service task. ice_dcb_rebuild() runs from ice_rebuild() (reset, also PCI
AER resume), and from ice_link_event() in the service task, on a link
change while DCB is enabled. So a MIB change from the link partner, a reset
or a link change, concurrent with "tc qdisc add ... mqprio" or "dcb ets
set" may deadlock.
Take rtnl first in both functions. ice_dcb_rebuild() now holds rtnl also
on its success path, which only sends a few AQ commands. None of its
callers hold rtnl.
Lockdep splat, on the first MIB change that needs a DCB reconfiguration
after "dcb ets set" was used:
CPU0 CPU1
---- ----
lock(&pf->tc_mutex);
lock(rtnl_mutex);
lock(&pf->tc_mutex);
lock(rtnl_mutex);
Reproducer, with the FW LLDP agent in its default (willing) mode:
PF=eth2
ethtool --set-priv-flags $PF fw-lldp-agent off
dcb ets set dev $PF tc-tsa all:ets tc-bw all:0 0:100 prio-tc all:0
ethtool --set-priv-flags $PF fw-lldp-agent on
and then, on the link partner, send an ETS recommendation with a different
UP2TC mapping (python3 lldp-ets.py <partner-ifname>):
import socket, struct, sys
ifc = sys.argv[1]
mac = open(f"/sys/class/net/{ifc}/address").read().strip()
mac = bytes.fromhex(mac.replace(":", ""))
tlv = lambda t, v: struct.pack("!H", t << 9 | len(v)) + v
# not willing, prio 4 -> TC1, TC0/TC1 ETS 50/50
ets = bytes([0, 0, 0, 0x10, 0, 50, 50] + [0] * 6 + [2, 2] + [0] * 6)
lldp = (tlv(1, b"\x04" + mac) + tlv(2, b"\x03" + mac) +
tlv(3, b"\x00\x78") +
tlv(127, b"\x00\x80\xc2\x09" + ets) + # ETS Configuration
tlv(127, b"\x00\x80\xc2\x0a" + ets) + # ETS Recommendation
tlv(0, b""))
s = socket.socket(socket.AF_PACKET, socket.SOCK_RAW)
s.bind((ifc, 0))
s.send(bytes.fromhex("0180c200000e") + mac + b"\x88\xcc" + lldp)
Fixes: 242b5e068b25 ("ice: Fix DCB rebuild after reset")
Signed-off-by: Przemek Kitszel <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 0bc6dd375687..3034f461f513 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -537,6 +537,7 @@ void ice_dcb_rebuild(struct ice_pf *pf)
struct ice_dcbx_cfg *err_cfg;
int ret;
+ rtnl_lock();
mutex_lock(&pf->tc_mutex);
ret = ice_query_port_ets(pf->hw.port_info, &buf, sizeof(buf), NULL);
@@ -570,14 +571,16 @@ void ice_dcb_rebuild(struct ice_pf *pf)
}
mutex_unlock(&pf->tc_mutex);
+ rtnl_unlock();
return;
dcb_error:
dev_err(dev, "Disabling DCB until new settings occur\n");
err_cfg = kzalloc_obj(*err_cfg);
if (!err_cfg) {
mutex_unlock(&pf->tc_mutex);
+ rtnl_unlock();
return;
}
@@ -591,10 +594,11 @@ void ice_dcb_rebuild(struct ice_pf *pf)
* Suppress the Coverity warning with the following comment...
*/
/* coverity[check_return] */
- ice_pf_dcb_cfg(pf, err_cfg, false);
+ ice_pf_dcb_cfg(pf, err_cfg, true);
kfree(err_cfg);
mutex_unlock(&pf->tc_mutex);
+ rtnl_unlock();
}
/**
@@ -1055,6 +1059,7 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
}
/* That a DCB change has happened is now determined */
+ rtnl_lock();
mutex_lock(&pf->tc_mutex);
/* store the old configuration */
@@ -1105,25 +1110,23 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
pending_handled = true;
}
- rtnl_lock();
/* disable VSIs affected by DCB changes */
ice_dcb_ena_dis_vsi(pf, false, true);
ret = ice_query_port_ets(pi, &buf, sizeof(buf), NULL);
if (ret) {
dev_err(dev, "Query Port ETS failed\n");
- goto unlock_rtnl;
+ goto out;
}
/* changes in configuration update VSI */
ice_pf_dcb_recfg(pf, false);
/* enable previously downed VSIs */
ice_dcb_ena_dis_vsi(pf, true, true);
-unlock_rtnl:
- rtnl_unlock();
out:
mutex_unlock(&pf->tc_mutex);
+ rtnl_unlock();
/* Send Execute Pending MIB Change event if it is a Pending event */
if (!pending_handled)
--
2.54.0
