[PATCH net 1/2] net/sched: flag inconsistent qdisc dumps

Reshma Sreekumar <[email protected]>
Newsgroups org.kernel.vger.linux-kselftest,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.