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

[Jesper Dangaard Brouer <hawk@kernel.org>]

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.


> The existing SKB_DROP_REASON_QDISC_OVERLIMIT is used in sfq_drop() when
> the overall qdisc limit is exceeded and packets are dropped from the
> longest queue.
> 
> These detailed drop reasons enable production monitoring systems to
> distinguish between different SFQ drop scenarios and generate specific
> metrics for:
> - Flow table exhaustion (flows exceeded)
> - Per-flow congestion (depth limit exceeded)
> - Global qdisc congestion (overall limit exceeded)
> 
> This granular visibility allows operators to identify issues related
> to traffic patterns, and optimize SFQ configuration based on
> real-world drop patterns.
> 
> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> ---
>   include/net/dropreason-core.h |   12 ++++++++++++
>   net/sched/sch_sfq.c           |    8 ++++----
>   2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
> index 58d91ccc56e0..e395d0ff9904 100644
> --- a/include/net/dropreason-core.h
> +++ b/include/net/dropreason-core.h
> @@ -69,6 +69,8 @@
>   	FN(QDISC_DROP)			\
>   	FN(QDISC_OVERLIMIT)		\
>   	FN(QDISC_CONGESTED)		\
> +	FN(QDISC_MAXFLOWS)		\
> +	FN(QDISC_MAXDEPTH)		\
>   	FN(CAKE_FLOOD)			\
>   	FN(FQ_BAND_LIMIT)		\
>   	FN(FQ_HORIZON_LIMIT)		\

FQ drop reason names can be seen above.

> @@ -384,6 +386,16 @@ enum skb_drop_reason {
>   	 * due to congestion.
>   	 */
>   	SKB_DROP_REASON_QDISC_CONGESTED,
> +	/**
> +	 * @SKB_DROP_REASON_QDISC_MAXFLOWS: dropped by qdisc when the maximum
> +	 * number of flows is exceeded.
> +	 */
> +	SKB_DROP_REASON_QDISC_MAXFLOWS,
> +	/**
> +	 * @SKB_DROP_REASON_QDISC_MAXDEPTH: dropped by qdisc when a flow
> +	 * exceeds its maximum queue depth limit.
> +	 */
> +	SKB_DROP_REASON_QDISC_MAXDEPTH,
>   	/**
>   	 * @SKB_DROP_REASON_CAKE_FLOOD: dropped by the flood protection part of
>   	 * CAKE qdisc AQM algorithm (BLUE).
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index 96eb2f122973..e91d74127600 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -302,7 +302,7 @@ static unsigned int sfq_drop(struct Qdisc *sch, struct sk_buff **to_free)
>   		sfq_dec(q, x);
>   		sch->q.qlen--;
>   		qdisc_qstats_backlog_dec(sch, skb);
> -		qdisc_drop(skb, sch, to_free);
> +		qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_OVERLIMIT);
>   		return len;
>   	}
>   
> @@ -363,7 +363,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
>   	if (x == SFQ_EMPTY_SLOT) {
>   		x = q->dep[0].next; /* get a free slot */
>   		if (x >= SFQ_MAX_FLOWS)
> -			return qdisc_drop(skb, sch, to_free);
> +			return qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_MAXFLOWS);
>   		q->ht[hash] = x;
>   		slot = &q->slots[x];
>   		slot->hash = hash;
> @@ -420,14 +420,14 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
>   	if (slot->qlen >= q->maxdepth) {
>   congestion_drop:
>   		if (!sfq_headdrop(q))
> -			return qdisc_drop(skb, sch, to_free);
> +			return qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_MAXDEPTH);
>   
>   		/* We know we have at least one packet in queue */
>   		head = slot_dequeue_head(slot);
>   		delta = qdisc_pkt_len(head) - qdisc_pkt_len(skb);
>   		sch->qstats.backlog -= delta;
>   		slot->backlog -= delta;
> -		qdisc_drop(head, sch, to_free);
> +		qdisc_drop_reason(head, sch, to_free, SKB_DROP_REASON_QDISC_MAXDEPTH);
>   
>   		slot_queue_add(slot, skb);
>   		qdisc_tree_reduce_backlog(sch, 0, delta);
> 
>