* [PATCH net-next v2 0/4] net: inline rollback_registered() functions
@ 2021-01-19 20:25 Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 1/4] net: move net_set_todo inside rollback_registered() Jakub Kicinski
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Jakub Kicinski @ 2021-01-19 20:25 UTC (permalink / raw)
To: davem; +Cc: netdev, Jakub Kicinski
After recent changes to the error path of register_netdevice()
we no longer need a version of unregister_netdevice_many() which
does not set net_todo. We can inline the rollback_registered()
functions into respective unregister_netdevice() calls.
v2: - add missing list_del() in the last patch
Jakub Kicinski (4):
net: move net_set_todo inside rollback_registered()
net: inline rollback_registered()
net: move rollback_registered_many()
net: inline rollback_registered_many()
net/core/dev.c | 210 +++++++++++++++++++++++--------------------------
1 file changed, 98 insertions(+), 112 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v2 1/4] net: move net_set_todo inside rollback_registered()
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
@ 2021-01-19 20:25 ` Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 2/4] net: inline rollback_registered() Jakub Kicinski
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2021-01-19 20:25 UTC (permalink / raw)
To: davem; +Cc: netdev, Jakub Kicinski
Commit 93ee31f14f6f ("[NET]: Fix free_netdev on register_netdev
failure.") moved net_set_todo() outside of rollback_registered()
so that rollback_registered() can be used in the failure path of
register_netdevice() but without risking a double free.
Since commit cf124db566e6 ("net: Fix inconsistent teardown and
release of private netdev state."), however, we have a better
way of handling that condition, since destructors don't call
free_netdev() directly.
After the change in commit c269a24ce057 ("net: make free_netdev()
more lenient with unregistering devices") we can now move
net_set_todo() back.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/dev.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6b90520a01b1..5f928b51c6b0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9546,8 +9546,10 @@ static void rollback_registered_many(struct list_head *head)
synchronize_net();
- list_for_each_entry(dev, head, unreg_list)
+ list_for_each_entry(dev, head, unreg_list) {
dev_put(dev);
+ net_set_todo(dev);
+ }
}
static void rollback_registered(struct net_device *dev)
@@ -10104,7 +10106,6 @@ int register_netdevice(struct net_device *dev)
/* Expect explicit free_netdev() on failure */
dev->needs_free_netdev = false;
rollback_registered(dev);
- net_set_todo(dev);
goto out;
}
/*
@@ -10727,8 +10728,6 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head)
list_move_tail(&dev->unreg_list, head);
} else {
rollback_registered(dev);
- /* Finish processing unregister after unlock */
- net_set_todo(dev);
}
}
EXPORT_SYMBOL(unregister_netdevice_queue);
@@ -10742,12 +10741,8 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
*/
void unregister_netdevice_many(struct list_head *head)
{
- struct net_device *dev;
-
if (!list_empty(head)) {
rollback_registered_many(head);
- list_for_each_entry(dev, head, unreg_list)
- net_set_todo(dev);
list_del(head);
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v2 2/4] net: inline rollback_registered()
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 1/4] net: move net_set_todo inside rollback_registered() Jakub Kicinski
@ 2021-01-19 20:25 ` Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 3/4] net: move rollback_registered_many() Jakub Kicinski
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2021-01-19 20:25 UTC (permalink / raw)
To: davem; +Cc: netdev, Jakub Kicinski
rollback_registered() is a local helper, it's common for driver
code to call unregister_netdevice_queue(dev, NULL) when they
want to unregister netdevices under rtnl_lock. Inline
rollback_registered() and adjust the only remaining caller.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/dev.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5f928b51c6b0..85e9d4b7ddf2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9552,15 +9552,6 @@ static void rollback_registered_many(struct list_head *head)
}
}
-static void rollback_registered(struct net_device *dev)
-{
- LIST_HEAD(single);
-
- list_add(&dev->unreg_list, &single);
- rollback_registered_many(&single);
- list_del(&single);
-}
-
static netdev_features_t netdev_sync_upper_features(struct net_device *lower,
struct net_device *upper, netdev_features_t features)
{
@@ -10105,7 +10096,7 @@ int register_netdevice(struct net_device *dev)
if (ret) {
/* Expect explicit free_netdev() on failure */
dev->needs_free_netdev = false;
- rollback_registered(dev);
+ unregister_netdevice_queue(dev, NULL);
goto out;
}
/*
@@ -10727,7 +10718,11 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head)
if (head) {
list_move_tail(&dev->unreg_list, head);
} else {
- rollback_registered(dev);
+ LIST_HEAD(single);
+
+ list_add(&dev->unreg_list, &single);
+ rollback_registered_many(&single);
+ list_del(&single);
}
}
EXPORT_SYMBOL(unregister_netdevice_queue);
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v2 3/4] net: move rollback_registered_many()
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 1/4] net: move net_set_todo inside rollback_registered() Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 2/4] net: inline rollback_registered() Jakub Kicinski
@ 2021-01-19 20:25 ` Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 4/4] net: inline rollback_registered_many() Jakub Kicinski
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2021-01-19 20:25 UTC (permalink / raw)
To: davem; +Cc: netdev, Jakub Kicinski
Move rollback_registered_many() and add a temporary
forward declaration to make merging the code into
unregister_netdevice_many() easier to review.
No functional changes.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/dev.c | 188 +++++++++++++++++++++++++------------------------
1 file changed, 95 insertions(+), 93 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 85e9d4b7ddf2..a7841d03c910 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9459,99 +9459,6 @@ static void net_set_todo(struct net_device *dev)
dev_net(dev)->dev_unreg_count++;
}
-static void rollback_registered_many(struct list_head *head)
-{
- struct net_device *dev, *tmp;
- LIST_HEAD(close_head);
-
- BUG_ON(dev_boot_phase);
- ASSERT_RTNL();
-
- list_for_each_entry_safe(dev, tmp, head, unreg_list) {
- /* Some devices call without registering
- * for initialization unwind. Remove those
- * devices and proceed with the remaining.
- */
- if (dev->reg_state == NETREG_UNINITIALIZED) {
- pr_debug("unregister_netdevice: device %s/%p never was registered\n",
- dev->name, dev);
-
- WARN_ON(1);
- list_del(&dev->unreg_list);
- continue;
- }
- dev->dismantle = true;
- BUG_ON(dev->reg_state != NETREG_REGISTERED);
- }
-
- /* If device is running, close it first. */
- list_for_each_entry(dev, head, unreg_list)
- list_add_tail(&dev->close_list, &close_head);
- dev_close_many(&close_head, true);
-
- list_for_each_entry(dev, head, unreg_list) {
- /* And unlink it from device chain. */
- unlist_netdevice(dev);
-
- dev->reg_state = NETREG_UNREGISTERING;
- }
- flush_all_backlogs();
-
- synchronize_net();
-
- list_for_each_entry(dev, head, unreg_list) {
- struct sk_buff *skb = NULL;
-
- /* Shutdown queueing discipline. */
- dev_shutdown(dev);
-
- dev_xdp_uninstall(dev);
-
- /* Notify protocols, that we are about to destroy
- * this device. They should clean all the things.
- */
- call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
-
- if (!dev->rtnl_link_ops ||
- dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U, 0,
- GFP_KERNEL, NULL, 0);
-
- /*
- * Flush the unicast and multicast chains
- */
- dev_uc_flush(dev);
- dev_mc_flush(dev);
-
- netdev_name_node_alt_flush(dev);
- netdev_name_node_free(dev->name_node);
-
- if (dev->netdev_ops->ndo_uninit)
- dev->netdev_ops->ndo_uninit(dev);
-
- if (skb)
- rtmsg_ifinfo_send(skb, dev, GFP_KERNEL);
-
- /* Notifier chain MUST detach us all upper devices. */
- WARN_ON(netdev_has_any_upper_dev(dev));
- WARN_ON(netdev_has_any_lower_dev(dev));
-
- /* Remove entries from kobject tree */
- netdev_unregister_kobject(dev);
-#ifdef CONFIG_XPS
- /* Remove XPS queueing entries */
- netif_reset_xps_queues_gt(dev, 0);
-#endif
- }
-
- synchronize_net();
-
- list_for_each_entry(dev, head, unreg_list) {
- dev_put(dev);
- net_set_todo(dev);
- }
-}
-
static netdev_features_t netdev_sync_upper_features(struct net_device *lower,
struct net_device *upper, netdev_features_t features)
{
@@ -10698,6 +10605,8 @@ void synchronize_net(void)
}
EXPORT_SYMBOL(synchronize_net);
+static void rollback_registered_many(struct list_head *head);
+
/**
* unregister_netdevice_queue - remove device from the kernel
* @dev: device
@@ -10743,6 +10652,99 @@ void unregister_netdevice_many(struct list_head *head)
}
EXPORT_SYMBOL(unregister_netdevice_many);
+static void rollback_registered_many(struct list_head *head)
+{
+ struct net_device *dev, *tmp;
+ LIST_HEAD(close_head);
+
+ BUG_ON(dev_boot_phase);
+ ASSERT_RTNL();
+
+ list_for_each_entry_safe(dev, tmp, head, unreg_list) {
+ /* Some devices call without registering
+ * for initialization unwind. Remove those
+ * devices and proceed with the remaining.
+ */
+ if (dev->reg_state == NETREG_UNINITIALIZED) {
+ pr_debug("unregister_netdevice: device %s/%p never was registered\n",
+ dev->name, dev);
+
+ WARN_ON(1);
+ list_del(&dev->unreg_list);
+ continue;
+ }
+ dev->dismantle = true;
+ BUG_ON(dev->reg_state != NETREG_REGISTERED);
+ }
+
+ /* If device is running, close it first. */
+ list_for_each_entry(dev, head, unreg_list)
+ list_add_tail(&dev->close_list, &close_head);
+ dev_close_many(&close_head, true);
+
+ list_for_each_entry(dev, head, unreg_list) {
+ /* And unlink it from device chain. */
+ unlist_netdevice(dev);
+
+ dev->reg_state = NETREG_UNREGISTERING;
+ }
+ flush_all_backlogs();
+
+ synchronize_net();
+
+ list_for_each_entry(dev, head, unreg_list) {
+ struct sk_buff *skb = NULL;
+
+ /* Shutdown queueing discipline. */
+ dev_shutdown(dev);
+
+ dev_xdp_uninstall(dev);
+
+ /* Notify protocols, that we are about to destroy
+ * this device. They should clean all the things.
+ */
+ call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
+
+ if (!dev->rtnl_link_ops ||
+ dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
+ skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U, 0,
+ GFP_KERNEL, NULL, 0);
+
+ /*
+ * Flush the unicast and multicast chains
+ */
+ dev_uc_flush(dev);
+ dev_mc_flush(dev);
+
+ netdev_name_node_alt_flush(dev);
+ netdev_name_node_free(dev->name_node);
+
+ if (dev->netdev_ops->ndo_uninit)
+ dev->netdev_ops->ndo_uninit(dev);
+
+ if (skb)
+ rtmsg_ifinfo_send(skb, dev, GFP_KERNEL);
+
+ /* Notifier chain MUST detach us all upper devices. */
+ WARN_ON(netdev_has_any_upper_dev(dev));
+ WARN_ON(netdev_has_any_lower_dev(dev));
+
+ /* Remove entries from kobject tree */
+ netdev_unregister_kobject(dev);
+#ifdef CONFIG_XPS
+ /* Remove XPS queueing entries */
+ netif_reset_xps_queues_gt(dev, 0);
+#endif
+ }
+
+ synchronize_net();
+
+ list_for_each_entry(dev, head, unreg_list) {
+ dev_put(dev);
+ net_set_todo(dev);
+ }
+}
+
/**
* unregister_netdev - remove device from the kernel
* @dev: device
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v2 4/4] net: inline rollback_registered_many()
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
` (2 preceding siblings ...)
2021-01-19 20:25 ` [PATCH net-next v2 3/4] net: move rollback_registered_many() Jakub Kicinski
@ 2021-01-19 20:25 ` Jakub Kicinski
2021-01-19 23:18 ` [PATCH net-next v2 0/4] net: inline rollback_registered() functions Edwin Peer
2021-01-21 5:10 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2021-01-19 20:25 UTC (permalink / raw)
To: davem; +Cc: netdev, Jakub Kicinski
Similar to the change for rollback_registered() -
rollback_registered_many() was a part of unregister_netdevice_many()
minus the net_set_todo(), which is no longer needed.
Functionally this patch moves the list_empty() check back after:
BUG_ON(dev_boot_phase);
ASSERT_RTNL();
but I can't find any reason why that would be an issue.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/dev.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index a7841d03c910..dee6488f8a31 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5709,7 +5709,7 @@ static void flush_all_backlogs(void)
}
/* we can have in flight packet[s] on the cpus we are not flushing,
- * synchronize_net() in rollback_registered_many() will take care of
+ * synchronize_net() in unregister_netdevice_many() will take care of
* them
*/
for_each_cpu(cpu, &flush_cpus)
@@ -10605,8 +10605,6 @@ void synchronize_net(void)
}
EXPORT_SYMBOL(synchronize_net);
-static void rollback_registered_many(struct list_head *head);
-
/**
* unregister_netdevice_queue - remove device from the kernel
* @dev: device
@@ -10630,8 +10628,7 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head)
LIST_HEAD(single);
list_add(&dev->unreg_list, &single);
- rollback_registered_many(&single);
- list_del(&single);
+ unregister_netdevice_many(&single);
}
}
EXPORT_SYMBOL(unregister_netdevice_queue);
@@ -10644,15 +10641,6 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
* we force a list_del() to make sure stack wont be corrupted later.
*/
void unregister_netdevice_many(struct list_head *head)
-{
- if (!list_empty(head)) {
- rollback_registered_many(head);
- list_del(head);
- }
-}
-EXPORT_SYMBOL(unregister_netdevice_many);
-
-static void rollback_registered_many(struct list_head *head)
{
struct net_device *dev, *tmp;
LIST_HEAD(close_head);
@@ -10660,6 +10648,9 @@ static void rollback_registered_many(struct list_head *head)
BUG_ON(dev_boot_phase);
ASSERT_RTNL();
+ if (list_empty(head))
+ return;
+
list_for_each_entry_safe(dev, tmp, head, unreg_list) {
/* Some devices call without registering
* for initialization unwind. Remove those
@@ -10743,7 +10734,10 @@ static void rollback_registered_many(struct list_head *head)
dev_put(dev);
net_set_todo(dev);
}
+
+ list_del(head);
}
+EXPORT_SYMBOL(unregister_netdevice_many);
/**
* unregister_netdev - remove device from the kernel
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 0/4] net: inline rollback_registered() functions
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
` (3 preceding siblings ...)
2021-01-19 20:25 ` [PATCH net-next v2 4/4] net: inline rollback_registered_many() Jakub Kicinski
@ 2021-01-19 23:18 ` Edwin Peer
2021-01-21 5:05 ` Jakub Kicinski
2021-01-21 5:10 ` patchwork-bot+netdevbpf
5 siblings, 1 reply; 8+ messages in thread
From: Edwin Peer @ 2021-01-19 23:18 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David S . Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 784 bytes --]
On Tue, Jan 19, 2021 at 12:32 PM Jakub Kicinski <kuba@kernel.org> wrote:
> After recent changes to the error path of register_netdevice()
> we no longer need a version of unregister_netdevice_many() which
> does not set net_todo. We can inline the rollback_registered()
> functions into respective unregister_netdevice() calls.
>
> v2: - add missing list_del() in the last patch
>
> Jakub Kicinski (4):
> net: move net_set_todo inside rollback_registered()
> net: inline rollback_registered()
> net: move rollback_registered_many()
> net: inline rollback_registered_many()
>
> net/core/dev.c | 210 +++++++++++++++++++++++--------------------------
> 1 file changed, 98 insertions(+), 112 deletions(-)
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Regards,
Edwin Peer
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 0/4] net: inline rollback_registered() functions
2021-01-19 23:18 ` [PATCH net-next v2 0/4] net: inline rollback_registered() functions Edwin Peer
@ 2021-01-21 5:05 ` Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2021-01-21 5:05 UTC (permalink / raw)
To: Edwin Peer; +Cc: David S . Miller, netdev
On Tue, 19 Jan 2021 15:18:46 -0800 Edwin Peer wrote:
> On Tue, Jan 19, 2021 at 12:32 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> > After recent changes to the error path of register_netdevice()
> > we no longer need a version of unregister_netdevice_many() which
> > does not set net_todo. We can inline the rollback_registered()
> > functions into respective unregister_netdevice() calls.
> >
> > v2: - add missing list_del() in the last patch
> >
> > Jakub Kicinski (4):
> > net: move net_set_todo inside rollback_registered()
> > net: inline rollback_registered()
> > net: move rollback_registered_many()
> > net: inline rollback_registered_many()
> >
> > net/core/dev.c | 210 +++++++++++++++++++++++--------------------------
> > 1 file changed, 98 insertions(+), 112 deletions(-)
>
> Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 0/4] net: inline rollback_registered() functions
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
` (4 preceding siblings ...)
2021-01-19 23:18 ` [PATCH net-next v2 0/4] net: inline rollback_registered() functions Edwin Peer
@ 2021-01-21 5:10 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-21 5:10 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Tue, 19 Jan 2021 12:25:17 -0800 you wrote:
> After recent changes to the error path of register_netdevice()
> we no longer need a version of unregister_netdevice_many() which
> does not set net_todo. We can inline the rollback_registered()
> functions into respective unregister_netdevice() calls.
>
> v2: - add missing list_del() in the last patch
>
> [...]
Here is the summary with links:
- [net-next,v2,1/4] net: move net_set_todo inside rollback_registered()
https://git.kernel.org/netdev/net-next/c/2014beea7eb1
- [net-next,v2,2/4] net: inline rollback_registered()
https://git.kernel.org/netdev/net-next/c/037e56bd965e
- [net-next,v2,3/4] net: move rollback_registered_many()
https://git.kernel.org/netdev/net-next/c/bcfe2f1a3818
- [net-next,v2,4/4] net: inline rollback_registered_many()
https://git.kernel.org/netdev/net-next/c/0cbe1e57a7b9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-21 5:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 20:25 [PATCH net-next v2 0/4] net: inline rollback_registered() functions Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 1/4] net: move net_set_todo inside rollback_registered() Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 2/4] net: inline rollback_registered() Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 3/4] net: move rollback_registered_many() Jakub Kicinski
2021-01-19 20:25 ` [PATCH net-next v2 4/4] net: inline rollback_registered_many() Jakub Kicinski
2021-01-19 23:18 ` [PATCH net-next v2 0/4] net: inline rollback_registered() functions Edwin Peer
2021-01-21 5:05 ` Jakub Kicinski
2021-01-21 5:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).