[PATCH v1 net-next 0/7] ip_tunnel: Support per-netns device unregistration.

[PATCH v1 net-next 0/7] ip_tunnel: Support per-netns device unregistration.

[Kuniyuki Iwashima]

Patch 1 - 3 are prep patches to unlink ip_tunnel device from
the hash table in ip_tunnel_dellink() only.

Patch 4 removes ip_tunnel_del() from ip_tunnel_uninit() to
ip_tunnel_dellink().

Patch 5 & 6 adds mutex to protect the per-netns hash table.

Patch 7 uses unregister_netdevice_queue_net() to support
cross-netns device unregistration.


Kuniyuki Iwashima (7):
  ipmr: Call ->dellink() to remove DVMRP tunnel device.
  ip_tunnel: Set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net().
  ip_tunnel: Don't pass rtnl_link_ops to ip_tunnel_delete_net().
  ip_tunnel: Centralise ip_tunnel_del() to ip_tunnel_dellink().
  ip_tunnel: Unify error paths in ip_tunnel_newlink() and
    ip_tunnel_changelink().
  ip_tunnel: Protect ip_tunnel_net.tunnels[] with mutex.
  ip_tunnel: Support per-netns device unregistration.

 include/net/ip_tunnels.h |   2 +-
 net/ipv4/ip_gre.c        |   6 +-
 net/ipv4/ip_tunnel.c     | 127 +++++++++++++++++++++++++++------------
 net/ipv4/ip_vti.c        |   2 +-
 net/ipv4/ipip.c          |   2 +-
 net/ipv4/ipmr.c          |  12 +++-
 6 files changed, 103 insertions(+), 48 deletions(-)

-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net-next 1/7] ipmr: Call ->dellink() to remove DVMRP tunnel device.

[Kuniyuki Iwashima]

ipmr.c uses unregister_netdevice() to remove DVMRP tunnel devices
created in ipmr_new_tunnel().

This is fine because currently ipip_link_ops.dellink() is no-op
for non-fallback tunnel devices.

However, we will move ip_tunnel_del() from ip_tunnel_uninit() to
ip_tunnel_dellink().

Removing DVMRP tunnel devices by unregister_netdevice() would leave
them in the hash table.

Let's call ->dellink for DVMRP tunnel devices.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/ipmr.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index e5f2b1c6150d..02edf9d7f4b5 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -479,6 +479,7 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
 {
 	struct net_device *tunnel_dev, *new_dev;
 	struct ip_tunnel_parm_kern p = { };
+	LIST_HEAD(dev_kill_list);
 	int err;
 
 	tunnel_dev = __dev_get_by_name(net, "tunl0");
@@ -520,7 +521,8 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
 	return new_dev;
 
 out_unregister:
-	unregister_netdevice(new_dev);
+	new_dev->rtnl_link_ops->dellink(new_dev, &dev_kill_list);
+	unregister_netdevice_many(&dev_kill_list);
 out:
 	return ERR_PTR(-ENOBUFS);
 }
@@ -733,8 +735,12 @@ static int vif_delete(struct mr_table *mrt, int vifi, int notify,
 		ip_rt_multicast_event(in_dev);
 	}
 
-	if (v->flags & (VIFF_TUNNEL | VIFF_REGISTER) && !notify)
-		unregister_netdevice_queue(dev, head);
+	if (!notify) {
+		if (v->flags & VIFF_TUNNEL)
+			dev->rtnl_link_ops->dellink(dev, head);
+		else if (v->flags & VIFF_REGISTER)
+			unregister_netdevice_queue(dev, head);
+	}
 
 	netdev_put(dev, &v->dev_tracker);
 	return 0;
-- 
2.55.0.1003.g10538fe699-goog

Re: [PATCH v1 net-next 1/7] ipmr: Call ->dellink() to remove DVMRP tunnel device.

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The commit message of "ipmr: Call ->dellink() to remove DVMRP
  tunnel device." states the rationale inverted: "This is fine because…

--

ipmr: Call ->dellink() to remove DVMRP tunnel device.

The patch converts the two places in net/ipv4/ipmr.c that removed DVMRP
tunnel devices with unregister_netdevice()/unregister_netdevice_queue()
into calls to the device's ->dellink() followed by unregistration, in
preparation for moving ip_tunnel_del() from ip_tunnel_uninit() to
ip_tunnel_dellink().

> ipmr.c uses unregister_netdevice() to remove DVMRP tunnel devices
> created in ipmr_new_tunnel().
>
> This is fine because currently ipip_link_ops.dellink() is no-op
> for non-fallback tunnel devices.

[Severity: Low]
Is this sentence inverted?  ipip_link_ops sets .dellink = ip_tunnel_dellink,
and that function looks like this:

net/ipv4/ip_tunnel.c:ip_tunnel_dellink() {
	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);

	if (itn->fb_tunnel_dev != dev) {
		ip_tunnel_del(itn, netdev_priv(dev));
		unregister_netdevice_queue(dev, head);
	}
}

So it does real work (unhash plus queue unregister) for non-fallback
devices, and is a no-op only for the fallback device (itn->fb_tunnel_dev).

The dvmrp%d devices ipmr creates go through tunl0's
ndo_tunnel_ctl(SIOCADDTUNNEL) -> ip_tunnel_create -> ip_tunnel_add, so
they are non-fallback and hashed in itn->tunnels[].

Isn't the reason the current unregister_netdevice() calls in ipmr.c are
harmless instead that ip_tunnel_uninit(), which runs as ndo_uninit from
the unregister path, also calls ip_tunnel_del()?

net/ipv4/ip_tunnel.c:ip_tunnel_uninit() {
	itn = net_generic(net, tunnel->ip_tnl_net_id);
	ip_tunnel_del(itn, netdev_priv(dev));
	...
}

ip_tunnel_del() uses hlist_del_init_rcu(), so calling it from both
->dellink() and ndo_uninit is idempotent and this conversion is
behaviour-preserving today.

Could the message be reworded to cite ip_tunnel_uninit() rather than
describing ->dellink() as a no-op for non-fallback devices?  As written it
suggests this preparatory patch has no interaction with ipmr, which is the
opposite of what the follow-up "ip_tunnel: Centralise ip_tunnel_del() to
ip_tunnel_dellink()." depends on.  The code change itself looks fine, and
dev->rtnl_link_ops is always set for ioctl-created tunnels by
__ip_tunnel_create(), so the new ->dellink() dereferences in
ipmr_new_tunnel() and vif_delete() cannot be NULL.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907225846.3787676-1-kuniyu%40google.com

Re: [PATCH v1 net-next 1/7] ipmr: Call ->dellink() to remove DVMRP tunnel device.

[Kuniyuki Iwashima]

On Tue, Sep 8, 2026 at 6:58 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 0 · Low: 1
>
> - [Low] The commit message of "ipmr: Call ->dellink() to remove DVMRP
>   tunnel device." states the rationale inverted: "This is fine because…
>
> --
>
> ipmr: Call ->dellink() to remove DVMRP tunnel device.
>
> The patch converts the two places in net/ipv4/ipmr.c that removed DVMRP
> tunnel devices with unregister_netdevice()/unregister_netdevice_queue()
> into calls to the device's ->dellink() followed by unregistration, in
> preparation for moving ip_tunnel_del() from ip_tunnel_uninit() to
> ip_tunnel_dellink().
>
> > ipmr.c uses unregister_netdevice() to remove DVMRP tunnel devices
> > created in ipmr_new_tunnel().
> >
> > This is fine because currently ipip_link_ops.dellink() is no-op
> > for non-fallback tunnel devices.
>
> [Severity: Low]
> Is this sentence inverted?

Oh right, will update the commit mesasge.


>  ipip_link_ops sets .dellink = ip_tunnel_dellink,
> and that function looks like this:
>
> net/ipv4/ip_tunnel.c:ip_tunnel_dellink() {
>         itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
>
>         if (itn->fb_tunnel_dev != dev) {
>                 ip_tunnel_del(itn, netdev_priv(dev));
>                 unregister_netdevice_queue(dev, head);
>         }
> }
>
> So it does real work (unhash plus queue unregister) for non-fallback
> devices, and is a no-op only for the fallback device (itn->fb_tunnel_dev).
>
> The dvmrp%d devices ipmr creates go through tunl0's
> ndo_tunnel_ctl(SIOCADDTUNNEL) -> ip_tunnel_create -> ip_tunnel_add, so
> they are non-fallback and hashed in itn->tunnels[].
>
> Isn't the reason the current unregister_netdevice() calls in ipmr.c are
> harmless instead that ip_tunnel_uninit(), which runs as ndo_uninit from
> the unregister path, also calls ip_tunnel_del()?
>
> net/ipv4/ip_tunnel.c:ip_tunnel_uninit() {
>         itn = net_generic(net, tunnel->ip_tnl_net_id);
>         ip_tunnel_del(itn, netdev_priv(dev));
>         ...
> }
>
> ip_tunnel_del() uses hlist_del_init_rcu(), so calling it from both
> ->dellink() and ndo_uninit is idempotent and this conversion is
> behaviour-preserving today.
>
> Could the message be reworded to cite ip_tunnel_uninit() rather than
> describing ->dellink() as a no-op for non-fallback devices?  As written it
> suggests this preparatory patch has no interaction with ipmr, which is the
> opposite of what the follow-up "ip_tunnel: Centralise ip_tunnel_del() to
> ip_tunnel_dellink()." depends on.  The code change itself looks fine, and
> dev->rtnl_link_ops is always set for ioctl-created tunnels by
> __ip_tunnel_create(), so the new ->dellink() dereferences in
> ipmr_new_tunnel() and vif_delete() cannot be NULL.
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907225846.3787676-1-kuniyu%40google.com

[PATCH v1 net-next 2/7] ip_tunnel: Set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net().

[Kuniyuki Iwashima]

ip_tunnel_dellink() ignores itn->fb_tunnel_dev, so the per-netns
fallback tunnel device cannot be removed by userspace.

This also makes default_device_exit_batch() impossible to remove
the device since it calls ->dellink().

So, ip_tunnel_delete_net() has to iterate devices in the dying netns
and call unregister_netdevice_queue() directly.

But then, this duplicates ip_tunnel_del() in ip_tunnel_dellink()
and ip_tunnel_uninit().

Let's set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net() and
remove for_each_netdev_safe() in ip_tunnel_delete_net().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/ip_tunnel.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index e6bcf01411d0..d29e2ba7cdd3 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -1155,26 +1155,19 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
 			  struct list_head *head)
 {
 	struct ip_tunnel_net *itn = net_generic(net, id);
-	struct net_device *dev, *aux;
 	int h;
 
 	ASSERT_RTNL_NET(net);
 
-	for_each_netdev_safe(net, dev, aux)
-		if (dev->rtnl_link_ops == ops)
-			unregister_netdevice_queue(dev, head);
+	WRITE_ONCE(itn->fb_tunnel_dev, NULL);
 
 	for (h = 0; h < IP_TNL_HASH_SIZE; h++) {
-		struct ip_tunnel *t;
-		struct hlist_node *n;
 		struct hlist_head *thead = &itn->tunnels[h];
+		struct hlist_node *n;
+		struct ip_tunnel *t;
 
 		hlist_for_each_entry_safe(t, n, thead, hash_node)
-			/* If dev is in the same netns, it has already
-			 * been added to the list by the previous loop.
-			 */
-			if (!net_eq(dev_net(t->dev), net))
-				unregister_netdevice_queue(t->dev, head);
+			unregister_netdevice_queue(t->dev, head);
 	}
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_delete_net);
@@ -1310,8 +1303,6 @@ void ip_tunnel_uninit(struct net_device *dev)
 
 	itn = net_generic(net, tunnel->ip_tnl_net_id);
 	ip_tunnel_del(itn, netdev_priv(dev));
-	if (itn->fb_tunnel_dev == dev)
-		WRITE_ONCE(itn->fb_tunnel_dev, NULL);
 
 	dst_cache_reset(&tunnel->dst_cache);
 }
-- 
2.55.0.1003.g10538fe699-goog

Re: [PATCH v1 net-next 2/7] ip_tunnel: Set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net().

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Claim (INVALIDATED by verification): 'ip_tunnel_delete_net() no
  longer uses its struct rtnl_link_ops *ops argument yet the prototype…

--

ip_tunnel: Set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net().

This clears itn->fb_tunnel_dev at the start of ip_tunnel_delete_net() so
that the fallback device is treated like any other tunnel device, drops
the for_each_netdev_safe() scan that matched dev->rtnl_link_ops == ops,
and removes the now redundant fb_tunnel_dev clearing from
ip_tunnel_uninit().

No Link: tags present in the commit header.

> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index e6bcf01411d0b..d29e2ba7cdd3a 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -1155,26 +1155,19 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
>  			  struct list_head *head)
>  {
>  	struct ip_tunnel_net *itn = net_generic(net, id);
> -	struct net_device *dev, *aux;
>  	int h;
>  
>  	ASSERT_RTNL_NET(net);
>  
> -	for_each_netdev_safe(net, dev, aux)
> -		if (dev->rtnl_link_ops == ops)
> -			unregister_netdevice_queue(dev, head);
> +	WRITE_ONCE(itn->fb_tunnel_dev, NULL);

[Severity: Low]
With the for_each_netdev_safe() scan gone, is the ops argument of
ip_tunnel_delete_net() still used anywhere in the function?

The prototype in include/net/ip_tunnels.h and all five callers
(ipgre_exit_rtnl_net(), ipgre_tap_exit_rtnl_net(),
erspan_exit_rtnl_net(), vti_exit_rtnl_net(), ipip_exit_rtnl_net()) still
pass their link ops:

	void ip_tunnel_delete_net(struct net *net, unsigned int id,
				  struct rtnl_link_ops *ops,
				  struct list_head *dev_to_kill);

The follow-up patch in this series, "ip_tunnel: Don't pass rtnl_link_ops
to ip_tunnel_delete_net().", removes the argument from the declaration
and from all five call sites, so the unused parameter only exists for one
commit and does not produce a compiler warning. Would it be simpler to
fold that removal into this patch, or to note in the changelog that the
argument becomes unused here?

A second, related observation about coverage: the removed loop was the
only place that picked up devices by netns membership, so after this
change ip_tunnel_delete_net(net) only walks the itn hashes of net. A
device that lives in the dying netns but is hashed in another netns' itn
(created with link-netns, e.g. "ip -n nsB link add gre1 link-netns nsA
type gre ..." followed by "ip netns del nsB") is no longer queued here.

Tracing that case, it still gets destroyed by default_device_exit_batch():

net/core/dev.c:default_device_exit_batch() {
	...
	for_each_netdev_reverse(net, dev) {
		if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink)
			dev->rtnl_link_ops->dellink(dev, &dev_kill_list);
		else
			unregister_netdevice_queue(dev, &dev_kill_list);
	}
	...
}

which reaches ip_tunnel_dellink() and, since fb_tunnel_dev != dev there,
does ip_tunnel_del() plus unregister. So nothing appears to be stranded.
Could the changelog mention this change in which devices
ip_tunnel_delete_net() queues, since the last patch of the series
("ip_tunnel: Support per-netns device unregistration.") is where that new
behaviour is finally described?

>  
>  	for (h = 0; h < IP_TNL_HASH_SIZE; h++) {
> -		struct ip_tunnel *t;
> -		struct hlist_node *n;
>  		struct hlist_head *thead = &itn->tunnels[h];
> +		struct hlist_node *n;
> +		struct ip_tunnel *t;
>  
>  		hlist_for_each_entry_safe(t, n, thead, hash_node)
> -			/* If dev is in the same netns, it has already
> -			 * been added to the list by the previous loop.
> -			 */
> -			if (!net_eq(dev_net(t->dev), net))
> -				unregister_netdevice_queue(t->dev, head);
> +			unregister_netdevice_queue(t->dev, head);
>  	}
>  }
>  EXPORT_SYMBOL_GPL(ip_tunnel_delete_net);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907225846.3787676-1-kuniyu%40google.com

Re: [PATCH v1 net-next 2/7] ip_tunnel: Set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net().

[Kuniyuki Iwashima]

On Tue, Sep 8, 2026 at 6:58 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 0 · Low: 1
>
> - [Low] Claim (INVALIDATED by verification): 'ip_tunnel_delete_net() no
>   longer uses its struct rtnl_link_ops *ops argument yet the prototype…
>
> --
>
> ip_tunnel: Set itn->fb_tunnel_dev to NULL in ip_tunnel_delete_net().
>
> This clears itn->fb_tunnel_dev at the start of ip_tunnel_delete_net() so
> that the fallback device is treated like any other tunnel device, drops
> the for_each_netdev_safe() scan that matched dev->rtnl_link_ops == ops,
> and removes the now redundant fb_tunnel_dev clearing from
> ip_tunnel_uninit().
>
> No Link: tags present in the commit header.
>
> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> > index e6bcf01411d0b..d29e2ba7cdd3a 100644
> > --- a/net/ipv4/ip_tunnel.c
> > +++ b/net/ipv4/ip_tunnel.c
> > @@ -1155,26 +1155,19 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
> >                         struct list_head *head)
> >  {
> >       struct ip_tunnel_net *itn = net_generic(net, id);
> > -     struct net_device *dev, *aux;
> >       int h;
> >
> >       ASSERT_RTNL_NET(net);
> >
> > -     for_each_netdev_safe(net, dev, aux)
> > -             if (dev->rtnl_link_ops == ops)
> > -                     unregister_netdevice_queue(dev, head);
> > +     WRITE_ONCE(itn->fb_tunnel_dev, NULL);
>
> [Severity: Low]
> With the for_each_netdev_safe() scan gone, is the ops argument of
> ip_tunnel_delete_net() still used anywhere in the function?

This is removed in the next patch.


>
> The prototype in include/net/ip_tunnels.h and all five callers
> (ipgre_exit_rtnl_net(), ipgre_tap_exit_rtnl_net(),
> erspan_exit_rtnl_net(), vti_exit_rtnl_net(), ipip_exit_rtnl_net()) still
> pass their link ops:
>
>         void ip_tunnel_delete_net(struct net *net, unsigned int id,
>                                   struct rtnl_link_ops *ops,
>                                   struct list_head *dev_to_kill);
>
> The follow-up patch in this series, "ip_tunnel: Don't pass rtnl_link_ops
> to ip_tunnel_delete_net().", removes the argument from the declaration
> and from all five call sites, so the unused parameter only exists for one
> commit and does not produce a compiler warning. Would it be simpler to
> fold that removal into this patch, or to note in the changelog that the
> argument becomes unused here?
>
> A second, related observation about coverage: the removed loop was the
> only place that picked up devices by netns membership, so after this
> change ip_tunnel_delete_net(net) only walks the itn hashes of net. A
> device that lives in the dying netns but is hashed in another netns' itn
> (created with link-netns, e.g. "ip -n nsB link add gre1 link-netns nsA
> type gre ..." followed by "ip netns del nsB") is no longer queued here.
>
> Tracing that case, it still gets destroyed by default_device_exit_batch():
>
> net/core/dev.c:default_device_exit_batch() {
>         ...
>         for_each_netdev_reverse(net, dev) {
>                 if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink)
>                         dev->rtnl_link_ops->dellink(dev, &dev_kill_list);
>                 else
>                         unregister_netdevice_queue(dev, &dev_kill_list);
>         }
>         ...
> }
>
> which reaches ip_tunnel_dellink() and, since fb_tunnel_dev != dev there,
> does ip_tunnel_del() plus unregister. So nothing appears to be stranded.
> Could the changelog mention this change in which devices
> ip_tunnel_delete_net() queues, since the last patch of the series
> ("ip_tunnel: Support per-netns device unregistration.") is where that new
> behaviour is finally described?
>
> >
> >       for (h = 0; h < IP_TNL_HASH_SIZE; h++) {
> > -             struct ip_tunnel *t;
> > -             struct hlist_node *n;
> >               struct hlist_head *thead = &itn->tunnels[h];
> > +             struct hlist_node *n;
> > +             struct ip_tunnel *t;
> >
> >               hlist_for_each_entry_safe(t, n, thead, hash_node)
> > -                     /* If dev is in the same netns, it has already
> > -                      * been added to the list by the previous loop.
> > -                      */
> > -                     if (!net_eq(dev_net(t->dev), net))
> > -                             unregister_netdevice_queue(t->dev, head);
> > +                     unregister_netdevice_queue(t->dev, head);
> >       }
> >  }
> >  EXPORT_SYMBOL_GPL(ip_tunnel_delete_net);
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907225846.3787676-1-kuniyu%40google.com

[PATCH v1 net-next 3/7] ip_tunnel: Don't pass rtnl_link_ops to ip_tunnel_delete_net().

[Kuniyuki Iwashima]

ip_tunnel_delete_net() no longer uses the 3rd argument,
struct rtnl_link_ops *ops.

Let's remove it.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/ip_tunnels.h | 1 -
 net/ipv4/ip_gre.c        | 6 +++---
 net/ipv4/ip_tunnel.c     | 1 -
 net/ipv4/ip_vti.c        | 2 +-
 net/ipv4/ipip.c          | 2 +-
 5 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 7c9aadfe8fe3..b0f9d02a7f18 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -398,7 +398,6 @@ int ip_tunnel_get_iflink(const struct net_device *dev);
 int ip_tunnel_init_net(struct net *net, unsigned int ip_tnl_net_id,
 		       struct rtnl_link_ops *ops, char *devname);
 void ip_tunnel_delete_net(struct net *net, unsigned int id,
-			  struct rtnl_link_ops *ops,
 			  struct list_head *dev_to_kill);
 
 void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 82309efd417e..5e877018e006 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1084,7 +1084,7 @@ static int __net_init ipgre_init_net(struct net *net)
 static void __net_exit ipgre_exit_rtnl(struct net *net,
 				       struct list_head *dev_to_kill)
 {
-	ip_tunnel_delete_net(net, ipgre_net_id, &ipgre_link_ops, dev_to_kill);
+	ip_tunnel_delete_net(net, ipgre_net_id, dev_to_kill);
 }
 
 static struct pernet_operations ipgre_net_ops = {
@@ -1728,7 +1728,7 @@ static int __net_init ipgre_tap_init_net(struct net *net)
 static void __net_exit ipgre_tap_exit_rtnl(struct net *net,
 					   struct list_head *dev_to_kill)
 {
-	ip_tunnel_delete_net(net, gre_tap_net_id, &ipgre_tap_ops, dev_to_kill);
+	ip_tunnel_delete_net(net, gre_tap_net_id, dev_to_kill);
 }
 
 static struct pernet_operations ipgre_tap_net_ops = {
@@ -1747,7 +1747,7 @@ static int __net_init erspan_init_net(struct net *net)
 static void __net_exit erspan_exit_rtnl(struct net *net,
 					struct list_head *dev_to_kill)
 {
-	ip_tunnel_delete_net(net, erspan_net_id, &erspan_link_ops, dev_to_kill);
+	ip_tunnel_delete_net(net, erspan_net_id, dev_to_kill);
 }
 
 static struct pernet_operations erspan_net_ops = {
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index d29e2ba7cdd3..1696ba1f3ed9 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -1151,7 +1151,6 @@ int ip_tunnel_init_net(struct net *net, unsigned int ip_tnl_net_id,
 EXPORT_SYMBOL_GPL(ip_tunnel_init_net);
 
 void ip_tunnel_delete_net(struct net *net, unsigned int id,
-			  struct rtnl_link_ops *ops,
 			  struct list_head *head)
 {
 	struct ip_tunnel_net *itn = net_generic(net, id);
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 3b80929994a0..c4f14d42df24 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -526,7 +526,7 @@ static int __net_init vti_init_net(struct net *net)
 static void __net_exit vti_exit_rtnl(struct net *net,
 				     struct list_head *dev_to_kill)
 {
-	ip_tunnel_delete_net(net, vti_net_id, &vti_link_ops, dev_to_kill);
+	ip_tunnel_delete_net(net, vti_net_id, dev_to_kill);
 }
 
 static struct pernet_operations vti_net_ops = {
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index f684baf8e58f..2ffe64e736e0 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -645,7 +645,7 @@ static int __net_init ipip_init_net(struct net *net)
 static void __net_exit ipip_exit_rtnl(struct net *net,
 				      struct list_head *dev_to_kill)
 {
-	ip_tunnel_delete_net(net, ipip_net_id, &ipip_link_ops, dev_to_kill);
+	ip_tunnel_delete_net(net, ipip_net_id, dev_to_kill);
 }
 
 static struct pernet_operations ipip_net_ops = {
-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net-next 4/7] ip_tunnel: Centralise ip_tunnel_del() to ip_tunnel_dellink().

[Kuniyuki Iwashima]

With the previous patch, itn->fb_tunnel_dev can be removed
via ->dellink().

However, ioctl(SIOCDELTUNNEL) still uses unregister_netdevice(),
which requires ip_tunnel_del() in ip_tunnel_uninit().

Let's use ip_tunnel_dellink() everywhere to remove ip_tunnel device
and remove ip_tunnel_del() in ip_tunnel_uninit().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/ip_tunnel.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 1696ba1f3ed9..5833f93c1964 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -897,10 +897,13 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
 int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 		  int cmd)
 {
-	int err = 0;
 	struct ip_tunnel *t = netdev_priv(dev);
+	struct ip_tunnel_net *itn;
+	LIST_HEAD(dev_kill_list);
 	struct net *net = t->net;
-	struct ip_tunnel_net *itn = net_generic(net, t->ip_tnl_net_id);
+	int err = 0;
+
+	itn = net_generic(net, t->ip_tnl_net_id);
 
 	switch (cmd) {
 	case SIOCGETTUNNEL:
@@ -984,7 +987,8 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 				goto done;
 			dev = t->dev;
 		}
-		unregister_netdevice(dev);
+
+		ip_tunnel_dellink(dev, &dev_kill_list);
 		err = 0;
 		break;
 
@@ -993,6 +997,8 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 	}
 
 done:
+	unregister_netdevice_many(&dev_kill_list);
+
 	return err;
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_ctl);
@@ -1166,7 +1172,7 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
 		struct ip_tunnel *t;
 
 		hlist_for_each_entry_safe(t, n, thead, hash_node)
-			unregister_netdevice_queue(t->dev, head);
+			ip_tunnel_dellink(t->dev, head);
 	}
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_delete_net);
@@ -1297,11 +1303,6 @@ EXPORT_SYMBOL_GPL(__ip_tunnel_init);
 void ip_tunnel_uninit(struct net_device *dev)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
-	struct net *net = tunnel->net;
-	struct ip_tunnel_net *itn;
-
-	itn = net_generic(net, tunnel->ip_tnl_net_id);
-	ip_tunnel_del(itn, netdev_priv(dev));
 
 	dst_cache_reset(&tunnel->dst_cache);
 }
-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net-next 5/7] ip_tunnel: Unify error paths in ip_tunnel_newlink() and ip_tunnel_changelink().

[Kuniyuki Iwashima]

The next patch will introduce per-netns mutex and acquire it
in ip_tunnel_newlink() and ip_tunnel_changelink().

To make the diff cleaner, let's unify the error paths.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/ip_tunnel.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 5833f93c1964..3ba03c2b3b90 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -1181,21 +1181,23 @@ int ip_tunnel_newlink(struct net *net, struct net_device *dev,
 		      struct nlattr *tb[], struct ip_tunnel_parm_kern *p,
 		      __u32 fwmark)
 {
-	struct ip_tunnel *nt;
 	struct ip_tunnel_net *itn;
+	struct ip_tunnel *nt;
+	int err = 0;
 	int mtu;
-	int err;
 
 	nt = netdev_priv(dev);
 	itn = net_generic(net, nt->ip_tnl_net_id);
 
 	if (nt->collect_md) {
 		if (rtnl_dereference(itn->collect_md_tun))
-			return -EEXIST;
+			err = -EEXIST;
 	} else {
 		if (ip_tunnel_find(itn, p, dev->type))
-			return -EEXIST;
+			err = -EEXIST;
 	}
+	if (err)
+		goto out;
 
 	nt->net = net;
 	nt->parms = *p;
@@ -1222,22 +1224,26 @@ int ip_tunnel_newlink(struct net *net, struct net_device *dev,
 		goto err_dev_set_mtu;
 
 	ip_tunnel_add(itn, nt);
-	return 0;
+out:
+	return err;
 
 err_dev_set_mtu:
 	unregister_netdevice(dev);
 err_register_netdevice:
-	return err;
+	goto out;
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_newlink);
 
 int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 			 struct ip_tunnel_parm_kern *p, __u32 fwmark)
 {
-	struct ip_tunnel *t;
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 	struct net *net = tunnel->net;
-	struct ip_tunnel_net *itn = net_generic(net, tunnel->ip_tnl_net_id);
+	struct ip_tunnel_net *itn;
+	struct ip_tunnel *t;
+	int err = 0;
+
+	itn = net_generic(net, tunnel->ip_tnl_net_id);
 
 	if (dev == itn->fb_tunnel_dev)
 		return -EINVAL;
@@ -1245,8 +1251,10 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 	t = ip_tunnel_find(itn, p, dev->type);
 
 	if (t) {
-		if (t->dev != dev)
-			return -EEXIST;
+		if (t->dev != dev) {
+			err = -EEXIST;
+			goto out;
+		}
 	} else {
 		t = tunnel;
 
@@ -1259,13 +1267,16 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 				nflags = IFF_POINTOPOINT;
 
 			if ((dev->flags ^ nflags) &
-			    (IFF_POINTOPOINT | IFF_BROADCAST))
-				return -EINVAL;
+			    (IFF_POINTOPOINT | IFF_BROADCAST)) {
+				err = -EINVAL;
+				goto out;
+			}
 		}
 	}
 
 	ip_tunnel_update(itn, t, dev, p, !tb[IFLA_MTU], fwmark);
-	return 0;
+out:
+	return err;
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_changelink);
 
-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net-next 6/7] ip_tunnel: Protect ip_tunnel_net.tunnels[] with mutex.

[Kuniyuki Iwashima]

struct ip_tunnel.net is the netns where encapsulated packets
flow into.

struct ip_tunnel is linked to ip_tunnel_net.tunnels[] of netns.

During netns dismantle or module unload, ip_tunnel_delete_net()
iterates the list and queues devices for destruction regardless
of the devices' netns.

Thus, once RTNL is removed, the list can be modified concurrently
from different netns due to device removal.

Let's protect it with per-netns mutex.

Note that dev_siocdevprivate() calls netdev_lock_ops() but
it must be NOP for tunnel devices to avoid AB-BA deadlock.
DEBUG_NET_WARN_ON_ONCE() is added to annotate the locking
explicitly.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/ip_tunnels.h |  1 +
 net/ipv4/ip_tunnel.c     | 42 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index b0f9d02a7f18..57a67900e2d3 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -215,6 +215,7 @@ struct ip_tunnel_net {
 	struct net_device *fb_tunnel_dev;
 	struct rtnl_link_ops *rtnl_link_ops;
 	struct hlist_head tunnels[IP_TNL_HASH_SIZE];
+	struct mutex tunnels_lock;
 	struct ip_tunnel __rcu *collect_md_tun;
 	int type;
 };
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 3ba03c2b3b90..9ad63f1af37a 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -219,7 +219,8 @@ static struct ip_tunnel *ip_tunnel_find(struct ip_tunnel_net *itn,
 
 	ip_tunnel_flags_copy(flags, parms->i_flags);
 
-	hlist_for_each_entry_rcu(t, head, hash_node, lockdep_rtnl_is_held()) {
+	hlist_for_each_entry_rcu(t, head, hash_node,
+				 lockdep_is_held(&itn->tunnels_lock)) {
 		if (local == t->parms.iph.saddr &&
 		    remote == t->parms.iph.daddr &&
 		    link == READ_ONCE(t->parms.link) &&
@@ -894,6 +895,16 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
 	netdev_state_change(dev);
 }
 
+static void __ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
+{
+	struct ip_tunnel *tunnel = netdev_priv(dev);
+	struct ip_tunnel_net *itn;
+
+	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
+	ip_tunnel_del(itn, tunnel);
+	unregister_netdevice_queue(dev, head);
+}
+
 int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 		  int cmd)
 {
@@ -903,8 +914,12 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 	struct net *net = t->net;
 	int err = 0;
 
+	DEBUG_NET_WARN_ON_ONCE(netdev_need_ops_lock(dev));
+
 	itn = net_generic(net, t->ip_tnl_net_id);
 
+	mutex_lock(&itn->tunnels_lock);
+
 	switch (cmd) {
 	case SIOCGETTUNNEL:
 		if (dev == itn->fb_tunnel_dev) {
@@ -988,7 +1003,7 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 			dev = t->dev;
 		}
 
-		ip_tunnel_dellink(dev, &dev_kill_list);
+		__ip_tunnel_dellink(dev, &dev_kill_list);
 		err = 0;
 		break;
 
@@ -997,6 +1012,8 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 	}
 
 done:
+	mutex_unlock(&itn->tunnels_lock);
+
 	unregister_netdevice_many(&dev_kill_list);
 
 	return err;
@@ -1093,8 +1110,9 @@ void ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
 	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
 
 	if (itn->fb_tunnel_dev != dev) {
-		ip_tunnel_del(itn, netdev_priv(dev));
-		unregister_netdevice_queue(dev, head);
+		mutex_lock(&itn->tunnels_lock);
+		__ip_tunnel_dellink(dev, head);
+		mutex_unlock(&itn->tunnels_lock);
 	}
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_dellink);
@@ -1126,6 +1144,8 @@ int ip_tunnel_init_net(struct net *net, unsigned int ip_tnl_net_id,
 	for (i = 0; i < IP_TNL_HASH_SIZE; i++)
 		INIT_HLIST_HEAD(&itn->tunnels[i]);
 
+	mutex_init(&itn->tunnels_lock);
+
 	if (!ops || !net_has_fallback_tunnels(net)) {
 		struct ip_tunnel_net *it_init_net;
 
@@ -1164,6 +1184,8 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
 
 	ASSERT_RTNL_NET(net);
 
+	mutex_lock(&itn->tunnels_lock);
+
 	WRITE_ONCE(itn->fb_tunnel_dev, NULL);
 
 	for (h = 0; h < IP_TNL_HASH_SIZE; h++) {
@@ -1172,8 +1194,10 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
 		struct ip_tunnel *t;
 
 		hlist_for_each_entry_safe(t, n, thead, hash_node)
-			ip_tunnel_dellink(t->dev, head);
+			__ip_tunnel_dellink(t->dev, head);
 	}
+
+	mutex_unlock(&itn->tunnels_lock);
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_delete_net);
 
@@ -1189,6 +1213,8 @@ int ip_tunnel_newlink(struct net *net, struct net_device *dev,
 	nt = netdev_priv(dev);
 	itn = net_generic(net, nt->ip_tnl_net_id);
 
+	mutex_lock(&itn->tunnels_lock);
+
 	if (nt->collect_md) {
 		if (rtnl_dereference(itn->collect_md_tun))
 			err = -EEXIST;
@@ -1225,6 +1251,8 @@ int ip_tunnel_newlink(struct net *net, struct net_device *dev,
 
 	ip_tunnel_add(itn, nt);
 out:
+	mutex_unlock(&itn->tunnels_lock);
+
 	return err;
 
 err_dev_set_mtu:
@@ -1248,6 +1276,8 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (dev == itn->fb_tunnel_dev)
 		return -EINVAL;
 
+	mutex_lock(&itn->tunnels_lock);
+
 	t = ip_tunnel_find(itn, p, dev->type);
 
 	if (t) {
@@ -1276,6 +1306,8 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 
 	ip_tunnel_update(itn, t, dev, p, !tb[IFLA_MTU], fwmark);
 out:
+	mutex_unlock(&itn->tunnels_lock);
+
 	return err;
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_changelink);
-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net-next 7/7] ip_tunnel: Support per-netns device unregistration.

[Kuniyuki Iwashima]

ip_tunnel_delete_net() iterates ip_tunnel devices whose link_net
is dying and queues them for destruction.

The devices may reside in different netns.

Let's use unregister_netdevice_queue_net() to support per-netns
device unregistration.

Even after ip_tunnel_delete_net() queues a cross-netns ip_tunnel
device, ip_tunnel_changelink(), ip_tunnel_dellink(), and
ip_tunnel_ctl() could be called concurrently for it (once RTNL is
removed).  In such a case, __rtnl_net_unlock() will perform the
unregistration.

In the example below, we can see the fallback tunnel device (gre0)
and the cross-netns device (gre1) are unregistered by different
processes:

  # bpftrace -e '#include <linux/netdevice.h>
  kprobe:ip_tunnel_uninit {
      $dev = (struct net_device *)arg0;
      printf("PID: %d | DEV: %s%s\n", pid, $dev->name, kstack());
  }
  kprobe:ipgre_exit_rtnl {
      printf("PID: %d%s\n", pid, kstack());
  }' &

  # ip netns add ns1
  # ip netns add ns2
  # ip -n ns1 link add name gre1 link-netns ns2 \
    type gre local 192.168.0.1 remote 192.168.1.1
  # ip netns del ns2

  PID: 12
          ipgre_exit_rtnl+5
          ops_undo_list+702
          cleanup_net+1122
          process_scheduled_works+2538
  ...
  PID: 12 | DEV: gre0  <------ fallback device (itn->fb_tunnel_dev).
          ip_tunnel_uninit+5
          unregister_netdevice_many_notify+7129
          unregister_netdevice_many_net+1050
          __rtnl_net_unlock+37
          ops_undo_list+754
          cleanup_net+1122
          process_scheduled_works+2538
  ...
  PID: 10 | DEV: gre1
          ip_tunnel_uninit+5
          unregister_netdevice_many_notify+7129
          unregister_netdevice_many_net+1050
          rtnl_net_work_func+136
          process_scheduled_works+2538

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/ip_tunnel.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 9ad63f1af37a..051d7fd68902 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -205,6 +205,11 @@ static void ip_tunnel_del(struct ip_tunnel_net *itn, struct ip_tunnel *t)
 	hlist_del_init_rcu(&t->hash_node);
 }
 
+static bool ip_tunnel_unregistering(struct ip_tunnel *t)
+{
+	return hlist_unhashed(&t->hash_node);
+}
+
 static struct ip_tunnel *ip_tunnel_find(struct ip_tunnel_net *itn,
 					struct ip_tunnel_parm_kern *parms,
 					int type)
@@ -895,20 +900,22 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
 	netdev_state_change(dev);
 }
 
-static void __ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
+static void __ip_tunnel_dellink(struct net *net, struct net_device *dev,
+				struct list_head *head)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 	struct ip_tunnel_net *itn;
 
 	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
 	ip_tunnel_del(itn, tunnel);
-	unregister_netdevice_queue(dev, head);
+	unregister_netdevice_queue_net(net, dev, head);
 }
 
 int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 		  int cmd)
 {
 	struct ip_tunnel *t = netdev_priv(dev);
+	struct net *orig_net = dev_net(dev);
 	struct ip_tunnel_net *itn;
 	LIST_HEAD(dev_kill_list);
 	struct net *net = t->net;
@@ -979,7 +986,7 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 			}
 		}
 
-		if (t) {
+		if (t && !ip_tunnel_unregistering(t)) {
 			err = 0;
 			ip_tunnel_update(itn, t, dev, p, true, 0);
 		} else {
@@ -1003,7 +1010,9 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
 			dev = t->dev;
 		}
 
-		__ip_tunnel_dellink(dev, &dev_kill_list);
+		if (!ip_tunnel_unregistering(t))
+			__ip_tunnel_dellink(orig_net, dev, &dev_kill_list);
+
 		err = 0;
 		break;
 
@@ -1111,7 +1120,8 @@ void ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
 
 	if (itn->fb_tunnel_dev != dev) {
 		mutex_lock(&itn->tunnels_lock);
-		__ip_tunnel_dellink(dev, head);
+		if (!ip_tunnel_unregistering(tunnel))
+			__ip_tunnel_dellink(dev_net(dev), dev, head);
 		mutex_unlock(&itn->tunnels_lock);
 	}
 }
@@ -1194,7 +1204,7 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
 		struct ip_tunnel *t;
 
 		hlist_for_each_entry_safe(t, n, thead, hash_node)
-			__ip_tunnel_dellink(t->dev, head);
+			__ip_tunnel_dellink(net, t->dev, head);
 	}
 
 	mutex_unlock(&itn->tunnels_lock);
@@ -1304,6 +1314,11 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 		}
 	}
 
+	if (ip_tunnel_unregistering(t)) {
+		err = -ENODEV;
+		goto out;
+	}
+
 	ip_tunnel_update(itn, t, dev, p, !tb[IFLA_MTU], fwmark);
 out:
 	mutex_unlock(&itn->tunnels_lock);
-- 
2.55.0.1003.g10538fe699-goog

Re: [PATCH v1 net-next 7/7] ip_tunnel: Support per-netns device unregistration.

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] ip_tunnel_ctl()'s SIOCADDTUNNEL arm can register a brand-new
  netdevice into, and re-hash a tunnel in, an ip_tunnel_net that…

--

ip_tunnel: Support per-netns device unregistration.

This switches __ip_tunnel_dellink() to unregister_netdevice_queue_net() so
ip_tunnel devices living in a different netns than the dying link_net are
queued per-netns, and adds an ip_tunnel_unregistering() guard to the
SIOCCHGTUNNEL and SIOCDELTUNNEL arms of ip_tunnel_ctl(), to
ip_tunnel_dellink() and to ip_tunnel_changelink().

> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 9ad63f1af37a6..051d7fd689029 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c

[ ... ]

> @@ -895,20 +900,22 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
>  	netdev_state_change(dev);
>  }
>  
> -static void __ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
> +static void __ip_tunnel_dellink(struct net *net, struct net_device *dev,
> +				struct list_head *head)
>  {
>  	struct ip_tunnel *tunnel = netdev_priv(dev);
>  	struct ip_tunnel_net *itn;
>  
>  	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
>  	ip_tunnel_del(itn, tunnel);
> -	unregister_netdevice_queue(dev, head);
> +	unregister_netdevice_queue_net(net, dev, head);
>  }
>  
>  int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
>  		  int cmd)
>  {
>  	struct ip_tunnel *t = netdev_priv(dev);
> +	struct net *orig_net = dev_net(dev);
>  	struct ip_tunnel_net *itn;
>  	LIST_HEAD(dev_kill_list);
>  	struct net *net = t->net;
> @@ -979,7 +986,7 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
>  			}
>  		}
>  
> -		if (t) {
> +		if (t && !ip_tunnel_unregistering(t)) {
>  			err = 0;
>  			ip_tunnel_update(itn, t, dev, p, true, 0);
>  		} else {

[Severity: High]
The SIOCCHGTUNNEL arm now consults ip_tunnel_unregistering(), and so do
SIOCDELTUNNEL, ip_tunnel_dellink() and ip_tunnel_changelink().  Should the
creation arm just above in ip_tunnel_ctl() get an equivalent check?  It is
the only path that mutates itn and registers a device with no liveness test:

net/ipv4/ip_tunnel.c:ip_tunnel_ctl() {
	...
	itn = net_generic(net, t->ip_tnl_net_id);   /* net == t->net */
	...
		t = ip_tunnel_find(itn, p, itn->type);

		if (cmd == SIOCADDTUNNEL) {
			if (!t) {
				t = ip_tunnel_create(net, itn, p);
	...
}

ip_tunnel_create() -> __ip_tunnel_create() does dev_net_set(dev, net) plus
register_netdevice(), then ip_tunnel_add(itn, nt) re-populates the hash.

Can that happen after ip_tunnel_delete_net() has already made its final
pass over the same itn?  With CONFIG_DEBUG_NET_SMALL_RTNL=y, this is the
sequence the commit message itself demonstrates with gre1:

cleanup_net()
  ops_undo_list() -> ipgre_exit_rtnl(ns2)
    ip_tunnel_delete_net(ns2)
      WRITE_ONCE(itn->fb_tunnel_dev, NULL);
      __ip_tunnel_dellink(ns2, gre1, head)
        unregister_netdevice_queue_net(ns2, gre1, head)

Since dev_net(gre1) is ns1 and not ns2, unregister_netdevice_queue_net()
only queues the device:

net/core/dev.c:unregister_netdevice_queue_net() {
	...
	list_add_tail(&dev->unreg_list_net, &net->dev_unreg_head);
	rtnl_net_queue_work(net);
	...
}

so gre1 stays registered after ops_exit_rtnl_list() drops RTNL, exactly as
the bpftrace output shows (gre1 is torn down later from
rtnl_net_work_func).  While it is still registered, an ioctl in ns1 can
reach ip_tunnel_ctl(gre1, p, SIOCADDTUNNEL) holding only
rtnl_net_lock(ns1):

dev_ioctl() -> dev_ifsioc() -> dev_siocdevprivate()
  -> ip_tunnel_siocdevprivate() -> ndo_tunnel_ctl -> ip_tunnel_ctl()

ip_tunnel_ctl() then uses net = t->net = ns2 and itn = net_generic(ns2, id),
the itn that was just drained.  ip_tunnel_find() returns NULL on the empty
hash, so a new device is registered into ns2 and hashed into that itn.

Does anything reclaim that device afterwards?  The last sweep of ns2's
device list is in default_device_exit_batch():

net/core/dev.c:default_device_exit_batch() {
	...
	unregister_netdevice_many(&dev_kill_list);
	rtnl_unlock();

	rtnl_net_flush_workqueue();
	...
}

The scan and rtnl_unlock() precede rtnl_net_flush_workqueue(), and gre1's
unregistration is still pending at that point, so an ioctl winning RTNL in
that gap can create a device in ns2 after the final sweep.  ops_free_list()
would then kfree() ns2's ip_tunnel_net while the new tunnel is hashed in
it, and cleanup_net() would free ns2's net->gen and struct net while a
registered netdevice still points at them through dev_net()/dev_list, since
netdevices hold no netns reference.

The per-tunnel guard added here, hlist_unhashed(&t->hash_node), cannot
cover a tunnel that does not exist yet.  Would a separate "this itn is
dying" test work here, for instance keying off the marker
ip_tunnel_delete_net() already publishes with
WRITE_ONCE(itn->fb_tunnel_dev, NULL)?

[ ... ]

> @@ -1194,7 +1204,7 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
>  		struct ip_tunnel *t;
>  
>  		hlist_for_each_entry_safe(t, n, thead, hash_node)
> -			__ip_tunnel_dellink(t->dev, head);
> +			__ip_tunnel_dellink(net, t->dev, head);
>  	}
>  
>  	mutex_unlock(&itn->tunnels_lock);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907225846.3787676-1-kuniyu%40google.com

Re: [PATCH v1 net-next 7/7] ip_tunnel: Support per-netns device unregistration.

[Kuniyuki Iwashima]

On Tue, Sep 8, 2026 at 6:58 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> - [High] ip_tunnel_ctl()'s SIOCADDTUNNEL arm can register a brand-new
>   netdevice into, and re-hash a tunnel in, an ip_tunnel_net that…
>
> --
>
> ip_tunnel: Support per-netns device unregistration.
>
> This switches __ip_tunnel_dellink() to unregister_netdevice_queue_net() so
> ip_tunnel devices living in a different netns than the dying link_net are
> queued per-netns, and adds an ip_tunnel_unregistering() guard to the
> SIOCCHGTUNNEL and SIOCDELTUNNEL arms of ip_tunnel_ctl(), to
> ip_tunnel_dellink() and to ip_tunnel_changelink().
>
> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> > index 9ad63f1af37a6..051d7fd689029 100644
> > --- a/net/ipv4/ip_tunnel.c
> > +++ b/net/ipv4/ip_tunnel.c
>
> [ ... ]
>
> > @@ -895,20 +900,22 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
> >       netdev_state_change(dev);
> >  }
> >
> > -static void __ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
> > +static void __ip_tunnel_dellink(struct net *net, struct net_device *dev,
> > +                             struct list_head *head)
> >  {
> >       struct ip_tunnel *tunnel = netdev_priv(dev);
> >       struct ip_tunnel_net *itn;
> >
> >       itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
> >       ip_tunnel_del(itn, tunnel);
> > -     unregister_netdevice_queue(dev, head);
> > +     unregister_netdevice_queue_net(net, dev, head);
> >  }
> >
> >  int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
> >                 int cmd)
> >  {
> >       struct ip_tunnel *t = netdev_priv(dev);
> > +     struct net *orig_net = dev_net(dev);
> >       struct ip_tunnel_net *itn;
> >       LIST_HEAD(dev_kill_list);
> >       struct net *net = t->net;
> > @@ -979,7 +986,7 @@ int ip_tunnel_ctl(struct net_device *dev, struct ip_tunnel_parm_kern *p,
> >                       }
> >               }
> >
> > -             if (t) {
> > +             if (t && !ip_tunnel_unregistering(t)) {
> >                       err = 0;
> >                       ip_tunnel_update(itn, t, dev, p, true, 0);
> >               } else {
>
> [Severity: High]
> The SIOCCHGTUNNEL arm now consults ip_tunnel_unregistering(), and so do
> SIOCDELTUNNEL, ip_tunnel_dellink() and ip_tunnel_changelink().  Should the
> creation arm just above in ip_tunnel_ctl() get an equivalent check?

No, ip_tunnel_find() is called under the mutex, so
"t && ip_tunnel_unregistering(t)" never happens.


> It is
> the only path that mutates itn and registers a device with no liveness test:
>
> net/ipv4/ip_tunnel.c:ip_tunnel_ctl() {
>         ...
>         itn = net_generic(net, t->ip_tnl_net_id);   /* net == t->net */
>         ...
>                 t = ip_tunnel_find(itn, p, itn->type);
>
>                 if (cmd == SIOCADDTUNNEL) {
>                         if (!t) {
>                                 t = ip_tunnel_create(net, itn, p);
>         ...
> }
>
> ip_tunnel_create() -> __ip_tunnel_create() does dev_net_set(dev, net) plus
> register_netdevice(), then ip_tunnel_add(itn, nt) re-populates the hash.
>
> Can that happen after ip_tunnel_delete_net() has already made its final
> pass over the same itn?

Good point, ip_tunnel_ctl() needs to check !itn->fb_tunnel_dev
or maybe_get_net().

---
pw-bot: cr


> With CONFIG_DEBUG_NET_SMALL_RTNL=y, this is the
> sequence the commit message itself demonstrates with gre1:
>
> cleanup_net()
>   ops_undo_list() -> ipgre_exit_rtnl(ns2)
>     ip_tunnel_delete_net(ns2)
>       WRITE_ONCE(itn->fb_tunnel_dev, NULL);
>       __ip_tunnel_dellink(ns2, gre1, head)
>         unregister_netdevice_queue_net(ns2, gre1, head)
>
> Since dev_net(gre1) is ns1 and not ns2, unregister_netdevice_queue_net()
> only queues the device:
>
> net/core/dev.c:unregister_netdevice_queue_net() {
>         ...
>         list_add_tail(&dev->unreg_list_net, &net->dev_unreg_head);
>         rtnl_net_queue_work(net);
>         ...
> }
>
> so gre1 stays registered after ops_exit_rtnl_list() drops RTNL, exactly as
> the bpftrace output shows (gre1 is torn down later from
> rtnl_net_work_func).  While it is still registered, an ioctl in ns1 can
> reach ip_tunnel_ctl(gre1, p, SIOCADDTUNNEL) holding only
> rtnl_net_lock(ns1):
>
> dev_ioctl() -> dev_ifsioc() -> dev_siocdevprivate()
>   -> ip_tunnel_siocdevprivate() -> ndo_tunnel_ctl -> ip_tunnel_ctl()
>
> ip_tunnel_ctl() then uses net = t->net = ns2 and itn = net_generic(ns2, id),
> the itn that was just drained.  ip_tunnel_find() returns NULL on the empty
> hash, so a new device is registered into ns2 and hashed into that itn.
>
> Does anything reclaim that device afterwards?  The last sweep of ns2's
> device list is in default_device_exit_batch():
>
> net/core/dev.c:default_device_exit_batch() {
>         ...
>         unregister_netdevice_many(&dev_kill_list);
>         rtnl_unlock();
>
>         rtnl_net_flush_workqueue();
>         ...
> }
>
> The scan and rtnl_unlock() precede rtnl_net_flush_workqueue(), and gre1's
> unregistration is still pending at that point, so an ioctl winning RTNL in
> that gap can create a device in ns2 after the final sweep.  ops_free_list()
> would then kfree() ns2's ip_tunnel_net while the new tunnel is hashed in
> it, and cleanup_net() would free ns2's net->gen and struct net while a
> registered netdevice still points at them through dev_net()/dev_list, since
> netdevices hold no netns reference.
>
> The per-tunnel guard added here, hlist_unhashed(&t->hash_node), cannot
> cover a tunnel that does not exist yet.  Would a separate "this itn is
> dying" test work here, for instance keying off the marker
> ip_tunnel_delete_net() already publishes with
> WRITE_ONCE(itn->fb_tunnel_dev, NULL)?
>
> [ ... ]
>
> > @@ -1194,7 +1204,7 @@ void ip_tunnel_delete_net(struct net *net, unsigned int id,
> >               struct ip_tunnel *t;
> >
> >               hlist_for_each_entry_safe(t, n, thead, hash_node)
> > -                     __ip_tunnel_dellink(t->dev, head);
> > +                     __ip_tunnel_dellink(net, t->dev, head);
> >       }
> >
> >       mutex_unlock(&itn->tunnels_lock);
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907225846.3787676-1-kuniyu%40google.com