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

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.

[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?

[ ... ]

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