Commit 31265c1e authored by Jiri Pirko's avatar Jiri Pirko Committed by Jakub Kicinski
Browse files

net: devlink: store copy netdevice ifindex and ifname to allow port_fill() without RTNL held



To avoid a need to take RTNL mutex in port_fill() function, benefit from
the introduce infrastructure that tracks netdevice notifier events.
Store the ifindex and ifname upon register and change name events.
Remove the rtnl_held bool propagated down to port_fill() function as it
is no longer needed.

Signed-off-by: default avatarJiri Pirko <jiri@nvidia.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent d0f51726
Loading
Loading
Loading
Loading
+2 −0
Original line number Original line Diff line number Diff line
@@ -129,6 +129,8 @@ struct devlink_port {
	union {
	union {
		struct {
		struct {
			struct net_device *netdev;
			struct net_device *netdev;
			int ifindex;
			char ifname[IFNAMSIZ];
		} type_eth;
		} type_eth;
		struct {
		struct {
			struct ib_device *ibdev;
			struct ib_device *ibdev;
+27 −41
Original line number Original line Diff line number Diff line
@@ -1279,8 +1279,7 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
static int devlink_nl_port_fill(struct sk_buff *msg,
static int devlink_nl_port_fill(struct sk_buff *msg,
				struct devlink_port *devlink_port,
				struct devlink_port *devlink_port,
				enum devlink_command cmd, u32 portid, u32 seq,
				enum devlink_command cmd, u32 portid, u32 seq,
				int flags, struct netlink_ext_ack *extack,
				int flags, struct netlink_ext_ack *extack)
				bool rtnl_held)
{
{
	struct devlink *devlink = devlink_port->devlink;
	struct devlink *devlink = devlink_port->devlink;
	void *hdr;
	void *hdr;
@@ -1294,9 +1293,6 @@ static int devlink_nl_port_fill(struct sk_buff *msg,
	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, devlink_port->index))
	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, devlink_port->index))
		goto nla_put_failure;
		goto nla_put_failure;


	/* Hold rtnl lock while accessing port's netdev attributes. */
	if (!rtnl_held)
		rtnl_lock();
	spin_lock_bh(&devlink_port->type_lock);
	spin_lock_bh(&devlink_port->type_lock);
	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_TYPE, devlink_port->type))
	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_TYPE, devlink_port->type))
		goto nla_put_failure_type_locked;
		goto nla_put_failure_type_locked;
@@ -1305,13 +1301,11 @@ static int devlink_nl_port_fill(struct sk_buff *msg,
			devlink_port->desired_type))
			devlink_port->desired_type))
		goto nla_put_failure_type_locked;
		goto nla_put_failure_type_locked;
	if (devlink_port->type == DEVLINK_PORT_TYPE_ETH) {
	if (devlink_port->type == DEVLINK_PORT_TYPE_ETH) {
		struct net_device *netdev = devlink_port->type_eth.netdev;
		if (devlink_port->type_eth.netdev &&

		if (netdev &&
		    (nla_put_u32(msg, DEVLINK_ATTR_PORT_NETDEV_IFINDEX,
		    (nla_put_u32(msg, DEVLINK_ATTR_PORT_NETDEV_IFINDEX,
				 netdev->ifindex) ||
				 devlink_port->type_eth.ifindex) ||
		     nla_put_string(msg, DEVLINK_ATTR_PORT_NETDEV_NAME,
		     nla_put_string(msg, DEVLINK_ATTR_PORT_NETDEV_NAME,
				    netdev->name)))
				    devlink_port->type_eth.ifname)))
			goto nla_put_failure_type_locked;
			goto nla_put_failure_type_locked;
	}
	}
	if (devlink_port->type == DEVLINK_PORT_TYPE_IB) {
	if (devlink_port->type == DEVLINK_PORT_TYPE_IB) {
@@ -1323,8 +1317,6 @@ static int devlink_nl_port_fill(struct sk_buff *msg,
			goto nla_put_failure_type_locked;
			goto nla_put_failure_type_locked;
	}
	}
	spin_unlock_bh(&devlink_port->type_lock);
	spin_unlock_bh(&devlink_port->type_lock);
	if (!rtnl_held)
		rtnl_unlock();
	if (devlink_nl_port_attrs_put(msg, devlink_port))
	if (devlink_nl_port_attrs_put(msg, devlink_port))
		goto nla_put_failure;
		goto nla_put_failure;
	if (devlink_nl_port_function_attrs_put(msg, devlink_port, extack))
	if (devlink_nl_port_function_attrs_put(msg, devlink_port, extack))
@@ -1339,15 +1331,13 @@ static int devlink_nl_port_fill(struct sk_buff *msg,


nla_put_failure_type_locked:
nla_put_failure_type_locked:
	spin_unlock_bh(&devlink_port->type_lock);
	spin_unlock_bh(&devlink_port->type_lock);
	if (!rtnl_held)
		rtnl_unlock();
nla_put_failure:
nla_put_failure:
	genlmsg_cancel(msg, hdr);
	genlmsg_cancel(msg, hdr);
	return -EMSGSIZE;
	return -EMSGSIZE;
}
}


static void __devlink_port_notify(struct devlink_port *devlink_port,
static void devlink_port_notify(struct devlink_port *devlink_port,
				  enum devlink_command cmd, bool rtnl_held)
				enum devlink_command cmd)
{
{
	struct devlink *devlink = devlink_port->devlink;
	struct devlink *devlink = devlink_port->devlink;
	struct sk_buff *msg;
	struct sk_buff *msg;
@@ -1362,8 +1352,7 @@ static void __devlink_port_notify(struct devlink_port *devlink_port,
	if (!msg)
	if (!msg)
		return;
		return;


	err = devlink_nl_port_fill(msg, devlink_port, cmd, 0, 0, 0, NULL,
	err = devlink_nl_port_fill(msg, devlink_port, cmd, 0, 0, 0, NULL);
				   rtnl_held);
	if (err) {
	if (err) {
		nlmsg_free(msg);
		nlmsg_free(msg);
		return;
		return;
@@ -1373,12 +1362,6 @@ static void __devlink_port_notify(struct devlink_port *devlink_port,
				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
				0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
}
}


static void devlink_port_notify(struct devlink_port *devlink_port,
				enum devlink_command cmd)
{
	__devlink_port_notify(devlink_port, cmd, false);
}

static void devlink_rate_notify(struct devlink_rate *devlink_rate,
static void devlink_rate_notify(struct devlink_rate *devlink_rate,
				enum devlink_command cmd)
				enum devlink_command cmd)
{
{
@@ -1542,7 +1525,7 @@ static int devlink_nl_cmd_port_get_doit(struct sk_buff *skb,


	err = devlink_nl_port_fill(msg, devlink_port, DEVLINK_CMD_PORT_NEW,
	err = devlink_nl_port_fill(msg, devlink_port, DEVLINK_CMD_PORT_NEW,
				   info->snd_portid, info->snd_seq, 0,
				   info->snd_portid, info->snd_seq, 0,
				   info->extack, false);
				   info->extack);
	if (err) {
	if (err) {
		nlmsg_free(msg);
		nlmsg_free(msg);
		return err;
		return err;
@@ -1572,8 +1555,7 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
						   DEVLINK_CMD_NEW,
						   DEVLINK_CMD_NEW,
						   NETLINK_CB(cb->skb).portid,
						   NETLINK_CB(cb->skb).portid,
						   cb->nlh->nlmsg_seq,
						   cb->nlh->nlmsg_seq,
						   NLM_F_MULTI, cb->extack,
						   NLM_F_MULTI, cb->extack);
						   false);
			if (err) {
			if (err) {
				devl_unlock(devlink);
				devl_unlock(devlink);
				devlink_put(devlink);
				devlink_put(devlink);
@@ -1785,8 +1767,7 @@ static int devlink_port_new_notify(struct devlink *devlink,
	}
	}


	err = devlink_nl_port_fill(msg, devlink_port, DEVLINK_CMD_NEW,
	err = devlink_nl_port_fill(msg, devlink_port, DEVLINK_CMD_NEW,
				   info->snd_portid, info->snd_seq, 0, NULL,
				   info->snd_portid, info->snd_seq, 0, NULL);
				   false);
	if (err)
	if (err)
		goto out;
		goto out;


@@ -10062,7 +10043,7 @@ static void devlink_port_type_netdev_checks(struct devlink_port *devlink_port,


static void __devlink_port_type_set(struct devlink_port *devlink_port,
static void __devlink_port_type_set(struct devlink_port *devlink_port,
				    enum devlink_port_type type,
				    enum devlink_port_type type,
				    void *type_dev, bool rtnl_held)
				    void *type_dev)
{
{
	struct net_device *netdev = type_dev;
	struct net_device *netdev = type_dev;


@@ -10081,6 +10062,13 @@ static void __devlink_port_type_set(struct devlink_port *devlink_port,
	switch (type) {
	switch (type) {
	case DEVLINK_PORT_TYPE_ETH:
	case DEVLINK_PORT_TYPE_ETH:
		devlink_port->type_eth.netdev = netdev;
		devlink_port->type_eth.netdev = netdev;
		if (netdev) {
			ASSERT_RTNL();
			devlink_port->type_eth.ifindex = netdev->ifindex;
			BUILD_BUG_ON(sizeof(devlink_port->type_eth.ifname) !=
				     sizeof(netdev->name));
			strcpy(devlink_port->type_eth.ifname, netdev->name);
		}
		break;
		break;
	case DEVLINK_PORT_TYPE_IB:
	case DEVLINK_PORT_TYPE_IB:
		devlink_port->type_ib.ibdev = type_dev;
		devlink_port->type_ib.ibdev = type_dev;
@@ -10089,7 +10077,7 @@ static void __devlink_port_type_set(struct devlink_port *devlink_port,
		break;
		break;
	}
	}
	spin_unlock_bh(&devlink_port->type_lock);
	spin_unlock_bh(&devlink_port->type_lock);
	__devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW, rtnl_held);
	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
}
}


/**
/**
@@ -10104,8 +10092,7 @@ void devlink_port_type_eth_set(struct devlink_port *devlink_port)
	dev_warn(devlink_port->devlink->dev,
	dev_warn(devlink_port->devlink->dev,
		 "devlink port type for port %d set to Ethernet without a software interface reference, device type not supported by the kernel?\n",
		 "devlink port type for port %d set to Ethernet without a software interface reference, device type not supported by the kernel?\n",
		 devlink_port->index);
		 devlink_port->index);
	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH, NULL,
	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH, NULL);
				false);
}
}
EXPORT_SYMBOL_GPL(devlink_port_type_eth_set);
EXPORT_SYMBOL_GPL(devlink_port_type_eth_set);


@@ -10118,8 +10105,7 @@ EXPORT_SYMBOL_GPL(devlink_port_type_eth_set);
void devlink_port_type_ib_set(struct devlink_port *devlink_port,
void devlink_port_type_ib_set(struct devlink_port *devlink_port,
			      struct ib_device *ibdev)
			      struct ib_device *ibdev)
{
{
	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_IB, ibdev,
	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_IB, ibdev);
				false);
}
}
EXPORT_SYMBOL_GPL(devlink_port_type_ib_set);
EXPORT_SYMBOL_GPL(devlink_port_type_ib_set);


@@ -10137,8 +10123,7 @@ void devlink_port_type_clear(struct devlink_port *devlink_port)
		dev_warn(devlink_port->devlink->dev,
		dev_warn(devlink_port->devlink->dev,
			 "devlink port type for port %d cleared without a software interface reference, device type not supported by the kernel?\n",
			 "devlink port type for port %d cleared without a software interface reference, device type not supported by the kernel?\n",
			 devlink_port->index);
			 devlink_port->index);
	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_NOTSET, NULL,
	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_NOTSET, NULL);
				false);
}
}
EXPORT_SYMBOL_GPL(devlink_port_type_clear);
EXPORT_SYMBOL_GPL(devlink_port_type_clear);


@@ -10161,16 +10146,17 @@ static int devlink_netdevice_event(struct notifier_block *nb,
		 * netdevice register
		 * netdevice register
		 */
		 */
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH,
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH,
					NULL, true);
					NULL);
		break;
		break;
	case NETDEV_REGISTER:
	case NETDEV_REGISTER:
	case NETDEV_CHANGENAME:
		/* Set the netdev on top of previously set type. Note this
		/* Set the netdev on top of previously set type. Note this
		 * event happens also during net namespace change so here
		 * event happens also during net namespace change so here
		 * we take into account netdev pointer appearing in this
		 * we take into account netdev pointer appearing in this
		 * namespace.
		 * namespace.
		 */
		 */
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH,
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH,
					netdev, true);
					netdev);
		break;
		break;
	case NETDEV_UNREGISTER:
	case NETDEV_UNREGISTER:
		/* Clear netdev pointer, but not the type. This event happens
		/* Clear netdev pointer, but not the type. This event happens
@@ -10178,14 +10164,14 @@ static int devlink_netdevice_event(struct notifier_block *nb,
		 * pointer to netdev that is going to another net namespace.
		 * pointer to netdev that is going to another net namespace.
		 */
		 */
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH,
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH,
					NULL, true);
					NULL);
		break;
		break;
	case NETDEV_PRE_UNINIT:
	case NETDEV_PRE_UNINIT:
		/* Clear the type and the netdev pointer. Happens one during
		/* Clear the type and the netdev pointer. Happens one during
		 * netdevice unregister.
		 * netdevice unregister.
		 */
		 */
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_NOTSET,
		__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_NOTSET,
					NULL, true);
					NULL);
		break;
		break;
	}
	}