Commit 346b3416 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'gtp-geneve-suppress-list_del-splat-during-exit_batch_rtnl' 

Kuniyuki Iwashima says:

====================
gtp/geneve: Suppress list_del() splat during ->exit_batch_rtnl().

The common pattern in tunnel device's ->exit_batch_rtnl() is iterating
two netdev lists for each netns: (i) for_each_netdev() to clean up
devices in the netns, and (ii) the device type specific list to clean
up devices in other netns.

	list_for_each_entry(net, net_list, exit_list) {
		for_each_netdev_safe(net, dev, next) {
			/* (i)  call unregister_netdevice_queue(dev, list) */
		}

		list_for_each_entry_safe(xxx, xxx_next, &net->yyy, zzz) {
			/* (ii) call unregister_netdevice_queue(xxx->dev, list) */
		}
	}

Then, ->exit_batch_rtnl() could touch the same device twice.

Say we have two netns A & B and device B that is created in netns A and
moved to netns B.

  1. cleanup_net() processes netns A and then B.

  2. ->exit_batch_rtnl() finds the device B while iterating netns A's (ii)

  [ device B is not yet unlinked from netns B as
    unregister_netdevice_many() has not been called. ]

  3. ->exit_batch_rtnl() finds the device B while iterating netns B's (i)

gtp and geneve calls ->dellink() at 2. and 3. that calls list_del() for (ii)
and unregister_netdevice_queue().

Calling unregister_netdevice_queue() twice is fine because it uses
list_move_tail(), but the 2nd list_del() triggers a splat when
CONFIG_DEBUG_LIST is enabled.

Possible solution is either of

 (a) Use list_del_init() in ->dellink()

 (b) Iterate dev with empty ->unreg_list for (i) like

	#define for_each_netdev_alive(net, d)				\
		list_for_each_entry(d, &(net)->dev_base_head, dev_list)	\
			if (list_empty(&d->unreg_list))

 (c) Remove (i) and delegate it to default_device_exit_batch().

This series avoids the 2nd ->dellink() by (c) to suppress the splat for
gtp and geneve.

Note that IPv4/IPv6 tunnels calls just unregister_netdevice() during
->exit_batch_rtnl() and dev is unlinked from (ii) later in ->ndo_uninit(),
so they are safe.

Also, pfcp has the same pattern but is safe because
unregister_netdevice_many() is called for each netns.
====================

Link: https://patch.msgid.link/20250217203705.40342-1-kuniyu@amazon.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents a92c3228 62fab6ee

drivers/net/geneve.c

+0 −7
Original line number Diff line number Diff line
@@ -1902,14 +1902,7 @@ static void geneve_destroy_tunnels(struct net *net, struct list_head *head) 
{

	struct geneve_net *gn = net_generic(net, geneve_net_id);

	struct geneve_dev *geneve, *next;

	struct net_device *dev, *aux;



	/* gather any geneve devices that were moved into this ns */

	for_each_netdev_safe(net, dev, aux)

		if (dev->rtnl_link_ops == &geneve_link_ops)

			geneve_dellink(dev, head);



	/* now gather any other geneve devices that were created in this ns */

	list_for_each_entry_safe(geneve, next, &gn->geneve_list, next)

		geneve_dellink(geneve->dev, head);

}

drivers/net/gtp.c

+0 −5
Original line number Diff line number Diff line
@@ -2481,11 +2481,6 @@ static void __net_exit gtp_net_exit_batch_rtnl(struct list_head *net_list, 
	list_for_each_entry(net, net_list, exit_list) {

		struct gtp_net *gn = net_generic(net, gtp_net_id);

		struct gtp_dev *gtp, *gtp_next;

		struct net_device *dev;



		for_each_netdev(net, dev)

			if (dev->rtnl_link_ops == &gtp_link_ops)

				gtp_dellink(dev, dev_to_kill);



		list_for_each_entry_safe(gtp, gtp_next, &gn->gtp_dev_list, list)

			gtp_dellink(gtp->dev, dev_to_kill);