From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mail.toke.dk; dkim=pass header.d=mojatatu.com header.i=@mojatatu.com header.a=rsa-sha256 header.s=google header.b=rkDGDqCt; arc=none (Message is not ARC signed); dmarc=none Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) by mail.toke.dk (Postfix) with ESMTPS id 88F3B185F89E for ; Mon, 19 Oct 2026 18:00:58 +0200 (CEST) Received: by mail-qv1-xf31.google.com with SMTP id 6a1803df08f44-91b63bd2030so17891236d6.3 for ; Mon, 19 Oct 2026 09:00:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mojatatu.com; s=google; t=1792425657; x=1793030457; darn=lists.bufferbloat.net; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=cKS2dTyhA01XmkSWpi13gcyzgdEFBA2w22z0cfu/IS8=; b=rkDGDqCtlGGi5u0wHQA6Fag35e3mflMvxJDUHm6f3Z4sm0WZgtwVbJqXJAl/Dvb4Y1 5DU8yU0OUwmhP3BZC1WQ1z0NyK11bWkBKGZ2M6XzYXJXRJNazLFOfu8V0IsjTUvGEJpG oYfch98K7woLOM/z/wk8MwjMWsD6OX9pQ3Hvk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792425657; x=1793030457; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=cKS2dTyhA01XmkSWpi13gcyzgdEFBA2w22z0cfu/IS8=; b=zY5p4ZU6WK2eU6lwUM83rmy+XXBd2rS9E80QRwb8cnCkqA/rJpzfnaaEwkehW779TN PkRivaAjYn8CiIkZ59wksiNxjAIX5OnD8eWE64W+Yyug3d5J0j7v+bSmhTkEowg36Rm/ AUbGnGbrgwovyp/K539z4YPmt23+V4muyWcQu81tgZWr0hpkoYQ6FHBLDPWcJhimLJpH j1VKQEUWgraKYAVAz+2Np3tHF2U5QLrjgjwssKYJVPqwJnEToz8PVH9IO5SyUrtSTpZl 3okA16IoyrGCRSRumR0Y+WGmgySnYAgDqR9/pz0v0jMnNbP++ZvYV2Ju+9T0PdqXhd35 7nxg== X-Forwarded-Encrypted: i=1; AKwUvBz1AkZgwuU2plrCZQFdB3kKc5Zww3Nxv6b+M0uIMyoV/e6A1mtf7txPncREXTwlj1kzTB8y@lists.bufferbloat.net X-Gm-Message-State: AFq9FYJ9YMgFD8aEZesjpkSqF4uqSVNUu1bctDWR6xTvjTVYuHABgnsT PVo2bNSXQvfQ16N5FZtZ7Qt2yYcBcbbdA9W2y2Pp9EUyQjl0kdB3bmaLJibqcqX3Z01m4AF9uUL h1YE= X-Gm-Gg: AYBFou1LRCTLp6dhJEQVWd7MZnhrhZAjEn2/kzdbOvGxJpIU7JqYYWDA34eDBMDHXDp TEvH02aWUE/Z5u9KjDd3atnhD7il8eZI0MtXXgqvrpqAv6RpainXq5c8YhKOFmULCIXko/HqkJI dK8xda6comTZBcNYQlANp8fwkzERfCfrfuPsTD1iNz1nDs5Pxo3vktw8oxvMtx1TJG4s2vrz9Cw WEGrNMdipXXuF//lHAPBbrC2c084wRNAt3W/iDyDfacdmPW+NQM5EIfkQD5U3DKTpVSg9exqphE 5KEheaumduuQPGP7Z0XmLwtMDiRYLPKSdw5epmGs/Pm6Rnxtj8JKGr601n2/1UkOvL2wLerIIWk w9Inr/onKpvZtV4XTG3O+MObpl8JwHo8YoqmY7xEmmYZZrvOefxUsTd+kPjh/gMM48w5EE9xfgf IWb1n/W01blkKqNtZItBlMToIiMiG8nd05MToaNHMa45f+LWs7K4Kpia5c0z8isUDrfxxk43VZK /cCBBq/xLLBrh9C6ITA28WLIaPqbqudKVITEG4/Mx2j X-Received: by 2002:a05:6214:5904:b0:91b:7656:ffea with SMTP id 6a1803df08f44-91bdf195a90mr133081436d6.35.1792425656552; Mon, 19 Oct 2026 09:00:56 -0700 (PDT) Received: from mbili.waya ([64.203.83.2]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-91be9d8acb5sm64846846d6.0.2026.10.19.09.00.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Oct 2026 09:00:54 -0700 (PDT) From: Jamal Hadi Salim To: netdev@vger.kernel.org Cc: Jamal Hadi Salim , Jiri Pirko , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Victor Nogueira , =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= , =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= , cake@lists.bufferbloat.net, Sashiko Date: Mon, 19 Oct 2026 12:00:35 -0400 Message-Id: X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: A7FYM55VBHJVGI7W4LB7F4B6W2QRO7SZ X-Message-ID-Hash: A7FYM55VBHJVGI7W4LB7F4B6W2QRO7SZ X-MailFrom: jhs@mojatatu.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list Subject: [Cake] [PATCH net-next v7 4/4] net/sched: cake: hash before segmenting and reject an overlong split List-Id: Cake - FQ_codel the next generation Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The Fix: with CAKE_FLAG_SPLIT_GSO, cake computes the flow/host hash keys, then splits a GSO skb into segments it accounts individually. Key extraction must run on the unsegmented skb -- skb_gso_segment() can rewrite an encapsulated packet's headers to the inner ones -- and the resolution must wait until the packet is admitted, since it commits the set-associative tags, host indices and way counters that a later rejection would leave mutated (a GSO skb whose segmentation fails was dropped with that state already committed). Split cake_hash() into a side-effect-free cake_hash_keys(), run on the unsegmented skb, and cake_hash_resolve(), run only once the packet is admitted. Move the split-GSO path ahead of the resolution: sum the segment list, test the total against the ceiling before linking any segment, and drop the whole list on rejection so no persistent state is committed. The list is deferred to to_free so the tail segment's sock_wfree() never runs under the qdisc lock; a fraglist GSO, where skb_segment_list() returns the original skb as the head, drops the caller's reference and queues the list once instead of self-linking it. Tag every deferred segment with QDISC_DROP_OVERLIMIT: __skb_gso_segment() keeps SKB_GSO_CB at cb offset 32, aliasing tc_skb_cb.drop_reason, and __copy_skb_header() copies it into every segment, so tagging only the head left the rest with GSO scratch (a headroom-sized value) as their drop reason, which __tcf_kfree_skb_list() would report as an unrelated core reason. Read q->config->flow_mode once with READ_ONCE() in cake_enqueue() and pass that value to both cake_hash_keys() and cake_hash_resolve(). cake_config_change() writes the mode before cake_change() takes sch_tree_lock(), and the split introduced two separate loads with the segmentation work between them; a concurrent reconfiguration could extract keys in one mode and resolve them in another. Link: https://lore.kernel.org/netdev/179231933068.3135.80340792633077986@kernel.org/ Reviewed-by: Victor Nogueira Signed-off-by: Jamal Hadi Salim --- v6 -> v7: the split-GSO reject path now sets QDISC_DROP_OVERLIMIT on every deferred segment, not only on the head/original skb; the GSO cb aliases tc_skb_cb.drop_reason and is copied into every segment, so untagged segments carried GSO scratch as their drop reason. The flow_mode is likewise snapshotted once with READ_ONCE() and passed to both cake_hash_keys() and cake_hash_resolve(). (Sashiko nipa v6 review, gpt-6-astra-2-5, gpt-6-astra-5-9) --- net/sched/sch_cake.c | 130 ++++++++++++++++++++++++++++++++----------- 1 file changed, 99 insertions(+), 31 deletions(-) diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index 91c33295b30b..730c90af14bd 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -706,19 +706,28 @@ static u16 cake_get_flow_quantum(struct cake_tin_data *q, get_random_u16()) >> 16; } -static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb, - int flow_mode, u16 flow_override, u16 host_override) +struct cake_hash_keys { + u32 srchost_hash; + u32 dsthost_hash; + u32 flow_hash; +}; + +static void cake_hash_keys(const struct sk_buff *skb, int flow_mode, + u16 flow_override, u16 host_override, + struct cake_hash_keys *out) { bool hash_flows = (!flow_override && !!(flow_mode & CAKE_FLOW_FLOWS)); bool hash_hosts = (!host_override && !!(flow_mode & CAKE_FLOW_HOSTS)); bool nat_enabled = !!(flow_mode & CAKE_FLOW_NAT_FLAG); - u32 flow_hash = 0, srchost_hash = 0, dsthost_hash = 0; - u16 reduced_hash, srchost_idx, dsthost_idx; struct flow_keys keys, host_keys; bool use_skbhash = skb->l4_hash; + out->flow_hash = 0; + out->srchost_hash = 0; + out->dsthost_hash = 0; + if (unlikely(flow_mode == CAKE_FLOW_NONE)) - return 0; + return; /* If both overrides are set, or we can use the SKB hash and nat mode is * disabled, we can skip packet dissection entirely. If nat mode is @@ -753,50 +762,64 @@ static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb, switch (host_keys.control.addr_type) { case FLOW_DISSECTOR_KEY_IPV4_ADDRS: host_keys.addrs.v4addrs.src = 0; - dsthost_hash = flow_hash_from_keys(&host_keys); + out->dsthost_hash = flow_hash_from_keys(&host_keys); host_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src; host_keys.addrs.v4addrs.dst = 0; - srchost_hash = flow_hash_from_keys(&host_keys); + out->srchost_hash = flow_hash_from_keys(&host_keys); break; case FLOW_DISSECTOR_KEY_IPV6_ADDRS: memset(&host_keys.addrs.v6addrs.src, 0, sizeof(host_keys.addrs.v6addrs.src)); - dsthost_hash = flow_hash_from_keys(&host_keys); + out->dsthost_hash = flow_hash_from_keys(&host_keys); host_keys.addrs.v6addrs.src = keys.addrs.v6addrs.src; memset(&host_keys.addrs.v6addrs.dst, 0, sizeof(host_keys.addrs.v6addrs.dst)); - srchost_hash = flow_hash_from_keys(&host_keys); + out->srchost_hash = flow_hash_from_keys(&host_keys); break; default: - dsthost_hash = 0; - srchost_hash = 0; + out->dsthost_hash = 0; + out->srchost_hash = 0; } /* This *must* be after the above switch, since as a * side-effect it sorts the src and dst addresses. */ if (hash_flows && !use_skbhash) - flow_hash = flow_hash_from_keys(&keys); + out->flow_hash = flow_hash_from_keys(&keys); skip_hash: if (flow_override) - flow_hash = flow_override - 1; + out->flow_hash = flow_override - 1; else if (use_skbhash && (flow_mode & CAKE_FLOW_FLOWS)) - flow_hash = skb->hash; + out->flow_hash = skb->hash; if (host_override) { - dsthost_hash = host_override - 1; - srchost_hash = host_override - 1; + out->dsthost_hash = host_override - 1; + out->srchost_hash = host_override - 1; } if (!(flow_mode & CAKE_FLOW_FLOWS)) { if (flow_mode & CAKE_FLOW_SRC_IP) - flow_hash ^= srchost_hash; + out->flow_hash ^= out->srchost_hash; if (flow_mode & CAKE_FLOW_DST_IP) - flow_hash ^= dsthost_hash; + out->flow_hash ^= out->dsthost_hash; } +} + +/* Commit the flow into the set-associative table (q->tags[], host + * indices, bulk counters). Only after the packet is admitted. + */ +static u32 cake_hash_resolve(struct cake_tin_data *q, + const struct cake_hash_keys *keys, int flow_mode) +{ + u32 flow_hash = keys->flow_hash, srchost_hash = keys->srchost_hash, + dsthost_hash = keys->dsthost_hash; + u16 reduced_hash, srchost_idx, dsthost_idx; + + if (unlikely(flow_mode == CAKE_FLOW_NONE)) + return 0; reduced_hash = flow_hash % CAKE_QUEUES; @@ -1761,13 +1784,17 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, { u32 idx, tin, prev_qlen, prev_backlog, drop_id; struct cake_sched_data *q = qdisc_priv(sch); + int flow_mode = READ_ONCE(q->config->flow_mode); int len = qdisc_pkt_len(skb), ret; u16 flow_override, host_override; + struct sk_buff *segs = NULL; + struct cake_hash_keys keys; struct sk_buff *ack = NULL; ktime_t now = ktime_get(); struct cake_tin_data *b; struct cake_flow *flow; bool same_flow = false; + u64 slen = 0; /* Classify first: a terminal TC action consumes the packet. */ if (cake_tcf_classify(sch, skb, &flow_override, &host_override, &ret)) { @@ -1784,12 +1811,59 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, QDISC_DROP_OVERLIMIT); } - /* choose flow to insert into */ + /* Select the tin and extract the keys before segmenting: + * skb_gso_segment() rewrites encapsulated headers, which + * changes the keys. + */ b = cake_select_tin(sch, skb); - idx = cake_hash(b, skb, q->config->flow_mode, flow_override, - host_override) + 1; + cake_hash_keys(skb, flow_mode, flow_override, host_override, &keys); + + if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) { + struct sk_buff *nskb, *seg; + netdev_features_t features = netif_skb_features(skb); + + segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK); + if (IS_ERR_OR_NULL(segs)) + return qdisc_drop(skb, sch, to_free); + + /* The split path accounts the segment sum, not the + * stab-adjusted qdisc_pkt_len(), so check it here. + */ + skb_list_walk_safe(segs, seg, nskb) + slen += seg->len; + + if (unlikely((u64)sch->qstats.backlog + slen > QDISC_MAX_BACKLOG)) { + qdisc_qstats_overlimit(sch); + /* __skb_gso_segment() puts SKB_GSO_CB at cb + * offset 32, over tc_skb_cb.drop_reason; every + * segment inherits it. Tag each one with the + * qdisc drop reason. + */ + skb_list_walk_safe(segs, seg, nskb) + tcf_set_qdisc_drop_reason(seg, + QDISC_DROP_OVERLIMIT); + /* skb_gso_segment() moved sock_wfree to the tail + * segment, so defer the list to to_free rather + * than free it under the qdisc lock. + * + * Fraglist GSO returns the original skb as head + * (segs == skb), so drop our ref first. + */ + if (unlikely(segs == skb)) { + consume_skb(skb); + return qdisc_drop_all(segs, sch, to_free); + } + __qdisc_drop_all(segs, to_free); + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); + } + } + + /* Admitted: resolve the flow. The mode is read once above, so + * extraction and resolution agree if cake_change() intervenes. + */ + idx = cake_hash_resolve(b, &keys, flow_mode); tin = (u32)(b - q->tins); - idx--; flow = &b->flows[idx]; /* ensure shaper state isn't stale */ @@ -1816,14 +1890,9 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, if (unlikely(len > b->max_skblen)) WRITE_ONCE(b->max_skblen, len); - if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) { - struct sk_buff *segs, *nskb, *seg; - netdev_features_t features = netif_skb_features(skb); - unsigned int slen = 0, numsegs = 0; - - segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK); - if (IS_ERR_OR_NULL(segs)) - return qdisc_drop(skb, sch, to_free); + if (segs) { + struct sk_buff *nskb, *seg; + unsigned int numsegs = 0; skb_list_walk_safe(segs, seg, nskb) { skb_mark_not_on_list(seg); @@ -1837,7 +1906,6 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, qdisc_qlen_inc(sch); numsegs++; q->buffer_used += seg->truesize; - slen += seg->len; WRITE_ONCE(b->packets, b->packets + 1); } -- 2.43.0