Cake - FQ_codel the next generation
 help / color / mirror / Atom feed
* [Cake] [PATCH net-next v6 0/4] net/sched: cap the accounted backlog before it can wrap
@ 2026-10-14  9:55 Jamal Hadi Salim
  2026-10-14  9:55 ` [Cake] [PATCH net-next v6 1/4] net/sched: cake, fq_codel: return the classifier result to the caller Jamal Hadi Salim
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jamal Hadi Salim @ 2026-10-14  9:55 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Victor Nogueira,
	Toke Høiland-Jørgensen, cake, Sashiko

This series bounds the accounted backlog of the AQM qdiscs so that it
cannot wrap mod 2^32.

v1 widened the counters to u64 and was rejected; from v2 on the fix drops
at enqueue instead. The v5 posting was a single patch; at the reviewer's
request it is split here into four:

  1/4  classifier refactor: cake_tcf_classify() and fq_codel_classify()
       return their result to the caller.
  2/4  cake: use a dedicated split-GSO list iterator.
  3/4  the fix: drop at enqueue once the stored backlog is within one
       maximum-size packet of the wrap point, and clamp the stabless GSO
       producer in qdisc_pkt_len_segs_init(). Covers fq_codel, cake,
       codel, pie, fq_pie, dualpi2, RED, pfifo, plug and gred.
  4/4  cake: split cake_hash() into side-effect-free key extraction (on
       the unsegmented skb) and set-associative resolution (after
       admission), and move the split-GSO path ahead of the resolution.

The commit logs and the in-code comments are rewritten for readability.

The guard half is a net fix, but the CAKE hash split is new-code net-next
material, so the series targets net-next with no Fixes: (the aggregate
wrap predates every touched qdisc).

Changes in v6:
  - Rewrite as a four-patch series per Toke Høiland-Jørgensen's review of
    v5: split the classifier refactor, the split-GSO iterator, the overflow
    guard and the cake hash split into separate patches, and drop the
    verbose commit prose and the in-code comments that read as
    machine-written.
  - Considered the Sashiko gemini v5 TC_ACT_CONSUMED report; not folded.
    No in-tree producer can deliver TC_ACT_CONSUMED to a qdisc classify
    switch, so the fall-through it describes is unreachable: act_ct returns
    it only when bound to a clsact or ingress qdisc (11cb63b0d1a0), and
    act_mirred returns it only for a redirect that does not clone, which
    requires an ingress skb. cls_bpf and act_bpf map an unrecognized opcode
    such as CONSUMED to TC_ACT_UNSPEC.

v5 Considered Comments from Sashiko nipa and Simon:
  - cake: the cake_tcf_classify() block comment had an embedded "/*"
    ("else *flow/*host"), which trips a -Wcomment / W=1 build warning.
    Simon Horman reported it on v4 and noted it likely suppressed the
    Sashiko nipa review; reworded to drop the nested comment.
  - cake: reword the same comment so it no longer says the function
    returns true for "a matched no usable flow" result (Sashiko nipa v3
    Low): cake_tcf_classify() returns true only for a terminal TC action;
    a matched-but-unusable classid leaves the overrides at 0 and the
    packet is hashed as before (fq_codel_classify() differs).
  - commit log: soften the claim that the 1 MiB headroom always bounds a
    single packet (Sashiko nipa v3 High).

v4  fold the Sashiko nipa v3 review:
  - cake: compute the flow/host hash keys from the packet as received
    (before skb_gso_segment(), which can rewrite an encapsulated GSO
    packet's headers to the inner ones), but defer the set-associative
    resolution that writes q->tags[], the flow host indices, the way
    counters and the host bulk-flow counts until after the split-GSO
    segment-sum admission check, so a packet rejected by that check
    commits no flow state.
  - cake: in the split-GSO rejection, handle a fraglist GSO (whose
    skb_segment_list() returns the original skb as the list head with an
    extra reference, segs == skb) by dropping the caller's reference and
    queueing the list once, instead of calling __qdisc_drop_all() and
    qdisc_drop_reason() on the same skb, which self-linked *to_free.

v3  fold the v2 review (Eric Dumazet) and Sashiko gemini v2:
  - Test the stored backlog via qdisc_backlog_at_max() instead of
    (u64)backlog + qdisc_pkt_len() > QDISC_MAX_BACKLOG: no double-count
    of the reserved 1 MiB headroom, no per-enqueue 64-bit add.
  - cake: run the external classifier first, honor its terminal TC
    actions, then the ceiling check, then the hash and the shaper state;
    the SPLIT_GSO segment sum is checked before any flow/host state is
    committed and the list is deferred to to_free rather than freed
    under the qdisc lock.
  - fq_codel: run the external classifier (terminal actions honored)
    first, then the ceiling check before fq_codel_hash(); document the
    fq_codel_drop() NULL flow->head consequence.
  - pfifo: same ceiling guard, since RED reads a (graftable) child's
    backlog and SFB's default child is pfifo.

v2 approach rewrite per list discussion of v1:
  v1 approached the wrap by widening the per-flow backlog counters to
  u64. Eric Dumazet rejected the framing ("Storing 4GB in a qdisc is
  absolutely insane") and asked for a drop at enqueue once the backlog
  approaches the wrap point. Sebastian Moeller noted tc-stab is the
  generic overhead-accounting mechanism, so the fix must not penalize
  normal stab use.
  v2 replaces the u64 widening with an enqueue-side pre-check against
  QDISC_MAX_BACKLOG and drops with QDISC_DROP_OVERLIMIT. No counter is
  widened, so the 32-bit sch->qstats.backlog the AQM reads can no longer
  wrap. Same-pattern siblings fixed in one patch: fq_codel, cake, codel,
  pie, fq_pie, dualpi2, RED.

Jamal Hadi Salim (4):
  net/sched: cake, fq_codel: return the classifier result to the caller
  net/sched: cake: use a dedicated split-GSO list iterator
  net/sched: cap the accounted backlog before it can wrap
  net/sched: cake: hash before segmenting and reject an overlong split

 include/net/pkt_sched.h   |   1 -
 include/net/sch_generic.h |  15 ++++
 net/core/dev.c            |  15 +++-
 net/sched/sch_cake.c      | 183 +++++++++++++++++++++++++++-----------
 net/sched/sch_codel.c     |  16 ++--
 net/sched/sch_dualpi2.c   |   1 +
 net/sched/sch_fifo.c      |   6 ++
 net/sched/sch_fq_codel.c  |  55 +++++++++---
 net/sched/sch_fq_pie.c    |   3 +-
 net/sched/sch_gred.c      |  10 +++
 net/sched/sch_pie.c       |   3 +-
 net/sched/sch_plug.c      |   9 +-
 net/sched/sch_red.c       |  25 ++++--
 13 files changed, 262 insertions(+), 80 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-14  9:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14  9:55 [Cake] [PATCH net-next v6 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
2026-10-14  9:55 ` [Cake] [PATCH net-next v6 1/4] net/sched: cake, fq_codel: return the classifier result to the caller Jamal Hadi Salim
2026-10-14  9:55 ` [Cake] [PATCH net-next v6 2/4] net/sched: cake: use a dedicated split-GSO list iterator Jamal Hadi Salim
2026-10-14  9:55 ` [Cake] [PATCH net-next v6 3/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
2026-10-14  9:55 ` [Cake] [PATCH net-next v6 4/4] net/sched: cake: hash before segmenting and reject an overlong split Jamal Hadi Salim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox