[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1bbbb306-d497-4143-a714-b126ecc41a06@kernel.org>
Date: Fri, 16 Jan 2026 11:40:06 +0100
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: netdev@...r.kernel.org
Cc: bpf@...r.kernel.org, Eric Dumazet <eric.dumazet@...il.com>,
"David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Toke Høiland-Jørgensen
<toke@...e.dk>, carges@...udflare.com, kernel-team@...udflare.com
Subject: Re: [PATCH net-next v1] net: sched: sfq: add detailed drop reasons
for monitoring
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@...nel.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);
>
>
Powered by blists - more mailing lists