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

[PATCH net-next 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 the 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.

Eric, LMK if you prefer to keep the current flow, maybe it's just me.

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     |  8 ++---
 net/ipv6/route.c          | 12 +++----
 5 files changed, 67 insertions(+), 37 deletions(-)

-- 
2.40.1

[PATCH net-next 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>
---
 include/linux/netdevice.h |  4 +++
 net/core/dev.c            | 75 ++++++++++++++++++++++++++-------------
 net/ethtool/netlink.c     |  8 ++---
 net/ipv6/route.c          | 12 +++----
 4 files changed, 65 insertions(+), 34 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c2f0c6002a84..732d7a226e93 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 6d6f8a7fe6b4..0e9419d220bf 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 08120095cc68..107ef80e48e1 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -113,7 +113,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],
@@ -129,7 +130,8 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
 			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");
@@ -142,8 +144,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

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

[Eric Dumazet]

On Fri, Jun 9, 2023 at 8:32 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>
> ---
>  include/linux/netdevice.h |  4 +++
>  net/core/dev.c            | 75 ++++++++++++++++++++++++++-------------
>  net/ethtool/netlink.c     |  8 ++---
>  net/ipv6/route.c          | 12 +++----
>  4 files changed, 65 insertions(+), 34 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c2f0c6002a84..732d7a226e93 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 6d6f8a7fe6b4..0e9419d220bf 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 08120095cc68..107ef80e48e1 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -113,7 +113,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],

You forgot to change dev_put(dev) at line 126

> @@ -129,7 +130,8 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
>                         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");
> @@ -142,8 +144,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;
>  }

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

[Jakub Kicinski]

On Fri, 9 Jun 2023 20:50:07 +0200 Eric Dumazet wrote:
> > -               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],  
> 
> You forgot to change dev_put(dev) at line 126

Oh, good catch!

> Oh well, this would need GFP_ATOMIC, I guess this might be the reason
> you kept netdev_tracker_alloc()
> that can run after rcu_read_unlock()

Yes :(

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

[Eric Dumazet]

On Fri, Jun 9, 2023 at 8:32 PM Jakub Kicinski <kuba@kernel.org> wrote:
>


> +/**
> + *     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);


What about making instead dev_get_by_name(net, name) a wrapper around
the real thing ?

static inline struct net_device *dev_get_by_name(struct net *net,
const char *name)
{
    return netdev_get_by_name(net, name, NULL, 0);
}

This means netdev_get_by_name() could directly use netdev_hold()
instead of netdev_tracker_alloc() which is a bit confusing IMO.


diff --git a/net/core/dev.c b/net/core/dev.c
index b3c13e0419356b943e90b1f46dd7e035c6ec1a9c..b99f25c7fa0aad1eeb7fa117aaea2a0e16813fe0
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -759,9 +759,11 @@ 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
+ *     netdev_get_by_name              - find a device by its name
  *     @net: the applicable net namespace
  *     @name: name to find
+ *     @tracker: tracker
+ *     @gfp: allocation flag for tracker
  *
  *     Find an interface by name. This can be called from any
  *     context and does its own locking. The returned handle has
@@ -770,17 +772,18 @@ EXPORT_SYMBOL(dev_get_by_name_rcu);
  *     matching device is found.
  */

-struct net_device *dev_get_by_name(struct net *net, const char *name)
+struct net_device *netdev_get_by_name(struct net *net, const char *name,
+                                    netdevice_tracker *tracker, gfp_t gfp))
 {
        struct net_device *dev;

        rcu_read_lock();
        dev = dev_get_by_name_rcu(net, name);
-       dev_hold(dev);
+       netdev_hold(dev, tracker, gfp);
        rcu_read_unlock();
        return dev;
 }
-EXPORT_SYMBOL(dev_get_by_name);
+EXPORT_SYMBOL(netdev_get_by_name);

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

[Eric Dumazet]

On Fri, Jun 9, 2023 at 8:57 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Jun 9, 2023 at 8:32 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
>
>
> > +/**
> > + *     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);
>
>
> What about making instead dev_get_by_name(net, name) a wrapper around
> the real thing ?
>
> static inline struct net_device *dev_get_by_name(struct net *net,
> const char *name)
> {
>     return netdev_get_by_name(net, name, NULL, 0);
> }
>
> This means netdev_get_by_name() could directly use netdev_hold()
> instead of netdev_tracker_alloc() which is a bit confusing IMO.
>
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b3c13e0419356b943e90b1f46dd7e035c6ec1a9c..b99f25c7fa0aad1eeb7fa117aaea2a0e16813fe0
> 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -759,9 +759,11 @@ 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
> + *     netdev_get_by_name              - find a device by its name
>   *     @net: the applicable net namespace
>   *     @name: name to find
> + *     @tracker: tracker
> + *     @gfp: allocation flag for tracker
>   *
>   *     Find an interface by name. This can be called from any
>   *     context and does its own locking. The returned handle has
> @@ -770,17 +772,18 @@ EXPORT_SYMBOL(dev_get_by_name_rcu);
>   *     matching device is found.
>   */
>
> -struct net_device *dev_get_by_name(struct net *net, const char *name)
> +struct net_device *netdev_get_by_name(struct net *net, const char *name,
> +                                    netdevice_tracker *tracker, gfp_t gfp))
>  {
>         struct net_device *dev;
>
>         rcu_read_lock();
>         dev = dev_get_by_name_rcu(net, name);
> -       dev_hold(dev);
> +       netdev_hold(dev, tracker, gfp);

Oh well, this would need GFP_ATOMIC, I guess this might be the reason
you kept netdev_tracker_alloc()
that can run after rcu_read_unlock()

>         rcu_read_unlock();
>         return dev;
>  }
> -EXPORT_SYMBOL(dev_get_by_name);
> +EXPORT_SYMBOL(netdev_get_by_name);

[PATCH net-next 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().

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 2/2] netpoll: allocate netdev tracker right away

[Eric Dumazet]

On Fri, Jun 9, 2023 at 8:32 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> 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().
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>

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

Thanks.