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

[Jakub Kicinski <kuba@kernel.org>]

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?