From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mail.toke.dk; dkim=none; arc=none (Message is not ARC signed); dmarc=fail (Used From Domain Record) header.from=toke.dk policy.dmarc=none From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Jamal Hadi Salim , 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?B?QmrDtnJuIFTDtnBlbA==?= , cake@lists.bufferbloat.net, Sashiko In-Reply-To: References: Date: Tue, 20 Oct 2026 19:42:54 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87fqy0tgip.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 73CVUFKEGKHTJKJREVFGT5GIJ744RFGG X-Message-ID-Hash: 73CVUFKEGKHTJKJREVFGT5GIJ744RFGG X-MailFrom: toke@toke.dk 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] Re: [PATCH net-next v8 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: Jamal Hadi Salim writes: > 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. > > 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 Thank you for sticking with the series! :) Acked-by: Toke H=C3=B8iland-J=C3=B8rgensen