netdev - Re: [Cake] [PATCH net-next] net_sched: sch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a5d46i9c.fsf@toke.dk>
Date: Tue, 10 Dec 2024 09:42:55 +0100
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Dave Taht <dave.taht@...il.com>, Jakub Kicinski <kuba@...nel.org>
Cc: Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org, Jamal Hadi Salim
 <jhs@...atatu.com>, cake@...ts.bufferbloat.net, Eric Dumazet
 <edumazet@...gle.com>, Simon Horman <horms@...nel.org>, Cong Wang
 <xiyou.wangcong@...il.com>, Paolo Abeni <pabeni@...hat.com>, "David S.
 Miller" <davem@...emloft.net>
Subject: Re: [Cake] [PATCH net-next] net_sched: sch_cake: Add drop reasons

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

> On Mon, Dec 9, 2024 at 3:52 PM Jakub Kicinski via Cake
> <cake@...ts.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