Re: [PATCH net-next v1] net: sched: sfq: add detailed drop reasons for monitoring

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

Jesper Dangaard Brouer <hawk@kernel.org> writes:

> Hi Eric,
>
> I need an opinion on naming for drop_reasons below.
>
> On 15/01/2026 13.23, Jesper Dangaard Brouer wrote:
>> Add specific drop reasons to SFQ qdisc to improve packet drop observability
>> and monitoring capabilities. This change replaces generic qdisc_drop()
>> calls with qdisc_drop_reason() to provide granular metrics about different
>> drop scenarios in production environments.
>> 
>> Two new drop reasons are introduced:
>> 
>> - SKB_DROP_REASON_QDISC_MAXFLOWS: Used when a new flow cannot be created
>>    because the maximum number of flows (flows parameter) has been
>>    reached and no free flow slots are available.
>> 
>> - SKB_DROP_REASON_QDISC_MAXDEPTH: Used when a flow's queue length exceeds
>>    the per-flow depth limit (depth parameter), triggering either tail drop
>>    or head drop depending on headdrop configuration.
>
> I noticed commit 5765c7f6e317 ("net_sched: sch_fq: add three 
> drop_reason") (Author: Eric Dumazet).
>
>   SKB_DROP_REASON_FQ_BAND_LIMIT: Per-band packet limit exceeded
>   SKB_DROP_REASON_FQ_HORIZON_LIMIT: Packet timestamp too far in future
>   SKB_DROP_REASON_FQ_FLOW_LIMIT: Per-flow packet limit exceeded
>
> Should I/we make SKB_DROP_REASON_QDISC_MAXDEPTH specific for SFQ ?
> Like naming it = SKB_DROP_REASON_SFQ_MAXDEPTH ?
>
> Currently SKB_DROP_REASON_QDISC_MAXDEPTH is only used in SFQ, but it
> might be usable in other qdisc as well.  Except that I noticed the
> meaning of SKB_DROP_REASON_FQ_FLOW_LIMIT which is basically the same.
> This made me think that perhaps I should also make it qdisc specific.
> I'm considering adding a per-flow limit to fq_codel as I'm seeing prod
> issues with the global 10240 packet limit. This also need a similar flow
> depth limit drop reason. I'm undecided which way to go, please advice.

IMO, we should be reusing drop reasons where it makes sense (so
s/FQ/QDISC/ SKB_DROP_REASON_FQ_FLOW_LIMIT), but not sure if these are
considered UAPI (i.e., can we change the name of the existing one)?

-Toke