[PATCH net-next 0/4] net/sched: tc_dump_qdisc() optimizations

[PATCH net-next 0/4] net/sched: tc_dump_qdisc() optimizations

[Eric Dumazet]

Before converting tc_dump_qdisc() to RCU, we make the following changes:

- Use for_each_netdev_dump() instead of for_each_netdev()

- Only dump qdiscs of a single device at user space request.

Eric Dumazet (4):
  net/sched: propagate tc_fill_tclass() error
  net/sched: tc_dump_qdisc_root() refactor
  net/sched: switch tc_dump_qdisc() to for_each_netdev_dump()
  net/sched: speedup tc_dump_qdisc() when tcm_ifindex is provided

 net/sched/sch_api.c | 140 +++++++++++++++++++++++---------------------
 1 file changed, 74 insertions(+), 66 deletions(-)

-- 
2.54.0.545.g6539524ca2-goog

[PATCH net-next 1/4] net/sched: propagate tc_fill_tclass() error

[Eric Dumazet]

Change tc_fill_tclass() to return -EMSGSIZE when skb is too small.

Change its caller to propagate this error (instead of -EINVAL)

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_api.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index ed869a5ffc7377b7c19e66ae5fc9788e709488da..32ccd4672083aa19340520155aeba6d8b6ff546c 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1987,15 +1987,16 @@ static int tc_fill_tclass(struct sk_buff *skb, struct Qdisc *q,
 out_nlmsg_trim:
 nla_put_failure:
 	nlmsg_trim(skb, b);
-	return -1;
+	return -EMSGSIZE;
 }
 
 static int tclass_notify(struct net *net, struct sk_buff *oskb,
 			 struct nlmsghdr *n, struct Qdisc *q,
 			 unsigned long cl, int event, struct netlink_ext_ack *extack)
 {
-	struct sk_buff *skb;
 	u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+	struct sk_buff *skb;
+	int ret;
 
 	if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
 		return 0;
@@ -2004,9 +2005,10 @@ static int tclass_notify(struct net *net, struct sk_buff *oskb,
 	if (!skb)
 		return -ENOBUFS;
 
-	if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0, event, extack) < 0) {
+	ret = tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0, event, extack);
+	if (ret < 0) {
 		kfree_skb(skb);
-		return -EINVAL;
+		return ret;
 	}
 
 	return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
@@ -2017,17 +2019,19 @@ static int tclass_get_notify(struct net *net, struct sk_buff *oskb,
 			     struct nlmsghdr *n, struct Qdisc *q,
 			     unsigned long cl, struct netlink_ext_ack *extack)
 {
-	struct sk_buff *skb;
 	u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+	struct sk_buff *skb;
+	int ret;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOBUFS;
 
-	if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0, RTM_NEWTCLASS,
-			   extack) < 0) {
+	ret = tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
+			     RTM_NEWTCLASS, extack);
+	if (ret < 0) {
 		kfree_skb(skb);
-		return -EINVAL;
+		return ret;
 	}
 
 	return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
@@ -2041,7 +2045,7 @@ static int tclass_del_notify(struct net *net,
 			     struct netlink_ext_ack *extack)
 {
 	u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
-	struct sk_buff *skb;
+	struct sk_buff *skb = NULL;
 	int err = 0;
 
 	if (!cops->delete)
@@ -2052,13 +2056,12 @@ static int tclass_del_notify(struct net *net,
 		if (!skb)
 			return -ENOBUFS;
 
-		if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
-				   RTM_DELTCLASS, extack) < 0) {
+		err = tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
+				     RTM_DELTCLASS, extack);
+		if (err < 0) {
 			kfree_skb(skb);
-			return -EINVAL;
+			return err;
 		}
-	} else {
-		skb = NULL;
 	}
 
 	err = cops->delete(q, cl, extack);
-- 
2.54.0.545.g6539524ca2-goog

[PATCH net-next 2/4] net/sched: tc_dump_qdisc_root() refactor

[Eric Dumazet]

Change tc_fill_qdisc() to return -EMSGSIZE when skb is too small.

Change tc_dump_qdisc_root() to propagate tc_fill_qdisc() error to its callers.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_api.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 32ccd4672083aa19340520155aeba6d8b6ff546c..322eab3bd2b1327af323c44333318bdd52c7ee52 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -976,7 +976,7 @@ static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
 out_nlmsg_trim:
 nla_put_failure:
 	nlmsg_trim(skb, b);
-	return -1;
+	return -EMSGSIZE;
 }
 
 static bool tc_qdisc_dump_ignore(struct Qdisc *q, bool dump_invisible)
@@ -1833,16 +1833,16 @@ static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 		return 0;
 
 	q = root;
-	if (q_idx < s_q_idx) {
-		q_idx++;
-	} else {
-		if (!tc_qdisc_dump_ignore(q, dump_invisible) &&
-		    tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).portid,
-				  cb->nlh->nlmsg_seq, NLM_F_MULTI,
-				  RTM_NEWQDISC, NULL) <= 0)
-			goto done;
-		q_idx++;
+	if (!(q_idx < s_q_idx) &&
+	    !tc_qdisc_dump_ignore(q, dump_invisible)) {
+		ret = tc_fill_qdisc(skb, q, q->parent,
+				    NETLINK_CB(cb->skb).portid,
+				    cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				    RTM_NEWQDISC, NULL);
+		if (ret < 0)
+			goto out;
 	}
+	q_idx++;
 
 	/* If dumping singletons, there is no qdisc_dev(root) and the singleton
 	 * itself has already been dumped.
@@ -1854,24 +1854,21 @@ static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 		goto out;
 
 	hash_for_each(qdisc_dev(root)->qdisc_hash, b, q, hash) {
-		if (q_idx < s_q_idx) {
-			q_idx++;
-			continue;
+		if (!(q_idx < s_q_idx) &&
+		    !tc_qdisc_dump_ignore(q, dump_invisible)) {
+			ret = tc_fill_qdisc(skb, q, q->parent,
+					    NETLINK_CB(cb->skb).portid,
+					    cb->nlh->nlmsg_seq, NLM_F_MULTI,
+					    RTM_NEWQDISC, NULL);
+			if (ret < 0)
+				goto out;
 		}
-		if (!tc_qdisc_dump_ignore(q, dump_invisible) &&
-		    tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).portid,
-				  cb->nlh->nlmsg_seq, NLM_F_MULTI,
-				  RTM_NEWQDISC, NULL) <= 0)
-			goto done;
 		q_idx++;
 	}
 
 out:
 	*q_idx_p = q_idx;
 	return ret;
-done:
-	ret = -1;
-	goto out;
 }
 
 static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
-- 
2.54.0.545.g6539524ca2-goog

[PATCH net-next 3/4] net/sched: switch tc_dump_qdisc() to for_each_netdev_dump()

[Eric Dumazet]

Use for_each_netdev_dump() instead of for_each_netdev().

This is more scalable, and will ease RCU conversion.

This also offer better behavior when other threads
are adding or deleting netdevices concurrently.

This enables dumping qdiscs for a single device
at user space request in the following patch.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_api.c | 61 ++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 322eab3bd2b1327af323c44333318bdd52c7ee52..c4e8059acdd324441e0d069f6878bbfcab5a0cc7 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1873,18 +1873,18 @@ static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 
 static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	struct net *net = sock_net(skb->sk);
-	int idx, q_idx;
-	int s_idx, s_q_idx;
-	struct net_device *dev;
 	const struct nlmsghdr *nlh = cb->nlh;
+	struct net *net = sock_net(skb->sk);
 	struct nlattr *tca[TCA_MAX + 1];
+	struct {
+		unsigned long ifindex;
+		int q_idx;
+	} *ctx = (void *)cb->ctx;
+	unsigned long s_ifindex;
+	struct net_device *dev;
+	int s_q_idx, q_idx;
 	int err;
 
-	s_idx = cb->args[0];
-	s_q_idx = q_idx = cb->args[1];
-
-	idx = 0;
 	ASSERT_RTNL();
 
 	err = nlmsg_parse_deprecated(nlh, sizeof(struct tcmsg), tca, TCA_MAX,
@@ -1892,42 +1892,41 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err < 0)
 		return err;
 
-	for_each_netdev(net, dev) {
+	s_ifindex = ctx->ifindex;
+	s_q_idx = ctx->q_idx;
+
+	for_each_netdev_dump(net, dev, ctx->ifindex) {
 		struct netdev_queue *dev_queue;
+		struct Qdisc *q;
 
-		if (idx < s_idx)
-			goto cont;
-		if (idx > s_idx)
+		if (ctx->ifindex > s_ifindex)
 			s_q_idx = 0;
 		q_idx = 0;
 
 		netdev_lock_ops(dev);
-		if (tc_dump_qdisc_root(rtnl_dereference(dev->qdisc),
-				       skb, cb, &q_idx, s_q_idx,
-				       true, tca[TCA_DUMP_INVISIBLE]) < 0) {
-			netdev_unlock_ops(dev);
-			goto done;
-		}
+		q = rtnl_dereference(dev->qdisc);
+		err = tc_dump_qdisc_root(q, skb, cb, &q_idx, s_q_idx,
+					 true, tca[TCA_DUMP_INVISIBLE]);
+		if (err < 0)
+			goto error_unlock;
 
 		dev_queue = dev_ingress_queue(dev);
-		if (dev_queue &&
-		    tc_dump_qdisc_root(rtnl_dereference(dev_queue->qdisc_sleeping),
-				       skb, cb, &q_idx, s_q_idx, false,
-				       tca[TCA_DUMP_INVISIBLE]) < 0) {
-			netdev_unlock_ops(dev);
-			goto done;
+		if (dev_queue) {
+			q = rtnl_dereference(dev_queue->qdisc_sleeping);
+			err = tc_dump_qdisc_root(q, skb, cb, &q_idx, s_q_idx,
+						 false, tca[TCA_DUMP_INVISIBLE]);
+			if (err < 0)
+				goto error_unlock;
 		}
 		netdev_unlock_ops(dev);
-
-cont:
-		idx++;
 	}
+	return skb->len;
 
-done:
-	cb->args[0] = idx;
-	cb->args[1] = q_idx;
+error_unlock:
+	netdev_unlock_ops(dev);
+	ctx->q_idx = q_idx;
 
-	return skb->len;
+	return err;
 }
 
 
-- 
2.54.0.545.g6539524ca2-goog

[PATCH net-next 4/4] net/sched: speedup tc_dump_qdisc() when tcm_ifindex is provided

[Eric Dumazet]

There is no point dumping qdiscs for all devices when user space
wants them for a single device:

tc -s -d qdisc show dev eth1

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_api.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index c4e8059acdd324441e0d069f6878bbfcab5a0cc7..8d0e4eb72103d9b074131e13e31830cb266eadba 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1874,6 +1874,7 @@ static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
+	const struct tcmsg *tcm = nlmsg_data(nlh);
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tca[TCA_MAX + 1];
 	struct {
@@ -1892,6 +1893,9 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err < 0)
 		return err;
 
+	if (!ctx->ifindex)
+		ctx->ifindex = tcm->tcm_ifindex;
+
 	s_ifindex = ctx->ifindex;
 	s_q_idx = ctx->q_idx;
 
@@ -1899,8 +1903,13 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 		struct netdev_queue *dev_queue;
 		struct Qdisc *q;
 
-		if (ctx->ifindex > s_ifindex)
+		if (ctx->ifindex > s_ifindex) {
+			if (tcm->tcm_ifindex) {
+				ctx->ifindex = ULONG_MAX;
+				break;
+			}
 			s_q_idx = 0;
+		}
 		q_idx = 0;
 
 		netdev_lock_ops(dev);
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH net-next 0/4] net/sched: tc_dump_qdisc() optimizations

[Jakub Kicinski]

On Mon, 27 Apr 2026 13:25:51 +0000 Eric Dumazet wrote:
> Before converting tc_dump_qdisc() to RCU, we make the following changes:
> 
> - Use for_each_netdev_dump() instead of for_each_netdev()
> 
> - Only dump qdiscs of a single device at user space request.

Hah, what are the chances, I've been sitting on something like patch 4
since Apr 15th..

Re: [PATCH net-next 2/4] net/sched: tc_dump_qdisc_root() refactor

[Jakub Kicinski]

On Mon, 27 Apr 2026 13:25:53 +0000 Eric Dumazet wrote:
> -	if (q_idx < s_q_idx) {
> -		q_idx++;
> -	} else {
> -		if (!tc_qdisc_dump_ignore(q, dump_invisible) &&
> -		    tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).portid,
> -				  cb->nlh->nlmsg_seq, NLM_F_MULTI,
> -				  RTM_NEWQDISC, NULL) <= 0)
> -			goto done;
> -		q_idx++;
> +	if (!(q_idx < s_q_idx) &&
> +	    !tc_qdisc_dump_ignore(q, dump_invisible)) {
> +		ret = tc_fill_qdisc(skb, q, q->parent,
> +				    NETLINK_CB(cb->skb).portid,
> +				    cb->nlh->nlmsg_seq, NLM_F_MULTI,
> +				    RTM_NEWQDISC, NULL);
> +		if (ret < 0)
> +			goto out;
>  	}
> +	q_idx++;

Probably subjective but I'm not sure the

	if (!(q_idx < s_q_idx) &&

is more readable than the previous version?

Especially for:

>  	hash_for_each(qdisc_dev(root)->qdisc_hash, b, q, hash) {
> -		if (q_idx < s_q_idx) {
> -			q_idx++;
> -			continue;
> +		if (!(q_idx < s_q_idx) &&
> +		    !tc_qdisc_dump_ignore(q, dump_invisible)) {
> +			ret = tc_fill_qdisc(skb, q, q->parent,
> +					    NETLINK_CB(cb->skb).portid,
> +					    cb->nlh->nlmsg_seq, NLM_F_MULTI,
> +					    RTM_NEWQDISC, NULL);
> +			if (ret < 0)
> +				goto out;
>  		}

I feel like checking the ID + continue in netlink dump loop is fairly
idiomatic.

Re: [PATCH net-next 3/4] net/sched: switch tc_dump_qdisc() to for_each_netdev_dump()

[Jakub Kicinski]

On Mon, 27 Apr 2026 13:25:54 +0000 Eric Dumazet wrote:
> +	for_each_netdev_dump(net, dev, ctx->ifindex) {
>  		struct netdev_queue *dev_queue;
> +		struct Qdisc *q;
>  
> -		if (idx < s_idx)
> -			goto cont;
> -		if (idx > s_idx)
> +		if (ctx->ifindex > s_ifindex)
>  			s_q_idx = 0;

I don't think we need this conditional now that we use for_each_netdev_dump()
We can just add

	s_q_idx = 0;

right before closing bracket of the loop?

>  		q_idx = 0;

Re: [PATCH net-next 4/4] net/sched: speedup tc_dump_qdisc() when tcm_ifindex is provided

[Jakub Kicinski]

On Mon, 27 Apr 2026 13:25:55 +0000 Eric Dumazet wrote:
>  static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
>  {
>  	const struct nlmsghdr *nlh = cb->nlh;
> +	const struct tcmsg *tcm = nlmsg_data(nlh);

nit: breaks reverse xmas tree

I added this tcm assignment after parsing:

        err = nlmsg_parse_deprecated(nlh, sizeof(struct tcmsg), tca, TCA_MAX,
                                     rtm_tca_policy, cb->extack);
        if (err < 0)
                return err;
+       tcm = nlmsg_data(nlh);

because nlmsg_parse validates the struct is in range of skb->len

>  	struct net *net = sock_net(skb->sk);
>  	struct nlattr *tca[TCA_MAX + 1];
>  	struct {
> @@ -1892,6 +1893,9 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
>  	if (err < 0)
>  		return err;
>  
> +	if (!ctx->ifindex)
> +		ctx->ifindex = tcm->tcm_ifindex;
> +
>  	s_ifindex = ctx->ifindex;
>  	s_q_idx = ctx->q_idx;
>  
> @@ -1899,8 +1903,13 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
>  		struct netdev_queue *dev_queue;
>  		struct Qdisc *q;
>  
> -		if (ctx->ifindex > s_ifindex)
> +		if (ctx->ifindex > s_ifindex) {
> +			if (tcm->tcm_ifindex) {
> +				ctx->ifindex = ULONG_MAX;
> +				break;
> +			}
>  			s_q_idx = 0;
> +		}

nit2: personal preference but I had before the loop:

	if (tcm->tcm_ifindex && !ctx->ifindex)
		ctx->ifindex = tcm->tcm_ifindex;

in the loop:

	if (tcm->tcm_ifindex && ctx->ifindex != tcm->tcm_ifindex)
		break;

Re: [PATCH net-next 4/4] net/sched: speedup tc_dump_qdisc() when tcm_ifindex is provided

[Eric Dumazet]

On Mon, Apr 27, 2026 at 4:18 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 27 Apr 2026 13:25:55 +0000 Eric Dumazet wrote:
> >  static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
> >  {
> >       const struct nlmsghdr *nlh = cb->nlh;
> > +     const struct tcmsg *tcm = nlmsg_data(nlh);
>
> nit: breaks reverse xmas tree
>
> I added this tcm assignment after parsing:
>
>         err = nlmsg_parse_deprecated(nlh, sizeof(struct tcmsg), tca, TCA_MAX,
>                                      rtm_tca_policy, cb->extack);
>         if (err < 0)
>                 return err;
> +       tcm = nlmsg_data(nlh);
>
> because nlmsg_parse validates the struct is in range of skb->len

Note tc_get_qdisc() and tc_ctl_tclass() use tcm = nlmsg_data(n) before
validation.

Do they need to be changed?

>
> >       struct net *net = sock_net(skb->sk);
> >       struct nlattr *tca[TCA_MAX + 1];
> >       struct {
> > @@ -1892,6 +1893,9 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
> >       if (err < 0)
> >               return err;
> >
> > +     if (!ctx->ifindex)
> > +             ctx->ifindex = tcm->tcm_ifindex;
> > +
> >       s_ifindex = ctx->ifindex;
> >       s_q_idx = ctx->q_idx;
> >
> > @@ -1899,8 +1903,13 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
> >               struct netdev_queue *dev_queue;
> >               struct Qdisc *q;
> >
> > -             if (ctx->ifindex > s_ifindex)
> > +             if (ctx->ifindex > s_ifindex) {
> > +                     if (tcm->tcm_ifindex) {
> > +                             ctx->ifindex = ULONG_MAX;
> > +                             break;
> > +                     }
> >                       s_q_idx = 0;
> > +             }
>
> nit2: personal preference but I had before the loop:
>
>         if (tcm->tcm_ifindex && !ctx->ifindex)
>                 ctx->ifindex = tcm->tcm_ifindex;
>
> in the loop:
>
>         if (tcm->tcm_ifindex && ctx->ifindex != tcm->tcm_ifindex)
>                 break;

Re: [PATCH net-next 2/4] net/sched: tc_dump_qdisc_root() refactor

[Eric Dumazet]

On Mon, Apr 27, 2026 at 4:08 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 27 Apr 2026 13:25:53 +0000 Eric Dumazet wrote:
> > -     if (q_idx < s_q_idx) {
> > -             q_idx++;
> > -     } else {
> > -             if (!tc_qdisc_dump_ignore(q, dump_invisible) &&
> > -                 tc_fill_qdisc(skb, q, q->parent, NETLINK_CB(cb->skb).portid,
> > -                               cb->nlh->nlmsg_seq, NLM_F_MULTI,
> > -                               RTM_NEWQDISC, NULL) <= 0)
> > -                     goto done;
> > -             q_idx++;
> > +     if (!(q_idx < s_q_idx) &&
> > +         !tc_qdisc_dump_ignore(q, dump_invisible)) {
> > +             ret = tc_fill_qdisc(skb, q, q->parent,
> > +                                 NETLINK_CB(cb->skb).portid,
> > +                                 cb->nlh->nlmsg_seq, NLM_F_MULTI,
> > +                                 RTM_NEWQDISC, NULL);
> > +             if (ret < 0)
> > +                     goto out;
> >       }
> > +     q_idx++;
>
> Probably subjective but I'm not sure the
>
>         if (!(q_idx < s_q_idx) &&
>
> is more readable than the previous version?
>
> Especially for:
>
> >       hash_for_each(qdisc_dev(root)->qdisc_hash, b, q, hash) {
> > -             if (q_idx < s_q_idx) {
> > -                     q_idx++;
> > -                     continue;
> > +             if (!(q_idx < s_q_idx) &&
> > +                 !tc_qdisc_dump_ignore(q, dump_invisible)) {
> > +                     ret = tc_fill_qdisc(skb, q, q->parent,
> > +                                         NETLINK_CB(cb->skb).portid,
> > +                                         cb->nlh->nlmsg_seq, NLM_F_MULTI,
> > +                                         RTM_NEWQDISC, NULL);
> > +                     if (ret < 0)
> > +                             goto out;
> >               }
>
> I feel like checking the ID + continue in netlink dump loop is fairly
> idiomatic.

OK, this was removing one indent level.

I will keep the double q_idx++ then.

Re: [PATCH net-next 3/4] net/sched: switch tc_dump_qdisc() to for_each_netdev_dump()

[Eric Dumazet]

On Mon, Apr 27, 2026 at 4:13 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 27 Apr 2026 13:25:54 +0000 Eric Dumazet wrote:
> > +     for_each_netdev_dump(net, dev, ctx->ifindex) {
> >               struct netdev_queue *dev_queue;
> > +             struct Qdisc *q;
> >
> > -             if (idx < s_idx)
> > -                     goto cont;
> > -             if (idx > s_idx)
> > +             if (ctx->ifindex > s_ifindex)
> >                       s_q_idx = 0;
>
> I don't think we need this conditional now that we use for_each_netdev_dump()
> We can just add
>
>         s_q_idx = 0;
>
> right before closing bracket of the loop?

Ack thanks.

Re: [PATCH net-next 4/4] net/sched: speedup tc_dump_qdisc() when tcm_ifindex is provided

[Jakub Kicinski]

On Tue, 28 Apr 2026 08:40:52 -0700 Eric Dumazet wrote:
> > On Mon, 27 Apr 2026 13:25:55 +0000 Eric Dumazet wrote:  
> > >  static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
> > >  {
> > >       const struct nlmsghdr *nlh = cb->nlh;
> > > +     const struct tcmsg *tcm = nlmsg_data(nlh);  
> >
> > nit: breaks reverse xmas tree
> >
> > I added this tcm assignment after parsing:
> >
> >         err = nlmsg_parse_deprecated(nlh, sizeof(struct tcmsg), tca, TCA_MAX,
> >                                      rtm_tca_policy, cb->extack);
> >         if (err < 0)
> >                 return err;
> > +       tcm = nlmsg_data(nlh);
> >
> > because nlmsg_parse validates the struct is in range of skb->len  
> 
> Note tc_get_qdisc() and tc_ctl_tclass() use tcm = nlmsg_data(n) before
> validation.
> 
> Do they need to be changed?

To be clear - I just mean that as a coding nit pick.
I seem to recall that some AI was flagging this as a bug, it's not
really a bug. But if we're touching the code I think it's easier to
everyone to follow if the nlmsg_data() is after the validation.

Re: [PATCH net-next 4/4] net/sched: speedup tc_dump_qdisc() when tcm_ifindex is provided

[Eric Dumazet]

On Tue, Apr 28, 2026 at 10:20 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 28 Apr 2026 08:40:52 -0700 Eric Dumazet wrote:
> > > On Mon, 27 Apr 2026 13:25:55 +0000 Eric Dumazet wrote:
> > > >  static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
> > > >  {
> > > >       const struct nlmsghdr *nlh = cb->nlh;
> > > > +     const struct tcmsg *tcm = nlmsg_data(nlh);
> > >
> > > nit: breaks reverse xmas tree
> > >
> > > I added this tcm assignment after parsing:
> > >
> > >         err = nlmsg_parse_deprecated(nlh, sizeof(struct tcmsg), tca, TCA_MAX,
> > >                                      rtm_tca_policy, cb->extack);
> > >         if (err < 0)
> > >                 return err;
> > > +       tcm = nlmsg_data(nlh);
> > >
> > > because nlmsg_parse validates the struct is in range of skb->len
> >
> > Note tc_get_qdisc() and tc_ctl_tclass() use tcm = nlmsg_data(n) before
> > validation.
> >
> > Do they need to be changed?
>
> To be clear - I just mean that as a coding nit pick.
> I seem to recall that some AI was flagging this as a bug, it's not
> really a bug. But if we're touching the code I think it's easier to
> everyone to follow if the nlmsg_data() is after the validation.

I included your suggestion in V2.

I will send a cleanup later.