Re: [PATCH net-next v2 07/11] net: bridge: don't recurse on the port's netdev ops lock

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Jakub

On 3.6.2026 03:28, Jakub Kicinski wrote:
> port_cost() calls __ethtool_get_link_ksettings() on the port device,
> which will soon take the port's ops lock. br_port_carrier_check()
> is reached via the NETDEV_CHANGE notifier from linkwatch, which
> already holds the port's ops lock, so the call would deadlock.
> 
> Make port_cost() expect the port's ops lock held and switch to
> netif_get_link_ksettings(). The only other caller is new_nbp(),
> make sure it takes the lock explicitly.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/bridge/br_if.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index d39571e13744..049d1d25bc26 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -19,6 +19,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/slab.h>
>  #include <net/dsa.h>
> +#include <net/netdev_lock.h>
>  #include <net/sock.h>
>  #include <linux/if_vlan.h>
>  #include <net/switchdev.h>
> @@ -30,13 +31,13 @@
>   * Determine initial path cost based on speed.
>   * using recommendations from 802.1d standard
>   *
> - * Since driver might sleep need to not be holding any locks.
> + * Since driver might sleep, we need to not be holding any bridge 
> spinlocks.
>   */
>  static int port_cost(struct net_device *dev)
>  {
>  	struct ethtool_link_ksettings ecmd;
> 
> -	if (!__ethtool_get_link_ksettings(dev, &ecmd)) {
> +	if (!netif_get_link_ksettings(dev, &ecmd)) {
>  		switch (ecmd.base.speed) {
>  		case SPEED_10000:
>  			return 2;
> @@ -436,7 +437,9 @@ static struct net_bridge_port *new_nbp(struct 
> net_bridge *br,
>  	p->br = br;
>  	netdev_hold(dev, &p->dev_tracker, GFP_KERNEL);
>  	p->dev = dev;
> +	netdev_lock_ops(dev);
>  	p->path_cost = port_cost(dev);
> +	netdev_unlock_ops(dev);
>  	p->priority = 0x8000 >> BR_PORT_BITS;
>  	p->port_no = index;
>  	p->flags = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD;

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks
Nicolai