Re: [PATCH net] net/sched: Mark qdisc for deletion if graft cannot delete

[Jakub Kicinski <kuba@kernel.org>]

On Sat,  7 Mar 2026 16:20:58 -0500 Jamal Hadi Salim wrote:
> Note: We tried a couple of different approaches that had smaller code
> footprint but were a bit fugly. The first approach was to use recursion
> on the qdisc hash table to iterate the descendants of the qdisc; however,
> the challenge here is if the graph depth is "high" - we may overflow the
> stack. The second approach was to use a breadth first search to achieve
> the same goal; the challenge here was it was a quadratic algorithm.

Lots of complexity when realistically only ingress/clsact support 
the unlocked operations. Can we not just take rtnl before the
references and not bother all the real qdiscs with this @#%$ ?
(diff just to illustrate the point not even compiled)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 4829c27446e3..21b461f3323d 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2255,6 +2255,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	int err;
 	int tp_created;
 	bool rtnl_held = false;
+	bool rtnl_take = false
 	u32 flags;
 
 replay:
@@ -2290,11 +2291,17 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 		}
 	}
 
+	/* Realistically only INGRESS supports unlocked ops */
+	if (parent != TC_H_INGRESS) {
+		rtnl_held = true;
+		rtnl_lock();
+	}
+
 	/* Find head of filter chain. */
 
 	err = __tcf_qdisc_find(net, &q, &parent, t->tcm_ifindex, false, extack);
 	if (err)
-		return err;
+		goto errout;
 
 	if (tcf_proto_check_kind(tca[TCA_KIND], name)) {
 		NL_SET_ERR_MSG(extack, "Specified TC filter name too long");
@@ -2306,11 +2313,12 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	 * block is shared (no qdisc found), qdisc is not unlocked, classifier
 	 * type is not specified, classifier is not unlocked.
 	 */
-	if (rtnl_held ||
+	if (rtnl_take ||
 	    (q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_UNLOCKED)) ||
 	    !tcf_proto_is_unlocked(name)) {
+		if (!rtnl_held)
+			rtnl_lock();
 		rtnl_held = true;
-		rtnl_lock();
 	}
 
 	err = __tcf_qdisc_cl_find(q, parent, &cl, t->tcm_ifindex, extack);
@@ -2451,17 +2459,16 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	}
 	tcf_block_release(q, block, rtnl_held);
 
-	if (rtnl_held)
-		rtnl_unlock();
-
 	if (err == -EAGAIN) {
 		/* Take rtnl lock in case EAGAIN is caused by concurrent flush
 		 * of target chain.
 		 */
-		rtnl_held = true;
+		rtnl_take = true;
 		/* Replay the request. */
 		goto replay;
 	}
+	if (rtnl_held)
+		rtnl_unlock();
 	return err;
 
 errout_locked: