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: <20241210172802.410c76a6@kernel.org>
Date: Tue, 10 Dec 2024 17:28:02 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Toke Høiland-Jørgensen <toke@...hat.com>
Cc: Dave Taht <dave.taht@...il.com>, 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

On Tue, 10 Dec 2024 09:42:55 +0100 Toke Høiland-Jørgensen wrote:
> > 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.

Why does type of the qdisc matter if the qdisc was overlimit?

> 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?

Can you tell me why I'd need to inspect the skb->cb[] in cake if packet
is overlimit? Actually, none of the fields of the cb are initialized
when the packet is dropped for overlimit, AFAIU.

If someone is doing serious / advanced debug they mostly care about
access to the qdisc and can trivially check if its ops match the
expected symbol. (Speaking from experience, I've been debugging FQ
packet loss on Nov 23rd.)

If someone is just doing high level drop attribution having to list all
possible qdiscs under "qdisc discard" is purely pain.

Can we start with OVERLIMIT and CONGESTION as generic values and we can
specialize if anyone has a clear need?