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:45:29 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87cxt4tgee.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: WVY4QJHEKNCBP4AD4EJ6LPSKWB2XCDEB X-Message-ID-Hash: WVY4QJHEKNCBP4AD4EJ6LPSKWB2XCDEB 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 1/4] net/sched: cake: return the classifier result to the caller 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: > cake runs an optional external classifier from its enqueue path and then > computes the flow hash inside the helper. Have the helper return the > filter's overrides instead, so the caller owns the flow selection. > > This is preparation for the accounting changes that follow. Packet > classification and flow selection are unchanged. > > Reviewed-by: Victor Nogueira > Signed-off-by: Jamal Hadi Salim One nit below, in case you need to respin for other reasons; but I guess I can live with it as-is, so: Acked-by: Toke H=C3=B8iland-J=C3=B8rgensen > --- > > v7 -> v8: 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 fq_codel > keeps its original classify shape; the fq_codel backlog guard is placed > inline in sch_fq_codel.c in 0003 instead. (Toke H=C3=B8iland-J=C3=B8rgen= sen, v7 > review) > --- > net/sched/sch_cake.c | 35 ++++++++++++++++++++++------------- > 1 file changed, 22 insertions(+), 13 deletions(-) > > diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c > index dc93267029e7..6432d10c992e 100644 > --- a/net/sched/sch_cake.c > +++ b/net/sched/sch_cake.c > @@ -1712,18 +1712,24 @@ static struct cake_tin_data *cake_select_tin(stru= ct Qdisc *sch, > return &qd->tins[tin]; > } >=20=20 > -static u32 cake_classify(struct Qdisc *sch, struct cake_tin_data **t, > - struct sk_buff *skb, int flow_mode, int *qerr) > +/* Run the qdisc's optional external classifier. Returns true when a > + * terminal TC action consumed the packet, with the reason in *qerr; > + * otherwise *flow and *host hold the filter's overrides (0 if none). > + */ nit: I still think this comment is superfluous and should just be removed :/ > +static bool cake_tcf_classify(struct Qdisc *sch, struct sk_buff *skb, > + u16 *flow, u16 *host, int *qerr) > { > struct cake_sched_data *q =3D qdisc_priv(sch); > struct tcf_proto *filter; > struct tcf_result res; > - u16 flow =3D 0, host =3D 0; > int result; >=20=20 > + *flow =3D 0; > + *host =3D 0; > + > filter =3D rcu_dereference_bh(q->filter_list); > if (!filter) > - goto hash; > + return false; >=20=20 > *qerr =3D NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; > result =3D tcf_classify_qdisc(skb, filter, &res, false); > @@ -1737,17 +1743,15 @@ static u32 cake_classify(struct Qdisc *sch, struc= t cake_tin_data **t, > *qerr =3D NET_XMIT_SUCCESS | __NET_XMIT_STOLEN; > fallthrough; > case TC_ACT_SHOT: > - return 0; > + return true; > } > #endif > if (TC_H_MIN(res.classid) <=3D CAKE_QUEUES) > - flow =3D TC_H_MIN(res.classid); > + *flow =3D TC_H_MIN(res.classid); > if (TC_H_MAJ(res.classid) <=3D (CAKE_QUEUES << 16)) > - host =3D TC_H_MAJ(res.classid) >> 16; > + *host =3D TC_H_MAJ(res.classid) >> 16; > } > -hash: > - *t =3D cake_select_tin(sch, skb); > - return cake_hash(*t, skb, flow_mode, flow, host) + 1; > + return false; > } >=20=20 > static void cake_reconfigure(struct Qdisc *sch); > @@ -1758,20 +1762,25 @@ static s32 cake_enqueue(struct sk_buff *skb, stru= ct Qdisc *sch, > u32 idx, tin, prev_qlen, prev_backlog, drop_id; > struct cake_sched_data *q =3D qdisc_priv(sch); > int len =3D qdisc_pkt_len(skb), ret; > + u16 flow_override, host_override; > struct sk_buff *ack =3D NULL; > ktime_t now =3D ktime_get(); > struct cake_tin_data *b; > struct cake_flow *flow; > bool same_flow =3D false; >=20=20 > - /* choose flow to insert into */ > - idx =3D cake_classify(sch, &b, skb, q->config->flow_mode, &ret); > - if (idx =3D=3D 0) { > + /* Classify first: a terminal TC action consumes the packet. */ > + if (cake_tcf_classify(sch, skb, &flow_override, &host_override, &ret)) { > if (ret & __NET_XMIT_BYPASS) > qdisc_qstats_drop(sch); > __qdisc_drop(skb, to_free); > return ret; > } > + > + /* choose flow to insert into */ > + b =3D cake_select_tin(sch, skb); > + idx =3D cake_hash(b, skb, q->config->flow_mode, flow_override, > + host_override) + 1; > tin =3D (u32)(b - q->tins); > idx--; > flow =3D &b->flows[idx]; > --=20 > 2.43.0