[PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[Jakub Kicinski]

We still see dev_hold() / dev_put() calls without reference tracker
getting added in new code. dev_get_by_name() / dev_get_by_index()
seem to be one of the sources of those. Provide appropriate helpers.
Allocating the tracker can obviously be done with an additional call
to netdev_tracker_alloc(), but a single API feels cleaner.

v2:
 - fix a dev_put() in ethtool
v1: https://lore.kernel.org/all/20230609183207.1466075-1-kuba@kernel.org/

Jakub Kicinski (2):
  net: create device lookup API with reference tracking
  netpoll: allocate netdev tracker right away

 include/linux/netdevice.h |  4 +++
 net/core/dev.c            | 75 ++++++++++++++++++++++++++-------------
 net/core/netpoll.c        |  5 ++-
 net/ethtool/netlink.c     | 10 +++---
 net/ipv6/route.c          | 12 +++----
 5 files changed, 68 insertions(+), 38 deletions(-)

-- 
2.40.1

[PATCH net-next v2 1/2] net: create device lookup API with reference tracking

[Jakub Kicinski]

New users of dev_get_by_index() and dev_get_by_name() keep
getting added and it would be nice to steer them towards
the APIs with reference tracking.

Add variants of those calls which allocate the reference
tracker and use them in a couple of places.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - convert intermediate dev_put() -> netdev_put() in ethtool
v1: https://lore.kernel.org/all/20230609183207.1466075-1-kuba@kernel.org/
---
 include/linux/netdevice.h |  4 +++
 net/core/dev.c            | 75 ++++++++++++++++++++++++++-------------
 net/ethtool/netlink.c     | 10 +++---
 net/ipv6/route.c          | 12 +++----
 4 files changed, 66 insertions(+), 35 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2d6cb2bf2f05..acf706d49c2b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3124,6 +3124,10 @@ struct net_device *netdev_sk_get_lowest_dev(struct net_device *dev,
 					    struct sock *sk);
 struct net_device *dev_get_by_index(struct net *net, int ifindex);
 struct net_device *__dev_get_by_index(struct net *net, int ifindex);
+struct net_device *netdev_get_by_index(struct net *net, int ifindex,
+				       netdevice_tracker *tracker, gfp_t gfp);
+struct net_device *netdev_get_by_name(struct net *net, const char *name,
+				      netdevice_tracker *tracker, gfp_t gfp);
 struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex);
 struct net_device *dev_get_by_napi_id(unsigned int napi_id);
 int dev_restart(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index c2456b3667fe..63abb0463c24 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -758,18 +758,7 @@ struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
 }
 EXPORT_SYMBOL(dev_get_by_name_rcu);
 
-/**
- *	dev_get_by_name		- find a device by its name
- *	@net: the applicable net namespace
- *	@name: name to find
- *
- *	Find an interface by name. This can be called from any
- *	context and does its own locking. The returned handle has
- *	the usage count incremented and the caller must use dev_put() to
- *	release it when it is no longer needed. %NULL is returned if no
- *	matching device is found.
- */
-
+/* Deprecated for new users, call netdev_get_by_name() instead */
 struct net_device *dev_get_by_name(struct net *net, const char *name)
 {
 	struct net_device *dev;
@@ -782,6 +771,31 @@ struct net_device *dev_get_by_name(struct net *net, const char *name)
 }
 EXPORT_SYMBOL(dev_get_by_name);
 
+/**
+ *	netdev_get_by_name() - find a device by its name
+ *	@net: the applicable net namespace
+ *	@name: name to find
+ *	@tracker: tracking object for the acquired reference
+ *	@gfp: allocation flags for the tracker
+ *
+ *	Find an interface by name. This can be called from any
+ *	context and does its own locking. The returned handle has
+ *	the usage count incremented and the caller must use netdev_put() to
+ *	release it when it is no longer needed. %NULL is returned if no
+ *	matching device is found.
+ */
+struct net_device *netdev_get_by_name(struct net *net, const char *name,
+				      netdevice_tracker *tracker, gfp_t gfp)
+{
+	struct net_device *dev;
+
+	dev = dev_get_by_name(net, name);
+	if (dev)
+		netdev_tracker_alloc(dev, tracker, gfp);
+	return dev;
+}
+EXPORT_SYMBOL(netdev_get_by_name);
+
 /**
  *	__dev_get_by_index - find a device by its ifindex
  *	@net: the applicable net namespace
@@ -831,18 +845,7 @@ struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex)
 }
 EXPORT_SYMBOL(dev_get_by_index_rcu);
 
-
-/**
- *	dev_get_by_index - find a device by its ifindex
- *	@net: the applicable net namespace
- *	@ifindex: index of device
- *
- *	Search for an interface by index. Returns NULL if the device
- *	is not found or a pointer to the device. The device returned has
- *	had a reference added and the pointer is safe until the user calls
- *	dev_put to indicate they have finished with it.
- */
-
+/* Deprecated for new users, call netdev_get_by_index() instead */
 struct net_device *dev_get_by_index(struct net *net, int ifindex)
 {
 	struct net_device *dev;
@@ -855,6 +858,30 @@ struct net_device *dev_get_by_index(struct net *net, int ifindex)
 }
 EXPORT_SYMBOL(dev_get_by_index);
 
+/**
+ *	netdev_get_by_index() - find a device by its ifindex
+ *	@net: the applicable net namespace
+ *	@ifindex: index of device
+ *	@tracker: tracking object for the acquired reference
+ *	@gfp: allocation flags for the tracker
+ *
+ *	Search for an interface by index. Returns NULL if the device
+ *	is not found or a pointer to the device. The device returned has
+ *	had a reference added and the pointer is safe until the user calls
+ *	netdev_put() to indicate they have finished with it.
+ */
+struct net_device *netdev_get_by_index(struct net *net, int ifindex,
+				       netdevice_tracker *tracker, gfp_t gfp)
+{
+	struct net_device *dev;
+
+	dev = dev_get_by_index(net, ifindex);
+	if (dev)
+		netdev_tracker_alloc(dev, tracker, gfp);
+	return dev;
+}
+EXPORT_SYMBOL(netdev_get_by_index);
+
 /**
  *	dev_get_by_napi_id - find a device by napi_id
  *	@napi_id: ID of the NAPI struct
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 5dd5e8222c45..39a459b0111b 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -115,7 +115,8 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
 	if (tb[ETHTOOL_A_HEADER_DEV_INDEX]) {
 		u32 ifindex = nla_get_u32(tb[ETHTOOL_A_HEADER_DEV_INDEX]);
 
-		dev = dev_get_by_index(net, ifindex);
+		dev = netdev_get_by_index(net, ifindex, &req_info->dev_tracker,
+					  GFP_KERNEL);
 		if (!dev) {
 			NL_SET_ERR_MSG_ATTR(extack,
 					    tb[ETHTOOL_A_HEADER_DEV_INDEX],
@@ -125,13 +126,14 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
 		/* if both ifindex and ifname are passed, they must match */
 		if (devname_attr &&
 		    strncmp(dev->name, nla_data(devname_attr), IFNAMSIZ)) {
-			dev_put(dev);
+			netdev_put(dev, &req_info->dev_tracker);
 			NL_SET_ERR_MSG_ATTR(extack, header,
 					    "ifindex and name do not match");
 			return -ENODEV;
 		}
 	} else if (devname_attr) {
-		dev = dev_get_by_name(net, nla_data(devname_attr));
+		dev = netdev_get_by_name(net, nla_data(devname_attr),
+					 &req_info->dev_tracker, GFP_KERNEL);
 		if (!dev) {
 			NL_SET_ERR_MSG_ATTR(extack, devname_attr,
 					    "no device matches name");
@@ -144,8 +146,6 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
 	}
 
 	req_info->dev = dev;
-	if (dev)
-		netdev_tracker_alloc(dev, &req_info->dev_tracker, GFP_KERNEL);
 	req_info->flags = flags;
 	return 0;
 }
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 392aaa373b66..e510a4162ef8 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3503,6 +3503,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
 		 struct fib6_config *cfg, gfp_t gfp_flags,
 		 struct netlink_ext_ack *extack)
 {
+	netdevice_tracker *dev_tracker = &fib6_nh->fib_nh_dev_tracker;
 	struct net_device *dev = NULL;
 	struct inet6_dev *idev = NULL;
 	int addr_type;
@@ -3520,7 +3521,8 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
 
 	err = -ENODEV;
 	if (cfg->fc_ifindex) {
-		dev = dev_get_by_index(net, cfg->fc_ifindex);
+		dev = netdev_get_by_index(net, cfg->fc_ifindex,
+					  dev_tracker, gfp_flags);
 		if (!dev)
 			goto out;
 		idev = in6_dev_get(dev);
@@ -3554,11 +3556,11 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
 		/* hold loopback dev/idev if we haven't done so. */
 		if (dev != net->loopback_dev) {
 			if (dev) {
-				dev_put(dev);
+				netdev_put(dev, dev_tracker);
 				in6_dev_put(idev);
 			}
 			dev = net->loopback_dev;
-			dev_hold(dev);
+			netdev_hold(dev, dev_tracker, gfp_flags);
 			idev = in6_dev_get(dev);
 			if (!idev) {
 				err = -ENODEV;
@@ -3610,8 +3612,6 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
 	}
 
 	fib6_nh->fib_nh_dev = dev;
-	netdev_tracker_alloc(dev, &fib6_nh->fib_nh_dev_tracker, gfp_flags);
-
 	fib6_nh->fib_nh_oif = dev->ifindex;
 	err = 0;
 out:
@@ -3621,7 +3621,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
 	if (err) {
 		lwtstate_put(fib6_nh->fib_nh_lws);
 		fib6_nh->fib_nh_lws = NULL;
-		dev_put(dev);
+		netdev_put(dev, dev_tracker);
 	}
 
 	return err;
-- 
2.40.1

[PATCH net-next v2 2/2] netpoll: allocate netdev tracker right away

[Jakub Kicinski]

Commit 5fa5ae605821 ("netpoll: add net device refcount tracker to struct netpoll")
was part of one of the initial netdev tracker introduction patches.
It added an explicit netdev_tracker_alloc() for netpoll, presumably
because the flow of the function is somewhat odd.
After most of the core networking stack was converted to use
the tracking hold() variants, netpoll's call to old dev_hold()
stands out a bit.

np is allocated by the caller and ready to use, we can use
netdev_hold() here, even tho np->ndev will only be set to
ndev inside __netpoll_setup().

Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/netpoll.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e6a739b1afa9..543007f159f9 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -690,7 +690,7 @@ int netpoll_setup(struct netpoll *np)
 		err = -ENODEV;
 		goto unlock;
 	}
-	dev_hold(ndev);
+	netdev_hold(ndev, &np->dev_tracker, GFP_KERNEL);
 
 	if (netdev_master_upper_dev_get(ndev)) {
 		np_err(np, "%s is a slave device, aborting\n", np->dev_name);
@@ -783,12 +783,11 @@ int netpoll_setup(struct netpoll *np)
 	err = __netpoll_setup(np, ndev);
 	if (err)
 		goto put;
-	netdev_tracker_alloc(ndev, &np->dev_tracker, GFP_KERNEL);
 	rtnl_unlock();
 	return 0;
 
 put:
-	dev_put(ndev);
+	netdev_put(ndev, &np->dev_tracker);
 unlock:
 	rtnl_unlock();
 	return err;
-- 
2.40.1

Re: [PATCH net-next v2 1/2] net: create device lookup API with reference tracking

[Eric Dumazet]

On Mon, Jun 12, 2023 at 11:49 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> New users of dev_get_by_index() and dev_get_by_name() keep
> getting added and it would be nice to steer them towards
> the APIs with reference tracking.
>
> Add variants of those calls which allocate the reference
> tracker and use them in a couple of places.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net-next v2 1/2] net: create device lookup API with reference tracking

[David Ahern]

On 6/12/23 3:49 PM, Jakub Kicinski wrote:
> New users of dev_get_by_index() and dev_get_by_name() keep
> getting added and it would be nice to steer them towards
> the APIs with reference tracking.
> 
> Add variants of those calls which allocate the reference
> tracker and use them in a couple of places.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - convert intermediate dev_put() -> netdev_put() in ethtool
> v1: https://lore.kernel.org/all/20230609183207.1466075-1-kuba@kernel.org/
> ---
>  include/linux/netdevice.h |  4 +++
>  net/core/dev.c            | 75 ++++++++++++++++++++++++++-------------
>  net/ethtool/netlink.c     | 10 +++---
>  net/ipv6/route.c          | 12 +++----
>  4 files changed, 66 insertions(+), 35 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Mon, 12 Jun 2023 14:49:42 -0700 you wrote:
> We still see dev_hold() / dev_put() calls without reference tracker
> getting added in new code. dev_get_by_name() / dev_get_by_index()
> seem to be one of the sources of those. Provide appropriate helpers.
> Allocating the tracker can obviously be done with an additional call
> to netdev_tracker_alloc(), but a single API feels cleaner.
> 
> v2:
>  - fix a dev_put() in ethtool
> v1: https://lore.kernel.org/all/20230609183207.1466075-1-kuba@kernel.org/
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/2] net: create device lookup API with reference tracking
    (no matching commit)
  - [net-next,v2,2/2] netpoll: allocate netdev tracker right away
    https://git.kernel.org/netdev/net-next/c/48eed027d310

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[Jakub Kicinski]

On Thu, 15 Jun 2023 07:50:20 +0000 patchwork-bot+netdevbpf@kernel.org
wrote:
> Here is the summary with links:
>   - [net-next,v2,1/2] net: create device lookup API with reference tracking
>     (no matching commit)

Hi Konstantin!

Any recent changes to the pw-bot in commit matching?
We don't do any editing when applying, AFAIK, and it's 3rd or 4th case
within a week we get a no-match.

Lore
https://lore.kernel.org/all/20230612214944.1837648-2-kuba@kernel.org/

commit
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=70f7457ad6d655e65f1b93cbba2a519e4b11c946

patchwork
https://patchwork.kernel.org/project/netdevbpf/patch/20230612214944.1837648-2-kuba@kernel.org/

Re: [PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[Konstantin Ryabitsev]

On Thu, Jun 15, 2023 at 10:00:21AM -0700, Jakub Kicinski wrote:
> Any recent changes to the pw-bot in commit matching?
> We don't do any editing when applying, AFAIK, and it's 3rd or 4th case
> within a week we get a no-match.

Did you, by chance, set your diff.algorithm to "histogram"? I noticed that the
diffs in your submission are very different from what I get when I run "git
show". E.g. notice this block in your email:

    --- a/net/core/dev.c
    +++ b/net/core/dev.c
    @@ -758,18 +758,7 @@  struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
     }
     EXPORT_SYMBOL(dev_get_by_name_rcu);
     
    -/**
    - *	dev_get_by_name		- find a device by its name
    - *	@net: the applicable net namespace
    - *	@name: name to find
    - *
    - *	Find an interface by name. This can be called from any
    - *	context and does its own locking. The returned handle has
    - *	the usage count incremented and the caller must use dev_put() to
    - *	release it when it is no longer needed. %NULL is returned if no
    - *	matching device is found.
    - */
    -
    +/* Deprecated for new users, call netdev_get_by_name() instead */
     struct net_device *dev_get_by_name(struct net *net, const char *name)
     {
        struct net_device *dev;

When I run "git show 70f7457ad6d655e65f1b93cbba2a519e4b11c946", I get a very
different looking diff:

    --- a/net/core/dev.c
    +++ b/net/core/dev.c
    @@ -758,29 +758,43 @@ struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
     }
     EXPORT_SYMBOL(dev_get_by_name_rcu);

    +/* Deprecated for new users, call netdev_get_by_name() instead */
    +struct net_device *dev_get_by_name(struct net *net, const char *name)
    +{
    + struct net_device *dev;
    +
    + rcu_read_lock();
    + dev = dev_get_by_name_rcu(net, name);
    + dev_hold(dev);
    + rcu_read_unlock();
    ...

Unless I pass --histogram, in which case it starts to match yours. So, I'm
wondering if you have diff.algorithm set to "histogram" and this generates
patches that we can no longer match against commits, because we are generating
the diffs using the default algorithm.

-K

Re: [PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[Jakub Kicinski]

On Thu, 15 Jun 2023 15:41:07 -0400 Konstantin Ryabitsev wrote:
> On Thu, Jun 15, 2023 at 10:00:21AM -0700, Jakub Kicinski wrote:
> > Any recent changes to the pw-bot in commit matching?
> > We don't do any editing when applying, AFAIK, and it's 3rd or 4th case
> > within a week we get a no-match.  
> 
> Did you, by chance, set your diff.algorithm to "histogram"? I noticed that the
> diffs in your submission are very different from what I get when I run "git
> show". E.g. notice this block in your email:
> 
>     --- a/net/core/dev.c
>     +++ b/net/core/dev.c
>     @@ -758,18 +758,7 @@  struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
>      }
>      EXPORT_SYMBOL(dev_get_by_name_rcu);
>      
>     -/**
>     - *	dev_get_by_name		- find a device by its name
>     - *	@net: the applicable net namespace
>     - *	@name: name to find
>     - *
>     - *	Find an interface by name. This can be called from any
>     - *	context and does its own locking. The returned handle has
>     - *	the usage count incremented and the caller must use dev_put() to
>     - *	release it when it is no longer needed. %NULL is returned if no
>     - *	matching device is found.
>     - */
>     -
>     +/* Deprecated for new users, call netdev_get_by_name() instead */
>      struct net_device *dev_get_by_name(struct net *net, const char *name)
>      {
>         struct net_device *dev;
> 
> When I run "git show 70f7457ad6d655e65f1b93cbba2a519e4b11c946", I get a very
> different looking diff:
> 
>     --- a/net/core/dev.c
>     +++ b/net/core/dev.c
>     @@ -758,29 +758,43 @@ struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
>      }
>      EXPORT_SYMBOL(dev_get_by_name_rcu);
> 
>     +/* Deprecated for new users, call netdev_get_by_name() instead */
>     +struct net_device *dev_get_by_name(struct net *net, const char *name)
>     +{
>     + struct net_device *dev;
>     +
>     + rcu_read_lock();
>     + dev = dev_get_by_name_rcu(net, name);
>     + dev_hold(dev);
>     + rcu_read_unlock();
>     ...
> 
> Unless I pass --histogram, in which case it starts to match yours. So, I'm
> wondering if you have diff.algorithm set to "histogram" and this generates
> patches that we can no longer match against commits, because we are generating
> the diffs using the default algorithm.

Oh, that could well be it, I did! Linus asked people to do that
recently, I think in the -rc4 email. One of the RC emails, anyway.
IDK how many people listened to Linus. Technically it only matters
for maintainers who may send PRs to him, otherwise he said his diffstat
will be different than the one in the PR.

But it's not just me:
https://lore.kernel.org/all/168674222282.23990.8151831714077509932.git-patchwork-notify@kernel.org/
https://lore.kernel.org/all/168661442392.10094.4616497599019441750.git-patchwork-notify@kernel.org/
(there's a third one but it's is also Matthieu so CBA to get the lore
link, we're just counting people.)

Could the bot try matching in histogram and non-histogram mode?

Re: [PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[Konstantin Ryabitsev]

On Thu, Jun 15, 2023 at 01:17:47PM -0700, Jakub Kicinski wrote:
> Oh, that could well be it, I did! Linus asked people to do that
> recently, I think in the -rc4 email. One of the RC emails, anyway.
> IDK how many people listened to Linus. Technically it only matters
> for maintainers who may send PRs to him, otherwise he said his diffstat
> will be different than the one in the PR.
> 
> But it's not just me:
> https://lore.kernel.org/all/168674222282.23990.8151831714077509932.git-patchwork-notify@kernel.org/
> https://lore.kernel.org/all/168661442392.10094.4616497599019441750.git-patchwork-notify@kernel.org/
> (there's a third one but it's is also Matthieu so CBA to get the lore
> link, we're just counting people.)
> 
> Could the bot try matching in histogram and non-histogram mode?

Yes, I'll see if I can teach the bot to regenerate the diff with --histogram
when there is no match.

-K

Re: [PATCH net-next v2 0/2] net: create device lookup API with reference tracking

[Jakub Kicinski]

On Thu, 15 Jun 2023 16:22:28 -0400 Konstantin Ryabitsev wrote:
> Yes, I'll see if I can teach the bot to regenerate the diff with --histogram
> when there is no match.

Perfect, thank you!

Re: [PATCH net-next v2 1/2] net: create device lookup API with reference tracking

[Eric Dumazet]

On Mon, Jun 12, 2023 at 11:49 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> New users of dev_get_by_index() and dev_get_by_name() keep
> getting added and it would be nice to steer them towards
> the APIs with reference tracking.
>
> Add variants of those calls which allocate the reference
> tracker and use them in a couple of places.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---


> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 392aaa373b66..e510a4162ef8 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3503,6 +3503,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>                  struct fib6_config *cfg, gfp_t gfp_flags,
>                  struct netlink_ext_ack *extack)
>  {
> +       netdevice_tracker *dev_tracker = &fib6_nh->fib_nh_dev_tracker;
>         struct net_device *dev = NULL;
>         struct inet6_dev *idev = NULL;
>         int addr_type;
> @@ -3520,7 +3521,8 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>
>         err = -ENODEV;
>         if (cfg->fc_ifindex) {
> -               dev = dev_get_by_index(net, cfg->fc_ifindex);
> +               dev = netdev_get_by_index(net, cfg->fc_ifindex,
> +                                         dev_tracker, gfp_flags);
>                 if (!dev)
>                         goto out;
>                 idev = in6_dev_get(dev);
> @@ -3554,11 +3556,11 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>                 /* hold loopback dev/idev if we haven't done so. */
>                 if (dev != net->loopback_dev) {
>                         if (dev) {
> -                               dev_put(dev);
> +                               netdev_put(dev, dev_tracker);
>                                 in6_dev_put(idev);
>                         }
>                         dev = net->loopback_dev;
> -                       dev_hold(dev);
> +                       netdev_hold(dev, dev_tracker, gfp_flags);
>                         idev = in6_dev_get(dev);
>                         if (!idev) {
>                                 err = -ENODEV;
> @@ -3610,8 +3612,6 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>         }
>
>         fib6_nh->fib_nh_dev = dev;
> -       netdev_tracker_alloc(dev, &fib6_nh->fib_nh_dev_tracker, gfp_flags);
> -
>         fib6_nh->fib_nh_oif = dev->ifindex;
>         err = 0;
>  out:
> @@ -3621,7 +3621,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>         if (err) {
>                 lwtstate_put(fib6_nh->fib_nh_lws);
>                 fib6_nh->fib_nh_lws = NULL;
> -               dev_put(dev);
> +               netdev_put(dev, dev_tracker);
>         }
>
>         return err;
> --
> 2.40.1
>

Oops, ip6_validate_gw() is able to change dev under us (this is done
in ip6_route_check_nh())

Crazy, I know...

Re: [PATCH net-next v2 1/2] net: create device lookup API with reference tracking

[David Ahern]

On 6/16/23 2:21 AM, Eric Dumazet wrote:
> Oops, ip6_validate_gw() is able to change dev under us (this is done
> in ip6_route_check_nh())
> 
> Crazy, I know...

I presume you saw this with a test case or syzbot? If so, can you share?

ip6_route_check_nh sets but never resets the device:

        if (dev) {
                if (dev != res.nh->fib_nh_dev)
                        err = -EHOSTUNREACH;
        } else {
                *_dev = dev = res.nh->fib_nh_dev;
                dev_hold(dev);
                *idev = in6_dev_get(dev);
        }