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

[Reshma Sreekumar]

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@xxxxxxxxx>
---
 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