Commit b7e3a5d9 authored by Cosmin Ratiu's avatar Cosmin Ratiu Committed by Jakub Kicinski
Browse files

net/mlx5: qos: Restrict RTNL area to avoid a lock cycle 



A lock dependency cycle exists where:
1. mlx5_ib_roce_init -> mlx5_core_uplink_netdev_event_replay ->
mlx5_blocking_notifier_call_chain (takes notifier_rwsem) ->
mlx5e_mdev_notifier_event -> mlx5_netdev_notifier_register ->
register_netdevice_notifier_dev_net (takes rtnl)
=> notifier_rwsem -> rtnl

2. mlx5e_probe -> _mlx5e_probe ->
mlx5_core_uplink_netdev_set (takes uplink_netdev_lock) ->
mlx5_blocking_notifier_call_chain (takes notifier_rwsem)
=> uplink_netdev_lock -> notifier_rwsem

3: devlink_nl_rate_set_doit -> devlink_nl_rate_set ->
mlx5_esw_devlink_rate_leaf_tx_max_set -> esw_qos_devlink_rate_to_mbps ->
mlx5_esw_qos_max_link_speed_get (takes rtnl) ->
mlx5_esw_qos_lag_link_speed_get_locked ->
mlx5_uplink_netdev_get (takes uplink_netdev_lock)
=> rtnl -> uplink_netdev_lock
=> BOOM! (lock cycle)

Fix that by restricting the rtnl-protected section to just the necessary
part, the call to netdev_master_upper_dev_get and speed querying, so
that the last lock dependency is avoided and the cycle doesn't close.
This is safe because mlx5_uplink_netdev_get uses netdev_hold to keep the
uplink netdev alive while its master device is queried.

Use this opportunity to rename the ambiguously-named "hold_rtnl_lock"
argument to "take_rtnl" and remove the "_locked" suffix from
mlx5_esw_qos_lag_link_speed_get_locked.

Fixes: 6b4be64f ("net/mlx5e: Harden uplink netdev access against device unbind")
Signed-off-by: default avatarCosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: default avatarDragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: default avatarTariq Toukan <tariqt@nvidia.com>
Link: https://patch.msgid.link/20260316094603.6999-2-tariqt@nvidia.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent d0f9eca2

drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c

+9 −14
Original line number Diff line number Diff line
@@ -1489,24 +1489,24 @@ static int esw_qos_node_enable_tc_arbitration(struct mlx5_esw_sched_node *node, 
	return err;

}



static u32 mlx5_esw_qos_lag_link_speed_get_locked(struct mlx5_core_dev *mdev)

static u32 mlx5_esw_qos_lag_link_speed_get(struct mlx5_core_dev *mdev,

					   bool take_rtnl)

{

	struct ethtool_link_ksettings lksettings;

	struct net_device *slave, *master;

	u32 speed = SPEED_UNKNOWN;



	/* Lock ensures a stable reference to master and slave netdevice

	 * while port speed of master is queried.

	 */

	ASSERT_RTNL();



	slave = mlx5_uplink_netdev_get(mdev);

	if (!slave)

		goto out;



	if (take_rtnl)

		rtnl_lock();

	master = netdev_master_upper_dev_get(slave);

	if (master && !__ethtool_get_link_ksettings(master, &lksettings))

		speed = lksettings.base.speed;

	if (take_rtnl)

		rtnl_unlock();



out:

	mlx5_uplink_netdev_put(mdev, slave);
@@ -1514,20 +1514,15 @@ static u32 mlx5_esw_qos_lag_link_speed_get_locked(struct mlx5_core_dev *mdev) 
}



static int mlx5_esw_qos_max_link_speed_get(struct mlx5_core_dev *mdev, u32 *link_speed_max,

					   bool hold_rtnl_lock, struct netlink_ext_ack *extack)

					   bool take_rtnl,

					   struct netlink_ext_ack *extack)

{

	int err;



	if (!mlx5_lag_is_active(mdev))

		goto skip_lag;



	if (hold_rtnl_lock)

		rtnl_lock();



	*link_speed_max = mlx5_esw_qos_lag_link_speed_get_locked(mdev);



	if (hold_rtnl_lock)

		rtnl_unlock();

	*link_speed_max = mlx5_esw_qos_lag_link_speed_get(mdev, take_rtnl);



	if (*link_speed_max != (u32)SPEED_UNKNOWN)

		return 0;