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

On Tue, Sep 8, 2026 at 6:58 PM [off-list ref] 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.

quoted

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?

quoted

      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