net: hold netdev instance lock during ndo_open/ndo_stop

For the drivers that use shaper API, switch to the mode where
core stack holds the netdev lock. This affects two drivers:

* iavf - already grabs netdev lock in ndo_open/ndo_stop, so mostly
         remove these
* netdevsim - switch to _locked APIs to avoid deadlock

iavf_close diff is a bit confusing, the existing call looks like this:
  iavf_close() {
    netdev_lock()
    ..
    netdev_unlock()
    wait_event_timeout(down_waitqueue)
  }

I change it to the following:
  netdev_lock()
  iavf_close() {
    ..
    netdev_unlock()
    wait_event_timeout(down_waitqueue)
    netdev_lock() // reusing this lock call
  }
  netdev_unlock()

Since I'm reusing existing netdev_lock call, so it looks like I only
add netdev_unlock.

Cc: Saeed Mahameed <saeed@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250305163732.2766420-2-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 71f11f64b13d7436e635e4ae7eb4d188d3451b5d..9f4d223dffcfb5efa0e3a47308367069e2e06cfe 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -4562,22 +4562,21 @@ static int iavf_open(struct net_device *netdev)
        struct iavf_adapter *adapter = netdev_priv(netdev);
        int err;
 
+       netdev_assert_locked(netdev);
+
        if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED) {
                dev_err(&adapter->pdev->dev, "Unable to open device due to PF driver failure.\n");
                return -EIO;
        }
 
-       netdev_lock(netdev);
        while (!mutex_trylock(&adapter->crit_lock)) {
                /* If we are in __IAVF_INIT_CONFIG_ADAPTER state the crit_lock
                 * is already taken and iavf_open is called from an upper
                 * device's notifier reacting on NETDEV_REGISTER event.
                 * We have to leave here to avoid dead lock.
                 */
-               if (adapter->state == __IAVF_INIT_CONFIG_ADAPTER) {
-                       netdev_unlock(netdev);
+               if (adapter->state == __IAVF_INIT_CONFIG_ADAPTER)
                        return -EBUSY;
-               }
 
                usleep_range(500, 1000);
        }
@@ -4626,7 +4625,6 @@ static int iavf_open(struct net_device *netdev)
        iavf_irq_enable(adapter, true);
 
        mutex_unlock(&adapter->crit_lock);
-       netdev_unlock(netdev);
 
        return 0;
 
@@ -4639,7 +4637,6 @@ err_setup_tx:
        iavf_free_all_tx_resources(adapter);
 err_unlock:
        mutex_unlock(&adapter->crit_lock);
-       netdev_unlock(netdev);
 
        return err;
 }
@@ -4661,12 +4658,12 @@ static int iavf_close(struct net_device *netdev)
        u64 aq_to_restore;
        int status;
 
-       netdev_lock(netdev);
+       netdev_assert_locked(netdev);
+
        mutex_lock(&adapter->crit_lock);
 
        if (adapter->state <= __IAVF_DOWN_PENDING) {
                mutex_unlock(&adapter->crit_lock);
-               netdev_unlock(netdev);
                return 0;
        }
 
@@ -4719,6 +4716,7 @@ static int iavf_close(struct net_device *netdev)
        if (!status)
                netdev_warn(netdev, "Device resources not yet released\n");
 
+       netdev_lock(netdev);
        mutex_lock(&adapter->crit_lock);
        adapter->aq_required |= aq_to_restore;
        mutex_unlock(&adapter->crit_lock);

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index a41dc79e9c2e082367af156b10b61f04be8c41fb..aaa3b58e2e3e1868b5b79d937c7d563a5630f650 100644 (file)
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -402,7 +402,7 @@ static int nsim_init_napi(struct netdevsim *ns)
        for (i = 0; i < dev->num_rx_queues; i++) {
                rq = ns->rq[i];
 
-               netif_napi_add_config(dev, &rq->napi, nsim_poll, i);
+               netif_napi_add_config_locked(dev, &rq->napi, nsim_poll, i);
        }
 
        for (i = 0; i < dev->num_rx_queues; i++) {
@@ -422,7 +422,7 @@ err_pp_destroy:
        }
 
        for (i = 0; i < dev->num_rx_queues; i++)
-               __netif_napi_del(&ns->rq[i]->napi);
+               __netif_napi_del_locked(&ns->rq[i]->napi);
 
        return err;
 }
@@ -452,7 +452,7 @@ static void nsim_enable_napi(struct netdevsim *ns)
                struct nsim_rq *rq = ns->rq[i];
 
                netif_queue_set_napi(dev, i, NETDEV_QUEUE_TYPE_RX, &rq->napi);
-               napi_enable(&rq->napi);
+               napi_enable_locked(&rq->napi);
        }
 }
 
@@ -461,6 +461,8 @@ static int nsim_open(struct net_device *dev)
        struct netdevsim *ns = netdev_priv(dev);
        int err;
 
+       netdev_assert_locked(dev);
+
        err = nsim_init_napi(ns);
        if (err)
                return err;
@@ -478,8 +480,8 @@ static void nsim_del_napi(struct netdevsim *ns)
        for (i = 0; i < dev->num_rx_queues; i++) {
                struct nsim_rq *rq = ns->rq[i];
 
-               napi_disable(&rq->napi);
-               __netif_napi_del(&rq->napi);
+               napi_disable_locked(&rq->napi);
+               __netif_napi_del_locked(&rq->napi);
        }
        synchronize_net();
 
@@ -494,6 +496,8 @@ static int nsim_stop(struct net_device *dev)
        struct netdevsim *ns = netdev_priv(dev);
        struct netdevsim *peer;
 
+       netdev_assert_locked(dev);
+
        netif_carrier_off(dev);
        peer = rtnl_dereference(ns->peer);
        if (peer)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7ab86ec228b7fb25e64c295aca6da28dfeff8dec..33066b155c842646fb15bb7977cb3f16569a89fd 100644 (file)
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2753,6 +2753,29 @@ static inline void netdev_assert_locked_or_invisible(struct net_device *dev)
                netdev_assert_locked(dev);
 }
 
+static inline bool netdev_need_ops_lock(struct net_device *dev)
+{
+       bool ret = false;
+
+#if IS_ENABLED(CONFIG_NET_SHAPER)
+       ret |= !!dev->netdev_ops->net_shaper_ops;
+#endif
+
+       return ret;
+}
+
+static inline void netdev_lock_ops(struct net_device *dev)
+{
+       if (netdev_need_ops_lock(dev))
+               netdev_lock(dev);
+}
+
+static inline void netdev_unlock_ops(struct net_device *dev)
+{
+       if (netdev_need_ops_lock(dev))
+               netdev_unlock(dev);
+}
+
 void netif_napi_set_irq_locked(struct napi_struct *napi, int irq);
 
 static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2dc705604509eb780129ccf75b1bf65ebeeb153e..7a327c782ea46a4419df7029b2c65059f8a34b56 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1627,6 +1627,8 @@ static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack)
        if (ret)
                return ret;
 
+       netdev_lock_ops(dev);
+
        set_bit(__LINK_STATE_START, &dev->state);
 
        if (ops->ndo_validate_addr)
@@ -1646,6 +1648,8 @@ static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack)
                add_device_randomness(dev->dev_addr, dev->addr_len);
        }
 
+       netdev_unlock_ops(dev);
+
        return ret;
 }
 
@@ -1716,11 +1720,19 @@ static void __dev_close_many(struct list_head *head)
                 *      We allow it to be called even after a DETACH hot-plug
                 *      event.
                 */
+
+               /* TODO: move the lock up before clearing __LINK_STATE_START.
+                * Generates spurious lockdep warning.
+                */
+               netdev_lock_ops(dev);
+
                if (ops->ndo_stop)
                        ops->ndo_stop(dev);
 
                netif_set_up(dev, false);
                netpoll_poll_enable(dev);
+
+               netdev_unlock_ops(dev);
        }
 }
 

diff --git a/net/core/dev.h b/net/core/dev.h
index caa13e431a6bcfdc4aad1cbcaa4a9487e34e2fa6..25bb9d6afbce1ca15a1a34e331323826ce11793e 100644 (file)
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -134,9 +134,11 @@ static inline void netif_set_up(struct net_device *dev, bool value)
        else
                dev->flags &= ~IFF_UP;
 
-       netdev_lock(dev);
+       if (!netdev_need_ops_lock(dev))
+               netdev_lock(dev);
        dev->up = value;
-       netdev_unlock(dev);
+       if (!netdev_need_ops_lock(dev))
+               netdev_unlock(dev);
 }
 
 static inline void netif_set_gso_max_size(struct net_device *dev,