Commit 97d0dca7 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'mlxsw-avoid-non-tracker-helpers-when-holding-and-putting-netdevices'



Petr Machata says:

====================
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.

Suggested-by: default avatarPaolo Abeni <pabeni@redhat.com>
Link: https://lore.kernel.org/netdev/4c056da27c19d95ffeaba5acf1427ecadfc3f94c.camel@redhat.com/
====================

Link: https://lore.kernel.org/r/cover.1690471774.git.petrm@nvidia.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 5027d54a cb211620
Loading
Loading
Loading
Loading
+0 −17
Original line number Diff line number Diff line
@@ -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];
+0 −2
Original line number Diff line number Diff line
@@ -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);
+4 −3
Original line number Diff line number Diff line
@@ -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);
+15 −10
Original line number Diff line number Diff line
@@ -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];
@@ -7547,6 +7548,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 +7722,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 +7798,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;
	}
}
@@ -8306,6 +8309,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)
@@ -8317,7 +8321,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);
}

@@ -8337,7 +8341,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);
}
@@ -8409,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;
@@ -8466,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:
@@ -8508,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--;
@@ -9329,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;
};

@@ -9352,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);
}

@@ -9378,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;
+5 −4
Original line number Diff line number Diff line
@@ -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);