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

["Toke Høiland-Jørgensen" <toke@toke.dk>]

Jamal Hadi Salim <jhs@mojatatu.com> writes:

> On Mon, Oct 19, 2026 at 2:11 PM Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>>
>> Jamal Hadi Salim <jhs@mojatatu.com> writes:
>>
>> > The Fix:
>> > cake and fq_codel run an optional external classifier from their enqueue
>> > path and then compute the flow hash inside the helper. Have the helper
>> > return the filter's overrides (cake) or a 1-based flow index or a request
>> > to hash (fq_codel) instead, so the caller owns the flow selection.
>> >
>> > This is preparation for the accounting changes that follow. Packet
>> > classification and flow selection are unchanged.
>>
>> It's not clear to me why this change is needed at all for fq_codel?
>> fq_codel_hash just does a reciprocal_scale() of the skb hash, it doesn't
>> mutate any qdisc state. So why can't fq_codel_classify not just stay as
>> it is?
>
> It is not functionaly needed for fq_codel, it is more because these
> two are cousins and simplifies subsequent patches.
> Note in patch 4 we force the same semantics on cake as in fq_codel
> (classifier decides the drop, the caller owns flow selection). Keeping
> fq_codel's classify the same makes patch 3 hunks in the two files
> similar. There is another reason: the old helper encoded "drop" as a
> magic return and carried the 1-based +1 arithmetic on every return
> path.

But you're not making the same change to fq_pie, although its classify
function is line-for-line the same as fq_codel. I think it's better to
keep those two the same; you can add the new check to fq_codel between
the 'idx--' and 'codel_set_enqueue_time()' lines just fine.

> In any case, unless you feel strongly i would rather keep it.

Well, see above :)

-Toke