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

[Reshma Sreekumar]

On Thu, Jul 30, 2026 at 1:24 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
  > I dislike this patch; it will force dump operations to restart on
busy netns.
  >
  > Most devices are created with a new ifindex (dev_index_reserve() is
  > called with ifindex == 0)
  >
  > Forcing a given ifindex is rather unusual.

  Agreed.

  For context on why I went looking: we hit this on production hosts running
  kernels predating 748bbef5fc6a. An agent that reconciles tc configuration
  against a qdisc dump saw qdiscs that did exist as absent, tried to create them
  again (-EEXIST), and tore down configuration it believed had gone stale.
  Because nothing sets cb->seq there, a truncated reply is indistinguishable
  from a complete one, so there was no way to detect it and retry.

  748bbef5fc6a makes that far less likely, and I have not been able to reproduce
  the missing-qdisc case on 7.2-rc5. One case does remain, which this patch
  would not have fixed either: ctx->q_idx is a position within the per-device
  qdisc_hash walk, so a qdisc added or removed on the device at a batch boundary
  misaligns the skip count.

  Is that worth reporting to userspace, and how would you want it detected?


On Thu, Jul 30, 2026 at 12:25 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Thu, Jul 30, 2026 at 12:00 PM Reshma Sreekumar <reshmaisat@xxxxxxxxx> wrote:
> >
> > 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.
>
> I dislike this patch; it will force dump operations to restart on busy netns.
>
> Most devices are created with a new ifindex (dev_index_reserve() is
> called with ifindex == 0)
>
> Forcing a given ifindex is rather unusual.
>
> I suggest detecting this specific use case instead of slowing down everything.