Re: [Cake] [PATCH net-next] net_sched: sch_cake: Add drop reasons

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

Dave Taht <dave.taht@gmail.com> writes:

> On Mon, Dec 9, 2024 at 3:52 PM Jakub Kicinski via Cake
> <cake@lists.bufferbloat.net> wrote:
>>
>> On Mon, 09 Dec 2024 13:02:18 +0100 Toke Høiland-Jørgensen wrote:
>> > Add three qdisc-specific drop reasons for sch_cake:
>> >
>> >  1) SKB_DROP_REASON_CAKE_CONGESTED
>> >     Whenever a packet is dropped by the CAKE AQM algorithm because
>> >     congestion is detected.
>> >
>> >  2) SKB_DROP_REASON_CAKE_FLOOD
>> >     Whenever a packet is dropped by the flood protection part of the
>> >     CAKE AQM algorithm (BLUE).
>> >
>> >  3) SKB_DROP_REASON_CAKE_OVERLIMIT
>> >     Whenever the total queue limit for a CAKE instance is exceeded and a
>> >     packet is dropped to make room.
>>
>> Eric's patch was adding fairly FQ-specific reasons, other than flood
>> this seems like generic AQM stuff, no? From a very quick look the
>> congestion looks like fairly standard AQM, overlimit is also typical
>> for qdics?
>
> While I initially agreed with making this generic, preserving the qdisc from
> where the drop came lets you safely inspect the cb block (timestamp, etc),
> format of which varies by qdisc. You also get insight as to which
> qdisc was dropping.
>
> Downside is we'll end up with SKB_DROP_REASON_XXX_OVERLIMIT for
> each of the qdiscs. Etc.

Yeah, I agree that a generic "dropped by AQM" reason will be too generic
without knowing which qdisc dropped it. I guess any calls directly to
kfree_skb_reason() from the qdisc will provide the calling function, but
for qdisc_drop_reason() the drop will be deferred to __dev_queue_xmit(),
so no way of knowing where the drop came from, AFAICT?

-Toke