Netdev List
 help / color / mirror / Atom feed
From: Reshma Sreekumar <reshmaisat@gmail.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>,
	netdev@vger.kernel.org
Cc: "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>, Shuah Khan <shuah@kernel.org>,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Reshma Sreekumar <reshmaisat@gmail.com>
Subject: [PATCH net 1/2] net/sched: flag inconsistent qdisc dumps
Date: Thu, 30 Jul 2026 12:00:05 +0200	[thread overview]
Message-ID: <20260730100006.3547944-2-reshmaisat@gmail.com> (raw)
In-Reply-To: <20260730100006.3547944-1-reshmaisat@gmail.com>

tc_dump_qdisc() walks every netdev in the netns and can span many netlink
batches.  RTNL is only held while a single batch is filled, so devices can
be registered or unregistered in between.  Since 748bbef5fc6a ("net/sched:
switch tc_dump_qdisc() to for_each_netdev_dump()") the walk resumes on an
ifindex rather than a list position, but ifindexes are reused: a device
created between two batches can be given an ifindex the dump has already
walked past, and is then missed entirely.  Before that commit the resume
used a position in the device list, so any unregister shifted every later
device and silently dropped a run of qdiscs from the reply.

Unlike rtnl_dump_ifinfo(), which pairs for_each_netdev_dump() with
cb->seq = dev_base_seq and nl_dump_check_consistent(), nothing under
net/sched sets cb->seq at all -- and never has.  NLM_F_DUMP_INTR is
therefore never raised for a qdisc dump, an incomplete reply is
indistinguishable from a complete one, and userspace has no way to tell it
is reconciling against a partial listing.  Callers already handle the flag
by redoing the dump (iproute2 warns, netlink libraries retry), so raising
it is enough to make this self-correcting.

Use the same idiom as the link dump.  Both return paths are covered, as
the -EMSGSIZE path is the normal one for every batch but the last.

Tested by dumping the qdiscs of 400 dummy devices (mq + 16 fq + clsact
each) while unregistering devices ahead of the dump: NLM_F_DUMP_INTR is
raised on 482 of 972 dumps with this patch and on 0 of 1089 without it.

Signed-off-by: Reshma Sreekumar <reshmaisat@gmail.com>
---
 net/sched/sch_api.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 668bcd60d..ab31a75fa 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1949,12 +1949,26 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 		netdev_unlock_ops(dev);
 		s_q_idx = 0;
 	}
-	return skb->len;
+	err = skb->len;
+	goto out;
 
 error_unlock:
 	netdev_unlock_ops(dev);
 	ctx->q_idx = q_idx;
 
+out:
+	/* RTNL is dropped between dump batches, so devices may have been
+	 * registered or unregistered while this dump was in flight.  Because
+	 * ifindexes get reused, a device created in that window can be given an
+	 * ifindex this dump has already walked past and be missed entirely.
+	 * Advertise that, as rtnl_dump_ifinfo() does for RTM_GETLINK, so that
+	 * userspace redoes the dump instead of acting on a listing that silently
+	 * omits entries.
+	 */
+	cb->seq = READ_ONCE(net->dev_base_seq);
+	if (skb->len)
+		nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+
 	return err;
 }
 
-- 
2.43.0


  reply	other threads:[~2026-07-30 10:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-30 10:00 [PATCH net 0/2] net/sched: flag inconsistent qdisc dumps Reshma Sreekumar
2026-07-30 10:00 ` Reshma Sreekumar [this message]
2026-07-30 10:25   ` [PATCH net 1/2] " Eric Dumazet
2026-07-30 10:00 ` [PATCH net 2/2] selftests: net: check that a disturbed qdisc dump is flagged Reshma Sreekumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260730100006.3547944-2-reshmaisat@gmail.com \
    --to=reshmaisat@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox