On Tue, 30 Jul 2024 22:39:47 +0200 Paolo Abeni wrote:

+static int net_shaper_delete(struct net_device *dev, u32 handle,
+			     struct netlink_ext_ack *extack)
+{
+	struct net_shaper_info *parent, *shaper = sc_lookup(dev, handle);
+	struct xarray *xa = __sc_container(dev);
+	enum net_shaper_scope pscope;
+	u32 parent_handle;
+	int ret;
+
+	if (!xa || !shaper) {
+		NL_SET_ERR_MSG_FMT(extack, "Shaper %x not found", handle);

below the print format for shaper id is %d

also just point at the attribute with NL_SET_BAD_ATTR(),
we shouldn't bloat the kernel with strings for trivial errors

+		return -EINVAL;

ENOENT

+	}
+
+	if (is_detached(handle) && shaper->children > 0) {
+		NL_SET_ERR_MSG_FMT(extack, "Can't delete detached shaper %d with %d child nodes",
+				   handle, shaper->children);
+		return -EINVAL;

I'd say EBUSY

+	}
+
+	while (shaper) {
+		parent_handle = shaper->parent;
+		pscope = net_shaper_handle_scope(parent_handle);
+
+		ret = dev->netdev_ops->net_shaper_ops->delete(dev, handle,
+							      extack);
+		if (ret < 0)
+			return ret;
+
+		xa_lock(xa);
+		__xa_erase(xa, handle);
+		if (is_detached(handle))
+			idr_remove(&dev->net_shaper_data->detached_ids,
+				   net_shaper_handle_id(handle));
+		xa_unlock(xa);
+		kfree(shaper);
+		shaper = NULL;

IIUC child is the input / ingress node? (The parentage terminology 
is a bit ambiguous in this case, although I must admit I keep catching
myself trying to use it, too).
Does "deleting a queue" return it to the "implicit mux" at the 
global level? If we look at the delegation use case - when queue
is "deleted" from a container-controlled mux it should go back to
the group created by the orchestrator, not "escpate" to global scope,
right?

+		if (pscope == NET_SHAPER_SCOPE_DETACHED) {
+			parent = sc_lookup(dev, parent_handle);
+			if (parent && !--parent->children) {
+				shaper = parent;
+				handle = parent_handle;
+			}
+		}
+	}
+	return 0;
 }
 
 int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return -EOPNOTSUPP;
+	struct net_device *dev;
+	u32 handle;
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_HANDLE))
+		return -EINVAL;
+
+	ret = fetch_dev(info, &dev);
+	if (ret)
+		return ret;

Possibly a candidate for a "pre" handler, since multiple ops call it?