[PATCH iwl-net v2 0/5] ice: fix RTNL vs auxiliary device lock inversion

[PATCH iwl-net v2 0/5] ice: fix RTNL vs auxiliary device lock inversion

[Petr Oros]

ice takes the auxiliary device lock of its RDMA auxiliary device from
three places that run with the RTNL lock held. The RDMA auxiliary probe
takes the two in the opposite order, because __driver_attach() holds the
auxiliary device lock across ib_register_device(), which needs the RTNL
lock. The result is a hard deadlock of everything that needs the RTNL
lock, so the boot never completes:

  systemd-udevd   state:D
    really_probe                  <- holds device_lock(&adev->dev)
    icrdma_probe
    irdma_ib_register_device
    ib_register_device
    setup_device
    irdma_roce_port_immutable
    ib_query_port
    irdma_query_port
    ib_get_eth_speed              <- blocks on rtnl_lock()

  lldpad          state:D
    rtnetlink_rcv_msg             <- holds rtnl_lock()
    dcb_doit
    dcbnl_ieee_set
    ice_dcbnl_setets
    ice_pf_dcb_cfg
    ice_send_event_to_aux         <- blocks on device_lock(&adev->dev)

Lockdep cannot flag it, because device_initialize() marks dev->mutex
with lockdep_set_novalidate_class(). A PREEMPT_RT kernel with
CONFIG_DEBUG_RT_MUTEXES reports "rtmutex deadlock detected" instead.

The probe side cannot be changed, so the three ice callers use
device_trylock() and refuse the operation with -EBUSY when the driver
core holds the lock. Patches 1 and 2 are new in v2 and fix two separate
problems on the same sites, the auxiliary device pointer was tested
before pf->adev_mutex was taken, and ndo_setup_tc() took pf->adev_mutex
before pf->tc_mutex while the dcbnl handlers take them the other way
round. Patch 3 keeps the locks across the whole DCB reconfiguration so
that BEFORE_TC_CHANGE and AFTER_TC_CHANGE are either both delivered or
the request is refused before anything changes, and re-arms the service
task events. Patches 4 and 5 turn the remaining two locks into trylocks.

The proper fix is provider-owned registration as in i40e and mlx5, which
makes the inversion impossible by construction. That is a net-next change
across ice, irdma and iidc_rdma.h, this series is the backportable part.

Tested on an E810-XXV with two PFs, irdma bound to both auxiliary
devices, on net tip commit 191aa41548b2d2 ("MAINTAINERS: add Bobby
Eshleman as vsock reviewer") with and without this series. A reproducer
binds the irdma auxiliary driver while driving one of the three RTNL
held callers into the probe window:

                        without the series   with the series
  ethtool set_channels  deadlock 10/10       0/10, window entered 300x
  ndo_setup_tc mqprio   deadlock             0/10, window entered 50x
  dcbnl ets set         deadlock             0/10, window entered 30x

A window entry is a contended device_trylock(), so each one used to
close the cycle. 25 reboots on the patched kernel were clean.

Changes since v1:
- new patch 1 for the auxiliary device pointer race
- new patch 2 for the pf->tc_mutex order in ndo_setup_tc()
- patch 3 holds the auxiliary device locks across the whole DCB
  reconfiguration and sends AFTER_TC_CHANGE under them instead of
  dropping it when the lock is contended, and takes the locks before it
  consumes pf->oicr_err_reg in the service task
- patch 5 warns instead of netdev_dbg(), the qdisc teardown ignores the
  return value

Link to v1:
https://lore.kernel.org/all/20261019065301.3983537-1-poros@redhat.com/

Petr Oros (5):
  ice: check the auxiliary device pointer under adev_mutex
  ice: take tc_mutex before the auxiliary device locks in ndo_setup_tc
  ice: avoid RTNL deadlock when notifying the RDMA auxiliary driver
  ice: do not block on the aux device lock in ethtool set_channels
  ice: do not block on the aux device lock in ndo_setup_tc

 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 46 ++++++++++++++++---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 26 +++++++----
 drivers/net/ethernet/intel/ice/ice_idc.c     | 48 ++++++++++++++++----
 drivers/net/ethernet/intel/ice/ice_idc_int.h |  2 +
 drivers/net/ethernet/intel/ice/ice_main.c    | 47 +++++++++++++------
 5 files changed, 130 insertions(+), 39 deletions(-)


base-commit: 07bd391eb9572e74f073c49acef327611234f9bb
-- 
2.56.0

[PATCH iwl-net v2 1/5] ice: check the auxiliary device pointer under adev_mutex

[Petr Oros]

ice_set_channels() and the mqprio path of ice_setup_tc() test
cdev->adev before taking pf->adev_mutex and dereference it afterwards.
ice_unplug_aux_dev() clears the pointer under the mutex from the
service task, from ice_prepare_for_reset() and from the devlink RoCE
and iWARP parameter handlers, none of which hold the RTNL lock, so the
pointer can go away between the test and the device_lock() call.

Take pf->adev_mutex first and test the pointer under it. Without a
device the mutex is released again and the rebuild runs unlocked as
before.

Fixes: a6a0974aae42 ("ice: Prevent set_channel from changing queues while RDMA active")
Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v2:
- new patch, the pointer test raced with ice_unplug_aux_dev()
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 22 +++++++++++++-------
 drivers/net/ethernet/intel/ice/ice_main.c    | 22 +++++++++++++-------
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index bf9a821c543b7f..5482a64c81c95a 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3888,6 +3888,7 @@ static int ice_vsi_set_dflt_rss_lut(struct ice_vsi *vsi, int req_rss_size)
 static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
 {
 	struct ice_netdev_priv *np = netdev_priv(dev);
+	struct auxiliary_device *adev = NULL;
 	struct ice_vsi *vsi = np->vsi;
 	struct ice_pf *pf = vsi->back;
 	int new_rx = 0, new_tx = 0;
@@ -3942,14 +3943,19 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
 		return -EINVAL;
 	}
 
-	if (pf->cdev_info && pf->cdev_info->adev) {
+	if (pf->cdev_info) {
 		mutex_lock(&pf->adev_mutex);
-		device_lock(&pf->cdev_info->adev->dev);
-		locked = true;
-		if (pf->cdev_info->adev->dev.driver) {
-			netdev_err(dev, "Cannot change channels when RDMA is active\n");
-			ret = -EBUSY;
-			goto adev_unlock;
+		adev = pf->cdev_info->adev;
+		if (adev) {
+			device_lock(&adev->dev);
+			locked = true;
+			if (adev->dev.driver) {
+				netdev_err(dev, "Cannot change channels when RDMA is active\n");
+				ret = -EBUSY;
+				goto adev_unlock;
+			}
+		} else {
+			mutex_unlock(&pf->adev_mutex);
 		}
 	}
 
@@ -3965,7 +3971,7 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
 
 adev_unlock:
 	if (locked) {
-		device_unlock(&pf->cdev_info->adev->dev);
+		device_unlock(&adev->dev);
 		mutex_unlock(&pf->adev_mutex);
 	}
 	return ret;
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index d88835482d3aac..984479f1e1f6d9 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -9406,6 +9406,7 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 	struct ice_netdev_priv *np = netdev_priv(netdev);
 	enum flow_block_binder_type binder_type;
 	struct iidc_rdma_core_dev_info *cdev;
+	struct auxiliary_device *adev = NULL;
 	struct ice_pf *pf = np->vsi->back;
 	flow_setup_cb_t *flower_handler;
 	bool locked = false;
@@ -9438,14 +9439,19 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 		}
 
 		cdev = pf->cdev_info;
-		if (cdev && cdev->adev) {
+		if (cdev) {
 			mutex_lock(&pf->adev_mutex);
-			device_lock(&cdev->adev->dev);
-			locked = true;
-			if (cdev->adev->dev.driver) {
-				netdev_err(netdev, "Cannot change qdisc when RDMA is active\n");
-				err = -EBUSY;
-				goto adev_unlock;
+			adev = cdev->adev;
+			if (adev) {
+				device_lock(&adev->dev);
+				locked = true;
+				if (adev->dev.driver) {
+					netdev_err(netdev, "Cannot change qdisc when RDMA is active\n");
+					err = -EBUSY;
+					goto adev_unlock;
+				}
+			} else {
+				mutex_unlock(&pf->adev_mutex);
 			}
 		}
 
@@ -9456,7 +9462,7 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 
 adev_unlock:
 		if (locked) {
-			device_unlock(&cdev->adev->dev);
+			device_unlock(&adev->dev);
 			mutex_unlock(&pf->adev_mutex);
 		}
 		return err;
-- 
2.56.0

RE: [PATCH iwl-net v2 1/5] ice: check the auxiliary device pointer under adev_mutex

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Petr Oros <poros@redhat.com>
> Sent: Friday, October 23, 2026 9:03 AM
> To: netdev@vger.kernel.org
> Cc: Oros, Petr <poros@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@kernel.org>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>;
> Clark Williams <clrkwllms@kernel.org>; Steven Rostedt
> <rostedt@goodmis.org>; Ertman, David M <david.m.ertman@intel.com>;
> Mateusz Palczewski <mateusz.palczewski@intel.com>; Leon Romanovsky
> <leon@kernel.org>; Rafal Rogalski <rafalx.rogalski@intel.com>; Shiraz
> Saleem <shiraz.saleem@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; intel-wired-lan@lists.osuosl.org;
> linux-kernel@vger.kernel.org; linux-rt-devel@lists.linux.dev
> Subject: [PATCH iwl-net v2 1/5] ice: check the auxiliary device pointer
> under adev_mutex
> 
> ice_set_channels() and the mqprio path of ice_setup_tc() test
> cdev->adev before taking pf->adev_mutex and dereference it afterwards.
> ice_unplug_aux_dev() clears the pointer under the mutex from the service
> task, from ice_prepare_for_reset() and from the devlink RoCE and iWARP
> parameter handlers, none of which hold the RTNL lock, so the pointer can
> go away between the test and the device_lock() call.
> 
> Take pf->adev_mutex first and test the pointer under it. Without a
> device the mutex is released again and the rebuild runs unlocked as
> before.
> 
> Fixes: a6a0974aae42 ("ice: Prevent set_channel from changing queues
> while RDMA active")
> Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> v2:
> - new patch, the pointer test raced with ice_unplug_aux_dev()
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 22 +++++++++++++-------
>  drivers/net/ethernet/intel/ice/ice_main.c    | 22 +++++++++++++-------
>  2 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index bf9a821c543b7f..5482a64c81c95a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3888,6 +3888,7 @@ static int ice_vsi_set_dflt_rss_lut(struct ice_vsi
> *vsi, int req_rss_size)  static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)  {
>  	struct ice_netdev_priv *np = netdev_priv(dev);
> +	struct auxiliary_device *adev = NULL;
>  	struct ice_vsi *vsi = np->vsi;
>  	struct ice_pf *pf = vsi->back;
>  	int new_rx = 0, new_tx = 0;
> @@ -3942,14 +3943,19 @@ static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)
>  		return -EINVAL;
>  	}
> 
> -	if (pf->cdev_info && pf->cdev_info->adev) {
> +	if (pf->cdev_info) {
>  		mutex_lock(&pf->adev_mutex);
> -		device_lock(&pf->cdev_info->adev->dev);
> -		locked = true;
> -		if (pf->cdev_info->adev->dev.driver) {
> -			netdev_err(dev, "Cannot change channels when RDMA
> is active\n");
> -			ret = -EBUSY;
> -			goto adev_unlock;
> +		adev = pf->cdev_info->adev;
> +		if (adev) {
> +			device_lock(&adev->dev);
> +			locked = true;
> +			if (adev->dev.driver) {
> +				netdev_err(dev, "Cannot change channels when
> RDMA is active\n");
> +				ret = -EBUSY;
> +				goto adev_unlock;
> +			}
> +		} else {
> +			mutex_unlock(&pf->adev_mutex);
>  		}
>  	}
> 
> @@ -3965,7 +3971,7 @@ static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)
> 
>  adev_unlock:
>  	if (locked) {
> -		device_unlock(&pf->cdev_info->adev->dev);
> +		device_unlock(&adev->dev);
>  		mutex_unlock(&pf->adev_mutex);
>  	}
>  	return ret;
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index d88835482d3aac..984479f1e1f6d9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -9406,6 +9406,7 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
>  	struct ice_netdev_priv *np = netdev_priv(netdev);
>  	enum flow_block_binder_type binder_type;
>  	struct iidc_rdma_core_dev_info *cdev;
> +	struct auxiliary_device *adev = NULL;
>  	struct ice_pf *pf = np->vsi->back;
>  	flow_setup_cb_t *flower_handler;
>  	bool locked = false;
> @@ -9438,14 +9439,19 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
>  		}
> 
>  		cdev = pf->cdev_info;
> -		if (cdev && cdev->adev) {
> +		if (cdev) {
>  			mutex_lock(&pf->adev_mutex);
> -			device_lock(&cdev->adev->dev);
> -			locked = true;
> -			if (cdev->adev->dev.driver) {
> -				netdev_err(netdev, "Cannot change qdisc when
> RDMA is active\n");
> -				err = -EBUSY;
> -				goto adev_unlock;
> +			adev = cdev->adev;
> +			if (adev) {
> +				device_lock(&adev->dev);
> +				locked = true;
> +				if (adev->dev.driver) {
> +					netdev_err(netdev, "Cannot change
> qdisc when RDMA is active\n");
> +					err = -EBUSY;
> +					goto adev_unlock;
> +				}
> +			} else {
> +				mutex_unlock(&pf->adev_mutex);
>  			}
>  		}
> 
> @@ -9456,7 +9462,7 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
> 
>  adev_unlock:
>  		if (locked) {
> -			device_unlock(&cdev->adev->dev);
> +			device_unlock(&adev->dev);
>  			mutex_unlock(&pf->adev_mutex);
>  		}
>  		return err;
> --
> 2.56.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

[PATCH iwl-net v2 2/5] ice: take tc_mutex before the auxiliary device locks in ndo_setup_tc

[Petr Oros]

The mqprio path of ice_setup_tc() takes pf->adev_mutex and the auxiliary
device lock first and pf->tc_mutex after them. The dcbnl handlers take
pf->tc_mutex first and reach pf->adev_mutex through ice_pf_dcb_cfg() and
ice_send_event_to_aux(). Both run with the RTNL lock held, so the two
orders do not deadlock against each other, but they form a circular
dependency for lockdep, and the DCB reconfiguration is about to hold
pf->adev_mutex for longer.

Take pf->tc_mutex first, which is the order the dcbnl side has used
since pf->adev_mutex was introduced.

Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v2:
- new patch, ice_setup_tc() took pf->adev_mutex before pf->tc_mutex
  while the dcbnl handlers take them the other way round
---
 drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 984479f1e1f6d9..8693ca285d5f1e 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -9438,6 +9438,8 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 			return -EOPNOTSUPP;
 		}
 
+		mutex_lock(&pf->tc_mutex);
+
 		cdev = pf->cdev_info;
 		if (cdev) {
 			mutex_lock(&pf->adev_mutex);
@@ -9456,15 +9458,14 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 		}
 
 		/* setup traffic classifier for receive side */
-		mutex_lock(&pf->tc_mutex);
 		err = ice_setup_tc_mqprio_qdisc(netdev, type_data);
-		mutex_unlock(&pf->tc_mutex);
 
 adev_unlock:
 		if (locked) {
 			device_unlock(&adev->dev);
 			mutex_unlock(&pf->adev_mutex);
 		}
+		mutex_unlock(&pf->tc_mutex);
 		return err;
 	case TC_SETUP_QDISC_ETF:
 		return ice_offload_txtime(netdev, type_data);
-- 
2.56.0

RE: [PATCH iwl-net v2 2/5] ice: take tc_mutex before the auxiliary device locks in ndo_setup_tc

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Petr Oros <poros@redhat.com>
> Sent: Friday, October 23, 2026 9:03 AM
> To: netdev@vger.kernel.org
> Cc: Oros, Petr <poros@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@kernel.org>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>;
> Clark Williams <clrkwllms@kernel.org>; Steven Rostedt
> <rostedt@goodmis.org>; Ertman, David M <david.m.ertman@intel.com>;
> Mateusz Palczewski <mateusz.palczewski@intel.com>; Leon Romanovsky
> <leon@kernel.org>; Rafal Rogalski <rafalx.rogalski@intel.com>; Shiraz
> Saleem <shiraz.saleem@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; intel-wired-lan@lists.osuosl.org;
> linux-kernel@vger.kernel.org; linux-rt-devel@lists.linux.dev
> Subject: [PATCH iwl-net v2 2/5] ice: take tc_mutex before the auxiliary
> device locks in ndo_setup_tc
> 
> The mqprio path of ice_setup_tc() takes pf->adev_mutex and the auxiliary
> device lock first and pf->tc_mutex after them. The dcbnl handlers take
> pf->tc_mutex first and reach pf->adev_mutex through ice_pf_dcb_cfg() and
> ice_send_event_to_aux(). Both run with the RTNL lock held, so the two
> orders do not deadlock against each other, but they form a circular
> dependency for lockdep, and the DCB reconfiguration is about to hold
> pf->adev_mutex for longer.
> 
> Take pf->tc_mutex first, which is the order the dcbnl side has used
> since pf->adev_mutex was introduced.
> 
> Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> v2:
> - new patch, ice_setup_tc() took pf->adev_mutex before pf->tc_mutex
>   while the dcbnl handlers take them the other way round
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 984479f1e1f6d9..8693ca285d5f1e 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -9438,6 +9438,8 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
>  			return -EOPNOTSUPP;
>  		}
> 
> +		mutex_lock(&pf->tc_mutex);
> +
>  		cdev = pf->cdev_info;
>  		if (cdev) {
>  			mutex_lock(&pf->adev_mutex);
> @@ -9456,15 +9458,14 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
>  		}
> 
>  		/* setup traffic classifier for receive side */
> -		mutex_lock(&pf->tc_mutex);
>  		err = ice_setup_tc_mqprio_qdisc(netdev, type_data);
> -		mutex_unlock(&pf->tc_mutex);
> 
>  adev_unlock:
>  		if (locked) {
>  			device_unlock(&adev->dev);
>  			mutex_unlock(&pf->adev_mutex);
>  		}
> +		mutex_unlock(&pf->tc_mutex);
>  		return err;
>  	case TC_SETUP_QDISC_ETF:
>  		return ice_offload_txtime(netdev, type_data);
> --
> 2.56.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

[PATCH iwl-net v2 3/5] ice: avoid RTNL deadlock when notifying the RDMA auxiliary driver

[Petr Oros]

ice_send_event_to_aux() blocks on the auxiliary device lock, and the
dcbnl callers reach it with the RTNL lock held. The RDMA auxiliary probe
takes the two in the opposite order, because __driver_attach() holds the
auxiliary device lock across ib_register_device(), which needs the RTNL
lock:

  systemd-udevd   state:D
    really_probe                  <- holds device_lock(&adev->dev)
    icrdma_probe
    irdma_ib_register_device
    ib_register_device
    setup_device
    irdma_roce_port_immutable
    ib_query_port
    irdma_query_port
    ib_get_eth_speed              <- blocks on rtnl_lock()

  lldpad          state:D
    rtnetlink_rcv_msg             <- holds rtnl_lock()
    dcb_doit
    dcbnl_ieee_set
    ice_dcbnl_setets
    ice_pf_dcb_cfg
    ice_send_event_to_aux         <- blocks on device_lock(&adev->dev)

Everything else that needs the RTNL lock piles up behind lldpad and the
boot never completes. The probe side cannot be changed, so take the
auxiliary device lock with device_trylock() around the event and let the
caller decide what to do when it is contended.

The DCB reconfiguration sends IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, on which
the RDMA driver suspends its queue pairs, and
IIDC_RDMA_EVENT_AFTER_TC_CHANGE, on which it resumes them. Neither can be
dropped, the first would change the traffic classes behind the RDMA
driver's back and the second would leave its queue pairs suspended. So
ice_pf_dcb_cfg() takes the locks once before the first event, keeps them
across the whole reconfiguration and sends the second event before
releasing them. The driver core holds the lock while it binds, unbinds or
suspends the auxiliary device and in a few of its sysfs handlers, and the
request is then refused with -EBUSY. The RTNL lock is taken before the
auxiliary locks also for the callers that did not hold it, so every path
takes the two in the same order, and the first event is delivered with
the RTNL lock held on those paths too, as it already was for the dcbnl
callers. The second event is now sent on the error paths too, which used
to leave the queue pairs suspended after a failed commit.
ice_pf_dcb_recfg() still sends the second event on its own for the LLDP
MIB change path, which sends no first event, so a dropped one is ignored
by the RDMA driver there.

ice_pf_dcb_recfg() runs the fallback for non-contiguous traffic classes
only when the caller does not hold the locks. That fallback is a nested
ice_pf_dcb_cfg() and would block on pf->adev_mutex, so ice_pf_dcb_cfg()
runs it after it has released the locks. The other caller that passes
locked, ice_vsi_recfg_qs(), already held pf->adev_mutex there and would
have deadlocked the same way through ice_send_event_to_aux().

ice_pf_dcb_cfg() already returns -EBUSY when the custom Tx scheduler is
enabled, so the refused request takes the same path through its callers.

The critical error and MTU change events are sent from the service task,
which consumes their pending bits before sending, so those are re-armed
instead. The critical error event takes the locks before it consumes
pf->oicr_err_reg, so a contended lock leaves the register value to the
next run instead of merging it back underneath the interrupt handler.

Fixes: 348048e724a0 ("ice: Implement iidc operations")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v2:
- hold the auxiliary device locks across the whole DCB reconfiguration
  and send AFTER_TC_CHANGE under them instead of fire and forget
- take the RTNL lock before the auxiliary locks also for the callers
  that did not hold it
- send AFTER_TC_CHANGE on the error paths of ice_pf_dcb_cfg() too
- add ice_aux_dev_trylock() and ice_aux_dev_unlock(), the callers of
  ice_send_event_to_aux() hold the locks
- take the locks before consuming pf->oicr_err_reg in the service task
  instead of merging the value back when the lock is contended
- run the fallback for non-contiguous traffic classes after the locks
  are released, it is a nested ice_pf_dcb_cfg()

v1: https://lore.kernel.org/all/20261019065301.3983537-2-poros@redhat.com/
---
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 46 ++++++++++++++++---
 drivers/net/ethernet/intel/ice/ice_idc.c     | 48 ++++++++++++++++----
 drivers/net/ethernet/intel/ice/ice_idc_int.h |  2 +
 drivers/net/ethernet/intel/ice/ice_main.c    | 14 ++++--
 4 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 0bc6dd37568792..9916bd408071c1 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -5,6 +5,9 @@
 #include "ice_dcb_nl.h"
 #include "devlink/devlink.h"
 
+static bool ice_dcb_tc_contig(u8 *prio_table);
+static int ice_dcb_noncontig_cfg(struct ice_pf *pf);
+
 /**
  * ice_dcb_get_ena_tc - return bitmap of enabled TCs
  * @dcbcfg: DCB config to evaluate for enabled TCs
@@ -352,11 +355,16 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 	struct ice_aqc_port_ets_elem buf = { 0 };
 	struct ice_dcbx_cfg *old_cfg, *curr_cfg;
 	struct device *dev = ice_pf_to_dev(pf);
+	struct iidc_rdma_priv_dev_info *privd;
+	struct iidc_rdma_core_dev_info *cdev;
 	struct iidc_rdma_event *event;
 	int ret = ICE_DCB_NO_HW_CHG;
 	struct ice_vsi *pf_vsi;
+	bool noncontig = false;
+	bool dcb_ena;
 
 	curr_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg;
+	dcb_ena = test_bit(ICE_FLAG_DCB_ENA, pf->flags);
 
 	/* FW does not care if change happened */
 	if (!pf->hw.port_info->qos_cfg.is_sw_lldp)
@@ -405,16 +413,21 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 		goto free_cfg;
 	}
 
-	set_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
-	ice_send_event_to_aux(pf, event);
-	kfree(event);
-
 	/* avoid race conditions by holding the lock while disabling and
 	 * re-enabling the VSI
 	 */
 	if (!locked)
 		rtnl_lock();
 
+	if (!ice_aux_dev_trylock(pf)) {
+		assign_bit(ICE_FLAG_DCB_ENA, pf->flags, dcb_ena);
+		ret = -EBUSY;
+		goto unlock_rtnl;
+	}
+
+	set_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
+	ice_send_event_to_aux(pf, event);
+
 	/* disable VSIs affected by DCB changes */
 	ice_dcb_ena_dis_vsi(pf, false, true);
 
@@ -441,13 +454,28 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 		goto out;
 	}
 
-	ice_pf_dcb_recfg(pf, false);
+	ice_pf_dcb_recfg(pf, true);
+	noncontig = !ice_dcb_tc_contig(curr_cfg->etscfg.prio_table);
 
 out:
+	cdev = pf->cdev_info;
+	if (cdev) {
+		privd = cdev->iidc_priv;
+		ice_setup_dcb_qos_info(pf, &privd->qos_info);
+	}
+	clear_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
+	set_bit(IIDC_RDMA_EVENT_AFTER_TC_CHANGE, event->type);
+	ice_send_event_to_aux(pf, event);
+	ice_aux_dev_unlock(pf);
+
 	/* enable previously downed VSIs */
 	ice_dcb_ena_dis_vsi(pf, true, true);
+	if (noncontig)
+		ice_dcb_noncontig_cfg(pf);
+unlock_rtnl:
 	if (!locked)
 		rtnl_unlock();
+	kfree(event);
 free_cfg:
 	kfree(old_cfg);
 	return ret;
@@ -761,7 +789,8 @@ void ice_pf_dcb_recfg(struct ice_pf *pf, bool locked)
 			 */
 			if (!ice_dcb_tc_contig(dcbcfg->etscfg.prio_table)) {
 				tc_map = ICE_DFLT_TRAFFIC_CLASS;
-				ice_dcb_noncontig_cfg(pf);
+				if (!locked)
+					ice_dcb_noncontig_cfg(pf);
 			}
 		} else if (vsi->type == ICE_VSI_CHNL) {
 			tc_map = BIT(ice_get_first_droptc(vsi));
@@ -796,7 +825,10 @@ void ice_pf_dcb_recfg(struct ice_pf *pf, bool locked)
 			return;
 
 		set_bit(IIDC_RDMA_EVENT_AFTER_TC_CHANGE, event->type);
-		ice_send_event_to_aux(pf, event);
+		if (ice_aux_dev_trylock(pf)) {
+			ice_send_event_to_aux(pf, event);
+			ice_aux_dev_unlock(pf);
+		}
 		kfree(event);
 	}
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
index 102d63c3018b1f..018e7f4d46ae9a 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -30,34 +30,62 @@ ice_get_auxiliary_drv(struct iidc_rdma_core_dev_info *cdev)
 			    struct iidc_rdma_core_auxiliary_drv, adrv.driver);
 }
 
+/**
+ * ice_aux_dev_trylock - take the locks that guard the RDMA AUX driver
+ * @pf: pointer to PF struct
+ *
+ * Return: true when the locks are held, false when the auxiliary device lock
+ * was contended and nothing is held.
+ */
+bool ice_aux_dev_trylock(struct ice_pf *pf)
+{
+	struct iidc_rdma_core_dev_info *cdev = pf->cdev_info;
+
+	mutex_lock(&pf->adev_mutex);
+	if (cdev && cdev->adev && !device_trylock(&cdev->adev->dev)) {
+		mutex_unlock(&pf->adev_mutex);
+		return false;
+	}
+
+	return true;
+}
+
+/**
+ * ice_aux_dev_unlock - release the locks taken by ice_aux_dev_trylock()
+ * @pf: pointer to PF struct
+ */
+void ice_aux_dev_unlock(struct ice_pf *pf)
+{
+	struct iidc_rdma_core_dev_info *cdev = pf->cdev_info;
+
+	if (cdev && cdev->adev)
+		device_unlock(&cdev->adev->dev);
+	mutex_unlock(&pf->adev_mutex);
+}
+
 /**
  * ice_send_event_to_aux - send event to RDMA AUX driver
  * @pf: pointer to PF struct
  * @event: event struct
+ *
+ * The caller holds the locks taken by ice_aux_dev_trylock().
  */
 void ice_send_event_to_aux(struct ice_pf *pf, struct iidc_rdma_event *event)
 {
+	struct iidc_rdma_core_dev_info *cdev = pf->cdev_info;
 	struct iidc_rdma_core_auxiliary_drv *iadrv;
-	struct iidc_rdma_core_dev_info *cdev;
 
 	if (WARN_ON_ONCE(!in_task()))
 		return;
 
-	cdev = pf->cdev_info;
+	lockdep_assert_held(&pf->adev_mutex);
+
 	if (!cdev)
 		return;
 
-	mutex_lock(&pf->adev_mutex);
-	if (!cdev->adev)
-		goto finish;
-
-	device_lock(&cdev->adev->dev);
 	iadrv = ice_get_auxiliary_drv(cdev);
 	if (iadrv && iadrv->event_handler)
 		iadrv->event_handler(cdev, event);
-	device_unlock(&cdev->adev->dev);
-finish:
-	mutex_unlock(&pf->adev_mutex);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_idc_int.h b/drivers/net/ethernet/intel/ice/ice_idc_int.h
index 17dbfcfb6a2aff..4cccb4764271ae 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc_int.h
+++ b/drivers/net/ethernet/intel/ice/ice_idc_int.h
@@ -9,6 +9,8 @@
 
 struct ice_pf;
 
+bool ice_aux_dev_trylock(struct ice_pf *pf);
+void ice_aux_dev_unlock(struct ice_pf *pf);
 void ice_send_event_to_aux(struct ice_pf *pf, struct iidc_rdma_event *event);
 
 #endif /* !_ICE_IDC_INT_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 8693ca285d5f1e..9de375f5c4deb5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2313,13 +2313,16 @@ static void ice_service_task(struct work_struct *work)
 		struct iidc_rdma_event *event;
 
 		event = kzalloc_obj(*event);
-		if (event) {
+		if (event && ice_aux_dev_trylock(pf)) {
 			set_bit(IIDC_RDMA_EVENT_CRIT_ERR, event->type);
 			/* report the entire OICR value to AUX driver */
 			swap(event->reg, pf->oicr_err_reg);
 			ice_send_event_to_aux(pf, event);
-			kfree(event);
+			ice_aux_dev_unlock(pf);
+		} else {
+			set_bit(ICE_AUX_ERR_PENDING, pf->state);
 		}
+		kfree(event);
 	}
 
 	/* unplug aux dev per request, if an unplug request came in
@@ -2336,11 +2339,14 @@ static void ice_service_task(struct work_struct *work)
 		struct iidc_rdma_event *event;
 
 		event = kzalloc_obj(*event);
-		if (event) {
+		if (event && ice_aux_dev_trylock(pf)) {
 			set_bit(IIDC_RDMA_EVENT_AFTER_MTU_CHANGE, event->type);
 			ice_send_event_to_aux(pf, event);
-			kfree(event);
+			ice_aux_dev_unlock(pf);
+		} else {
+			set_bit(ICE_FLAG_MTU_CHANGED, pf->flags);
 		}
+		kfree(event);
 	}
 
 	ice_clean_adminq_subtask(pf);
-- 
2.56.0

Re: [PATCH iwl-net v2 3/5] ice: avoid RTNL deadlock when notifying the RDMA auxiliary driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ice: premature AFTER_TC_CHANGE and unlock before ice_dcb_noncontig_cfg()
- [High] ice: inconsistent QoS and DCB state on error path in ice_pf_dcb_cfg()
- [Medium] ice: unrolled state mutations when ice_pf_dcb_cfg() fails with -EBUSY
--

commit 0b49c36f284346b01578d0498826a110d7c3a315
Author: Petr Oros <poros@redhat.com>

ice: avoid RTNL deadlock when notifying the RDMA auxiliary driver

This commit avoids an ABBA deadlock between rtnl_lock and the RDMA
auxiliary device lock by acquiring the auxiliary device lock with
device_trylock() via ice_aux_dev_trylock() around RDMA event delivery.
In ice_pf_dcb_cfg(), the locks are held across the DCB reconfiguration
from IIDC_RDMA_EVENT_BEFORE_TC_CHANGE through
IIDC_RDMA_EVENT_AFTER_TC_CHANGE, and the non-contiguous traffic class
fallback is deferred until after the locks are released.

> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> index 0bc6dd3756879..9916bd408071c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> @@ -5,6 +5,9 @@
>  #include "ice_dcb_nl.h"
>  #include "devlink/devlink.h"
>  
> +static bool ice_dcb_tc_contig(u8 *prio_table);
> +static int ice_dcb_noncontig_cfg(struct ice_pf *pf);
> +
>  /**
>   * ice_dcb_get_ena_tc - return bitmap of enabled TCs
>   * @dcbcfg: DCB config to evaluate for enabled TCs
> @@ -352,11 +355,16 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
>  	struct ice_aqc_port_ets_elem buf = { 0 };
>  	struct ice_dcbx_cfg *old_cfg, *curr_cfg;
>  	struct device *dev = ice_pf_to_dev(pf);
> +	struct iidc_rdma_priv_dev_info *privd;
> +	struct iidc_rdma_core_dev_info *cdev;
>  	struct iidc_rdma_event *event;
>  	int ret = ICE_DCB_NO_HW_CHG;
>  	struct ice_vsi *pf_vsi;
> +	bool noncontig = false;
> +	bool dcb_ena;
>  
>  	curr_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg;
> +	dcb_ena = test_bit(ICE_FLAG_DCB_ENA, pf->flags);

[ ... ]

> @@ -405,16 +413,21 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
>  		goto free_cfg;
>  	}
>  
> -	set_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
> -	ice_send_event_to_aux(pf, event);
> -	kfree(event);
> -
>  	/* avoid race conditions by holding the lock while disabling and
>  	 * re-enabling the VSI
>  	 */
>  	if (!locked)
>  		rtnl_lock();
>  
> +	if (!ice_aux_dev_trylock(pf)) {
> +		assign_bit(ICE_FLAG_DCB_ENA, pf->flags, dcb_ena);
> +		ret = -EBUSY;
> +		goto unlock_rtnl;
> +	}

[Severity: Medium]
When ice_aux_dev_trylock(pf) fails here and returns -EBUSY, what happens to
the state mutations that already took place earlier in ice_pf_dcb_cfg() and
its callers?

Earlier in ice_pf_dcb_cfg(), when ice_dcb_get_num_tc(new_cfg) > 1,
ice_tear_down_devlink_rate_tree(pf) has already run before
ice_aux_dev_trylock(pf):

    if (ice_dcb_get_num_tc(new_cfg) > 1) {
        if (pf->hw.port_info->is_custom_tx_enabled) {
            ...
            return -EBUSY;
        }
        ice_tear_down_devlink_rate_tree(pf);

        set_bit(ICE_FLAG_DCB_ENA, pf->flags);
    }

Unlike the is_custom_tx_enabled check, failing at ice_aux_dev_trylock(pf)
reverts ICE_FLAG_DCB_ENA while leaving the devlink rate tree torn down.

In addition, callers that configure 1 TC (where is_custom_tx_enabled could
never trigger), such as ice_dcb_sw_dflt_cfg() and ice_dcb_init_cfg(), zero
pi->qos_cfg.local_dcbx_cfg with memset() before calling ice_pf_dcb_cfg():

    memset(&pi->qos_cfg.local_dcbx_cfg, 0, sizeof(*dcbcfg));
    ...
    ret = ice_pf_dcb_cfg(pf, dcbcfg, locked);

If ice_aux_dev_trylock(pf) returns -EBUSY before curr_cfg is populated,
does local_dcbx_cfg remain completely zeroed out? Similarly, in
ice_dcbnl_setapp(), a negative return from ice_pf_dcb_cfg() is converted to
ICE_DCB_NO_HW_CHG without rolling back dcb_ieee_setapp(), dscp_mapped,
desired_dcbx_cfg, or ice_aq_set_pfc_mode().

> +
> +	set_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
> +	ice_send_event_to_aux(pf, event);
> +
>  	/* disable VSIs affected by DCB changes */
>  	ice_dcb_ena_dis_vsi(pf, false, true);
>  
> @@ -441,13 +454,28 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
>  		goto out;
>  	}
>  
> -	ice_pf_dcb_recfg(pf, false);
> +	ice_pf_dcb_recfg(pf, true);
> +	noncontig = !ice_dcb_tc_contig(curr_cfg->etscfg.prio_table);
>  
>  out:
> +	cdev = pf->cdev_info;
> +	if (cdev) {
> +		privd = cdev->iidc_priv;
> +		ice_setup_dcb_qos_info(pf, &privd->qos_info);
> +	}
> +	clear_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
> +	set_bit(IIDC_RDMA_EVENT_AFTER_TC_CHANGE, event->type);
> +	ice_send_event_to_aux(pf, event);

[Severity: High]
Can jumping to out on error leave curr_cfg, ICE_FLAG_DCB_ENA, and
privd->qos_info inconsistent with the actual VSI configuration and cause
active RDMA queue pairs to transition to the error state?

In ice_pf_dcb_cfg(), if ice_set_dcb_cfg() succeeds (updating firmware and
PRTDCB_TUP2TC) but ice_query_port_ets() fails and jumps to out:

1. curr_cfg still contains new_cfg (unlike the ice_set_dcb_cfg() failure
   path, which restores old_cfg), and neither error path restores
   ICE_FLAG_DCB_ENA to dcb_ena.

2. Because ice_pf_dcb_recfg(pf, true) was skipped, vsi->tc_cfg.ena_tc still
   only has the old traffic classes enabled.

3. At out, ice_setup_dcb_qos_info(pf, &privd->qos_info) populates
   privd->qos_info from the new curr_cfg and PRTDCB_TUP2TC register, and
   ice_send_event_to_aux() delivers IIDC_RDMA_EVENT_AFTER_TC_CHANGE.

When irdma handles IIDC_RDMA_EVENT_AFTER_TC_CHANGE via
icrdma_iidc_event_handler() -> irdma_change_l2params() ->
irdma_sc_suspend_resume_qps(), it attempts to resume QPs on the new TCs in
qos_info->up2tc[] via irdma_ws_add() -> icrdma_lan_register_qset() ->
ice_add_rdma_qset(). Since vsi->tc_cfg.ena_tc was never updated for those
new TCs, ice_ena_vsi_rdma_qset() fails with -EINVAL, causing
irdma_sc_suspend_resume_qps() to call irdma_modify_qp_to_err() on the QPs.

> +	ice_aux_dev_unlock(pf);
> +
>  	/* enable previously downed VSIs */
>  	ice_dcb_ena_dis_vsi(pf, true, true);
> +	if (noncontig)
> +		ice_dcb_noncontig_cfg(pf);

[Severity: High]
When a DCB configuration has non-contiguous traffic classes in
curr_cfg->etscfg.prio_table, does deferring ice_dcb_noncontig_cfg(pf) until
after IIDC_RDMA_EVENT_AFTER_TC_CHANGE and ice_aux_dev_unlock(pf) break RDMA
QP resume?

Previously, ice_pf_dcb_cfg() called ice_pf_dcb_recfg(pf, false), which saw
!ice_dcb_tc_contig(dcbcfg->etscfg.prio_table) and immediately ran
ice_dcb_noncontig_cfg(pf) to reset local_dcbx_cfg and hardware PRTDCB_TUP2TC
back to the default TC0 configuration before populating privd->qos_info,
sending IIDC_RDMA_EVENT_AFTER_TC_CHANGE, or re-enabling the VSIs.

With this change:

1. ice_set_dcb_cfg() programs the non-contiguous prio_table into hardware
   PRTDCB_TUP2TC.

2. ice_pf_dcb_recfg(pf, true) configures the PF VSI with
   tc_map = ICE_DFLT_TRAFFIC_CLASS (only TC0 enabled in vsi->tc_cfg.ena_tc)
   and skips ice_dcb_noncontig_cfg(pf) because locked is true.

3. Before ice_dcb_noncontig_cfg(pf) runs, ice_setup_dcb_qos_info() reads
   PRTDCB_TUP2TC from hardware:

    dcbx_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg;
    up2tc = rd32(&pf->hw, PRTDCB_TUP2TC);

    qos_info->num_tc = ice_dcb_get_num_tc(dcbx_cfg);

    for (i = 0; i < IIDC_MAX_USER_PRIORITY; i++)
        qos_info->up2tc[i] = (up2tc >> (i * 3)) & 0x7;

   This populates qos_info->up2tc[] with the un-reset non-contiguous TCs
   (> 0) while vsi->tc_cfg.ena_tc only has BIT(0) set.

4. ice_send_event_to_aux() then sends IIDC_RDMA_EVENT_AFTER_TC_CHANGE. When
   irdma resumes QPs via irdma_sc_suspend_resume_qps(), irdma_ws_add() ->
   ice_add_rdma_qset() -> ice_ena_vsi_rdma_qset() fails with -EINVAL for
   any TC > 0, and irdma_modify_qp_to_err() transitions the QPs to the
   error state.

5. Finally, ice_aux_dev_unlock(pf) drops the locks and
   ice_dcb_ena_dis_vsi(pf, true, true) re-enables the VSIs before
   ice_dcb_noncontig_cfg(pf) resets the configuration, and if the second
   ice_aux_dev_trylock(pf) inside ice_dcb_noncontig_cfg() ->
   ice_dcb_sw_dflt_cfg() -> ice_pf_dcb_cfg() fails with -EBUSY, the fallback
   aborts with local_dcbx_cfg zeroed out.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023070258.730168-1-poros@redhat.com?part=3

[PATCH iwl-net v2 4/5] ice: do not block on the aux device lock in ethtool set_channels

[Petr Oros]

ice_set_channels() takes the auxiliary device lock to test whether the
RDMA driver is bound, and ethtool ops run with the RTNL lock held. This
is the same inversion against the RDMA auxiliary probe as the one fixed
in ice_send_event_to_aux(), reached through a different caller.

Use device_trylock() and report the device as busy when it fails. The
existing diagnostic is not reused, a contended lock does not mean the
RDMA driver is bound. It is a netdev_dbg(), the condition is transient
and userspace already learns about it from -EBUSY.

Fixes: a6a0974aae42 ("ice: Prevent set_channel from changing queues while RDMA active")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v2:
- rebased on the new patches 1 and 2, the trylock now sits inside the
  pointer test done under adev_mutex

v1: https://lore.kernel.org/all/20261019065301.3983537-3-poros@redhat.com/
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 5482a64c81c95a..dbc662d6233957 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3947,7 +3947,11 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
 		mutex_lock(&pf->adev_mutex);
 		adev = pf->cdev_info->adev;
 		if (adev) {
-			device_lock(&adev->dev);
+			if (!device_trylock(&adev->dev)) {
+				mutex_unlock(&pf->adev_mutex);
+				netdev_dbg(dev, "RDMA auxiliary device is busy, try again\n");
+				return -EBUSY;
+			}
 			locked = true;
 			if (adev->dev.driver) {
 				netdev_err(dev, "Cannot change channels when RDMA is active\n");
-- 
2.56.0

RE: [PATCH iwl-net v2 4/5] ice: do not block on the aux device lock in ethtool set_channels

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Petr Oros <poros@redhat.com>
> Sent: Friday, October 23, 2026 9:03 AM
> To: netdev@vger.kernel.org
> Cc: Oros, Petr <poros@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@kernel.org>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>;
> Clark Williams <clrkwllms@kernel.org>; Steven Rostedt
> <rostedt@goodmis.org>; Ertman, David M <david.m.ertman@intel.com>;
> Mateusz Palczewski <mateusz.palczewski@intel.com>; Leon Romanovsky
> <leon@kernel.org>; Rafal Rogalski <rafalx.rogalski@intel.com>; Shiraz
> Saleem <shiraz.saleem@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; intel-wired-lan@lists.osuosl.org;
> linux-kernel@vger.kernel.org; linux-rt-devel@lists.linux.dev
> Subject: [PATCH iwl-net v2 4/5] ice: do not block on the aux device lock
> in ethtool set_channels
> 
> ice_set_channels() takes the auxiliary device lock to test whether the
> RDMA driver is bound, and ethtool ops run with the RTNL lock held. This
> is the same inversion against the RDMA auxiliary probe as the one fixed
> in ice_send_event_to_aux(), reached through a different caller.
> 
> Use device_trylock() and report the device as busy when it fails. The
> existing diagnostic is not reused, a contended lock does not mean the
> RDMA driver is bound. It is a netdev_dbg(), the condition is transient
> and userspace already learns about it from -EBUSY.
> 
> Fixes: a6a0974aae42 ("ice: Prevent set_channel from changing queues
> while RDMA active")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> v2:
> - rebased on the new patches 1 and 2, the trylock now sits inside the
>   pointer test done under adev_mutex
> 
> v1: https://lore.kernel.org/all/20261019065301.3983537-3-
> poros@redhat.com/
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 5482a64c81c95a..dbc662d6233957 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3947,7 +3947,11 @@ static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)
>  		mutex_lock(&pf->adev_mutex);
>  		adev = pf->cdev_info->adev;
>  		if (adev) {
> -			device_lock(&adev->dev);
> +			if (!device_trylock(&adev->dev)) {
> +				mutex_unlock(&pf->adev_mutex);
> +				netdev_dbg(dev, "RDMA auxiliary device is
> busy, try again\n");
> +				return -EBUSY;
> +			}
>  			locked = true;
>  			if (adev->dev.driver) {
>  				netdev_err(dev, "Cannot change channels when
> RDMA is active\n");
> --
> 2.56.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

[PATCH iwl-net v2 5/5] ice: do not block on the aux device lock in ndo_setup_tc

[Petr Oros]

The TC_SETUP_QDISC_MQPRIO path of ice_setup_tc() takes the auxiliary
device lock to test whether the RDMA driver is bound, and ndo_setup_tc()
runs with the RTNL lock held. This closes the same cycle against the
RDMA auxiliary probe as the two preceding patches.

Use device_trylock() and report the device as busy when it fails. When
the qdisc is added, userspace gets -EBUSY and can retry. When it is
deleted, mqprio_disable_offload() ignores the return value, so the
hardware traffic classes and the channel VSIs stay in place until a
later qdisc teardown gets through, the same as the existing -EBUSY for
a bound RDMA driver. The message is a netdev_warn() because of that
second case, nothing else reports it.

Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v2:
- rebased on the new patches 1 and 2
- netdev_warn() instead of netdev_dbg(), the qdisc teardown cannot
  retry and ignores the return value

v1: https://lore.kernel.org/all/20261019065301.3983537-4-poros@redhat.com/
---
 drivers/net/ethernet/intel/ice/ice_main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 9de375f5c4deb5..7eb653a7ebacd5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -9451,7 +9451,12 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 			mutex_lock(&pf->adev_mutex);
 			adev = cdev->adev;
 			if (adev) {
-				device_lock(&adev->dev);
+				if (!device_trylock(&adev->dev)) {
+					mutex_unlock(&pf->adev_mutex);
+					netdev_warn(netdev, "RDMA auxiliary device is busy, qdisc not changed\n");
+					err = -EBUSY;
+					goto tc_unlock;
+				}
 				locked = true;
 				if (adev->dev.driver) {
 					netdev_err(netdev, "Cannot change qdisc when RDMA is active\n");
@@ -9471,6 +9476,7 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 			device_unlock(&adev->dev);
 			mutex_unlock(&pf->adev_mutex);
 		}
+tc_unlock:
 		mutex_unlock(&pf->tc_mutex);
 		return err;
 	case TC_SETUP_QDISC_ETF:
-- 
2.56.0

Re: [PATCH iwl-net v2 5/5] ice: do not block on the aux device lock in ndo_setup_tc

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ice: orphaned channel VSIs and memory leak when ice_setup_tc() fails teardown
--

commit 58934f25b30092ac8064d34365ba48edb6a32a06
Author: Petr Oros <poros@redhat.com>

ice: do not block on the aux device lock in ndo_setup_tc

This commit replaces device_lock() with device_trylock() on the RDMA
auxiliary device in the TC_SETUP_QDISC_MQPRIO path of ice_setup_tc() to
avoid a lock dependency cycle with the RTNL lock, returning -EBUSY when
the lock is contended.

> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 9de375f5c4deb..7eb653a7ebacd 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -9451,7 +9451,12 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
>  			mutex_lock(&pf->adev_mutex);
>  			adev = cdev->adev;
>  			if (adev) {
> -				device_lock(&adev->dev);
> +				if (!device_trylock(&adev->dev)) {
> +					mutex_unlock(&pf->adev_mutex);
> +					netdev_warn(netdev, "RDMA auxiliary device is busy, qdisc not changed\n");
> +					err = -EBUSY;
> +					goto tc_unlock;
> +				}

[Severity: High]
Can failing with -EBUSY during qdisc removal permanently leak the
struct ice_channel entries on vsi->ch_list and leave channel VSI state
orphaned?

The commit message notes that when the qdisc is deleted,
mqprio_disable_offload() ignores the return value so the hardware traffic
classes and channel VSIs stay in place until a later qdisc teardown gets
through.

However, in net/sched/sch_mqprio.c, mqprio_destroy() calls
mqprio_disable_offload() once while destroying the Qdisc:

net/sched/sch_mqprio.c:mqprio_destroy() {
	...
	if (priv->hw_offload && dev->netdev_ops->ndo_setup_tc)
		mqprio_disable_offload(sch);
	else
		netdev_set_num_tc(dev, 0);
}

Once mqprio_destroy() finishes, the Qdisc object is freed and no mqprio
Qdisc remains attached to the net_device, so no subsequent teardown call
will happen for it (including on unregister_netdev()).

During driver unload (ice_remove() -> ice_unload() -> ice_deinit() ->
ice_vsi_release_all()), ice_deinit_pf_sw() releases the main VSI without
calling ice_remove_q_channels(), and ice_vsi_release_all() skips
ICE_VSI_CHNL VSIs:

drivers/net/ethernet/intel/ice/ice_main.c:ice_vsi_release_all() {
	...
		if (pf->vsi[i]->type == ICE_VSI_CHNL)
			continue;

		err = ice_vsi_release(pf->vsi[i]);
	...
}

Moreover, if a user later adds a new offloaded mqprio qdisc (hw != 0) to
attempt another teardown, ice_setup_tc_mqprio_qdisc() only removes existing
channels when disabling offload:

drivers/net/ethernet/intel/ice/ice_main.c:ice_setup_tc_mqprio_qdisc() {
	...
	if (!hw && !test_bit(ICE_FLAG_TC_MQPRIO, pf->flags))
		ice_remove_q_channels(vsi, true);
	...
	if (hw) {
		...
		ret = ice_create_q_channels(vsi);
		...
	}
}

Wouldn't adding a new offloaded mqprio qdisc leave the old struct
ice_channel entries on vsi->ch_list, rebuild the main VSI rings, and append
duplicate channels via ice_create_q_channels(), potentially indexing past
vsi->tx_rings[] and vsi->rx_rings[] in ice_remove_q_channels() if the new
qdisc configures fewer queues?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023070258.730168-1-poros@redhat.com?part=5