From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mail.toke.dk; spf=none smtp.mailfrom=mojatatu.com; dkim=pass header.d=mojatatu-com.20230601.gappssmtp.com; arc=pass; dmarc=none Received: from mail-pf1-x434.google.com (mail-pf1-x434.google.com [IPv6:2607:f8b0:4864:20::434]) by mail.toke.dk (Postfix) with ESMTPS id CF0ECDF247D for ; Wed, 11 Mar 2026 17:22:55 +0100 (CET) Received: by mail-pf1-x434.google.com with SMTP id d2e1a72fcca58-829ac4670c4so76040b3a.0 for ; Wed, 11 Mar 2026 09:22:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1773246173; cv=none; d=google.com; s=arc-20240605; b=LW1hc9NlrHSKOnHSPNCW4v/I9YW9wbzSpeitAkELX1RoEtsJefmYpD927gOD3Py1uS pprC5GzQgEK1VX8HErqR3uns+sBctgq99DSW6ehpI8yFj0uySylFYGdAIwxcTbILefAA 8fWD3KViD+pAUOBoD2eTNv8WHEpON1U2LxYo73U9f/4STBARD8bX3bAwUYY1jJgT5kBr kis1A8gpdY4UDDOCspUqG6TJE7xrmqovUoLhgza08IlRZRjUsQwE16iQTM4cC3mBeWy9 7ObmjtXFHIlXuamRvx+18b4338TyzVkZk1J8aVeWTcQXT73np7wHbu/Z1eExIJXjcI3e c1Gg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=vfY7z/mH1QCzKBvw7XY9whOUf9+5X6AiAqhhH3gD6M4=; fh=hgvZhdXPa7/ilUwj3RHPWg2QgXqj8HDCRvCEofhAX9M=; b=PmzHvLkfsBscpGcrK7vo8BQW5Bl76UwkDw8xXSUcK5gL/ApVHOwfqb9KCmqul/vE0o YA8Jmu6FPaQZV9f6BIbCn744Caw8fYnJ/vgIy8alnS/AG3Qf9ZGm3c+ba15sSZKlTk3U 5iVco8xs7SLFbxeuDXAJfHxdM2mO/qBOefskAA45tt8lmAfxWTX49Sl5Ft1YMBgX30be PUqdCiYJ/r9IoURR3AlZKt6Pu6ixMqM+cGBKchTpsMvohg0E/4kA/J6b4czlF0z5GsvC KY5Gno/x32jnY9W6l3LYp36/jKzzKMtRaSoZXve4bWsLdiZgZLXtMkmPccbIHeHpvI4K EaGw==; darn=lists.bufferbloat.net ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mojatatu-com.20230601.gappssmtp.com; s=20230601; t=1773246173; x=1773850973; darn=lists.bufferbloat.net; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=vfY7z/mH1QCzKBvw7XY9whOUf9+5X6AiAqhhH3gD6M4=; b=FDecoXwggJzwyLfoWW7vQ5Q3OCkIGuyBxOqdPV/llntH8D2dqtK6J+HDvY5Dhz0k6k R09bqAiXCVXeAU87p0WqcFw0T/8l8zV0gW8Xxc9N26BU62zsYUR9ZseDlZdZ9I/osGox JDvDrUgoLb8jnYIa3v9YCoSpgLo7Qi6tDLfyrSJhe2guV8eyadN9OWWyySWO1iI9+uyt iXfBbEeK1WVKPTS08TMRDleRelWcTRc/OQaJyLhw725TXY3MquU7LqZWRGBFoa9u0Dd4 NtruzISKsmsIBeQJSzrle4k+LjAkL3g+p7jCqbEyLbRMUgqyY3JArij1kCT3AfAvaeCx g/wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1773246173; x=1773850973; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=vfY7z/mH1QCzKBvw7XY9whOUf9+5X6AiAqhhH3gD6M4=; b=Qn2BhHI8e7fUrHpYgFwf8Axd4M8MhFe6OFfy0oA/GQdH3VWY3c88CsdqBG4IPv95tC aklD2jOOibxFqntXtsmzbfw7AinsPlYjLe8ePs20QEsFurBDIiEivAiVGA+6AxBrHs/g s+O34fRUpqToOYBrvNorAqV5JBxNZaBnDuBv3j6+lyNkcLhPv80pPEMobCW+prF03pzK eHSrmx94c0v9C0/XW403GeN2ctE6C2KGDX9e8DEFB9dPNyOxRRnVd4r4IE5sfVI7yqJG dAW+7D4Bxd7lQdX/Q43N6IFe3zPmGdfgwhNqjEu0e49nnhR+5XhcmYplOra/HJy76EWV u00w== X-Forwarded-Encrypted: i=1; AJvYcCV0kxpjR1uiop81VxKJkIhBVHrjpO/WSrpsBxrE8qwnjBCNgrTdtlYh8morWzJTO48yWU3x@lists.bufferbloat.net X-Gm-Message-State: AOJu0YxuUt1Qef1DUdA474TjpJJPfKjLekHv4oAD+C4mEFvaRaSJfKlQ A4cf8nOY1HVOTgoePRBHUFayQUkB8ZGBig2gEa7eiH+d1lnK7ofXipZfHeSnWeqpymv1FSZgvzT 4rhCai22q6UjCfmwlNBmgmQPOsEYUN6gdHJT2dRrr X-Gm-Gg: ATEYQzxFRhtMAe0UmTOtAVpQ3eGRFS6pkLlzZXZlOZkDI3xUGX9pGo6cjlrKryi7oFN GZboePYDHi6hJmpjh59f7tMo7+mLF9lzycfzpCkYxkuh1A0NRyHLZp9/exDbMd2tTJYTeFPgRHe Lzd6lo4I5JccRAXJMpdK9HJczXxcbeSPQ6aaPk5QvO2YfArnEpqJ4NEy+rx9wibYP+HF6U4JEX1 +gDRQiyS60Oz2madtVrEx7TRw/r8U5qpxwFitxahARYH9Dvt7YndbAq2kAF8umfWyO/oSJf6ma1 HKnBB7p2TqUxTetUDoyp4r5IPg== X-Received: by 2002:a05:6a00:7603:b0:829:9a7b:db84 with SMTP id d2e1a72fcca58-829f715d0c1mr2313680b3a.49.1773246172868; Wed, 11 Mar 2026 09:22:52 -0700 (PDT) MIME-Version: 1.0 References: <20260307212058.169511-1-jhs@mojatatu.com> <20260310184713.7e810431@kernel.org> In-Reply-To: <20260310184713.7e810431@kernel.org> From: Jamal Hadi Salim Date: Wed, 11 Mar 2026 12:22:41 -0400 X-Gm-Features: AaiRm52amjkmnAH6dCMjUuecqHixg9RGz4S-GvYmjS08m4Y_7VN4P7VgDYfbo_8 Message-ID: To: Jakub Kicinski Cc: netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, horms@kernel.org, jiri@resnulli.us, toke@toke.dk, vinicius.gomes@intel.com, stephen@networkplumber.org, vladbu@nvidia.com, cake@lists.bufferbloat.net, bpf@vger.kernel.org, ghandatmanas@gmail.com, km.kim1503@gmail.com, security@kernel.org, Victor Nogueira Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 3ATJQDFUTUP66MYUAELWFW3DZGJV6SWF X-Message-ID-Hash: 3ATJQDFUTUP66MYUAELWFW3DZGJV6SWF X-MailFrom: jhs@mojatatu.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list Subject: [Cake] Re: [PATCH net] net/sched: Mark qdisc for deletion if graft cannot delete List-Id: Cake - FQ_codel the next generation Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Tue, Mar 10, 2026 at 9:47=E2=80=AFPM Jakub Kicinski wr= ote: > > 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; howeve= r, > > the challenge here is if the graph depth is "high" - we may overflow th= e > > 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) > Two of the several (I think 4!) patches we had took a similar path. I am trying to remember at least one variant was bad for performance and the other was unstable. Let's see if we can revive it and take a closer look. BTW - none were pretty, it was maybe half the lines of code but touched many things. cheers, jamal > 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, stru= ct nlmsghdr *n, > int err; > int tp_created; > bool rtnl_held =3D false; > + bool rtnl_take =3D false > u32 flags; > > replay: > @@ -2290,11 +2291,17 @@ static int tc_new_tfilter(struct sk_buff *skb, st= ruct nlmsghdr *n, > } > } > > + /* Realistically only INGRESS supports unlocked ops */ > + if (parent !=3D TC_H_INGRESS) { > + rtnl_held =3D true; > + rtnl_lock(); > + } > + > /* Find head of filter chain. */ > > err =3D __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, st= ruct nlmsghdr *n, > * block is shared (no qdisc found), qdisc is not unlocked, class= ifier > * 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 =3D true; > - rtnl_lock(); > } > > err =3D __tcf_qdisc_cl_find(q, parent, &cl, t->tcm_ifindex, extac= k); > @@ -2451,17 +2459,16 @@ static int tc_new_tfilter(struct sk_buff *skb, st= ruct nlmsghdr *n, > } > tcf_block_release(q, block, rtnl_held); > > - if (rtnl_held) > - rtnl_unlock(); > - > if (err =3D=3D -EAGAIN) { > /* Take rtnl lock in case EAGAIN is caused by concurrent = flush > * of target chain. > */ > - rtnl_held =3D true; > + rtnl_take =3D true; > /* Replay the request. */ > goto replay; > } > + if (rtnl_held) > + rtnl_unlock(); > return err; > > errout_locked: