Re: [PATCH net-next v8 1/4] net/sched: cake: return the classifier result to the caller

On Tue, Oct 20, 2026 at 1:45 PM Toke Høiland-Jørgensen [off-list ref] wrote:

Jamal Hadi Salim [off-list ref] writes:

quoted

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 <redacted>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

One nit below, in case you need to respin for other reasons; but I guess
I can live with it as-is, so:

If sashiko comes up with something code-specific that is meaningful, I
will send another round and will factor this in.

In this and other series i learnt one small lesson that cost a lot of churn:
If sashiko complains about some pre-existing issue, I shouldnt jump in
and say "that is sensible, why not add it on the next update" ;->
It then becomes a recursion of sorts, sashiko starts asking for more
and more ;->
My temptation is to put as many small related issues as possible in
one series instead of sending one at a time and wrestling in the mud
with sashiko every week..

cheers,
jamal

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>

quoted

---

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øiland-Jørgensen, 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(struct Qdisc *sch,
      return &qd->tins[tin];
 }

-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 :/

quoted

+static bool cake_tcf_classify(struct Qdisc *sch, struct sk_buff *skb,
+                           u16 *flow, u16 *host, int *qerr)
 {
      struct cake_sched_data *q = qdisc_priv(sch);
      struct tcf_proto *filter;
      struct tcf_result res;
-     u16 flow = 0, host = 0;
      int result;

+     *flow = 0;
+     *host = 0;
+
      filter = rcu_dereference_bh(q->filter_list);
      if (!filter)
-             goto hash;
+             return false;

      *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
      result = tcf_classify_qdisc(skb, filter, &res, false);

@@ -1737,17 +1743,15 @@ static u32 cake_classify(struct Qdisc *sch, struct cake_tin_data **t,
                      *qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
                      fallthrough;
              case TC_ACT_SHOT:
-                     return 0;
+                     return true;
              }
 #endif
              if (TC_H_MIN(res.classid) <= CAKE_QUEUES)
-                     flow = TC_H_MIN(res.classid);
+                     *flow = TC_H_MIN(res.classid);
              if (TC_H_MAJ(res.classid) <= (CAKE_QUEUES << 16))
-                     host = TC_H_MAJ(res.classid) >> 16;
+                     *host = TC_H_MAJ(res.classid) >> 16;
      }
-hash:
-     *t = cake_select_tin(sch, skb);
-     return cake_hash(*t, skb, flow_mode, flow, host) + 1;
+     return false;
 }

 static void cake_reconfigure(struct Qdisc *sch);

@@ -1758,20 +1762,25 @@ 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 len = qdisc_pkt_len(skb), ret;
+     u16 flow_override, host_override;
      struct sk_buff *ack = NULL;
      ktime_t now = ktime_get();
      struct cake_tin_data *b;
      struct cake_flow *flow;
      bool same_flow = false;

-     /* choose flow to insert into */
-     idx = cake_classify(sch, &b, skb, q->config->flow_mode, &ret);
-     if (idx == 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 = cake_select_tin(sch, skb);
+     idx = cake_hash(b, skb, q->config->flow_mode, flow_override,
+                     host_override) + 1;
      tin = (u32)(b - q->tins);
      idx--;
      flow = &b->flows[idx];
--

2.43.0