Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v9 0/4] net/sched: cap the accounted backlog before it can wrap
@ 2026-11-10 14:40 Jamal Hadi Salim
  2026-11-10 14:40 ` [PATCH net-next v9 1/4] net/sched: cake: return the classifier result to the caller Jamal Hadi Salim
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-10 14:40 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, Björn Töpel, 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 into four:

  1/4  cake: classifier refactor: cake_tcf_classify() returns its 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 cannot
       represent the packet's accounted length -- the standing
       QDISC_MAX_BACKLOG ceiling for ordinary packets, a per-packet
       representability check for the larger wire estimates. 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), move the split-GSO path ahead of the resolution, and
       tag every deferred split segment.

For the sashikos: This series targets net-next with no Fixes: (the
aggregate wrap predates every touched qdisc).
Unfortunately something broke in the sashiko reviews. Review for v7
showed up a while later after v8.  So you will see that this series
has fixes (majority in comments and commit logs) from both.

Changes in v9 (late Sashiko nipa reviews of v7 and v8, delivered on lore):
  - 1/4: reword the "The Fix:" heading; this patch is preparation, not a
    fix.
  - 1/4: the classifier helper's comments now say the caller must drop
    the skb, since a terminal action does not take ownership of it.
  - 3/4: bound the child backlog as well as RED's aggregate, and re-derive
    the aggregate from snapshots taken before the reservation when the
    enqueue does not succeed. A child that returns NET_XMIT_CN may or may
    not have reduced its ancestors already: pfifo_head_drop, sfq and
    fq_codel do, while hhf and skbprio evict a packet and return without
    qdisc_tree_reduce_backlog(). Keep what the child actually accounted
    for and pass any difference on to the ancestors above RED, so neither
    counter can diverge. Verified with two A/B witnesses: RED over a
    grafted hhf child (control ends with red_backlog 2042016 !=
    child_backlog 6177600) and RED under an HTB class over a grafted
    pfifo_head_drop limit 1, where an unequal shrinking replacement
    otherwise wraps the aggregate and wedges the new admission guard --
    control 4294966052, fixed 92 with RED == child == HTB.
  - 3/4: note that CAKE's own split-GSO branch is the second exception;
    the segments it accounts are summed and checked against
    QDISC_MAX_BACKLOG by 4/4 before any of them is linked.
  - 4/4: add the split-GSO "Conditions to recreate the bug" recipe for the
    segment-sum rejection at the split ceiling.
  - 4/4: the IS_ERR_OR_NULL(segs) segmentation-failure return uses
    qdisc_drop_reason(..., QDISC_DROP_GENERIC); __skb_gso_segment() has
    already clobbered tc_skb_cb.drop_reason through SKB_GSO_CB, so the old
    qdisc_drop() reported an unrelated core reason and skipped
    trace_qdisc_drop.
  - 4/4: pass the flow_mode snapshot to the later flowchain calls
    (cake_get_flow_quantum(), cake_inc_srchost_bulk_flow_count(),
    cake_inc_dsthost_bulk_flow_count()) as well, so one enqueue sees one
    mode (KCSAN-visible race and stale host index).

Changes in v8:
  - 1/4 is cake-only. fq_codel_classify() is line-for-line the same as
    fq_pie_classify(), and fq_pie is not changed by this series, so the
    two stay in lockstep: fq_codel keeps its original classify shape and
    the fq_codel backlog guard is placed inline in its enqueue path
    (3/4) instead. (Toke Høiland-Jørgensen)
  - 3/4: the fq_codel guard sits between idx-- and
    codel_set_enqueue_time(), so a dropped packet cannot touch flow
    state. The commit message is reworded as suggested. (Toke
    Høiland-Jørgensen)
  - 4/4: the original skb is consumed unconditionally right after a
    successful split -- that is the caller's reference, since a fraglist
    GSO takes the list's own reference on the same skb in
    skb_segment_list() -- so the reject branch tags every deferred
    segment and drops the whole list with qdisc_drop_all(), and the
    admitted path no longer consumes the skb after the walk. (Toke
    Høiland-Jørgensen)

Changes in v7:
  - 1/4: complete the fq_codel_classify() comment (Sashiko nipa v6
    main-2-1, Low). The comment listed only the terminal-action and
    no-class-ID cases; the function also returns true when no filter
    matched or when the selected class minor is above q->flows_cnt,
    and those packets are BYPASS drops, not a fallback to hashing as
    in cake. Comment-only, no code change; folded into this resend
    since the series is revised anyway.
  - 3/4: do not clamp the shared qdisc_pkt_len() (Sashiko nipa v6
    main-2-6, Medium). The clamp silently undercharged BPF
    __sk_buff.wire_len, act_police and the HTB/TBF shapers for a valid
    large GSO estimate. The initializer now stays truthful for every
    input and the overflow protection is enqueue admission: each
    affected qdisc drops a packet once the stored backlog cannot
    represent its accounted length -- the standing QDISC_MAX_BACKLOG
    ceiling covers ordinary packets, and a per-packet representability
    check covers the larger wire estimates, which a supported
    small-MSS BIG TCP aggregate can legitimately carry above 1 MiB
    while its raw length stays within GSO_MAX_SIZE. The consumers read
    the real length, and sch_handle_ingress() keeps complete pkt_segs
    metadata.
  - 4/4: tag every deferred split-GSO segment with QDISC_DROP_OVERLIMIT
    (Sashiko nipa v6 gpt-6-astra-2-5, Medium). __skb_gso_segment() stores
    SKB_GSO_CB at cb offset 32, aliasing tc_skb_cb.drop_reason, and the cb
    is copied into every segment, so tagging only the head left the rest
    with GSO scratch as their drop reason.
  - 4/4: snapshot q->config->flow_mode once with READ_ONCE() and pass it
    to both hash helpers (Sashiko nipa v6 gpt-6-astra-5-9, Low). The split
    introduced two loads with the segmentation work between them.
  - Not folded: the 3/4 intermediate-commit split-GSO limitation (4/4
    closes it; the pre-series path was already unbounded) and the
    Fixes:/stable suggestions (intentional net-next target). The gemini
    v6 TC_ACT_CONSUMED High is the v5 finding already rebutted as
    unreachable from a qdisc classify switch.

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; drop the machine-written
    prose). Considered the Sashiko gemini v5 TC_ACT_CONSUMED report and
    did not fold it (unreachable; see below). Reworded the cake block
    comment that had an embedded "/*" (Simon Horman's W=1 warning) and
    reworded a comment that overstated the classifier's return cases. The
    TC_ACT_CONSUMED switch case was dropped from the final 0001 because it
    still reached __qdisc_drop(); no in-tree producer can deliver
    TC_ACT_CONSUMED to a qdisc classify switch (act_ct returns it only on
    a clsact/ingress qdisc, act_mirred only for a non-cloning ingress
    redirect, and cls_bpf/act_bpf remap an unknown verdict to UNSPEC).

v5/v4/v3/v2 history: see the v6 posting. In short, v3 adopted the stored
    backlog compare qdisc_backlog_at_max() (no double-count of the 1 MiB
    headroom, no per-enqueue 64-bit add), moved the classifier and the
    ceiling check ahead of the hash/shaper state, and added pfifo; v4
    split the hash keys from the set-associative resolution and handled
    the fraglist-GSO self-link; v2 replaced the rejected u64 widening with
    the enqueue drop; v1 widened the counters.

Jamal Hadi Salim (4):
  net/sched: cake: 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 |  20 +++++
 net/sched/sch_cake.c      | 185 ++++++++++++++++++++++++++------------
 net/sched/sch_codel.c     |  16 ++--
 net/sched/sch_dualpi2.c   |   1 +
 net/sched/sch_fifo.c      |   6 ++
 net/sched/sch_fq_codel.c  |   9 ++
 net/sched/sch_fq_pie.c    |   3 +-
 net/sched/sch_gred.c      |   9 ++
 net/sched/sch_pie.c       |   3 +-
 net/sched/sch_plug.c      |   9 +-
 net/sched/sch_red.c       |  46 ++++++++--
 12 files changed, 237 insertions(+), 71 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-12 15:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10 14:40 [PATCH net-next v9 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
2026-11-10 14:40 ` [PATCH net-next v9 1/4] net/sched: cake: return the classifier result to the caller Jamal Hadi Salim
2026-11-10 14:40 ` [PATCH net-next v9 2/4] net/sched: cake: use a dedicated split-GSO list iterator Jamal Hadi Salim
2026-11-12 15:13   ` Toke Høiland-Jørgensen
2026-11-10 14:40 ` [PATCH net-next v9 3/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
2026-11-12 15:18   ` Toke Høiland-Jørgensen
2026-11-10 14:40 ` [PATCH net-next v9 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