From patchwork Wed Jan 15 03:53:09 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939813 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2DF6D136E3F for ; Wed, 15 Jan 2025 03:53:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913218; cv=none; b=QeL8NN0djQ6c7rXSGePhza83BSmfj6PhBMYEoC1WNRv+UsHRAIQy0uq3YxW8C2VtOkMFj0yPCwr52DKXTUkH9cEB9s/4W2kIcgZfgBfbHnK6sIEeUlDLNdT47SeFwoAgbBXvDEtGXo5SVLEovGcdi/SSc9OSle/Ysntpciwa/D4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913218; c=relaxed/simple; bh=VEZO3YHXsT0tELic0irZ/BWk60tEucx1ZHaN+NZN+gU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=HVWFllYpiELRyOrFM5vjyfBKaI6j1H2kuR4Jd6fSmvBx/TYfVp4LNGv6RAUUPOHyiv9qZ9UOsIZkGloGlZpCRGmTlujQ/yrw6M5L2SINW1sSbh37HrvsOqPRjgXCIqDth37UJl+M5MzAi1bHV4zA7BNWpLlhpBNJYBi3bZ2Opsc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=W6TPtU9g; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="W6TPtU9g" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 44F2CC4CEE4; Wed, 15 Jan 2025 03:53:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913217; bh=VEZO3YHXsT0tELic0irZ/BWk60tEucx1ZHaN+NZN+gU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W6TPtU9gFuOkc/I+L69DxVjEH9ToN22l9XJ6P+eFcQ7RS9Mt7207HneMhRAdA/Bbf ZIchrO5kRwOsD2Wno9vDohGOfA8Yz0BdVBxVuSzntgMTQUGJ6ZJYtRW/13fgV6rRSS 5M0aIc8vD5G5Yo7KdgaMBEBHr5Bsb54l5vXJFkcu4PsoTnvawGTvwMmVmHNdDFAr5o TMDkaU2CazngzNiBXDKNXg1vaqCMjRvmCmJ7z2LP3YqGsVgPJEAATaxA7DrQDfd7cA xJWHlLfkZRWNFUQd7RHORR9l8tMXF7QD0Se7IViWsigKSQOXtyM1R+4pjnZRhtuW0z xDKhXR9n7dhtQ== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski , anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com, jiri@resnulli.us Subject: [PATCH net-next v2 01/11] net: add netdev_lock() / netdev_unlock() helpers Date: Tue, 14 Jan 2025 19:53:09 -0800 Message-ID: <20250115035319.559603-2-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Add helpers for locking the netdev instance, use it in drivers and the shaper code. This will make grepping for the lock usage much easier, as we extend the lock to cover more fields. Reviewed-by: Joe Damato Signed-off-by: Jakub Kicinski Reviewed-by: Eric Dumazet Reviewed-by: Kuniyuki Iwashima Reviewed-by: Przemek Kitszel --- CC: anthony.l.nguyen@intel.com CC: przemyslaw.kitszel@intel.com CC: jiri@resnulli.us --- include/linux/netdevice.h | 23 ++++++- drivers/net/ethernet/intel/iavf/iavf_main.c | 74 ++++++++++----------- drivers/net/netdevsim/ethtool.c | 4 +- net/shaper/shaper.c | 6 +- 4 files changed, 63 insertions(+), 44 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index bced03fb349e..891c5bdb894c 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2444,8 +2444,12 @@ struct net_device { u32 napi_defer_hard_irqs; /** - * @lock: protects @net_shaper_hierarchy, feel free to use for other - * netdev-scope protection. Ordering: take after rtnl_lock. + * @lock: netdev-scope lock, protects a small selection of fields. + * Should always be taken using netdev_lock() / netdev_unlock() helpers. + * Drivers are free to use it for other protection. + * + * Protects: @net_shaper_hierarchy. + * Ordering: take after rtnl_lock. */ struct mutex lock; @@ -2671,6 +2675,21 @@ void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index, enum netdev_queue_type type, struct napi_struct *napi); +static inline void netdev_lock(struct net_device *dev) +{ + mutex_lock(&dev->lock); +} + +static inline void netdev_unlock(struct net_device *dev) +{ + mutex_unlock(&dev->lock); +} + +static inline void netdev_assert_locked(struct net_device *dev) +{ + lockdep_assert_held(&dev->lock); +} + static inline void netif_napi_set_irq(struct napi_struct *napi, int irq) { napi->irq = irq; diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 7740f446c73f..ab908d620285 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -1977,7 +1977,7 @@ static void iavf_finish_config(struct work_struct *work) * The dev->lock is needed to update the queue number */ rtnl_lock(); - mutex_lock(&adapter->netdev->lock); + netdev_lock(adapter->netdev); mutex_lock(&adapter->crit_lock); if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES) && @@ -1997,7 +1997,7 @@ static void iavf_finish_config(struct work_struct *work) netif_set_real_num_tx_queues(adapter->netdev, pairs); if (adapter->netdev->reg_state != NETREG_REGISTERED) { - mutex_unlock(&adapter->netdev->lock); + netdev_unlock(adapter->netdev); netdev_released = true; err = register_netdevice(adapter->netdev); if (err) { @@ -2027,7 +2027,7 @@ static void iavf_finish_config(struct work_struct *work) out: mutex_unlock(&adapter->crit_lock); if (!netdev_released) - mutex_unlock(&adapter->netdev->lock); + netdev_unlock(adapter->netdev); rtnl_unlock(); } @@ -2724,10 +2724,10 @@ static void iavf_watchdog_task(struct work_struct *work) struct iavf_hw *hw = &adapter->hw; u32 reg_val; - mutex_lock(&netdev->lock); + netdev_lock(netdev); if (!mutex_trylock(&adapter->crit_lock)) { if (adapter->state == __IAVF_REMOVE) { - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; } @@ -2741,35 +2741,35 @@ static void iavf_watchdog_task(struct work_struct *work) case __IAVF_STARTUP: iavf_startup(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, msecs_to_jiffies(30)); return; case __IAVF_INIT_VERSION_CHECK: iavf_init_version_check(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, msecs_to_jiffies(30)); return; case __IAVF_INIT_GET_RESOURCES: iavf_init_get_resources(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, msecs_to_jiffies(1)); return; case __IAVF_INIT_EXTENDED_CAPS: iavf_init_process_extended_caps(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, msecs_to_jiffies(1)); return; case __IAVF_INIT_CONFIG_ADAPTER: iavf_init_config_adapter(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, msecs_to_jiffies(1)); return; @@ -2781,7 +2781,7 @@ static void iavf_watchdog_task(struct work_struct *work) * as it can loop forever */ mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; } if (++adapter->aq_wait_count > IAVF_AQ_MAX_ERR) { @@ -2790,7 +2790,7 @@ static void iavf_watchdog_task(struct work_struct *work) adapter->flags |= IAVF_FLAG_PF_COMMS_FAILED; iavf_shutdown_adminq(hw); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, (5 * HZ)); return; @@ -2798,7 +2798,7 @@ static void iavf_watchdog_task(struct work_struct *work) /* Try again from failed step*/ iavf_change_state(adapter, adapter->last_state); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, HZ); return; case __IAVF_COMM_FAILED: @@ -2811,7 +2811,7 @@ static void iavf_watchdog_task(struct work_struct *work) iavf_change_state(adapter, __IAVF_INIT_FAILED); adapter->flags &= ~IAVF_FLAG_PF_COMMS_FAILED; mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; } reg_val = rd32(hw, IAVF_VFGEN_RSTAT) & @@ -2831,14 +2831,14 @@ static void iavf_watchdog_task(struct work_struct *work) adapter->aq_required = 0; adapter->current_op = VIRTCHNL_OP_UNKNOWN; mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, msecs_to_jiffies(10)); return; case __IAVF_RESETTING: mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, HZ * 2); return; @@ -2869,7 +2869,7 @@ static void iavf_watchdog_task(struct work_struct *work) case __IAVF_REMOVE: default: mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; } @@ -2881,14 +2881,14 @@ static void iavf_watchdog_task(struct work_struct *work) dev_err(&adapter->pdev->dev, "Hardware reset detected\n"); iavf_schedule_reset(adapter, IAVF_FLAG_RESET_PENDING); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); queue_delayed_work(adapter->wq, &adapter->watchdog_task, HZ * 2); return; } mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); restart_watchdog: if (adapter->state >= __IAVF_DOWN) queue_work(adapter->wq, &adapter->adminq_task); @@ -3015,12 +3015,12 @@ static void iavf_reset_task(struct work_struct *work) /* When device is being removed it doesn't make sense to run the reset * task, just return in such a case. */ - mutex_lock(&netdev->lock); + netdev_lock(netdev); if (!mutex_trylock(&adapter->crit_lock)) { if (adapter->state != __IAVF_REMOVE) queue_work(adapter->wq, &adapter->reset_task); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; } @@ -3068,7 +3068,7 @@ static void iavf_reset_task(struct work_struct *work) reg_val); iavf_disable_vf(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; /* Do not attempt to reinit. It's dead, Jim. */ } @@ -3209,7 +3209,7 @@ static void iavf_reset_task(struct work_struct *work) wake_up(&adapter->reset_waitqueue); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return; reset_err: @@ -3220,7 +3220,7 @@ static void iavf_reset_task(struct work_struct *work) iavf_disable_vf(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); dev_err(&adapter->pdev->dev, "failed to allocate resources during reinit\n"); } @@ -3692,10 +3692,10 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data) if (test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) return 0; - mutex_lock(&netdev->lock); + netdev_lock(netdev); netif_set_real_num_rx_queues(netdev, total_qps); netif_set_real_num_tx_queues(netdev, total_qps); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return ret; } @@ -4365,7 +4365,7 @@ static int iavf_open(struct net_device *netdev) return -EIO; } - mutex_lock(&netdev->lock); + 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 @@ -4373,7 +4373,7 @@ static int iavf_open(struct net_device *netdev) * We have to leave here to avoid dead lock. */ if (adapter->state == __IAVF_INIT_CONFIG_ADAPTER) { - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return -EBUSY; } @@ -4424,7 +4424,7 @@ static int iavf_open(struct net_device *netdev) iavf_irq_enable(adapter, true); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return 0; @@ -4437,7 +4437,7 @@ static int iavf_open(struct net_device *netdev) iavf_free_all_tx_resources(adapter); err_unlock: mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return err; } @@ -4459,12 +4459,12 @@ static int iavf_close(struct net_device *netdev) u64 aq_to_restore; int status; - mutex_lock(&netdev->lock); + netdev_lock(netdev); mutex_lock(&adapter->crit_lock); if (adapter->state <= __IAVF_DOWN_PENDING) { mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return 0; } @@ -4498,7 +4498,7 @@ static int iavf_close(struct net_device *netdev) iavf_free_traffic_irqs(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); /* We explicitly don't free resources here because the hardware is * still active and can DMA into memory. Resources are cleared in @@ -5375,7 +5375,7 @@ static int iavf_suspend(struct device *dev_d) netif_device_detach(netdev); - mutex_lock(&netdev->lock); + netdev_lock(netdev); mutex_lock(&adapter->crit_lock); if (netif_running(netdev)) { @@ -5387,7 +5387,7 @@ static int iavf_suspend(struct device *dev_d) iavf_reset_interrupt_capability(adapter); mutex_unlock(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); return 0; } @@ -5486,7 +5486,7 @@ static void iavf_remove(struct pci_dev *pdev) if (netdev->reg_state == NETREG_REGISTERED) unregister_netdev(netdev); - mutex_lock(&netdev->lock); + netdev_lock(netdev); mutex_lock(&adapter->crit_lock); dev_info(&adapter->pdev->dev, "Removing device\n"); iavf_change_state(adapter, __IAVF_REMOVE); @@ -5523,7 +5523,7 @@ static void iavf_remove(struct pci_dev *pdev) mutex_destroy(&hw->aq.asq_mutex); mutex_unlock(&adapter->crit_lock); mutex_destroy(&adapter->crit_lock); - mutex_unlock(&netdev->lock); + netdev_unlock(netdev); iounmap(hw->hw_addr); pci_release_regions(pdev); diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c index 5fe1eaef99b5..3f44a11aec83 100644 --- a/drivers/net/netdevsim/ethtool.c +++ b/drivers/net/netdevsim/ethtool.c @@ -103,10 +103,10 @@ nsim_set_channels(struct net_device *dev, struct ethtool_channels *ch) struct netdevsim *ns = netdev_priv(dev); int err; - mutex_lock(&dev->lock); + netdev_lock(dev); err = netif_set_real_num_queues(dev, ch->combined_count, ch->combined_count); - mutex_unlock(&dev->lock); + netdev_unlock(dev); if (err) return err; diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c index 15463062fe7b..7101a48bce54 100644 --- a/net/shaper/shaper.c +++ b/net/shaper/shaper.c @@ -40,7 +40,7 @@ static void net_shaper_lock(struct net_shaper_binding *binding) { switch (binding->type) { case NET_SHAPER_BINDING_TYPE_NETDEV: - mutex_lock(&binding->netdev->lock); + netdev_lock(binding->netdev); break; } } @@ -49,7 +49,7 @@ static void net_shaper_unlock(struct net_shaper_binding *binding) { switch (binding->type) { case NET_SHAPER_BINDING_TYPE_NETDEV: - mutex_unlock(&binding->netdev->lock); + netdev_unlock(binding->netdev); break; } } @@ -1398,7 +1398,7 @@ void net_shaper_set_real_num_tx_queues(struct net_device *dev, /* Only drivers implementing shapers support ensure * the lock is acquired in advance. */ - lockdep_assert_held(&dev->lock); + netdev_assert_locked(dev); /* Take action only when decreasing the tx queue number. */ for (i = txq; i < dev->real_num_tx_queues; ++i) { From patchwork Wed Jan 15 03:53:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939814 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B06A322E3F4 for ; Wed, 15 Jan 2025 03:53:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913218; cv=none; b=Y5rbqMG5QL0HgTP6RhpuDcc0/f6AmCcC3YUP3BZcNS6ar6ppUmkdZ4egyfnDIJzufpuTOb5WeOrZ7hEPUreJO2hRTb4wwALpgaRONWzdh/MMoqhn92zC0atODTDd/akTV11f0qtMjUTfMKzqV8pns0y5N/OJPrF21xWGfj6RKUk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913218; c=relaxed/simple; bh=9JEyl5sdkje6p3P7Kzrigfid34QwP8UKD23Ziugmw+Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=uos6hf9MWOxpD+gTy2pSMwcvR4cNWa5wfwZrwzN+8+YKOMOzP9ewuqad1CC/+//+GAFlcOKp5V8ozcrLgzlRk279aWRp+KfN5Q4NTIYuC9ecFP+n7kGbJk54Q9bu2qg324dVH6VKMJLKgedWJyhXoFHsIJqGJ4cxIWMRjOH31ew= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=YRy8UV3z; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="YRy8UV3z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D884EC4CEE2; Wed, 15 Jan 2025 03:53:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913218; bh=9JEyl5sdkje6p3P7Kzrigfid34QwP8UKD23Ziugmw+Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YRy8UV3z05haUCU97iJrfkf4zuCXmtZw43P7TL2AcmGzqgrfJlVxTk35n+FXdFbjd HO5MZ6qVqTj5IpKAe0WvcG5z9qqaDEBZ46wPrYqRQjZLJMrHqu11jfutzrQ3arJsXI a9VWr+G2qNJQEJvXGav8L3mbZhHI3h1FwggV4HxXAEegh4B/WkuIbCWDd0GSCzj+hw zC01M6ECX6Hb7oUG+rgfAOlbVeJXcnAOvDepu11se106j3FIFU5l/96ePH/r7z3CZx mxFTDb2FQoFldms6gFu6iBfAxraepvUXh8pMhUlnZkagWMTBWltmqOHSB0F87feLrU XTQzl0v/70XHg== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 02/11] net: make netdev_lock() protect netdev->reg_state Date: Tue, 14 Jan 2025 19:53:10 -0800 Message-ID: <20250115035319.559603-3-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Protect writes to netdev->reg_state with netdev_lock(). From now on holding netdev_lock() is sufficient to prevent the net_device from getting unregistered, so code which wants to hold just a single netdev around no longer needs to hold rtnl_lock. We do not protect the NETREG_UNREGISTERED -> NETREG_RELEASED transition. We'd need to move mutex_destroy(netdev->lock) to .release, but the real reason is that trying to stop the unregistration process mid-way would be unsafe / crazy. Taking references on such devices is not safe, either. So the intended semantics are to lock REGISTERED devices. Reviewed-by: Joe Damato Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- v2: - reorder with next patch v1: https://lore.kernel.org/20250114035118.110297-4-kuba@kernel.org --- include/linux/netdevice.h | 2 +- net/core/dev.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 891c5bdb894c..30963c5d409b 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2448,7 +2448,7 @@ struct net_device { * Should always be taken using netdev_lock() / netdev_unlock() helpers. * Drivers are free to use it for other protection. * - * Protects: @net_shaper_hierarchy. + * Protects: @reg_state, @net_shaper_hierarchy. * Ordering: take after rtnl_lock. */ struct mutex lock; diff --git a/net/core/dev.c b/net/core/dev.c index fda4e1039bf0..6603c08768f6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -10668,7 +10668,9 @@ int register_netdevice(struct net_device *dev) ret = netdev_register_kobject(dev); + netdev_lock(dev); WRITE_ONCE(dev->reg_state, ret ? NETREG_UNREGISTERED : NETREG_REGISTERED); + netdev_unlock(dev); if (ret) goto err_uninit_notify; @@ -10942,7 +10944,9 @@ void netdev_run_todo(void) continue; } + netdev_lock(dev); WRITE_ONCE(dev->reg_state, NETREG_UNREGISTERED); + netdev_unlock(dev); linkwatch_sync_dev(dev); } @@ -11548,7 +11552,9 @@ void unregister_netdevice_many_notify(struct list_head *head, list_for_each_entry(dev, head, unreg_list) { /* And unlink it from device chain. */ unlist_netdevice(dev); + netdev_lock(dev); WRITE_ONCE(dev->reg_state, NETREG_UNREGISTERING); + netdev_unlock(dev); } flush_all_backlogs(); From patchwork Wed Jan 15 03:53:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939815 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 12A5822E40A for ; Wed, 15 Jan 2025 03:53:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913219; cv=none; b=l7BN1y+awSbe9dtDQSDJ+prseXfmSenaMQzZBC9by9OcPoBQApChNFR7GiN5QxGiHDofNaJqolHdOHP2QNvV1mgf2T3+bQrFY+cAuTmRV7E3ewz59rEuvD8CssaNtt115HPbhycu5bgLZ8V4bQEOOm9pBA2DSJzHaDGlTBB4lXE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913219; c=relaxed/simple; bh=y/Qa4jSrQoepeNwZ4Prd7/DoPeFA/ecn7vKCQgm7NcU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=HCPXEd6QcOGaw45DtuB90V2SpgoEoKaH/2Eua7d4WjgMVHULIN94UvlTjpHtQDD2YVWL0lksvlkZxqtGCkMBc4/uEFhFGrKaMG/G6Msxq66HjCQmCfYNWiiioR3PyhaxPgVmO4lP7yyLop2RO1OcKfdF3dvg+Kme3W3DYn6uWkk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HsnNUxWj; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HsnNUxWj" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5E516C4CEDF; Wed, 15 Jan 2025 03:53:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913218; bh=y/Qa4jSrQoepeNwZ4Prd7/DoPeFA/ecn7vKCQgm7NcU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HsnNUxWjRpZGBhaK57SuEhxRNhm63odmiPrKfhcgLZhG9Kvh6Ejwu1lzAJmWBjjD8 I4u/u28whWl7vmu+P3nC4F4pQZRGZoc9OlMGx32J+RUD3XmZ+Tu/fYrPRgOf8+1mfd ikrLrofc21PvWuhfGx5+rLrnDaws/GF0Z1KFktl51yTS6CePeK3I+LnKBDFyKEQzE5 nH5PV+rHJKb0ZdHNNlTzr36jtDCMxxFFesTwddD+7CMtKb5/lWKRZRNRJHzZqY3Egf JDeg1/trl201KF3EwXikwPx3FsTRReRsX3/1+TbfGU1siw8FH05r3cHsmBdGg7arHV y/ntkTIe3wODw== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 03/11] net: add helpers for lookup and walking netdevs under netdev_lock() Date: Tue, 14 Jan 2025 19:53:11 -0800 Message-ID: <20250115035319.559603-4-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Add helpers for accessing netdevs under netdev_lock(). There's some careful handling needed to find the device and lock it safely, without it getting unregistered, and without taking rtnl_lock (the latter being the whole point of the new locking, after all). Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- v2: - add missing READ_ONCE() when under RCU v1: https://lore.kernel.org/20250114035118.110297-3-kuba@kernel.org --- net/core/dev.h | 16 +++++++ net/core/dev.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/net/core/dev.h b/net/core/dev.h index d8966847794c..25ae732c0775 100644 --- a/net/core/dev.h +++ b/net/core/dev.h @@ -2,6 +2,7 @@ #ifndef _NET_CORE_DEV_H #define _NET_CORE_DEV_H +#include #include #include #include @@ -23,8 +24,23 @@ struct sd_flow_limit { extern int netdev_flow_limit_table_len; struct napi_struct *netdev_napi_by_id(struct net *net, unsigned int napi_id); +struct napi_struct * +netdev_napi_by_id_lock(struct net *net, unsigned int napi_id); struct net_device *dev_get_by_napi_id(unsigned int napi_id); +struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex); +struct net_device *__netdev_put_lock(struct net_device *dev); +struct net_device * +netdev_xa_find_lock(struct net *net, struct net_device *dev, + unsigned long *index); + +DEFINE_FREE(netdev_unlock, struct net_device *, if (_T) netdev_unlock(_T)); + +#define for_each_netdev_lock_scoped(net, var_name, ifindex) \ + for (struct net_device *var_name __free(netdev_unlock) = NULL; \ + (var_name = netdev_xa_find_lock(net, var_name, &ifindex)); \ + ifindex++) + #ifdef CONFIG_PROC_FS int __init dev_proc_init(void); #else diff --git a/net/core/dev.c b/net/core/dev.c index 6603c08768f6..c871e2697fb6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -783,6 +783,49 @@ struct napi_struct *netdev_napi_by_id(struct net *net, unsigned int napi_id) return napi; } +/** + * netdev_napi_by_id_lock() - find a device by NAPI ID and lock it + * @net: the applicable net namespace + * @napi_id: ID of a NAPI of a target device + * + * Find a NAPI instance with @napi_id. Lock its device. + * The device must be in %NETREG_REGISTERED state for lookup to succeed. + * netdev_unlock() must be called to release it. + * + * Return: pointer to NAPI, its device with lock held, NULL if not found. + */ +struct napi_struct * +netdev_napi_by_id_lock(struct net *net, unsigned int napi_id) +{ + struct napi_struct *napi; + struct net_device *dev; + + rcu_read_lock(); + napi = netdev_napi_by_id(net, napi_id); + if (!napi || READ_ONCE(napi->dev->reg_state) != NETREG_REGISTERED) { + rcu_read_unlock(); + return NULL; + } + + dev = napi->dev; + dev_hold(dev); + rcu_read_unlock(); + + dev = __netdev_put_lock(dev); + if (!dev) + return NULL; + + rcu_read_lock(); + napi = netdev_napi_by_id(net, napi_id); + if (napi && napi->dev != dev) + napi = NULL; + rcu_read_unlock(); + + if (!napi) + netdev_unlock(dev); + return napi; +} + /** * __dev_get_by_name - find a device by its name * @net: the applicable net namespace @@ -971,6 +1014,73 @@ struct net_device *dev_get_by_napi_id(unsigned int napi_id) return napi ? napi->dev : NULL; } +/* Release the held reference on the net_device, and if the net_device + * is still registered try to lock the instance lock. If device is being + * unregistered NULL will be returned (but the reference has been released, + * either way!) + * + * This helper is intended for locking net_device after it has been looked up + * using a lockless lookup helper. Lock prevents the instance from going away. + */ +struct net_device *__netdev_put_lock(struct net_device *dev) +{ + netdev_lock(dev); + if (dev->reg_state > NETREG_REGISTERED) { + netdev_unlock(dev); + dev_put(dev); + return NULL; + } + dev_put(dev); + return dev; +} + +/** + * netdev_get_by_index_lock() - find a device by its ifindex + * @net: the applicable net namespace + * @ifindex: index of device + * + * Search for an interface by index. If a valid device + * with @ifindex is found it will be returned with netdev->lock held. + * netdev_unlock() must be called to release it. + * + * Return: pointer to a device with lock held, NULL if not found. + */ +struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex) +{ + struct net_device *dev; + + dev = dev_get_by_index(net, ifindex); + if (!dev) + return NULL; + + return __netdev_put_lock(dev); +} + +struct net_device * +netdev_xa_find_lock(struct net *net, struct net_device *dev, + unsigned long *index) +{ + if (dev) + netdev_unlock(dev); + + do { + rcu_read_lock(); + dev = xa_find(&net->dev_by_index, index, ULONG_MAX, XA_PRESENT); + if (!dev) { + rcu_read_unlock(); + return NULL; + } + dev_hold(dev); + rcu_read_unlock(); + + dev = __netdev_put_lock(dev); + if (dev) + return dev; + + (*index)++; + } while (true); +} + static DEFINE_SEQLOCK(netdev_rename_lock); void netdev_copy_name(struct net_device *dev, char *name) From patchwork Wed Jan 15 03:53:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939816 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 53DFE22E41D for ; Wed, 15 Jan 2025 03:53:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913219; cv=none; b=n6ygpK9ZRQ47RstB0wtYnwZfEX7JGwyZggPQ8/XGv4lF1PZQVvZdZLS2kqlcdaoPx9gMgJ2LpB+y5x8T3fSHmYwrEyg9La04cccOZ0K4CMFapYEN4Ru2Mlo9Vcr9UPLcaeSOn4bSYoOMMNg6vIMJUWC/F3eqlBbdi9qzEo3oS3Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913219; c=relaxed/simple; bh=0s2TaLPITNZgbrpX6AdtBSLUabw5dyWkhwRq5Z93L7o=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=j/IIgktajU5+ZgQ2zOh7DZSNTi5ae7AM2hgZVhHgAjgb9vY8jQB4AFSTPLxlDs+xitNcplNXg6wwbNqoKzBFumGqQdkX/XhUD+CrptNMHw9gQYRYUBq6EaFu4G/zDXzY/4FKd9stRiFmG95KT5H5nJp/Qrs10kQaFkfRraTM4G0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lv/rlPfI; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lv/rlPfI" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D3AF9C4CEE2; Wed, 15 Jan 2025 03:53:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913219; bh=0s2TaLPITNZgbrpX6AdtBSLUabw5dyWkhwRq5Z93L7o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lv/rlPfI/N89JdyYF7Cd+upntyCzbNeT5JcCmmPBv5Bl5w03f+RU1BLp4dcjZttsk YYQYSpymziP5YBNk33GVULTuhOKAuC71HKdrBBDZNK0sojbYRGiJe8wACYYZ1Ekyw3 4oR6+djdU1KgfCeBwhR0GmoGmbEQcs9E10y05WMHP5c6+sZgEHJ+q2jBmJeOb4KBux ra+eizzOevrKP945SIXf0crYGuWqKpRqOFPZKGmKhx40SJnKEbnv8j5Nr9fHvB8Rsk oZJII9vsT0k0+1MrgIZdzbokr4H8J5tXEml6Ys6+gWwRRtFZ1FFkpq2nAsqqPXlSJi kTY2q2PUB6b9A== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 04/11] net: add netdev->up protected by netdev_lock() Date: Tue, 14 Jan 2025 19:53:12 -0800 Message-ID: <20250115035319.559603-5-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Some uAPI (netdev netlink) hide net_device's sub-objects while the interface is down to ensure uniform behavior across drivers. To remove the rtnl_lock dependency from those uAPIs we need a way to safely tell if the device is down or up. Add an indication of whether device is open or closed, protected by netdev->lock. The semantics are the same as IFF_UP, but taking netdev_lock around every write to ->flags would be a lot of code churn. We don't want to blanket the entire open / close path by netdev_lock, because it will prevent us from applying it to specific structures - core helpers won't be able to take that lock from any function called by the drivers on open/close paths. So the state of the flag is "pessimistic", as in it may report false negatives, but never false positives. Reviewed-by: Joe Damato Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- v2: - rework the kdoc to please Sphinx v1: https://lore.kernel.org/20250114035118.110297-5-kuba@kernel.org --- include/linux/netdevice.h | 14 +++++++++++++- net/core/dev.h | 12 ++++++++++++ net/core/dev.c | 4 ++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 30963c5d409b..fdf3a8d93185 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2443,12 +2443,24 @@ struct net_device { unsigned long gro_flush_timeout; u32 napi_defer_hard_irqs; + /** + * @up: copy of @state's IFF_UP, but safe to read with just @lock. + * May report false negatives while the device is being opened + * or closed (@lock does not protect .ndo_open, or .ndo_close). + */ + bool up; + /** * @lock: netdev-scope lock, protects a small selection of fields. * Should always be taken using netdev_lock() / netdev_unlock() helpers. * Drivers are free to use it for other protection. * - * Protects: @reg_state, @net_shaper_hierarchy. + * Protects: + * @net_shaper_hierarchy, @reg_state + * + * Partially protects (writers must hold both @lock and rtnl_lock): + * @up + * * Ordering: take after rtnl_lock. */ struct mutex lock; diff --git a/net/core/dev.h b/net/core/dev.h index 25ae732c0775..ef37e2dd44f4 100644 --- a/net/core/dev.h +++ b/net/core/dev.h @@ -128,6 +128,18 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags, void unregister_netdevice_many_notify(struct list_head *head, u32 portid, const struct nlmsghdr *nlh); +static inline void netif_set_up(struct net_device *dev, bool value) +{ + if (value) + dev->flags |= IFF_UP; + else + dev->flags &= ~IFF_UP; + + netdev_lock(dev); + dev->up = value; + netdev_unlock(dev); +} + static inline void netif_set_gso_max_size(struct net_device *dev, unsigned int size) { diff --git a/net/core/dev.c b/net/core/dev.c index c871e2697fb6..4cba553a4742 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1618,7 +1618,7 @@ static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack) if (ret) clear_bit(__LINK_STATE_START, &dev->state); else { - dev->flags |= IFF_UP; + netif_set_up(dev, true); dev_set_rx_mode(dev); dev_activate(dev); add_device_randomness(dev->dev_addr, dev->addr_len); @@ -1697,7 +1697,7 @@ static void __dev_close_many(struct list_head *head) if (ops->ndo_stop) ops->ndo_stop(dev); - dev->flags &= ~IFF_UP; + netif_set_up(dev, false); netpoll_poll_enable(dev); } } From patchwork Wed Jan 15 03:53:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939817 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1C67F22F384 for ; Wed, 15 Jan 2025 03:53:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913220; cv=none; b=hJl82UyNQCwjmMdTPHub88yYvl8LeYWmXfYzstVF/+xxfhcrXyVVC9Cw8dqJr9GSCftyBvwXew/1/LjQA9pc7D9fSIRaGBoS6gEq/qizSSO/DF2GHY/BbfA7Z13Nh7AjCJ7NtF6yqowbGzUHr8nEznIPDchEKOWUL2W7k0Y92R8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913220; c=relaxed/simple; bh=YU5BJr5+AhcU+7nxIPi6zQfeg2hjxzOYzMVdeA1MHpM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=abyBJjMlGwiHaMAhhIrstHhSdlHq+tVl1mexKjEmGogveOs2UirHtIDw0tFAAb5BVDzCm1hYqiVUeQ1+hkGcVwcdXbEMox2q2M1FuqVgpXwfWDGObBXTk35tPovZ1xnoiASMxoZwnY2lgi+hgeiNEbpjCC2x730oN+XUHYZjyzU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=iIyNeoRt; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="iIyNeoRt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 58F21C4CEE5; Wed, 15 Jan 2025 03:53:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913219; bh=YU5BJr5+AhcU+7nxIPi6zQfeg2hjxzOYzMVdeA1MHpM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=iIyNeoRtvtKOWld4LblK/EvtREqfdrjq+4DaolrEwkRTQ9J/S2h4EXtt2na28azdv y8pkI4NqcxivrYkIeVXpqMCy7fhqaUlqthreOrXRv7fBU3stePMhGFdZnK5KG9A/rp jkLRmXPjhh5HQnJkOOk6vp5HTMel5iQRzbFrGWyvGipEeQKnX+msu5htLhjp3zA5We sZn5Z0/vAlBdV8XAvzbHOSWAVLi5bitibTUe52jkLThuVHM03wiVRg2xarL0GBJ9hC 0eldhJfokwFoihItTycOQnZUhSqClTYdRTY4STkJ/a37/FtG7c0oGjALvqsHIz/cRR uWufz0IiMIjkw== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 05/11] net: protect netdev->napi_list with netdev_lock() Date: Tue, 14 Jan 2025 19:53:13 -0800 Message-ID: <20250115035319.559603-6-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Hold netdev->lock when NAPIs are getting added or removed. This will allow safe access to NAPI instances of a net_device without rtnl_lock. Create a family of helpers which assume the lock is already taken. Switch iavf to them, as it makes extensive use of netdev->lock, already. Reviewed-by: Joe Damato Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- include/linux/netdevice.h | 54 ++++++++++++++++++--- drivers/net/ethernet/intel/iavf/iavf_main.c | 6 +-- net/core/dev.c | 15 ++++-- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index fdf3a8d93185..e8c8a5ea7326 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2456,7 +2456,7 @@ struct net_device { * Drivers are free to use it for other protection. * * Protects: - * @net_shaper_hierarchy, @reg_state + * @napi_list, @net_shaper_hierarchy, @reg_state * * Partially protects (writers must hold both @lock and rtnl_lock): * @up @@ -2712,8 +2712,19 @@ static inline void netif_napi_set_irq(struct napi_struct *napi, int irq) */ #define NAPI_POLL_WEIGHT 64 -void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi, - int (*poll)(struct napi_struct *, int), int weight); +void netif_napi_add_weight_locked(struct net_device *dev, + struct napi_struct *napi, + int (*poll)(struct napi_struct *, int), + int weight); + +static inline void +netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi, + int (*poll)(struct napi_struct *, int), int weight) +{ + netdev_lock(dev); + netif_napi_add_weight_locked(dev, napi, poll, weight); + netdev_unlock(dev); +} /** * netif_napi_add() - initialize a NAPI context @@ -2731,6 +2742,13 @@ netif_napi_add(struct net_device *dev, struct napi_struct *napi, netif_napi_add_weight(dev, napi, poll, NAPI_POLL_WEIGHT); } +static inline void +netif_napi_add_locked(struct net_device *dev, struct napi_struct *napi, + int (*poll)(struct napi_struct *, int)) +{ + netif_napi_add_weight_locked(dev, napi, poll, NAPI_POLL_WEIGHT); +} + static inline void netif_napi_add_tx_weight(struct net_device *dev, struct napi_struct *napi, @@ -2741,6 +2759,15 @@ netif_napi_add_tx_weight(struct net_device *dev, netif_napi_add_weight(dev, napi, poll, weight); } +static inline void +netif_napi_add_config_locked(struct net_device *dev, struct napi_struct *napi, + int (*poll)(struct napi_struct *, int), int index) +{ + napi->index = index; + napi->config = &dev->napi_config[index]; + netif_napi_add_weight_locked(dev, napi, poll, NAPI_POLL_WEIGHT); +} + /** * netif_napi_add_config - initialize a NAPI context with persistent config * @dev: network device @@ -2752,9 +2779,9 @@ static inline void netif_napi_add_config(struct net_device *dev, struct napi_struct *napi, int (*poll)(struct napi_struct *, int), int index) { - napi->index = index; - napi->config = &dev->napi_config[index]; - netif_napi_add_weight(dev, napi, poll, NAPI_POLL_WEIGHT); + netdev_lock(dev); + netif_napi_add_config_locked(dev, napi, poll, index); + netdev_unlock(dev); } /** @@ -2774,6 +2801,8 @@ static inline void netif_napi_add_tx(struct net_device *dev, netif_napi_add_tx_weight(dev, napi, poll, NAPI_POLL_WEIGHT); } +void __netif_napi_del_locked(struct napi_struct *napi); + /** * __netif_napi_del - remove a NAPI context * @napi: NAPI context @@ -2782,7 +2811,18 @@ static inline void netif_napi_add_tx(struct net_device *dev, * containing @napi. Drivers might want to call this helper to combine * all the needed RCU grace periods into a single one. */ -void __netif_napi_del(struct napi_struct *napi); +static inline void __netif_napi_del(struct napi_struct *napi) +{ + netdev_lock(napi->dev); + __netif_napi_del_locked(napi); + netdev_unlock(napi->dev); +} + +static inline void netif_napi_del_locked(struct napi_struct *napi) +{ + __netif_napi_del_locked(napi); + synchronize_net(); +} /** * netif_napi_del - remove a NAPI context diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index ab908d620285..2db97c5d9f9e 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -1800,8 +1800,8 @@ static int iavf_alloc_q_vectors(struct iavf_adapter *adapter) q_vector->v_idx = q_idx; q_vector->reg_idx = q_idx; cpumask_copy(&q_vector->affinity_mask, cpu_possible_mask); - netif_napi_add(adapter->netdev, &q_vector->napi, - iavf_napi_poll); + netif_napi_add_locked(adapter->netdev, &q_vector->napi, + iavf_napi_poll); } return 0; @@ -1827,7 +1827,7 @@ static void iavf_free_q_vectors(struct iavf_adapter *adapter) for (q_idx = 0; q_idx < num_q_vectors; q_idx++) { struct iavf_q_vector *q_vector = &adapter->q_vectors[q_idx]; - netif_napi_del(&q_vector->napi); + netif_napi_del_locked(&q_vector->napi); } kfree(adapter->q_vectors); adapter->q_vectors = NULL; diff --git a/net/core/dev.c b/net/core/dev.c index 4cba553a4742..7511207057e5 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6909,9 +6909,12 @@ netif_napi_dev_list_add(struct net_device *dev, struct napi_struct *napi) list_add_rcu(&napi->dev_list, higher); /* adds after higher */ } -void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi, - int (*poll)(struct napi_struct *, int), int weight) +void netif_napi_add_weight_locked(struct net_device *dev, + struct napi_struct *napi, + int (*poll)(struct napi_struct *, int), + int weight) { + netdev_assert_locked(dev); if (WARN_ON(test_and_set_bit(NAPI_STATE_LISTED, &napi->state))) return; @@ -6952,7 +6955,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi, dev->threaded = false; netif_napi_set_irq(napi, -1); } -EXPORT_SYMBOL(netif_napi_add_weight); +EXPORT_SYMBOL(netif_napi_add_weight_locked); void napi_disable(struct napi_struct *n) { @@ -7023,8 +7026,10 @@ static void flush_gro_hash(struct napi_struct *napi) } /* Must be called in process context */ -void __netif_napi_del(struct napi_struct *napi) +void __netif_napi_del_locked(struct napi_struct *napi) { + netdev_assert_locked(napi->dev); + if (!test_and_clear_bit(NAPI_STATE_LISTED, &napi->state)) return; @@ -7044,7 +7049,7 @@ void __netif_napi_del(struct napi_struct *napi) napi->thread = NULL; } } -EXPORT_SYMBOL(__netif_napi_del); +EXPORT_SYMBOL(__netif_napi_del_locked); static int __napi_poll(struct napi_struct *n, bool *repoll) { From patchwork Wed Jan 15 03:53:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939819 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2F53322F399 for ; Wed, 15 Jan 2025 03:53:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; cv=none; b=sSRw5w+J6JXmwEq0y4rJVuG4GxKXU3qo+K+KIVSUl3xgpUwLRNzgwwKY8YVSnYOoAHD5KY1auhclyx+VpqIGBmPcfEaabMl3bcAWpvaSVwibvWQgFrCR7Mv5YbgLCr58cHZwZTl2TiChmki4M1t0I5kHdEznXbJN6iofQgMsGbY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; c=relaxed/simple; bh=OgxCKVDd6gB1ntWzMAk5eOTDn1EsoLc45FuC2+NwPOo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=MxIJxk7fuXQrs/kvSce30ocJz97tE1PWqyvRQysamUBWXyymnFZ9kub2mTp3r0DbyZlTMFlzrlE+gqp5Jr9eqnIzYsFFRqJ3B2TcJm5Ob8jkN4Lj8pA7pXBjIWS6WQ/m32FxyBC9Q7WV41BmrPYk+TSBUx+CXN87Vh6xgwwAHzs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hbXAj4Kd; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hbXAj4Kd" Received: by smtp.kernel.org (Postfix) with ESMTPSA id CA33EC4CEDF; Wed, 15 Jan 2025 03:53:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913220; bh=OgxCKVDd6gB1ntWzMAk5eOTDn1EsoLc45FuC2+NwPOo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hbXAj4Kd422g9S0/j+9fnLalDsSWYDIueIFLvTq6KUquJDM3oGvJ+X9HlfN//DSF3 uYXB5XrnbZHDpJ8uBjlSVd6i3tjKnfD2YocbmS3G79uZT6yeD2Xvc8oqL/io9S2BFF DVOO0IoqCkzjADsa0pPS8SPlR+ET6laHXaOLdsWtudKhYJvPcMvbSbrPTIjN3NJ78j nrOibjgEfkkhGI4cOxIhpiL99fjPqyNaODrpKtpG122K2ZgyNgu0tj0pTK+dIFcm17 zWA16TvXVYUfcVvgb0OCdpSj22SMvUdHNfUT+MVJByhJW60fdgbMPcSRKm1om3yimC zjLtokM5pQrsg== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski , Francois Romieu , pcnet32@frontier.com, anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com, marcin.s.wojtas@gmail.com Subject: [PATCH net-next v2 06/11] net: protect NAPI enablement with netdev_lock() Date: Tue, 14 Jan 2025 19:53:14 -0800 Message-ID: <20250115035319.559603-7-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Wrap napi_enable() / napi_disable() with netdev_lock(). Provide the "already locked" flavor of the API. iavf needs the usual adjustment. A number of drivers call napi_enable() under a spin lock, so they have to be modified to take netdev_lock() first, then spin lock then call napi_enable_locked(). Protecting napi_enable() implies that napi->napi_id is protected by netdev_lock(). Acked-by: Francois Romieu # via-velocity Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- v2: - use napi_disabled_locked() in via-velocity v1: https://lore.kernel.org/20250114035118.110297-7-kuba@kernel.org CC: pcnet32@frontier.com CC: anthony.l.nguyen@intel.com CC: przemyslaw.kitszel@intel.com CC: marcin.s.wojtas@gmail.com CC: romieu@fr.zoreil.com --- include/linux/netdevice.h | 11 ++---- drivers/net/ethernet/amd/pcnet32.c | 11 +++++- drivers/net/ethernet/intel/iavf/iavf_main.c | 4 +- drivers/net/ethernet/marvell/mvneta.c | 5 ++- drivers/net/ethernet/via/via-velocity.c | 6 ++- net/core/dev.c | 41 +++++++++++++++++---- 6 files changed, 56 insertions(+), 22 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index e8c8a5ea7326..4ab33fbadd9f 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -382,7 +382,7 @@ struct napi_struct { struct sk_buff *skb; struct list_head rx_list; /* Pending GRO_NORMAL skbs */ int rx_count; /* length of rx_list */ - unsigned int napi_id; + unsigned int napi_id; /* protected by netdev_lock */ struct hrtimer timer; struct task_struct *thread; unsigned long gro_flush_timeout; @@ -570,16 +570,11 @@ static inline bool napi_complete(struct napi_struct *n) int dev_set_threaded(struct net_device *dev, bool threaded); -/** - * napi_disable - prevent NAPI from scheduling - * @n: NAPI context - * - * Stop NAPI from being scheduled on this context. - * Waits till any outstanding processing completes. - */ void napi_disable(struct napi_struct *n); +void napi_disable_locked(struct napi_struct *n); void napi_enable(struct napi_struct *n); +void napi_enable_locked(struct napi_struct *n); /** * napi_synchronize - wait until NAPI is not running diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c index 72db9f9e7bee..c6bd803f5b0c 100644 --- a/drivers/net/ethernet/amd/pcnet32.c +++ b/drivers/net/ethernet/amd/pcnet32.c @@ -462,7 +462,7 @@ static void pcnet32_netif_start(struct net_device *dev) val = lp->a->read_csr(ioaddr, CSR3); val &= 0x00ff; lp->a->write_csr(ioaddr, CSR3, val); - napi_enable(&lp->napi); + napi_enable_locked(&lp->napi); } /* @@ -889,6 +889,7 @@ static int pcnet32_set_ringparam(struct net_device *dev, if (netif_running(dev)) pcnet32_netif_stop(dev); + netdev_lock(dev); spin_lock_irqsave(&lp->lock, flags); lp->a->write_csr(ioaddr, CSR0, CSR0_STOP); /* stop the chip */ @@ -920,6 +921,7 @@ static int pcnet32_set_ringparam(struct net_device *dev, } spin_unlock_irqrestore(&lp->lock, flags); + netdev_unlock(dev); netif_info(lp, drv, dev, "Ring Param Settings: RX: %d, TX: %d\n", lp->rx_ring_size, lp->tx_ring_size); @@ -985,6 +987,7 @@ static int pcnet32_loopback_test(struct net_device *dev, uint64_t * data1) if (netif_running(dev)) pcnet32_netif_stop(dev); + netdev_lock(dev); spin_lock_irqsave(&lp->lock, flags); lp->a->write_csr(ioaddr, CSR0, CSR0_STOP); /* stop the chip */ @@ -1122,6 +1125,7 @@ static int pcnet32_loopback_test(struct net_device *dev, uint64_t * data1) lp->a->write_bcr(ioaddr, 20, 4); /* return to 16bit mode */ } spin_unlock_irqrestore(&lp->lock, flags); + netdev_unlock(dev); return rc; } /* end pcnet32_loopback_test */ @@ -2101,6 +2105,7 @@ static int pcnet32_open(struct net_device *dev) return -EAGAIN; } + netdev_lock(dev); spin_lock_irqsave(&lp->lock, flags); /* Check for a valid station address */ if (!is_valid_ether_addr(dev->dev_addr)) { @@ -2266,7 +2271,7 @@ static int pcnet32_open(struct net_device *dev) goto err_free_ring; } - napi_enable(&lp->napi); + napi_enable_locked(&lp->napi); /* Re-initialize the PCNET32, and start it when done. */ lp->a->write_csr(ioaddr, 1, (lp->init_dma_addr & 0xffff)); @@ -2300,6 +2305,7 @@ static int pcnet32_open(struct net_device *dev) lp->a->read_csr(ioaddr, CSR0)); spin_unlock_irqrestore(&lp->lock, flags); + netdev_unlock(dev); return 0; /* Always succeed */ @@ -2315,6 +2321,7 @@ static int pcnet32_open(struct net_device *dev) err_free_irq: spin_unlock_irqrestore(&lp->lock, flags); + netdev_unlock(dev); free_irq(dev->irq, dev); return rc; } diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 2db97c5d9f9e..cbfaaa5b7d02 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -1180,7 +1180,7 @@ static void iavf_napi_enable_all(struct iavf_adapter *adapter) q_vector = &adapter->q_vectors[q_idx]; napi = &q_vector->napi; - napi_enable(napi); + napi_enable_locked(napi); } } @@ -1196,7 +1196,7 @@ static void iavf_napi_disable_all(struct iavf_adapter *adapter) for (q_idx = 0; q_idx < q_vectors; q_idx++) { q_vector = &adapter->q_vectors[q_idx]; - napi_disable(&q_vector->napi); + napi_disable_locked(&q_vector->napi); } } diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index fe6261b81540..cc97474852ef 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -4392,6 +4392,7 @@ static int mvneta_cpu_online(unsigned int cpu, struct hlist_node *node) if (pp->neta_armada3700) return 0; + netdev_lock(port->napi.dev); spin_lock(&pp->lock); /* * Configuring the driver for a new CPU while the driver is @@ -4418,7 +4419,7 @@ static int mvneta_cpu_online(unsigned int cpu, struct hlist_node *node) /* Mask all ethernet port interrupts */ on_each_cpu(mvneta_percpu_mask_interrupt, pp, true); - napi_enable(&port->napi); + napi_enable_locked(&port->napi); /* * Enable per-CPU interrupts on the CPU that is @@ -4439,6 +4440,8 @@ static int mvneta_cpu_online(unsigned int cpu, struct hlist_node *node) MVNETA_CAUSE_LINK_CHANGE); netif_tx_start_all_queues(pp->dev); spin_unlock(&pp->lock); + netdev_unlock(port->napi.dev); + return 0; } diff --git a/drivers/net/ethernet/via/via-velocity.c b/drivers/net/ethernet/via/via-velocity.c index dd4a07c97eee..5aa93144a4f5 100644 --- a/drivers/net/ethernet/via/via-velocity.c +++ b/drivers/net/ethernet/via/via-velocity.c @@ -2320,7 +2320,8 @@ static int velocity_change_mtu(struct net_device *dev, int new_mtu) if (ret < 0) goto out_free_tmp_vptr_1; - napi_disable(&vptr->napi); + netdev_lock(dev); + napi_disable_locked(&vptr->napi); spin_lock_irqsave(&vptr->lock, flags); @@ -2342,12 +2343,13 @@ static int velocity_change_mtu(struct net_device *dev, int new_mtu) velocity_give_many_rx_descs(vptr); - napi_enable(&vptr->napi); + napi_enable_locked(&vptr->napi); mac_enable_int(vptr->mac_regs); netif_start_queue(dev); spin_unlock_irqrestore(&vptr->lock, flags); + netdev_unlock(dev); velocity_free_rings(tmp_vptr); diff --git a/net/core/dev.c b/net/core/dev.c index 7511207057e5..9cf93868ac7e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6957,11 +6957,13 @@ void netif_napi_add_weight_locked(struct net_device *dev, } EXPORT_SYMBOL(netif_napi_add_weight_locked); -void napi_disable(struct napi_struct *n) +void napi_disable_locked(struct napi_struct *n) { unsigned long val, new; might_sleep(); + netdev_assert_locked(n->dev); + set_bit(NAPI_STATE_DISABLE, &n->state); val = READ_ONCE(n->state); @@ -6984,16 +6986,25 @@ void napi_disable(struct napi_struct *n) clear_bit(NAPI_STATE_DISABLE, &n->state); } -EXPORT_SYMBOL(napi_disable); +EXPORT_SYMBOL(napi_disable_locked); /** - * napi_enable - enable NAPI scheduling - * @n: NAPI context + * napi_disable() - prevent NAPI from scheduling + * @n: NAPI context * - * Resume NAPI from being scheduled on this context. - * Must be paired with napi_disable. + * Stop NAPI from being scheduled on this context. + * Waits till any outstanding processing completes. + * Takes netdev_lock() for associated net_device. */ -void napi_enable(struct napi_struct *n) +void napi_disable(struct napi_struct *n) +{ + netdev_lock(n->dev); + napi_disable_locked(n); + netdev_unlock(n->dev); +} +EXPORT_SYMBOL(napi_disable); + +void napi_enable_locked(struct napi_struct *n) { unsigned long new, val = READ_ONCE(n->state); @@ -7010,6 +7021,22 @@ void napi_enable(struct napi_struct *n) new |= NAPIF_STATE_THREADED; } while (!try_cmpxchg(&n->state, &val, new)); } +EXPORT_SYMBOL(napi_enable_locked); + +/** + * napi_enable() - enable NAPI scheduling + * @n: NAPI context + * + * Enable scheduling of a NAPI instance. + * Must be paired with napi_disable(). + * Takes netdev_lock() for associated net_device. + */ +void napi_enable(struct napi_struct *n) +{ + netdev_lock(n->dev); + napi_enable_locked(n); + netdev_unlock(n->dev); +} EXPORT_SYMBOL(napi_enable); static void flush_gro_hash(struct napi_struct *napi) From patchwork Wed Jan 15 03:53:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939818 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2F4A722F397 for ; Wed, 15 Jan 2025 03:53:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; cv=none; b=R5fFrt6p8ozq2Ai17CuyAR14ZmmSeEKIBbksuOo/PPvXfFYK0/zhUSz6e6dp1NUfZ1vCCNdphsuwj53oJFcn43f3yGwBDygtoCG96Co3MSZUJh0VQqs+s34ZWdeIJQH2TZ0kWfEcHIXZdCBds7Pq/bWsMnXG+W0YtBT595yLqqY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; c=relaxed/simple; bh=4jodZjNT8rZQ3LqPSS9vu73sgZn78TXGULYqlc9RbVY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CbkgSyDM6WBVk6xzU5XXWMmIDz68cLRUBTSnw1Rb/fK9w416f+QOB177ZdLyHftwvMnESisYhPM8y6orSvnel8QKNNHlOStwBRC1MaNF3fKKH6h3l69vmVlewAMIV3SFje0bEgOaAe6AeouofZlpZAFR5l98of1HoscD0rhazOs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=NJ8zSO82; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="NJ8zSO82" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 72E93C4CEE6; Wed, 15 Jan 2025 03:53:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913220; bh=4jodZjNT8rZQ3LqPSS9vu73sgZn78TXGULYqlc9RbVY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NJ8zSO82bwvIqiBJCXG2bo8gNi1POz/W0RjZM1JYLv+cdsNF1+5mRScE6Z6beP6yR hHh3FEbABy2pEnD1UhHB00sXP9GT3ntCwL+7ACI+K4kZ+teAIgDnPB7whRXEd+ZOTh i3+A0g0ieyab7CRc4lWpKnB14eyFu8FyyxVJJQTHaHmZeJl99bJFN+btswlcFPdk2j KbC0cFCjNP8O+S5PKpO6vKXLl0QXiPyAe2xcgf00hto8/CqHimgMv/1DNuM9Xw6UVs xKuXv2evT2c6ww/Vjn9JjUskQqDaZuJOzk8ozIrUkdN958mLoT5p9NxEFGdBk/fxh1 Gv/Sq7NHdjDEQ== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 07/11] net: make netdev netlink ops hold netdev_lock() Date: Tue, 14 Jan 2025 19:53:15 -0800 Message-ID: <20250115035319.559603-8-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org In prep for dropping rtnl_lock, start locking netdev->lock in netlink genl ops. We need to be using netdev->up instead of flags & IFF_UP. We can remove the RCU lock protection for the NAPI since NAPI list is protected by netdev->lock already. Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- net/core/dev.h | 1 - net/core/dev.c | 3 ++- net/core/netdev-genl.c | 46 +++++++++++++++++++++++------------------- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/net/core/dev.h b/net/core/dev.h index ef37e2dd44f4..a5b166bbd169 100644 --- a/net/core/dev.h +++ b/net/core/dev.h @@ -23,7 +23,6 @@ struct sd_flow_limit { extern int netdev_flow_limit_table_len; -struct napi_struct *netdev_napi_by_id(struct net *net, unsigned int napi_id); struct napi_struct * netdev_napi_by_id_lock(struct net *net, unsigned int napi_id); struct net_device *dev_get_by_napi_id(unsigned int napi_id); diff --git a/net/core/dev.c b/net/core/dev.c index 9cf93868ac7e..9734c3f5b862 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -767,7 +767,8 @@ static struct napi_struct *napi_by_id(unsigned int napi_id) } /* must be called under rcu_read_lock(), as we dont take a reference */ -struct napi_struct *netdev_napi_by_id(struct net *net, unsigned int napi_id) +static struct napi_struct * +netdev_napi_by_id(struct net *net, unsigned int napi_id) { struct napi_struct *napi; diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index c59619a2ec23..810a446ab62c 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -167,7 +167,7 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi, void *hdr; pid_t pid; - if (!(napi->dev->flags & IFF_UP)) + if (!napi->dev->up) return 0; hdr = genlmsg_iput(rsp, info); @@ -230,17 +230,16 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info) return -ENOMEM; rtnl_lock(); - rcu_read_lock(); - napi = netdev_napi_by_id(genl_info_net(info), napi_id); + napi = netdev_napi_by_id_lock(genl_info_net(info), napi_id); if (napi) { err = netdev_nl_napi_fill_one(rsp, napi, info); + netdev_unlock(napi->dev); } else { NL_SET_BAD_ATTR(info->extack, info->attrs[NETDEV_A_NAPI_ID]); err = -ENOENT; } - rcu_read_unlock(); rtnl_unlock(); if (err) { @@ -266,7 +265,7 @@ netdev_nl_napi_dump_one(struct net_device *netdev, struct sk_buff *rsp, unsigned int prev_id; int err = 0; - if (!(netdev->flags & IFF_UP)) + if (!netdev->up) return err; prev_id = UINT_MAX; @@ -303,13 +302,15 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) rtnl_lock(); if (ifindex) { - netdev = __dev_get_by_index(net, ifindex); - if (netdev) + netdev = netdev_get_by_index_lock(net, ifindex); + if (netdev) { err = netdev_nl_napi_dump_one(netdev, skb, info, ctx); - else + netdev_unlock(netdev); + } else { err = -ENODEV; + } } else { - for_each_netdev_dump(net, netdev, ctx->ifindex) { + for_each_netdev_lock_scoped(net, netdev, ctx->ifindex) { err = netdev_nl_napi_dump_one(netdev, skb, info, ctx); if (err < 0) break; @@ -358,17 +359,16 @@ int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info) napi_id = nla_get_u32(info->attrs[NETDEV_A_NAPI_ID]); rtnl_lock(); - rcu_read_lock(); - napi = netdev_napi_by_id(genl_info_net(info), napi_id); + napi = netdev_napi_by_id_lock(genl_info_net(info), napi_id); if (napi) { err = netdev_nl_napi_set_config(napi, info); + netdev_unlock(napi->dev); } else { NL_SET_BAD_ATTR(info->extack, info->attrs[NETDEV_A_NAPI_ID]); err = -ENOENT; } - rcu_read_unlock(); rtnl_unlock(); return err; @@ -442,7 +442,7 @@ netdev_nl_queue_fill(struct sk_buff *rsp, struct net_device *netdev, u32 q_idx, { int err; - if (!(netdev->flags & IFF_UP)) + if (!netdev->up) return -ENOENT; err = netdev_nl_queue_validate(netdev, q_idx, q_type); @@ -474,11 +474,13 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info) rtnl_lock(); - netdev = __dev_get_by_index(genl_info_net(info), ifindex); - if (netdev) + netdev = netdev_get_by_index_lock(genl_info_net(info), ifindex); + if (netdev) { err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info); - else + netdev_unlock(netdev); + } else { err = -ENODEV; + } rtnl_unlock(); @@ -499,7 +501,7 @@ netdev_nl_queue_dump_one(struct net_device *netdev, struct sk_buff *rsp, { int err = 0; - if (!(netdev->flags & IFF_UP)) + if (!netdev->up) return err; for (; ctx->rxq_idx < netdev->real_num_rx_queues; ctx->rxq_idx++) { @@ -532,13 +534,15 @@ int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) rtnl_lock(); if (ifindex) { - netdev = __dev_get_by_index(net, ifindex); - if (netdev) + netdev = netdev_get_by_index_lock(net, ifindex); + if (netdev) { err = netdev_nl_queue_dump_one(netdev, skb, info, ctx); - else + netdev_unlock(netdev); + } else { err = -ENODEV; + } } else { - for_each_netdev_dump(net, netdev, ctx->ifindex) { + for_each_netdev_lock_scoped(net, netdev, ctx->ifindex) { err = netdev_nl_queue_dump_one(netdev, skb, info, ctx); if (err < 0) break; From patchwork Wed Jan 15 03:53:16 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939820 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A44B322DC5D for ; Wed, 15 Jan 2025 03:53:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; cv=none; b=Rlv+KHYL/0LF6AN0efvnOwUCM2q6Gm7FYRH2lpjGvnib5dz0ki7sJAsmN7OdWB8ibwRcxrhJaCaZvFqc676Lcbu8d/uI7qYwp/KlIayVo2F48WuyF8r7VinF+FvF+DWvlAeHdkCLYxD0A83xeH6XsFf1uE5OVMRdfN0WDsNf3+4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; c=relaxed/simple; bh=XZMCyfzUH5kA6wnjtBdIQuynGd66w3NqmqTblX+cAHU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=LrDPyuh064Xv8jci+ZvcBDDjm6/3STDWBWonkkr16uiZo5pYJhHzdh6mInaouHGUm3Wj3bwoZ2yMfClCTXh7sOgGNbz1cdwLhleaj7P64PHBLppk48dxqWDSdiyhuSAqjZkLQCdU8Tdzane3nirJtiINXIzqaySYGjL8bdVkFek= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Mqs5rJ/y; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Mqs5rJ/y" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E7449C4CEE2; Wed, 15 Jan 2025 03:53:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913221; bh=XZMCyfzUH5kA6wnjtBdIQuynGd66w3NqmqTblX+cAHU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Mqs5rJ/yfVYd2lkgVhD54Dzk3bQDjwBk//EgdSbWfpkjLZM5FJ0PDBkJHulPC1qfi hE0yd25wOJ3puOpSfHwV+okWwSKWQhr75z6AfbxC82vhj6pqNEnH6TtaMrZ3QHjG5L QjTTsbcWNcMk7LSdvsTh7DA+xfdjA1G8NzZTV7U99WRxsLvwFsPMlkDacMo281j/ug L/VKWCKYK1NY6+xYk/wnrpv/0ExOOrHMez4AHJHRFtok3xMW9XmJWnmqcETArRdopz 7ThIbqbm5/ru8VQJowiKyBYOswU+xjJCQACQURMaCFrh9C4bqsYeffmpSnrIjJSuLB 8QptyevquH/gg== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski , leitao@debian.org Subject: [PATCH net-next v2 08/11] net: protect threaded status of NAPI with netdev_lock() Date: Tue, 14 Jan 2025 19:53:16 -0800 Message-ID: <20250115035319.559603-9-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Now that NAPI instances can't come and go without holding netdev->lock we can trivially switch from rtnl_lock() to netdev_lock() for setting netdev->threaded via sysfs. Note that since we do not lock netdev_lock around sysfs calls in the core we don't have to "trylock" like we do with rtnl_lock. Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- v2: - update the comment on dev_isalive() v1: https://lore.kernel.org/20250114035118.110297-9-kuba@kernel.org CC: leitao@debian.org --- include/linux/netdevice.h | 13 +++++++++++-- net/core/dev.c | 2 ++ net/core/net-sysfs.c | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 4ab33fbadd9f..bf3da95c9350 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -384,7 +384,7 @@ struct napi_struct { int rx_count; /* length of rx_list */ unsigned int napi_id; /* protected by netdev_lock */ struct hrtimer timer; - struct task_struct *thread; + struct task_struct *thread; /* protected by netdev_lock */ unsigned long gro_flush_timeout; unsigned long irq_suspend_timeout; u32 defer_hard_irqs; @@ -2451,11 +2451,13 @@ struct net_device { * Drivers are free to use it for other protection. * * Protects: - * @napi_list, @net_shaper_hierarchy, @reg_state + * @napi_list, @net_shaper_hierarchy, @reg_state, @threaded * * Partially protects (writers must hold both @lock and rtnl_lock): * @up * + * Also protects some fields in struct napi_struct. + * * Ordering: take after rtnl_lock. */ struct mutex lock; @@ -2697,6 +2699,13 @@ static inline void netdev_assert_locked(struct net_device *dev) lockdep_assert_held(&dev->lock); } +static inline void netdev_assert_locked_or_invisible(struct net_device *dev) +{ + if (dev->reg_state == NETREG_REGISTERED || + dev->reg_state == NETREG_UNREGISTERING) + netdev_assert_locked(dev); +} + static inline void netif_napi_set_irq(struct napi_struct *napi, int irq) { napi->irq = irq; diff --git a/net/core/dev.c b/net/core/dev.c index 9734c3f5b862..d90bb100285d 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6784,6 +6784,8 @@ int dev_set_threaded(struct net_device *dev, bool threaded) struct napi_struct *napi; int err = 0; + netdev_assert_locked_or_invisible(dev); + if (dev->threaded == threaded) return 0; diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 2d9afc6e2161..9365a7185a1d 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -36,7 +36,7 @@ static const char fmt_uint[] = "%u\n"; static const char fmt_ulong[] = "%lu\n"; static const char fmt_u64[] = "%llu\n"; -/* Caller holds RTNL or RCU */ +/* Caller holds RTNL, netdev->lock or RCU */ static inline int dev_isalive(const struct net_device *dev) { return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED; @@ -108,6 +108,36 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr, return ret; } +/* Same as netdev_store() but takes netdev_lock() instead of rtnl_lock() */ +static ssize_t +netdev_lock_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len, + int (*set)(struct net_device *, unsigned long)) +{ + struct net_device *netdev = to_net_dev(dev); + struct net *net = dev_net(netdev); + unsigned long new; + int ret; + + if (!ns_capable(net->user_ns, CAP_NET_ADMIN)) + return -EPERM; + + ret = kstrtoul(buf, 0, &new); + if (ret) + return ret; + + netdev_lock(netdev); + + if (dev_isalive(netdev)) { + ret = (*set)(netdev, new); + if (ret == 0) + ret = len; + } + netdev_unlock(netdev); + + return ret; +} + NETDEVICE_SHOW_RO(dev_id, fmt_hex); NETDEVICE_SHOW_RO(dev_port, fmt_dec); NETDEVICE_SHOW_RO(addr_assign_type, fmt_dec); @@ -638,7 +668,7 @@ static ssize_t threaded_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t len) { - return netdev_store(dev, attr, buf, len, modify_napi_threaded); + return netdev_lock_store(dev, attr, buf, len, modify_napi_threaded); } static DEVICE_ATTR_RW(threaded); From patchwork Wed Jan 15 03:53:17 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939821 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D7BEB22F3B1 for ; Wed, 15 Jan 2025 03:53:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; cv=none; b=Nuv2WUpsBRfiCBxqulKrXyBH7wROdNa3QFl20efCbkExxC+Rr04xuGhGiPLW9wXtBEm0T2xLO0QYw9XpzbIZ8DkU6Fh9HpvC3eoujssQpoQJRySJGQRGnjZZRK78Xekhn917y0rFWpmS48WEVfxCXykj661v+ET8/GrdBw8aRQQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913221; c=relaxed/simple; bh=D9w2XEcMFHLvKicI7xIIwq/in2BDlW4Yhb8NuHCc0B8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=rwZxOVNCExC5tVOIZL6z9WZlVgZZ09I94AWO4HcrFlyuyIVmXZI+eYRsDOOdWHIiSHi19aLxK1mWYnMT6HyOgQogtRcN7Io0bsdqwBf0DsAmcz4ZKWc8ON3tuvHrEJl+sz5RQCiA7dRq7dEib06mPWpidbE6qUJH8WKLFKr0+1w= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=sUDye+To; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="sUDye+To" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 69585C4CEE6; Wed, 15 Jan 2025 03:53:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913221; bh=D9w2XEcMFHLvKicI7xIIwq/in2BDlW4Yhb8NuHCc0B8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sUDye+TogU9uhf4JIOEfZQ0E7bWmknJHKSgJguYwgMNyCA0VglrjEu9soxQS6bC1I n3oE0TTlSwBi2ZFCwn6el8UYEinVbYDVPyAytS0wlaR3ughF44byZOldUVD9H4/CMW YJqLNAZJp3TvPyuFH6iyczEmFOsPGNPM6bSOtqP3euYxYz5VcqIVjej2lpF2vJVOPi a/Tz+HzpJOeb9hhtJyYz0iHxAqYqpkZGS0xJAJkQlQppnIsEITllO13qD/sXypzDOo bfRjosfS4AoIK0GdrJhQVqCUntsJYk9oyLFp6kTq3k1DUIfqBF/cHw5T4GlWZdrYmE eWEZpcz+jzYLg== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 09/11] net: protect napi->irq with netdev_lock() Date: Tue, 14 Jan 2025 19:53:17 -0800 Message-ID: <20250115035319.559603-10-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Take netdev_lock() in netif_napi_set_irq(). All NAPI "control fields" are now protected by that lock (most of the other ones are set during napi add/del). The napi_hash_node is fully protected by the hash spin lock, but close enough for the kdoc... Reviewed-by: Joe Damato Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- include/linux/netdevice.h | 10 +++++++++- net/core/dev.c | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index bf3da95c9350..390fb70667bf 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -388,6 +388,7 @@ struct napi_struct { unsigned long gro_flush_timeout; unsigned long irq_suspend_timeout; u32 defer_hard_irqs; + /* all fields past this point are write-protected by netdev_lock */ /* control-path-only fields follow */ struct list_head dev_list; struct hlist_node napi_hash_node; @@ -2706,11 +2707,18 @@ static inline void netdev_assert_locked_or_invisible(struct net_device *dev) netdev_assert_locked(dev); } -static inline void netif_napi_set_irq(struct napi_struct *napi, int irq) +static inline void netif_napi_set_irq_locked(struct napi_struct *napi, int irq) { napi->irq = irq; } +static inline void netif_napi_set_irq(struct napi_struct *napi, int irq) +{ + netdev_lock(napi->dev); + netif_napi_set_irq_locked(napi, irq); + netdev_unlock(napi->dev); +} + /* Default NAPI poll() weight * Device drivers are strongly advised to not use bigger value */ diff --git a/net/core/dev.c b/net/core/dev.c index d90bb100285d..495ceefcdc34 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6956,7 +6956,7 @@ void netif_napi_add_weight_locked(struct net_device *dev, */ if (dev->threaded && napi_kthread_create(napi)) dev->threaded = false; - netif_napi_set_irq(napi, -1); + netif_napi_set_irq_locked(napi, -1); } EXPORT_SYMBOL(netif_napi_add_weight_locked); From patchwork Wed Jan 15 03:53:18 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939822 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6099422F822 for ; Wed, 15 Jan 2025 03:53:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913222; cv=none; b=hmvjbgz+w6A5HYwcvRLaZTp/B/9MP10wVoJylx9Trpz2aNGMQ8CODV2ZE216z7TtPYOtI2QT8fW7R9wQFrj2pbiOFrQjqpxE9nqKI14X5euMxqiE/NnZkQ47H9Qaz3crG6hYFPmbECEPy4VWOunYZCwJcysOB6sQtEoAU76tCd0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913222; c=relaxed/simple; bh=vTnzmHtGahe7bsE6KbOJEQ3kZZy4qG4BsKrcxqQVetc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=KtXZYbJoBtX+Wk0gMa24XHb21uFb0dLxsMWv+hgZO3kRJ3NxYMD8XeWAAUzeBIRUlvFc8+QydHxoqPsnGZj0xL1cLW+67cMFqcUrR27EtSZYRuGjHYDqc0rrsRBeBnYM7UuWRUMivw3G3NOs3/2SbQxsJ/jIDEN/VDprKuzR64c= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TTXbVQ6Z; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TTXbVQ6Z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E222CC4CEDF; Wed, 15 Jan 2025 03:53:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913222; bh=vTnzmHtGahe7bsE6KbOJEQ3kZZy4qG4BsKrcxqQVetc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TTXbVQ6Z2b+YvNCfJAe/lnCKjc/0MEZ6ySH9Nq7iSM77eJRcM2SEJcYxued8nKbHL 3Q/GBqy7d7m2J+9WgxQ1x6xWT8FGxvI1ucLlWM4MEVXdesfTzLCTifhWTln78qwXit Q12l8MQp0+JNEU61KIH7hOnb43+ZSPhH55ReXlWsxhnhvQkN73PaizCInHIW1hJrr/ NPRPgpDHhN8miq6EnRysHLArJgZsq1JJUFQMqnU0FVb8DvPeeVUoFrRR/bcc2C4Hlu DdPX4RAbaKe0Ez12IVZRtdOHozJ/6jM8X6eOr9KySzf02s9KjvfgLAXfe3fE3PJcVA ALiQufa5Lo6GA== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 10/11] net: protect NAPI config fields with netdev_lock() Date: Tue, 14 Jan 2025 19:53:18 -0800 Message-ID: <20250115035319.559603-11-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Protect the following members of netdev and napi by netdev_lock: - defer_hard_irqs, - gro_flush_timeout, - irq_suspend_timeout. The first two are written via sysfs (which this patch switches to new lock), and netdev genl which holds both netdev and rtnl locks. irq_suspend_timeout is only written by netdev genl. Reviewed-by: Joe Damato Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- include/linux/netdevice.h | 7 ++++--- net/core/net-sysfs.c | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 390fb70667bf..6f1e5725aca9 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -384,11 +384,11 @@ struct napi_struct { int rx_count; /* length of rx_list */ unsigned int napi_id; /* protected by netdev_lock */ struct hrtimer timer; - struct task_struct *thread; /* protected by netdev_lock */ + /* all fields past this point are write-protected by netdev_lock */ + struct task_struct *thread; unsigned long gro_flush_timeout; unsigned long irq_suspend_timeout; u32 defer_hard_irqs; - /* all fields past this point are write-protected by netdev_lock */ /* control-path-only fields follow */ struct list_head dev_list; struct hlist_node napi_hash_node; @@ -2452,7 +2452,8 @@ struct net_device { * Drivers are free to use it for other protection. * * Protects: - * @napi_list, @net_shaper_hierarchy, @reg_state, @threaded + * @gro_flush_timeout, @napi_defer_hard_irqs, @napi_list, + * @net_shaper_hierarchy, @reg_state, @threaded * * Partially protects (writers must hold both @lock and rtnl_lock): * @up diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 9365a7185a1d..07cb99b114bd 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -450,7 +450,7 @@ static ssize_t gro_flush_timeout_store(struct device *dev, if (!capable(CAP_NET_ADMIN)) return -EPERM; - return netdev_store(dev, attr, buf, len, change_gro_flush_timeout); + return netdev_lock_store(dev, attr, buf, len, change_gro_flush_timeout); } NETDEVICE_SHOW_RW(gro_flush_timeout, fmt_ulong); @@ -470,7 +470,8 @@ static ssize_t napi_defer_hard_irqs_store(struct device *dev, if (!capable(CAP_NET_ADMIN)) return -EPERM; - return netdev_store(dev, attr, buf, len, change_napi_defer_hard_irqs); + return netdev_lock_store(dev, attr, buf, len, + change_napi_defer_hard_irqs); } NETDEVICE_SHOW_RW(napi_defer_hard_irqs, fmt_uint); From patchwork Wed Jan 15 03:53:19 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13939823 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D9FF022F3A0 for ; Wed, 15 Jan 2025 03:53:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913222; cv=none; b=FJ9oYAXkzaOskcZNqfr1XGBz82QOBt41OuQieozifdfrok1DyUtf+wM8Us+Z/ifelIWQ7C8YgYzPI7cEnSa0miSH7+jJ2mYs9W7Rp4GTAB+RhOW99dqymiid1RqOF2jaFMZ9sTGwqSUI615zEj5dtwgPQ/gK5Zxef7BtRIlVUGk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736913222; c=relaxed/simple; bh=rzT1dRW5dy8Gl8R8yjhI5yMch2GXNUo4fdukpwdqlEo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=hr76NRmXbtCtVSB4sBc8nnir3bXVV0+R+Be2jJM+z3QWTWpNVk0AYBB/AGvgFNLkwkYvKKe9PkyfSkKN2NqmB+iKZg0uNMAR1thp7YpX8cu5vxm0MEgmGYHeSxINBvCVz1JHpWGMT1fnY7c7YhK5YQFeCmiojAy30SjscgrgyUw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ZBAsxA+h; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ZBAsxA+h" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6792AC4CEE4; Wed, 15 Jan 2025 03:53:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736913222; bh=rzT1dRW5dy8Gl8R8yjhI5yMch2GXNUo4fdukpwdqlEo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZBAsxA+h9LF/E9vF2O1pmXTG1MI/fc+QTIBcWK8z8QC2qiYmcGQsI3aUyywv5hRfu 17bkaH3Ba8wdzbZXMmX2kBqVk5EJq+t9vQxXZUBPzc+YgBCjsAjy/+An1bDxsW+MmC QqAuVc1h1jn6zQcBv83VaPKTlh1SWzLzUdvr1cwTRYCrFcSCsSNk957O6iJ+4fQEJL 0lN0gOibJq3LAXpleVW3SejaGCtbtESd40fwrExiaQv8qC4iFg++/W1wb/KOCCXulv otS1RIRqTN4JuFRMz9mdKimuRtIFfbv9+fWjgzHt8RW8pC/JaXdaHKdzmTcNI14m9A HjcsSc1rorbrg== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com, Jakub Kicinski Subject: [PATCH net-next v2 11/11] netdev-genl: remove rtnl_lock protection from NAPI ops Date: Tue, 14 Jan 2025 19:53:19 -0800 Message-ID: <20250115035319.559603-12-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 In-Reply-To: <20250115035319.559603-1-kuba@kernel.org> References: <20250115035319.559603-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org NAPI lifetime, visibility and config are all fully under netdev_lock protection now. Reviewed-by: Joe Damato Reviewed-by: Eric Dumazet Signed-off-by: Jakub Kicinski Reviewed-by: Kuniyuki Iwashima --- net/core/netdev-genl.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index 810a446ab62c..715f85c6b62e 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -229,8 +229,6 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info) if (!rsp) return -ENOMEM; - rtnl_lock(); - napi = netdev_napi_by_id_lock(genl_info_net(info), napi_id); if (napi) { err = netdev_nl_napi_fill_one(rsp, napi, info); @@ -240,8 +238,6 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info) err = -ENOENT; } - rtnl_unlock(); - if (err) { goto err_free_msg; } else if (!rsp->len) { @@ -300,7 +296,6 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) if (info->attrs[NETDEV_A_NAPI_IFINDEX]) ifindex = nla_get_u32(info->attrs[NETDEV_A_NAPI_IFINDEX]); - rtnl_lock(); if (ifindex) { netdev = netdev_get_by_index_lock(net, ifindex); if (netdev) { @@ -317,7 +312,6 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) ctx->napi_id = 0; } } - rtnl_unlock(); return err; } @@ -358,8 +352,6 @@ int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info) napi_id = nla_get_u32(info->attrs[NETDEV_A_NAPI_ID]); - rtnl_lock(); - napi = netdev_napi_by_id_lock(genl_info_net(info), napi_id); if (napi) { err = netdev_nl_napi_set_config(napi, info); @@ -369,8 +361,6 @@ int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info) err = -ENOENT; } - rtnl_unlock(); - return err; }