diff mbox series

[07/10] devlink: Allow rate node parents from other devlinks

Message ID 20241113203317.2507537-8-cratiu@nvidia.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series None | expand

Commit Message

Cosmin Ratiu Nov. 13, 2024, 8:30 p.m. UTC
This commit make use of the unlocked parent devlink from
info->user_ptr[1] to assign a devlink rate node to the requested parent
node. Because it is not locked, none of its mutable fields can be used.
But parent setting only requires:
1. Verifying that the same rate domain is used. The rate domain is
   immutable once set, so this is safe.
2. Comparing devlink_rate->devlink with the requested parent devlink.
   As the shared devlink rate domain is locked, other entities cannot
   concurrently make changes to any of its rates.

Issue: 3645895
Change-Id: Iffd9ccb012c3bd2dec97648161b182679eb5ca78
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 net/devlink/rate.c | 44 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/net/devlink/rate.c b/net/devlink/rate.c
index daf366ca0575..2f12d65b6709 100644
--- a/net/devlink/rate.c
+++ b/net/devlink/rate.c
@@ -141,6 +141,27 @@  static int devlink_nl_rate_fill(struct sk_buff *msg,
 
 	nla_nest_end(msg, nla_tc_bw);
 
+	if (devlink_rate->parent) {
+		struct devlink_rate *parent = devlink_rate->parent;
+
+		if (nla_put_string(msg, DEVLINK_ATTR_RATE_PARENT_NODE_NAME, parent->name))
+			goto nla_put_failure;
+		if (parent->devlink != devlink) {
+			/* The parent devlink isn't locked, but a reference to
+			 * it is held so it cannot suddenly disappear.
+			 * And since there are rate nodes pointing to it,
+			 * the parent devlink is fully initialized and
+			 * the fields accessed here are valid and immutable.
+			 */
+			if (nla_put_string(msg, DEVLINK_ATTR_PARENT_DEV_BUS_NAME,
+					   parent->devlink->dev->bus->name))
+				goto nla_put_failure;
+			if (nla_put_string(msg, DEVLINK_ATTR_PARENT_DEV_NAME,
+					   dev_name(parent->devlink->dev)))
+				goto nla_put_failure;
+		}
+	}
+
 	genlmsg_end(msg, hdr);
 	return 0;
 
@@ -292,9 +313,17 @@  devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
 	const char *parent_name = nla_data(nla_parent);
 	const struct devlink_ops *ops = devlink->ops;
 	size_t len = strlen(parent_name);
+	struct devlink *parent_devlink;
 	struct devlink_rate *parent;
 	int err = -EOPNOTSUPP;
 
+	parent_devlink = info->user_ptr[1] ? : devlink;
+	if (parent_devlink != devlink) {
+		if (parent_devlink->rate_domain != devlink->rate_domain) {
+			NL_SET_ERR_MSG(info->extack, "Cross rate-domain parent is not allowed");
+			return -EINVAL;
+		}
+	}
 	parent = devlink_rate->parent;
 
 	if (parent && !len) {
@@ -312,7 +341,11 @@  devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
 		refcount_dec(&parent->refcnt);
 		devlink_rate->parent = NULL;
 	} else if (len) {
-		parent = devlink_rate_node_get_by_name(devlink, parent_name);
+		/* parent_devlink (if != devlink) isn't locked, but the rate
+		 * domain, immutable once set, is already locked and the parent
+		 * is only used to determine node owner via pointer comparison.
+		 */
+		parent = devlink_rate_node_get_by_name(parent_devlink, parent_name);
 		if (IS_ERR(parent))
 			return -ENODEV;
 
@@ -816,8 +849,8 @@  EXPORT_SYMBOL_GPL(devl_rate_leaf_destroy);
  * devl_rate_nodes_destroy - destroy all devlink rate nodes on device
  * @devlink: devlink instance
  *
- * Unset parent for all rate objects and destroy all rate nodes
- * on specified device.
+ * Unset parent for all rate objects that involve this device and destroy all
+ * rate nodes on it.
  */
 void devl_rate_nodes_destroy(struct devlink *devlink)
 {
@@ -828,7 +861,9 @@  void devl_rate_nodes_destroy(struct devlink *devlink)
 	devl_rate_domain_lock(devlink);
 
 	list_for_each_entry(devlink_rate, &devlink->rate_domain->rate_list, list) {
-		if (!devlink_rate->parent || devlink_rate->devlink != devlink)
+		if (!devlink_rate->parent ||
+		    (devlink_rate->devlink != devlink &&
+		     devlink_rate->parent->devlink != devlink))
 			continue;
 
 		refcount_dec(&devlink_rate->parent->refcnt);
@@ -838,6 +873,7 @@  void devl_rate_nodes_destroy(struct devlink *devlink)
 		else if (devlink_rate_is_node(devlink_rate))
 			ops->rate_node_parent_set(devlink_rate, NULL, devlink_rate->priv,
 						  NULL, NULL);
+		devlink_rate->parent = NULL;
 	}
 	list_for_each_entry_safe(devlink_rate, tmp, &devlink->rate_domain->rate_list, list) {
 		if (devlink_rate->devlink == devlink && devlink_rate_is_node(devlink_rate)) {