mlxsw: Avoid non-tracker helpers when holding and putting netdevices

PATCH Applied Petr Machata 3 participants 10 messages Jul 27, 2023 – Jul 28, 2023

Subsystems (3)

Networking Network drivers Mellanox Ethernet switch drivers

lore

Share & tools

lore

Petr Machata

author maintainer

2023-07-27 16:00 UTC #0 lore raw

cover mlxsw: Avoid non-tracker helpers when holding and putting netdevices

Using the tracking helpers, netdev_hold() and netdev_put(), makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled. For example, the following traceback shows the callpath to the point of an outstanding hold that was never put:

    unregister_netdevice: waiting for swp3 to become free. Usage count = 6
    ref_tracker: eth%d@ffff888123c9a580 has 1/5 users at
	mlxsw_sp_switchdev_event+0x6bd/0xcc0 [mlxsw_spectrum]
	notifier_call_chain+0xbf/0x3b0
	atomic_notifier_call_chain+0x78/0x200
	br_switchdev_fdb_notify+0x25f/0x2c0 [bridge]
	fdb_notify+0x16a/0x1a0 [bridge]
	[...]

In this patchset, get rid of all non-ref-tracking helpers in mlxsw.

- Patch #1 drops two functions that are not used anymore, but contain dev_hold() / dev_put() calls.

- Patch #2 avoids taking a reference in one function which is called under RTNL.

- The remaining patches convert individual hold/put sites one by one from trackerless to tracker-enabled.

Link https://lore.kernel.org/netdev/[email protected]/ open here

Petr Machata (7):
mlxsw: spectrum: Drop unused functions

    mlxsw_sp_port_lower_dev_hold/_put()

mlxsw: spectrum_nve: Do not take reference when looking up netdevice
mlxsw: spectrum_switchdev: Use tracker helpers to hold & put
netdevices
mlxsw: spectrum_router: FIB: Use tracker helpers to hold & put
netdevices
mlxsw: spectrum_router: hw_stats: Use tracker helpers to hold & put
netdevices
mlxsw: spectrum_router: RIF: Use tracker helpers to hold & put
netdevices
mlxsw: spectrum_router: IPv6 events: Use tracker helpers to hold & put
netdevices

 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 17 -------------
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  2 --
 .../ethernet/mellanox/mlxsw/spectrum_nve.c    |  7 +++---
 .../ethernet/mellanox/mlxsw/spectrum_router.c | 25 +++++++++++--------
 .../mellanox/mlxsw/spectrum_switchdev.c       |  9 ++++---
 5 files changed, 24 insertions(+), 36 deletions(-)

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #1 lore raw

PATCH 1/7 mlxsw: spectrum: Drop unused functions mlxsw_sp_port_lower_dev_hold/_put()

As of commit 151b89f6025a ("mlxsw: spectrum_router: Reuse work neighbor initialization in work scheduler"), the functions mlxsw_sp_port_lower_dev_hold() and mlxsw_sp_port_dev_put() have no users. Drop them.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 17 -----------------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h |  2 --
 2 files changed, 19 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum.c +0 ~~-17~~

index f0f6af3ec7c5..9dbd5edff0b0 100644
@@ -4112,23 +4112,6 @@ struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find_rcu(struct net_device *dev)
 	return (struct mlxsw_sp_port *)priv.data;
 }
 
-struct mlxsw_sp_port *mlxsw_sp_port_lower_dev_hold(struct net_device *dev)
-{
-	struct mlxsw_sp_port *mlxsw_sp_port;
-
-	rcu_read_lock();
-	mlxsw_sp_port = mlxsw_sp_port_dev_lower_find_rcu(dev);
-	if (mlxsw_sp_port)
-		dev_hold(mlxsw_sp_port->dev);
-	rcu_read_unlock();
-	return mlxsw_sp_port;
-}
-
-void mlxsw_sp_port_dev_put(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-	dev_put(mlxsw_sp_port->dev);
-}
-
 int mlxsw_sp_parsing_depth_inc(struct mlxsw_sp *mlxsw_sp)
 {
 	char mprs_pl[MLXSW_REG_MPRS_LEN];

drivers/net/ethernet/mellanox/mlxsw/spectrum.h +0 -2

index 65eaa181e0aa..62151f0531ae 100644
@@ -720,8 +720,6 @@ int mlxsw_sp_txhdr_ptp_data_construct(struct mlxsw_core *mlxsw_core,
 bool mlxsw_sp_port_dev_check(const struct net_device *dev);
 struct mlxsw_sp *mlxsw_sp_lower_get(struct net_device *dev);
 struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find(struct net_device *dev);
-struct mlxsw_sp_port *mlxsw_sp_port_lower_dev_hold(struct net_device *dev);
-void mlxsw_sp_port_dev_put(struct mlxsw_sp_port *mlxsw_sp_port);
 struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find_rcu(struct net_device *dev);
 int mlxsw_sp_parsing_depth_inc(struct mlxsw_sp *mlxsw_sp);
 void mlxsw_sp_parsing_depth_dec(struct mlxsw_sp *mlxsw_sp);

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #2 lore raw

PATCH 2/7 mlxsw: spectrum_nve: Do not take reference when looking up netdevice

In reply to Petr Machata

mlxsw_sp_nve_fid_disable() is always called under RTNL. It is therefore safe to call __dev_get_by_index() to get the netdevice pointer without bumping the reference count, because we can be sure the netdevice is not going away. That then obviates the need to put the netdevice later in the function.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c +4 -3

index d2b57a045aa4..5479a1c19d2e 100644
@@ -989,6 +989,9 @@ void mlxsw_sp_nve_fid_disable(struct mlxsw_sp *mlxsw_sp,
 	int nve_ifindex;
 	__be32 vni;
 
+	/* Necessary for __dev_get_by_index() below. */
+	ASSERT_RTNL();
+
 	mlxsw_sp_nve_flood_ip_flush(mlxsw_sp, fid);
 	mlxsw_sp_nve_fdb_flush_by_fid(mlxsw_sp, fid_index);
 	mlxsw_sp_nve_ipv6_addr_flush_by_fid(mlxsw_sp, fid_index);

@@ -997,15 +1000,13 @@ void mlxsw_sp_nve_fid_disable(struct mlxsw_sp *mlxsw_sp,
 		    mlxsw_sp_fid_vni(fid, &vni)))
 		goto out;
 
-	nve_dev = dev_get_by_index(mlxsw_sp_net(mlxsw_sp), nve_ifindex);
+	nve_dev = __dev_get_by_index(mlxsw_sp_net(mlxsw_sp), nve_ifindex);
 	if (!nve_dev)
 		goto out;
 
 	mlxsw_sp_nve_fdb_clear_offload(mlxsw_sp, fid, nve_dev, vni);
 	mlxsw_sp_fid_fdb_clear_offload(fid, nve_dev);
 
-	dev_put(nve_dev);
-
 out:
 	mlxsw_sp_fid_vni_clear(fid);
 	mlxsw_sp_nve_tunnel_fini(mlxsw_sp);

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #3 lore raw

PATCH 3/7 mlxsw: spectrum_switchdev: Use tracker helpers to hold & put netdevices

In reply to Petr Machata

Using the tracking helpers makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled.

Convert dev_hold() / dev_put() to netdev_hold() / netdev_put() in the switchdev module.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +5 -4

index dffb67c1038e..5376d4af5f91 100644
@@ -3380,6 +3380,7 @@ static void mlxsw_sp_fdb_notify_work(struct work_struct *work)
 
 struct mlxsw_sp_switchdev_event_work {
 	struct work_struct work;
+	netdevice_tracker dev_tracker;
 	union {
 		struct switchdev_notifier_fdb_info fdb_info;
 		struct switchdev_notifier_vxlan_fdb_info vxlan_fdb_info;

@@ -3536,8 +3537,8 @@ static void mlxsw_sp_switchdev_bridge_fdb_event_work(struct work_struct *work)
 out:
 	rtnl_unlock();
 	kfree(switchdev_work->fdb_info.addr);
+	netdev_put(dev, &switchdev_work->dev_tracker);
 	kfree(switchdev_work);
-	dev_put(dev);
 }
 
 static void

@@ -3692,8 +3693,8 @@ static void mlxsw_sp_switchdev_vxlan_fdb_event_work(struct work_struct *work)
 
 out:
 	rtnl_unlock();
+	netdev_put(dev, &switchdev_work->dev_tracker);
 	kfree(switchdev_work);
-	dev_put(dev);
 }
 
 static int

@@ -3793,7 +3794,7 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
 		 * upper device containig mlxsw_sp_port or just a
 		 * mlxsw_sp_port
 		 */
-		dev_hold(dev);
+		netdev_hold(dev, &switchdev_work->dev_tracker, GFP_ATOMIC);
 		break;
 	case SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE:
 	case SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE:

@@ -3803,7 +3804,7 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
 							    info);
 		if (err)
 			goto err_vxlan_work_prepare;
-		dev_hold(dev);
+		netdev_hold(dev, &switchdev_work->dev_tracker, GFP_ATOMIC);
 		break;
 	default:
 		kfree(switchdev_work);

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #4 lore raw

PATCH 4/7 mlxsw: spectrum_router: FIB: Use tracker helpers to hold & put netdevices

In reply to Petr Machata

Using the tracking helpers makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled.

Convert dev_hold() / dev_put() to netdev_hold() / netdev_put() in the router code that deals with FIB events.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +5 -3

index 57f0faac836c..0744497f2762 100644
@@ -7547,6 +7547,7 @@ struct mlxsw_sp_fib6_event_work {
 
 struct mlxsw_sp_fib_event_work {
 	struct work_struct work;
+	netdevice_tracker dev_tracker;
 	union {
 		struct mlxsw_sp_fib6_event_work fib6_work;
 		struct fib_entry_notifier_info fen_info;

@@ -7720,12 +7721,12 @@ static void mlxsw_sp_router_fibmr_event_work(struct work_struct *work)
 						    &fib_work->ven_info);
 		if (err)
 			dev_warn(mlxsw_sp->bus_info->dev, "MR VIF add failed.\n");
-		dev_put(fib_work->ven_info.dev);
+		netdev_put(fib_work->ven_info.dev, &fib_work->dev_tracker);
 		break;
 	case FIB_EVENT_VIF_DEL:
 		mlxsw_sp_router_fibmr_vif_del(mlxsw_sp,
 					      &fib_work->ven_info);
-		dev_put(fib_work->ven_info.dev);
+		netdev_put(fib_work->ven_info.dev, &fib_work->dev_tracker);
 		break;
 	}
 	mutex_unlock(&mlxsw_sp->router->lock);

@@ -7796,7 +7797,8 @@ mlxsw_sp_router_fibmr_event(struct mlxsw_sp_fib_event_work *fib_work,
 	case FIB_EVENT_VIF_ADD:
 	case FIB_EVENT_VIF_DEL:
 		memcpy(&fib_work->ven_info, info, sizeof(fib_work->ven_info));
-		dev_hold(fib_work->ven_info.dev);
+		netdev_hold(fib_work->ven_info.dev, &fib_work->dev_tracker,
+			    GFP_ATOMIC);
 		break;
 	}
 }

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #5 lore raw

PATCH 5/7 mlxsw: spectrum_router: hw_stats: Use tracker helpers to hold & put netdevices

In reply to Petr Machata

Using the tracking helpers makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled.

Convert dev_hold() / dev_put() to netdev_hold() / netdev_put() in the router code that deals with hw_stats events.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +3 -2

index 0744497f2762..3ecd34ad0743 100644
@@ -8308,6 +8308,7 @@ mlxsw_sp_router_port_l3_stats_report_delta(struct mlxsw_sp_rif *rif,
 struct mlxsw_sp_router_hwstats_notify_work {
 	struct work_struct work;
 	struct net_device *dev;
+	netdevice_tracker dev_tracker;
 };
 
 static void mlxsw_sp_router_hwstats_notify_work(struct work_struct *work)

@@ -8319,7 +8320,7 @@ static void mlxsw_sp_router_hwstats_notify_work(struct work_struct *work)
 	rtnl_lock();
 	rtnl_offload_xstats_notify(hws_work->dev);
 	rtnl_unlock();
-	dev_put(hws_work->dev);
+	netdev_put(hws_work->dev, &hws_work->dev_tracker);
 	kfree(hws_work);
 }

@@ -8339,7 +8340,7 @@ mlxsw_sp_router_hwstats_notify_schedule(struct net_device *dev)
 		return;
 
 	INIT_WORK(&hws_work->work, mlxsw_sp_router_hwstats_notify_work);
-	dev_hold(dev);
+	netdev_hold(dev, &hws_work->dev_tracker, GFP_KERNEL);
 	hws_work->dev = dev;
 	mlxsw_core_schedule_work(&hws_work->work);
 }

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #6 lore raw

PATCH 6/7 mlxsw: spectrum_router: RIF: Use tracker helpers to hold & put netdevices

In reply to Petr Machata

Using the tracking helpers makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled.

Convert dev_hold() / dev_put() to netdev_hold() / netdev_put() in the router code that deals with RIF allocation.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +4 -3

index 3ecd34ad0743..59d12cf45939 100644
@@ -71,6 +71,7 @@ static const struct rhashtable_params mlxsw_sp_crif_ht_params = {
 
 struct mlxsw_sp_rif {
 	struct mlxsw_sp_crif *crif; /* NULL for underlay RIF */
+	netdevice_tracker dev_tracker;
 	struct list_head neigh_list;
 	struct mlxsw_sp_fid *fid;
 	unsigned char addr[ETH_ALEN];

@@ -8412,7 +8413,7 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
 		err = -ENOMEM;
 		goto err_rif_alloc;
 	}
-	dev_hold(params->dev);
+	netdev_hold(params->dev, &rif->dev_tracker, GFP_KERNEL);
 	mlxsw_sp->router->rifs[rif_index] = rif;
 	rif->mlxsw_sp = mlxsw_sp;
 	rif->ops = ops;

@@ -8469,7 +8470,7 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
 		mlxsw_sp_fid_put(fid);
 err_fid_get:
 	mlxsw_sp->router->rifs[rif_index] = NULL;
-	dev_put(params->dev);
+	netdev_put(params->dev, &rif->dev_tracker);
 	mlxsw_sp_rif_free(rif);
 err_rif_alloc:
 err_crif_lookup:

@@ -8511,7 +8512,7 @@ static void mlxsw_sp_rif_destroy(struct mlxsw_sp_rif *rif)
 		/* Loopback RIFs are not associated with a FID. */
 		mlxsw_sp_fid_put(fid);
 	mlxsw_sp->router->rifs[rif->rif_index] = NULL;
-	dev_put(dev);
+	netdev_put(dev, &rif->dev_tracker);
 	mlxsw_sp_rif_free(rif);
 	mlxsw_sp_rif_index_free(mlxsw_sp, rif_index, rif_entries);
 	vr->rif_count--;

Signature · 1 line

2.41.0

Petr Machata

author maintainer

2023-07-27 16:00 UTC #7 lore raw

PATCH 7/7 mlxsw: spectrum_router: IPv6 events: Use tracker helpers to hold & put netdevices

In reply to Petr Machata

Using the tracking helpers makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled.

Convert dev_hold() / dev_put() to netdev_hold() / netdev_put() in the router code that deals with IPv6 address events.

Signed-off-by Petr Machata <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +3 -2

index 59d12cf45939..debd2c466f11 100644
@@ -9333,6 +9333,7 @@ struct mlxsw_sp_inet6addr_event_work {
 	struct work_struct work;
 	struct mlxsw_sp *mlxsw_sp;
 	struct net_device *dev;
+	netdevice_tracker dev_tracker;
 	unsigned long event;
 };

@@ -9356,7 +9357,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
 out:
 	mutex_unlock(&mlxsw_sp->router->lock);
 	rtnl_unlock();
-	dev_put(dev);
+	netdev_put(dev, &inet6addr_work->dev_tracker);
 	kfree(inet6addr_work);
 }

@@ -9382,7 +9383,7 @@ static int mlxsw_sp_inet6addr_event(struct notifier_block *nb,
 	inet6addr_work->mlxsw_sp = router->mlxsw_sp;
 	inet6addr_work->dev = dev;
 	inet6addr_work->event = event;
-	dev_hold(dev);
+	netdev_hold(dev, &inet6addr_work->dev_tracker, GFP_ATOMIC);
 	mlxsw_core_schedule_work(&inet6addr_work->work);
 
 	return NOTIFY_DONE;

Signature · 1 line

2.41.0

Eric Dumazet

Reviewed-by on the v1 series

2023-07-27 17:02 UTC #8 lore raw

In reply to Petr Machata

Quoted 15 lines · Petr Machata

Using the tracking helpers, netdev_hold() and netdev_put(), makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled. For example, the following traceback shows the callpath to the point of an outstanding hold that was never put:
    unregister_netdevice: waiting for swp3 to become free. Usage count = 6
    ref_tracker: eth%d@ffff888123c9a580 has 1/5 users at
        mlxsw_sp_switchdev_event+0x6bd/0xcc0 [mlxsw_spectrum]
        notifier_call_chain+0xbf/0x3b0
        atomic_notifier_call_chain+0x78/0x200
        br_switchdev_fdb_notify+0x25f/0x2c0 [bridge]
        fdb_notify+0x16a/0x1a0 [bridge]
        [...]

Quoted 1 line

In this patchset, get rid of all non-ref-tracking helpers in mlxsw.

For the series :

Reviewed-by Eric Dumazet <[email protected]>

[email protected]

CI Applied

2023-07-28 20:50 UTC #9 lore raw

In reply to Petr Machata

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 14 lines · On Thu, 27 Jul 2023 17:59:18 +0200 you wrote:

Using the tracking helpers, netdev_hold() and netdev_put(), makes it easier to debug netdevice refcount imbalances when CONFIG_NET_DEV_REFCNT_TRACKER is enabled. For example, the following traceback shows the callpath to the point of an outstanding hold that was never put:
    unregister_netdevice: waiting for swp3 to become free. Usage count = 6
    ref_tracker: eth%d@ffff888123c9a580 has 1/5 users at
	mlxsw_sp_switchdev_event+0x6bd/0xcc0 [mlxsw_spectrum]
	notifier_call_chain+0xbf/0x3b0
	atomic_notifier_call_chain+0x78/0x200
	br_switchdev_fdb_notify+0x25f/0x2c0 [bridge]
	fdb_notify+0x16a/0x1a0 [bridge]
	[...]

Quoted 1 line

[...]

Here is the summary with links:
- [net-next,1/7] mlxsw: spectrum: Drop unused functions mlxsw_sp_port_lower_dev_hold/_put()

    https://git.kernel.org/netdev/net-next/c/569f98b36b38
  - [net-next,2/7] mlxsw: spectrum_nve: Do not take reference when looking up netdevice
    https://git.kernel.org/netdev/net-next/c/16f8c846cd6f
  - [net-next,3/7] mlxsw: spectrum_switchdev: Use tracker helpers to hold & put netdevices
    https://git.kernel.org/netdev/net-next/c/1ae489ab43e0
  - [net-next,4/7] mlxsw: spectrum_router: FIB: Use tracker helpers to hold & put netdevices
    https://git.kernel.org/netdev/net-next/c/deeaa3716f4f
  - [net-next,5/7] mlxsw: spectrum_router: hw_stats: Use tracker helpers to hold & put netdevices
    https://git.kernel.org/netdev/net-next/c/b17b2d57b7c1
  - [net-next,6/7] mlxsw: spectrum_router: RIF: Use tracker helpers to hold & put netdevices
    https://git.kernel.org/netdev/net-next/c/d0e0e880122f
  - [net-next,7/7] mlxsw: spectrum_router: IPv6 events: Use tracker helpers to hold & put netdevices
    https://git.kernel.org/netdev/net-next/c/cb2116204169

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html