* [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
@ 2026-02-04 17:02 Jesper Dangaard Brouer
2026-02-04 17:02 ` [PATCH net-next RFC v1 1/3] net: sched: introduce qdisc-specific drop reason tracing Jesper Dangaard Brouer
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-04 17:02 UTC (permalink / raw)
To: netdev, Eric Dumazet, David S. Miller, Paolo Abeni,
Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, bpf, Jakub Kicinski, horms, jiri,
edumazet, xiyou.wangcong, jhs, carges, kernel-team
RFC: Seeking feedback on this refactoring approach.
This series refactors qdisc drop reason handling by introducing a dedicated
enum qdisc_drop_reason and trace_qdisc_drop tracepoint, providing qdisc
layer drop diagnostics with direct qdisc context visibility.
Background:
-----------
Identifying which qdisc dropped a packet via skb_drop_reason is difficult.
Normally, the kfree_skb tracepoint caller "location" hints at the dropping
code, but qdisc drops happen at a central point (__dev_queue_xmit), making
this unusable. As a workaround, commits 5765c7f6e317 ("net_sched: sch_fq:
add three drop_reason") and a42d71e322a8 ("net_sched: sch_cake: Add drop
reasons") encoded qdisc names directly in the drop reason enums.
This series provides a cleaner solution by creating a dedicated qdisc
tracepoint that naturally includes qdisc context (handle, parent, kind).
Solution:
---------
Create a new tracepoint trace_qdisc_drop that builds on top of existing
trace_qdisc_enqueue infrastructure. It includes qdisc handle, parent,
qdisc kind (name), and device information directly.
The existing SKB_DROP_REASON_QDISC_DROP is retained for backwards
compatibility via kfree_skb_reason(). The qdisc-specific drop reasons
(QDISC_DROP_*) provide fine-grained detail via the new tracepoint.
This implements the alternative approach described in:
https://lore.kernel.org/all/6be17a08-f8aa-4f91-9bd0-d9e1f0a92d90@kernel.org/
Series overview:
----------------
Patch 1: Introduce the qdisc drop reason infrastructure
- New enum qdisc_drop_reason in dropreason-qdisc.h
- New trace_qdisc_drop tracepoint with qdisc context
- Convert FQ, FQ_CoDel, CoDel, SFB, pfifo_fast to use new infrastructure
- Remove SKB_DROP_REASON_QDISC_* entries from dropreason-core.h
Patch 2: Convert SFQ as example for reviewers
- Demonstrates how to convert a flow-based qdisc
- Adds QDISC_DROP_MAXFLOWS for flow table exhaustion
- Renames FQ_FLOW_LIMIT to common FLOW_LIMIT (shared by FQ and SFQ)
Patch 3: Fix CAKE to use enum qdisc_drop_reason
- Change cobalt_should_drop() return type for type correctness
---
Jesper Dangaard Brouer (3):
net: sched: introduce qdisc-specific drop reason tracing
net: sched: sfq: convert to qdisc drop reasons
net: sched: sch_cake: use enum qdisc_drop_reason for cobalt_should_drop
include/net/dropreason-core.h | 36 -------------
include/net/dropreason-qdisc.h | 97 ++++++++++++++++++++++++++++++++++
include/net/sch_generic.h | 36 ++++++++-----
include/trace/events/qdisc.h | 51 ++++++++++++++++++
net/core/dev.c | 8 +--
net/sched/sch_cake.c | 26 ++++-----
net/sched/sch_codel.c | 5 +-
net/sched/sch_dualpi2.c | 8 ++-
net/sched/sch_fq.c | 7 ++-
net/sched/sch_fq_codel.c | 4 +-
net/sched/sch_fq_pie.c | 4 +-
net/sched/sch_generic.c | 20 ++++++-
net/sched/sch_gred.c | 4 +-
net/sched/sch_pie.c | 4 +-
net/sched/sch_red.c | 4 +-
net/sched/sch_sfb.c | 4 +-
net/sched/sch_sfq.c | 8 +--
17 files changed, 230 insertions(+), 96 deletions(-)
create mode 100644 include/net/dropreason-qdisc.h
--
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next RFC v1 1/3] net: sched: introduce qdisc-specific drop reason tracing
2026-02-04 17:02 [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jesper Dangaard Brouer
@ 2026-02-04 17:02 ` Jesper Dangaard Brouer
2026-02-04 17:03 ` [PATCH net-next RFC v1 2/3] net: sched: sfq: convert to qdisc drop reasons Jesper Dangaard Brouer
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-04 17:02 UTC (permalink / raw)
To: netdev, Eric Dumazet, David S. Miller, Paolo Abeni,
Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, bpf, Jakub Kicinski, horms, jiri,
edumazet, xiyou.wangcong, jhs, carges, kernel-team
Create new enum qdisc_drop_reason and trace_qdisc_drop tracepoint
for qdisc layer drop diagnostics with direct qdisc context visibility.
The new tracepoint includes qdisc handle, parent, kind (name), and
device information. Existing SKB_DROP_REASON_QDISC_DROP is retained
for backwards compatibility via kfree_skb_reason().
Convert FQ, FQ_CoDel, CoDel, SFB, and pfifo_fast to use the new
infrastructure.
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
---
include/net/dropreason-core.h | 36 -----------------
include/net/dropreason-qdisc.h | 87 ++++++++++++++++++++++++++++++++++++++++
include/net/sch_generic.h | 36 +++++++++++------
include/trace/events/qdisc.h | 51 +++++++++++++++++++++++
net/core/dev.c | 8 ++--
net/sched/sch_cake.c | 6 +--
net/sched/sch_codel.c | 5 +-
net/sched/sch_dualpi2.c | 8 +---
net/sched/sch_fq.c | 7 +--
net/sched/sch_fq_codel.c | 4 +-
net/sched/sch_fq_pie.c | 4 +-
net/sched/sch_generic.c | 20 ++++++++-
net/sched/sch_gred.c | 4 +-
net/sched/sch_pie.c | 4 +-
net/sched/sch_red.c | 4 +-
net/sched/sch_sfb.c | 4 +-
16 files changed, 206 insertions(+), 82 deletions(-)
create mode 100644 include/net/dropreason-qdisc.h
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index a7b7abd66e21..456a2f078278 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -68,12 +68,6 @@
FN(SECURITY_HOOK) \
FN(QDISC_DROP) \
FN(QDISC_BURST_DROP) \
- FN(QDISC_OVERLIMIT) \
- FN(QDISC_CONGESTED) \
- FN(CAKE_FLOOD) \
- FN(FQ_BAND_LIMIT) \
- FN(FQ_HORIZON_LIMIT) \
- FN(FQ_FLOW_LIMIT) \
FN(CPU_BACKLOG) \
FN(XDP) \
FN(TC_INGRESS) \
@@ -380,36 +374,6 @@ enum skb_drop_reason {
* limit is hit.
*/
SKB_DROP_REASON_QDISC_BURST_DROP,
- /**
- * @SKB_DROP_REASON_QDISC_OVERLIMIT: dropped by qdisc when a qdisc
- * instance exceeds its total buffer size limit.
- */
- SKB_DROP_REASON_QDISC_OVERLIMIT,
- /**
- * @SKB_DROP_REASON_QDISC_CONGESTED: dropped by a qdisc AQM algorithm
- * due to congestion.
- */
- SKB_DROP_REASON_QDISC_CONGESTED,
- /**
- * @SKB_DROP_REASON_CAKE_FLOOD: dropped by the flood protection part of
- * CAKE qdisc AQM algorithm (BLUE).
- */
- SKB_DROP_REASON_CAKE_FLOOD,
- /**
- * @SKB_DROP_REASON_FQ_BAND_LIMIT: dropped by fq qdisc when per band
- * limit is reached.
- */
- SKB_DROP_REASON_FQ_BAND_LIMIT,
- /**
- * @SKB_DROP_REASON_FQ_HORIZON_LIMIT: dropped by fq qdisc when packet
- * timestamp is too far in the future.
- */
- SKB_DROP_REASON_FQ_HORIZON_LIMIT,
- /**
- * @SKB_DROP_REASON_FQ_FLOW_LIMIT: dropped by fq qdisc when a flow
- * exceeds its limits.
- */
- SKB_DROP_REASON_FQ_FLOW_LIMIT,
/**
* @SKB_DROP_REASON_CPU_BACKLOG: failed to enqueue the skb to the per CPU
* backlog queue. This can be caused by backlog queue full (see
diff --git a/include/net/dropreason-qdisc.h b/include/net/dropreason-qdisc.h
new file mode 100644
index 000000000000..2175ab34921e
--- /dev/null
+++ b/include/net/dropreason-qdisc.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _LINUX_DROPREASON_QDISC_H
+#define _LINUX_DROPREASON_QDISC_H
+
+#define DEFINE_QDISC_DROP_REASON(FN, FNe) \
+ FN(UNSPEC) \
+ FN(GENERIC) \
+ FN(OVERLIMIT) \
+ FN(CONGESTED) \
+ FN(CAKE_FLOOD) \
+ FN(FQ_BAND_LIMIT) \
+ FN(FQ_HORIZON_LIMIT) \
+ FN(FQ_FLOW_LIMIT) \
+ FNe(MAX)
+
+#undef FN
+#undef FNe
+#define FN(reason) QDISC_DROP_##reason,
+#define FNe(reason) QDISC_DROP_##reason
+
+/**
+ * enum qdisc_drop_reason - reason why a qdisc dropped a packet
+ *
+ * Qdisc-specific drop reasons for packet drops that occur within the
+ * traffic control (TC) queueing discipline layer. These reasons provide
+ * detailed diagnostics about why packets were dropped by various qdisc
+ * algorithms, enabling fine-grained monitoring and troubleshooting of
+ * queue behavior.
+ */
+enum qdisc_drop_reason {
+ /**
+ * @QDISC_DROP_UNSPEC: unspecified qdisc drop reason
+ */
+ QDISC_DROP_UNSPEC,
+ /**
+ * @QDISC_DROP_GENERIC: generic/default qdisc drop, used when no
+ * more specific reason applies
+ */
+ QDISC_DROP_GENERIC,
+ /**
+ * @QDISC_DROP_OVERLIMIT: packet dropped because the qdisc queue
+ * length exceeded its configured limit (sch->limit). This typically
+ * indicates the queue is full and cannot accept more packets.
+ */
+ QDISC_DROP_OVERLIMIT,
+ /**
+ * @QDISC_DROP_CONGESTED: packet dropped due to active congestion
+ * control algorithms (e.g., CoDel, PIE, RED) detecting network
+ * congestion. The qdisc proactively dropped the packet to signal
+ * congestion to the sender and prevent bufferbloat.
+ */
+ QDISC_DROP_CONGESTED,
+ /**
+ * @QDISC_DROP_CAKE_FLOOD: CAKE qdisc dropped packet due to flood
+ * protection mechanism (BLUE algorithm). This indicates potential
+ * DoS/flood attack or unresponsive flow behavior.
+ */
+ QDISC_DROP_CAKE_FLOOD,
+ /**
+ * @QDISC_DROP_FQ_BAND_LIMIT: FQ (Fair Queue) dropped packet because
+ * the priority band's packet limit was reached. Each priority band
+ * in FQ has its own limit.
+ */
+ QDISC_DROP_FQ_BAND_LIMIT,
+ /**
+ * @QDISC_DROP_FQ_HORIZON_LIMIT: FQ dropped packet because its
+ * timestamp is too far in the future (beyond horizon). This prevents
+ * packets with unreasonable future timestamps from blocking the queue.
+ */
+ QDISC_DROP_FQ_HORIZON_LIMIT,
+ /**
+ * @QDISC_DROP_FQ_FLOW_LIMIT: FQ dropped packet because an individual
+ * flow exceeded its per-flow packet limit.
+ */
+ QDISC_DROP_FQ_FLOW_LIMIT,
+ /**
+ * @QDISC_DROP_MAX: the maximum of qdisc drop reasons, which
+ * shouldn't be used as a real 'reason' - only for tracing code gen
+ */
+ QDISC_DROP_MAX,
+};
+
+#undef FN
+#undef FNe
+
+#endif
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index c3a7268b567e..f3ee0bd5d0f3 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -20,12 +20,15 @@
#include <net/rtnetlink.h>
#include <net/flow_offload.h>
#include <linux/xarray.h>
+#include <net/dropreason-qdisc.h>
struct Qdisc_ops;
struct qdisc_walker;
struct tcf_walker;
struct module;
struct bpf_flow_keys;
+struct Qdisc;
+struct netdev_queue;
struct qdisc_rate_table {
struct tc_ratespec rate;
@@ -1106,36 +1109,43 @@ static inline struct tc_skb_cb *tc_skb_cb(const struct sk_buff *skb)
return cb;
}
+/* TC classifier accessors - use enum skb_drop_reason */
static inline enum skb_drop_reason
tcf_get_drop_reason(const struct sk_buff *skb)
{
- return tc_skb_cb(skb)->drop_reason;
+ return (enum skb_drop_reason)tc_skb_cb(skb)->drop_reason;
}
static inline void tcf_set_drop_reason(const struct sk_buff *skb,
enum skb_drop_reason reason)
{
- tc_skb_cb(skb)->drop_reason = reason;
+ tc_skb_cb(skb)->drop_reason = (enum qdisc_drop_reason)reason;
}
-static inline void tcf_kfree_skb_list(struct sk_buff *skb)
+/* Qdisc accessors - use enum qdisc_drop_reason */
+static inline enum qdisc_drop_reason
+tcf_get_qdisc_drop_reason(const struct sk_buff *skb)
{
- while (unlikely(skb)) {
- struct sk_buff *next = skb->next;
+ return tc_skb_cb(skb)->drop_reason;
+}
- prefetch(next);
- kfree_skb_reason(skb, tcf_get_drop_reason(skb));
- skb = next;
- }
+static inline void tcf_set_qdisc_drop_reason(const struct sk_buff *skb,
+ enum qdisc_drop_reason reason)
+{
+ tc_skb_cb(skb)->drop_reason = reason;
}
+void tcf_kfree_skb_list(struct sk_buff *skb, struct Qdisc *q,
+ struct netdev_queue *txq,
+ struct net_device *dev);
+
static inline void qdisc_dequeue_drop(struct Qdisc *q, struct sk_buff *skb,
- enum skb_drop_reason reason)
+ enum qdisc_drop_reason reason)
{
DEBUG_NET_WARN_ON_ONCE(!(q->flags & TCQ_F_DEQUEUE_DROPS));
DEBUG_NET_WARN_ON_ONCE(q->flags & TCQ_F_NOLOCK);
- tcf_set_drop_reason(skb, reason);
+ tcf_set_qdisc_drop_reason(skb, reason);
skb->next = q->to_free;
q->to_free = skb;
}
@@ -1312,9 +1322,9 @@ static inline int qdisc_drop(struct sk_buff *skb, struct Qdisc *sch,
static inline int qdisc_drop_reason(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free,
- enum skb_drop_reason reason)
+ enum qdisc_drop_reason reason)
{
- tcf_set_drop_reason(skb, reason);
+ tcf_set_qdisc_drop_reason(skb, reason);
return qdisc_drop(skb, sch, to_free);
}
diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
index ff33f41a9db7..d8a5c2677470 100644
--- a/include/trace/events/qdisc.h
+++ b/include/trace/events/qdisc.h
@@ -74,6 +74,57 @@ TRACE_EVENT(qdisc_enqueue,
__entry->ifindex, __entry->handle, __entry->parent, __entry->skbaddr)
);
+#undef FN
+#undef FNe
+#define FN(reason) TRACE_DEFINE_ENUM(QDISC_DROP_##reason);
+#define FNe(reason) TRACE_DEFINE_ENUM(QDISC_DROP_##reason);
+DEFINE_QDISC_DROP_REASON(FN, FNe)
+
+#undef FN
+#undef FNe
+#define FN(reason) { QDISC_DROP_##reason, #reason },
+#define FNe(reason) { QDISC_DROP_##reason, #reason }
+
+TRACE_EVENT(qdisc_drop,
+
+ TP_PROTO(struct Qdisc *qdisc, const struct netdev_queue *txq,
+ struct net_device *dev, struct sk_buff *skb,
+ enum qdisc_drop_reason reason),
+
+ TP_ARGS(qdisc, txq, dev, skb, reason),
+
+ TP_STRUCT__entry(
+ __field(struct Qdisc *, qdisc)
+ __field(const struct netdev_queue *, txq)
+ __field(void *, skbaddr)
+ __field(int, ifindex)
+ __field(u32, handle)
+ __field(u32, parent)
+ __field(enum qdisc_drop_reason, reason)
+ __string(kind, qdisc->ops->id)
+ ),
+
+ TP_fast_assign(
+ __entry->qdisc = qdisc;
+ __entry->txq = txq;
+ __entry->skbaddr = skb;
+ __entry->ifindex = dev ? dev->ifindex : 0;
+ __entry->handle = qdisc->handle;
+ __entry->parent = qdisc->parent;
+ __entry->reason = reason;
+ __assign_str(kind);
+ ),
+
+ TP_printk("drop ifindex=%d kind=%s handle=0x%X parent=0x%X skbaddr=%p reason=%s",
+ __entry->ifindex, __get_str(kind), __entry->handle,
+ __entry->parent, __entry->skbaddr,
+ __print_symbolic(__entry->reason,
+ DEFINE_QDISC_DROP_REASON(FN, FNe)))
+);
+
+#undef FN
+#undef FNe
+
TRACE_EVENT(qdisc_reset,
TP_PROTO(struct Qdisc *q),
diff --git a/net/core/dev.c b/net/core/dev.c
index 43de5af0d6ec..33262344b751 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4161,7 +4161,7 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
qdisc_calculate_pkt_len(skb, q);
- tcf_set_drop_reason(skb, SKB_DROP_REASON_QDISC_DROP);
+ tcf_set_qdisc_drop_reason(skb, QDISC_DROP_GENERIC);
if (q->flags & TCQ_F_NOLOCK) {
if (q->flags & TCQ_F_CAN_BYPASS && nolock_qdisc_is_empty(q) &&
@@ -4269,8 +4269,8 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
spin_unlock(root_lock);
free_skbs:
- tcf_kfree_skb_list(to_free);
- tcf_kfree_skb_list(to_free2);
+ tcf_kfree_skb_list(to_free, q, txq, dev);
+ tcf_kfree_skb_list(to_free2, q, txq, dev);
return rc;
}
@@ -5795,7 +5795,7 @@ static __latent_entropy void net_tx_action(void)
to_free = qdisc_run(q);
if (root_lock)
spin_unlock(root_lock);
- tcf_kfree_skb_list(to_free);
+ tcf_kfree_skb_list(to_free, q, NULL, qdisc_dev(q));
}
rcu_read_unlock();
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index fd56b7d88301..a2d1b292600d 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -548,7 +548,7 @@ static enum skb_drop_reason cobalt_should_drop(struct cobalt_vars *vars,
if (next_due && vars->dropping) {
/* Use ECN mark if possible, otherwise drop */
if (!(vars->ecn_marked = INET_ECN_set_ce(skb)))
- reason = SKB_DROP_REASON_QDISC_CONGESTED;
+ reason = QDISC_DROP_CONGESTED;
vars->count++;
if (!vars->count)
@@ -573,7 +573,7 @@ static enum skb_drop_reason cobalt_should_drop(struct cobalt_vars *vars,
/* Simple BLUE implementation. Lack of ECN is deliberate. */
if (vars->p_drop && reason == SKB_NOT_DROPPED_YET &&
get_random_u32() < vars->p_drop)
- reason = SKB_DROP_REASON_CAKE_FLOOD;
+ reason = QDISC_DROP_CAKE_FLOOD;
/* Overload the drop_next field as an activity timeout */
if (!vars->count)
@@ -1604,7 +1604,7 @@ static unsigned int cake_drop(struct Qdisc *sch, struct sk_buff **to_free)
if (q->config->rate_flags & CAKE_FLAG_INGRESS)
cake_advance_shaper(q, b, skb, now, true);
- qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_OVERLIMIT);
+ qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_OVERLIMIT);
sch->q.qlen--;
cake_heapify(q, 0);
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index c6551578f1cf..dc2be90666ff 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -52,7 +52,7 @@ static void drop_func(struct sk_buff *skb, void *ctx)
{
struct Qdisc *sch = ctx;
- qdisc_dequeue_drop(sch, skb, SKB_DROP_REASON_QDISC_CONGESTED);
+ qdisc_dequeue_drop(sch, skb, QDISC_DROP_CONGESTED);
qdisc_qstats_drop(sch);
}
@@ -86,8 +86,7 @@ static int codel_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
q = qdisc_priv(sch);
q->drop_overlimit++;
- return qdisc_drop_reason(skb, sch, to_free,
- SKB_DROP_REASON_QDISC_OVERLIMIT);
+ return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_OVERLIMIT);
}
static const struct nla_policy codel_policy[TCA_CODEL_MAX + 1] = {
diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index 6d7e6389758d..0b6a38704a3b 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -393,13 +393,11 @@ static int dualpi2_enqueue_skb(struct sk_buff *skb, struct Qdisc *sch,
qdisc_qstats_overlimit(sch);
if (skb_in_l_queue(skb))
qdisc_qstats_overlimit(q->l_queue);
- return qdisc_drop_reason(skb, sch, to_free,
- SKB_DROP_REASON_QDISC_OVERLIMIT);
+ return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_OVERLIMIT);
}
if (q->drop_early && must_drop(sch, q, skb)) {
- qdisc_drop_reason(skb, sch, to_free,
- SKB_DROP_REASON_QDISC_CONGESTED);
+ qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_CONGESTED);
return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
}
@@ -593,7 +591,7 @@ static struct sk_buff *dualpi2_qdisc_dequeue(struct Qdisc *sch)
while ((skb = dequeue_packet(sch, q, &credit_change, now))) {
if (!q->drop_early && must_drop(sch, q, skb)) {
drop_and_retry(q, skb, sch,
- SKB_DROP_REASON_QDISC_CONGESTED);
+ QDISC_DROP_CONGESTED);
continue;
}
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 6e5f2f4f2415..058801fefca1 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -541,7 +541,7 @@ static bool fq_packet_beyond_horizon(const struct sk_buff *skb,
return unlikely((s64)skb->tstamp > (s64)(now + q->horizon));
}
-#define FQDR(reason) SKB_DROP_REASON_FQ_##reason
+#define FQDR(reason) QDISC_DROP_FQ_##reason
static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
@@ -554,8 +554,7 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
band = fq_prio2band(q->prio2band, skb->priority & TC_PRIO_MAX);
if (unlikely(q->band_pkt_count[band] >= sch->limit)) {
q->stat_band_drops[band]++;
- return qdisc_drop_reason(skb, sch, to_free,
- FQDR(BAND_LIMIT));
+ return qdisc_drop_reason(skb, sch, to_free, FQDR(BAND_LIMIT));
}
now = ktime_get_ns();
@@ -581,7 +580,7 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
if (unlikely(f->qlen >= q->flow_plimit)) {
q->stat_flows_plimit++;
return qdisc_drop_reason(skb, sch, to_free,
- FQDR(FLOW_LIMIT));
+ QDISC_DROP_FQ_FLOW_LIMIT);
}
if (fq_flow_is_detached(f)) {
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index dc187c7f06b1..3e384a344bc3 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -168,7 +168,7 @@ static unsigned int fq_codel_drop(struct Qdisc *sch, unsigned int max_packets,
skb = dequeue_head(flow);
len += qdisc_pkt_len(skb);
mem += get_codel_cb(skb)->mem_usage;
- tcf_set_drop_reason(skb, SKB_DROP_REASON_QDISC_OVERLIMIT);
+ tcf_set_qdisc_drop_reason(skb, QDISC_DROP_OVERLIMIT);
__qdisc_drop(skb, to_free);
} while (++i < max_packets && len < threshold);
@@ -275,7 +275,7 @@ static void drop_func(struct sk_buff *skb, void *ctx)
{
struct Qdisc *sch = ctx;
- qdisc_dequeue_drop(sch, skb, SKB_DROP_REASON_QDISC_CONGESTED);
+ qdisc_dequeue_drop(sch, skb, QDISC_DROP_CONGESTED);
qdisc_qstats_drop(sch);
}
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 7b96bc3ff891..9fe997179d78 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -130,7 +130,7 @@ static inline void flow_queue_add(struct fq_pie_flow *flow,
static int fq_pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
- enum skb_drop_reason reason = SKB_DROP_REASON_QDISC_OVERLIMIT;
+ enum qdisc_drop_reason reason = QDISC_DROP_OVERLIMIT;
struct fq_pie_sched_data *q = qdisc_priv(sch);
struct fq_pie_flow *sel_flow;
int ret;
@@ -162,7 +162,7 @@ static int fq_pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
q->overmemory++;
}
- reason = SKB_DROP_REASON_QDISC_CONGESTED;
+ reason = QDISC_DROP_CONGESTED;
if (!pie_drop_early(sch, &q->p_params, &sel_flow->vars,
sel_flow->backlog, skb->len)) {
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 852e603c1755..3baf84eaaf79 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -25,11 +25,11 @@
#include <linux/skb_array.h>
#include <linux/if_macvlan.h>
#include <linux/bpf.h>
+#include <trace/events/qdisc.h>
#include <net/sch_generic.h>
#include <net/pkt_sched.h>
#include <net/dst.h>
#include <net/hotdata.h>
-#include <trace/events/qdisc.h>
#include <trace/events/net.h>
#include <net/xfrm.h>
@@ -37,6 +37,22 @@
const struct Qdisc_ops *default_qdisc_ops = &pfifo_fast_ops;
EXPORT_SYMBOL(default_qdisc_ops);
+void tcf_kfree_skb_list(struct sk_buff *skb, struct Qdisc *q,
+ struct netdev_queue *txq,
+ struct net_device *dev)
+{
+ while (unlikely(skb)) {
+ struct sk_buff *next = skb->next;
+ enum qdisc_drop_reason reason = tcf_get_qdisc_drop_reason(skb);
+
+ prefetch(next);
+ trace_qdisc_drop(q, txq, dev, skb, reason);
+ kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_DROP);
+ skb = next;
+ }
+}
+EXPORT_SYMBOL(tcf_kfree_skb_list);
+
static void qdisc_maybe_clear_missed(struct Qdisc *q,
const struct netdev_queue *txq)
{
@@ -741,7 +757,7 @@ static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
err = skb_array_produce(q, skb);
if (unlikely(err)) {
- tcf_set_drop_reason(skb, SKB_DROP_REASON_QDISC_OVERLIMIT);
+ tcf_set_qdisc_drop_reason(skb, QDISC_DROP_OVERLIMIT);
if (qdisc_is_percpu_stats(qdisc))
return qdisc_drop_cpu(skb, qdisc, to_free);
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 532fde548b88..66b72a09725f 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -251,10 +251,10 @@ static int gred_enqueue(struct sk_buff *skb, struct Qdisc *sch,
q->stats.pdrop++;
drop:
- return qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_OVERLIMIT);
+ return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_OVERLIMIT);
congestion_drop:
- qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_CONGESTED);
+ qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_CONGESTED);
return NET_XMIT_CN;
}
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 0a377313b6a9..16f3f629cb8e 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -85,7 +85,7 @@ EXPORT_SYMBOL_GPL(pie_drop_early);
static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
- enum skb_drop_reason reason = SKB_DROP_REASON_QDISC_OVERLIMIT;
+ enum qdisc_drop_reason reason = QDISC_DROP_OVERLIMIT;
struct pie_sched_data *q = qdisc_priv(sch);
bool enqueue = false;
@@ -94,7 +94,7 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
goto out;
}
- reason = SKB_DROP_REASON_QDISC_CONGESTED;
+ reason = QDISC_DROP_CONGESTED;
if (!pie_drop_early(sch, &q->params, &q->vars, sch->qstats.backlog,
skb->len)) {
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 479c42d11083..c8d3d09f15e3 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -70,7 +70,7 @@ static int red_use_nodrop(struct red_sched_data *q)
static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
- enum skb_drop_reason reason = SKB_DROP_REASON_QDISC_CONGESTED;
+ enum qdisc_drop_reason reason = QDISC_DROP_CONGESTED;
struct red_sched_data *q = qdisc_priv(sch);
struct Qdisc *child = q->qdisc;
unsigned int len;
@@ -108,7 +108,7 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
break;
case RED_HARD_MARK:
- reason = SKB_DROP_REASON_QDISC_OVERLIMIT;
+ reason = QDISC_DROP_OVERLIMIT;
qdisc_qstats_overlimit(sch);
if (red_use_harddrop(q) || !red_use_ecn(q)) {
q->stats.forced_drop++;
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index d2835f1168e1..013738662128 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -280,7 +280,7 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
- enum skb_drop_reason reason = SKB_DROP_REASON_QDISC_OVERLIMIT;
+ enum qdisc_drop_reason reason = QDISC_DROP_OVERLIMIT;
struct sfb_sched_data *q = qdisc_priv(sch);
unsigned int len = qdisc_pkt_len(skb);
struct Qdisc *child = q->qdisc;
@@ -381,7 +381,7 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
r = get_random_u16() & SFB_MAX_PROB;
- reason = SKB_DROP_REASON_QDISC_CONGESTED;
+ reason = QDISC_DROP_CONGESTED;
if (unlikely(r < p_min)) {
if (unlikely(p_min > SFB_MAX_PROB / 2)) {
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next RFC v1 2/3] net: sched: sfq: convert to qdisc drop reasons
2026-02-04 17:02 [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jesper Dangaard Brouer
2026-02-04 17:02 ` [PATCH net-next RFC v1 1/3] net: sched: introduce qdisc-specific drop reason tracing Jesper Dangaard Brouer
@ 2026-02-04 17:03 ` Jesper Dangaard Brouer
2026-02-04 17:03 ` [PATCH net-next RFC v1 3/3] net: sched: sch_cake: use enum qdisc_drop_reason for cobalt_should_drop Jesper Dangaard Brouer
2026-02-05 3:04 ` [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jakub Kicinski
3 siblings, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-04 17:03 UTC (permalink / raw)
To: netdev, Eric Dumazet, David S. Miller, Paolo Abeni,
Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, bpf, Jakub Kicinski, horms, jiri,
edumazet, xiyou.wangcong, jhs, carges, kernel-team
Convert SFQ to use the new qdisc-specific drop reason infrastructure.
This patch demonstrates how to convert a flow-based qdisc to use the
new enum qdisc_drop_reason. As part of this conversion:
- Add QDISC_DROP_MAXFLOWS for flow table exhaustion
- Rename FQ_FLOW_LIMIT to generic FLOW_LIMIT, now shared by FQ and SFQ
- Use QDISC_DROP_OVERLIMIT for sfq_drop() when overall limit exceeded
- Use QDISC_DROP_FLOW_LIMIT for per-flow depth limit exceeded
The FLOW_LIMIT reason is now a common drop reason for per-flow limits,
applicable to both FQ and SFQ qdiscs.
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
---
include/net/dropreason-qdisc.h | 22 ++++++++++++++++------
net/sched/sch_fq.c | 2 +-
net/sched/sch_sfq.c | 8 ++++----
3 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/include/net/dropreason-qdisc.h b/include/net/dropreason-qdisc.h
index 2175ab34921e..2c0a5c358a5f 100644
--- a/include/net/dropreason-qdisc.h
+++ b/include/net/dropreason-qdisc.h
@@ -8,10 +8,11 @@
FN(GENERIC) \
FN(OVERLIMIT) \
FN(CONGESTED) \
+ FN(FLOW_LIMIT) \
+ FN(MAXFLOWS) \
FN(CAKE_FLOOD) \
FN(FQ_BAND_LIMIT) \
FN(FQ_HORIZON_LIMIT) \
- FN(FQ_FLOW_LIMIT) \
FNe(MAX)
#undef FN
@@ -51,6 +52,20 @@ enum qdisc_drop_reason {
* congestion to the sender and prevent bufferbloat.
*/
QDISC_DROP_CONGESTED,
+ /**
+ * @QDISC_DROP_FLOW_LIMIT: packet dropped because an individual flow
+ * exceeded its per-flow packet/depth limit. Used by FQ and SFQ qdiscs
+ * to enforce per-flow fairness and prevent a single flow from
+ * monopolizing queue resources.
+ */
+ QDISC_DROP_FLOW_LIMIT,
+ /**
+ * @QDISC_DROP_MAXFLOWS: packet dropped because the qdisc's flow
+ * tracking table is full and no free slots are available to allocate
+ * for a new flow. This indicates flow table exhaustion in flow-based
+ * qdiscs that maintain per-flow state (e.g., SFQ).
+ */
+ QDISC_DROP_MAXFLOWS,
/**
* @QDISC_DROP_CAKE_FLOOD: CAKE qdisc dropped packet due to flood
* protection mechanism (BLUE algorithm). This indicates potential
@@ -69,11 +84,6 @@ enum qdisc_drop_reason {
* packets with unreasonable future timestamps from blocking the queue.
*/
QDISC_DROP_FQ_HORIZON_LIMIT,
- /**
- * @QDISC_DROP_FQ_FLOW_LIMIT: FQ dropped packet because an individual
- * flow exceeded its per-flow packet limit.
- */
- QDISC_DROP_FQ_FLOW_LIMIT,
/**
* @QDISC_DROP_MAX: the maximum of qdisc drop reasons, which
* shouldn't be used as a real 'reason' - only for tracing code gen
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 058801fefca1..ca615d5433db 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -580,7 +580,7 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
if (unlikely(f->qlen >= q->flow_plimit)) {
q->stat_flows_plimit++;
return qdisc_drop_reason(skb, sch, to_free,
- QDISC_DROP_FQ_FLOW_LIMIT);
+ QDISC_DROP_FLOW_LIMIT);
}
if (fq_flow_is_detached(f)) {
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 96eb2f122973..efb796976a5b 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, QDISC_DROP_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, QDISC_DROP_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, QDISC_DROP_FLOW_LIMIT);
/* 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, QDISC_DROP_FLOW_LIMIT);
slot_queue_add(slot, skb);
qdisc_tree_reduce_backlog(sch, 0, delta);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next RFC v1 3/3] net: sched: sch_cake: use enum qdisc_drop_reason for cobalt_should_drop
2026-02-04 17:02 [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jesper Dangaard Brouer
2026-02-04 17:02 ` [PATCH net-next RFC v1 1/3] net: sched: introduce qdisc-specific drop reason tracing Jesper Dangaard Brouer
2026-02-04 17:03 ` [PATCH net-next RFC v1 2/3] net: sched: sfq: convert to qdisc drop reasons Jesper Dangaard Brouer
@ 2026-02-04 17:03 ` Jesper Dangaard Brouer
2026-02-05 3:04 ` [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jakub Kicinski
3 siblings, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-04 17:03 UTC (permalink / raw)
To: netdev, Eric Dumazet, David S. Miller, Paolo Abeni,
Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, bpf, Jakub Kicinski, horms, jiri,
edumazet, xiyou.wangcong, jhs, carges, kernel-team
Change cobalt_should_drop() return type from enum skb_drop_reason to
enum qdisc_drop_reason to fix implicit enum conversion warnings.
Use QDISC_DROP_UNSPEC as the 'not dropped' sentinel instead of
SKB_NOT_DROPPED_YET. Both have the same compiled value (0), so the
comparison logic remains semantically equivalent.
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
---
net/sched/sch_cake.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index a2d1b292600d..da3c347574d2 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -497,13 +497,13 @@ static bool cobalt_queue_empty(struct cobalt_vars *vars,
/* Call this with a freshly dequeued packet for possible congestion marking.
* Returns true as an instruction to drop the packet, false for delivery.
*/
-static enum skb_drop_reason cobalt_should_drop(struct cobalt_vars *vars,
- struct cobalt_params *p,
- ktime_t now,
- struct sk_buff *skb,
- u32 bulk_flows)
+static enum qdisc_drop_reason cobalt_should_drop(struct cobalt_vars *vars,
+ struct cobalt_params *p,
+ ktime_t now,
+ struct sk_buff *skb,
+ u32 bulk_flows)
{
- enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
+ enum qdisc_drop_reason reason = QDISC_DROP_UNSPEC;
bool next_due, over_target;
ktime_t schedule;
u64 sojourn;
@@ -571,14 +571,14 @@ static enum skb_drop_reason cobalt_should_drop(struct cobalt_vars *vars,
}
/* Simple BLUE implementation. Lack of ECN is deliberate. */
- if (vars->p_drop && reason == SKB_NOT_DROPPED_YET &&
+ if (vars->p_drop && reason == QDISC_DROP_UNSPEC &&
get_random_u32() < vars->p_drop)
reason = QDISC_DROP_CAKE_FLOOD;
/* Overload the drop_next field as an activity timeout */
if (!vars->count)
vars->drop_next = ktime_add_ns(now, p->interval);
- else if (ktime_to_ns(schedule) > 0 && reason == SKB_NOT_DROPPED_YET)
+ else if (ktime_to_ns(schedule) > 0 && reason == QDISC_DROP_UNSPEC)
vars->drop_next = now;
return reason;
@@ -2004,7 +2004,7 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch)
{
struct cake_sched_data *q = qdisc_priv(sch);
struct cake_tin_data *b = &q->tins[q->cur_tin];
- enum skb_drop_reason reason;
+ enum qdisc_drop_reason reason;
ktime_t now = ktime_get();
struct cake_flow *flow;
struct list_head *head;
@@ -2225,7 +2225,7 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch)
!!(q->config->rate_flags &
CAKE_FLAG_INGRESS)));
/* Last packet in queue may be marked, shouldn't be dropped */
- if (reason == SKB_NOT_DROPPED_YET || !flow->head)
+ if (reason == QDISC_DROP_UNSPEC || !flow->head)
break;
/* drop this packet, get another one */
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-04 17:02 [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jesper Dangaard Brouer
` (2 preceding siblings ...)
2026-02-04 17:03 ` [PATCH net-next RFC v1 3/3] net: sched: sch_cake: use enum qdisc_drop_reason for cobalt_should_drop Jesper Dangaard Brouer
@ 2026-02-05 3:04 ` Jakub Kicinski
2026-02-05 8:48 ` Jesper Dangaard Brouer
3 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2026-02-05 3:04 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Eric Dumazet, David S. Miller, Paolo Abeni,
Toke Høiland-Jørgensen, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team
On Wed, 04 Feb 2026 18:02:49 +0100 Jesper Dangaard Brouer wrote:
> RFC: Seeking feedback on this refactoring approach.
>
> This series refactors qdisc drop reason handling by introducing a dedicated
> enum qdisc_drop_reason and trace_qdisc_drop tracepoint, providing qdisc
> layer drop diagnostics with direct qdisc context visibility.
I like this!
It will presumably require existing users to migrate over
but sooner we do it the fewer users that have to move?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-05 3:04 ` [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jakub Kicinski
@ 2026-02-05 8:48 ` Jesper Dangaard Brouer
2026-02-05 11:34 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-05 8:48 UTC (permalink / raw)
To: Jakub Kicinski, Eric Dumazet, Toke Høiland-Jørgensen
Cc: netdev, David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team
On 05/02/2026 04.04, Jakub Kicinski wrote:
> On Wed, 04 Feb 2026 18:02:49 +0100 Jesper Dangaard Brouer wrote:
>> RFC: Seeking feedback on this refactoring approach.
>>
>> This series refactors qdisc drop reason handling by introducing a dedicated
>> enum qdisc_drop_reason and trace_qdisc_drop tracepoint, providing qdisc
>> layer drop diagnostics with direct qdisc context visibility.
>
> I like this!
Awesome, then I will continue in this direction :-)
> It will presumably require existing users to migrate over
> but sooner we do it the fewer users that have to move?
I agree. Existing users will loose some details, but the patchset
results in fallback to SKB_DROP_REASON_QDISC_DROP as a drop reason,
which existing tools/users will still handle.
(The more specific drop reason is now avail via trace_qdisc_drop).
Eric and Toke is this acceptable for your users?
In patch 1/3 I kept the qdisc "name" as part of the enum for FQ and
CAKE. IMHO this should be changed for next patchset, but I offer you the
chance to review and suggest generic naming (else I will choose).
--Jesper
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-05 8:48 ` Jesper Dangaard Brouer
@ 2026-02-05 11:34 ` Toke Høiland-Jørgensen
2026-02-05 14:15 ` Antoine Tenart
0 siblings, 1 reply; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-02-05 11:34 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jakub Kicinski, Eric Dumazet
Cc: netdev, David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team, Antoine Tenart
Jesper Dangaard Brouer <hawk@kernel.org> writes:
> On 05/02/2026 04.04, Jakub Kicinski wrote:
>> On Wed, 04 Feb 2026 18:02:49 +0100 Jesper Dangaard Brouer wrote:
>>> RFC: Seeking feedback on this refactoring approach.
>>>
>>> This series refactors qdisc drop reason handling by introducing a dedicated
>>> enum qdisc_drop_reason and trace_qdisc_drop tracepoint, providing qdisc
>>> layer drop diagnostics with direct qdisc context visibility.
>>
>> I like this!
>
> Awesome, then I will continue in this direction :-)
>
>> It will presumably require existing users to migrate over
>> but sooner we do it the fewer users that have to move?
>
> I agree. Existing users will loose some details, but the patchset
> results in fallback to SKB_DROP_REASON_QDISC_DROP as a drop reason,
> which existing tools/users will still handle.
> (The more specific drop reason is now avail via trace_qdisc_drop).
>
> Eric and Toke is this acceptable for your users?
I like it! My only concern is that drop monitor tools will see two
events for each qdisc drop. I guess that could be relatively
straight-forwardly deduplicated in the tool, though? +Antoine who works
on Retis to give him a chance to complain if not :)
> In patch 1/3 I kept the qdisc "name" as part of the enum for FQ and
> CAKE. IMHO this should be changed for next patchset, but I offer you the
> chance to review and suggest generic naming (else I will choose).
You mean you'd just name it something like QDISC_DROP_FLOOD_PROTECTION
instead of QDISC_DROP_CAKE_FLOOD? No strong objections to that...
-Toke
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-05 11:34 ` Toke Høiland-Jørgensen
@ 2026-02-05 14:15 ` Antoine Tenart
2026-02-05 14:46 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 13+ messages in thread
From: Antoine Tenart @ 2026-02-05 14:15 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, Jakub Kicinski, Eric Dumazet, netdev,
David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team, Antoine Tenart
On Thu, Feb 05, 2026 at 12:34:18PM +0100, Toke Høiland-Jørgensen wrote:
> Jesper Dangaard Brouer <hawk@kernel.org> writes:
> > On 05/02/2026 04.04, Jakub Kicinski wrote:
> >
> >> It will presumably require existing users to migrate over
> >> but sooner we do it the fewer users that have to move?
> >
> > I agree. Existing users will loose some details, but the patchset
> > results in fallback to SKB_DROP_REASON_QDISC_DROP as a drop reason,
> > which existing tools/users will still handle.
> > (The more specific drop reason is now avail via trace_qdisc_drop).
> >
> > Eric and Toke is this acceptable for your users?
>
> I like it! My only concern is that drop monitor tools will see two
> events for each qdisc drop. I guess that could be relatively
> straight-forwardly deduplicated in the tool, though? +Antoine who works
> on Retis to give him a chance to complain if not :)
Thanks for the heads up. We'll have to add support for the new enum
(pretty straightforward), other than that things should just work.
I'm wondering why not using the drop reason subsys logic to combine the
qdisc specific enum while not conflicting with drop reasons and allowing
to inject the qdisc reasons into drop reason enabled helpers as well?
Thanks!
Antoine
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-05 14:15 ` Antoine Tenart
@ 2026-02-05 14:46 ` Toke Høiland-Jørgensen
2026-02-05 16:24 ` Antoine Tenart
0 siblings, 1 reply; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-02-05 14:46 UTC (permalink / raw)
To: Antoine Tenart
Cc: Jesper Dangaard Brouer, Jakub Kicinski, Eric Dumazet, netdev,
David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team, Antoine Tenart
Antoine Tenart <atenart@redhat.com> writes:
> On Thu, Feb 05, 2026 at 12:34:18PM +0100, Toke Høiland-Jørgensen wrote:
>> Jesper Dangaard Brouer <hawk@kernel.org> writes:
>> > On 05/02/2026 04.04, Jakub Kicinski wrote:
>> >
>> >> It will presumably require existing users to migrate over
>> >> but sooner we do it the fewer users that have to move?
>> >
>> > I agree. Existing users will loose some details, but the patchset
>> > results in fallback to SKB_DROP_REASON_QDISC_DROP as a drop reason,
>> > which existing tools/users will still handle.
>> > (The more specific drop reason is now avail via trace_qdisc_drop).
>> >
>> > Eric and Toke is this acceptable for your users?
>>
>> I like it! My only concern is that drop monitor tools will see two
>> events for each qdisc drop. I guess that could be relatively
>> straight-forwardly deduplicated in the tool, though? +Antoine who works
>> on Retis to give him a chance to complain if not :)
>
> Thanks for the heads up. We'll have to add support for the new enum
> (pretty straightforward), other than that things should just work.
>
> I'm wondering why not using the drop reason subsys logic to combine the
> qdisc specific enum while not conflicting with drop reasons and allowing
> to inject the qdisc reasons into drop reason enabled helpers as well?
See this prior discussion for how the proposal came to be:
https://lore.kernel.org/all/6be17a08-f8aa-4f91-9bd0-d9e1f0a92d90@kernel.org/
Basically, figuring out which qdisc a drop came from is not possible
with the current drop_reason code.
So it's not just new drop reasons, it's a separate tracepoint with more
information available (qdisc handle, name, etc). So you'll get two
events for qdisc drops with this series (from patch 1):
+ trace_qdisc_drop(q, txq, dev, skb, reason);
+ kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_DROP);
-Toke
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-05 14:46 ` Toke Høiland-Jørgensen
@ 2026-02-05 16:24 ` Antoine Tenart
2026-02-06 14:41 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 13+ messages in thread
From: Antoine Tenart @ 2026-02-05 16:24 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, Jakub Kicinski, Eric Dumazet, netdev,
David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team
On Thu, Feb 05, 2026 at 03:46:29PM +0100, Toke Høiland-Jørgensen wrote:
> Antoine Tenart <atenart@redhat.com> writes:
>
> > I'm wondering why not using the drop reason subsys logic to combine the
> > qdisc specific enum while not conflicting with drop reasons and allowing
> > to inject the qdisc reasons into drop reason enabled helpers as well?
>
> See this prior discussion for how the proposal came to be:
>
> https://lore.kernel.org/all/6be17a08-f8aa-4f91-9bd0-d9e1f0a92d90@kernel.org/
>
> Basically, figuring out which qdisc a drop came from is not possible
> with the current drop_reason code.
>
> So it's not just new drop reasons, it's a separate tracepoint with more
> information available (qdisc handle, name, etc). So you'll get two
> events for qdisc drops with this series (from patch 1):
>
> + trace_qdisc_drop(q, txq, dev, skb, reason);
> + kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_DROP);
Right and that's fine. Using the drop reason subsys logic you could turn
the above into:
+ trace_qdisc_drop(q, txq, dev, skb, reason);
+ kfree_skb_reason(skb, (u32)reason);
You can look at enum skb_drop_reason_subsys and how that is used within
OvS (net/openvswitch/drop.h) for example.
Antoine
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-05 16:24 ` Antoine Tenart
@ 2026-02-06 14:41 ` Jesper Dangaard Brouer
2026-02-06 15:04 ` Antoine Tenart
0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-06 14:41 UTC (permalink / raw)
To: Antoine Tenart, Toke Høiland-Jørgensen
Cc: Jakub Kicinski, Eric Dumazet, netdev, David S. Miller,
Paolo Abeni, bpf, horms, jiri, edumazet, xiyou.wangcong, jhs,
carges, kernel-team
On 05/02/2026 17.24, Antoine Tenart wrote:
> On Thu, Feb 05, 2026 at 03:46:29PM +0100, Toke Høiland-Jørgensen wrote:
>> Antoine Tenart <atenart@redhat.com> writes:
>>
>>> I'm wondering why not using the drop reason subsys logic to combine the
>>> qdisc specific enum while not conflicting with drop reasons and allowing
>>> to inject the qdisc reasons into drop reason enabled helpers as well?
>>
>> See this prior discussion for how the proposal came to be:
>>
>> https://lore.kernel.org/all/6be17a08-f8aa-4f91-9bd0-d9e1f0a92d90@kernel.org/
>>
>> Basically, figuring out which qdisc a drop came from is not possible
>> with the current drop_reason code.
>>
>> So it's not just new drop reasons, it's a separate tracepoint with more
>> information available (qdisc handle, name, etc). So you'll get two
>> events for qdisc drops with this series (from patch 1):
>>
>> + trace_qdisc_drop(q, txq, dev, skb, reason);
>> + kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_DROP);
>
> Right and that's fine. Using the drop reason subsys logic you could turn
> the above into:
>
> + trace_qdisc_drop(q, txq, dev, skb, reason);
> + kfree_skb_reason(skb, (u32)reason);
>
> You can look at enum skb_drop_reason_subsys and how that is used within
> OvS (net/openvswitch/drop.h) for example.
>
Thanks for the input Antoine, I've adapted V2 to use skb_drop_reason_subsys.
Can you point me at the OVS userspace code that consume these subsys
drop_reason's?
--Jesper
[V2]
https://lore.kernel.org/all/177032644012.1975497.16411100029657607833.stgit@firesoul/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-06 14:41 ` Jesper Dangaard Brouer
@ 2026-02-06 15:04 ` Antoine Tenart
2026-02-06 15:21 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 13+ messages in thread
From: Antoine Tenart @ 2026-02-06 15:04 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Toke Høiland-Jørgensen, Jakub Kicinski, Eric Dumazet,
netdev, David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team
On Fri, Feb 06, 2026 at 03:41:23PM +0100, Jesper Dangaard Brouer wrote:
> On 05/02/2026 17.24, Antoine Tenart wrote:
>
> Can you point me at the OVS userspace code that consume these subsys
> drop_reason's?
I think this was mostly added for debugging utilities and not consumed
by OvS directly but I don't know the OvS userspace code tbf. Anything
you're looking for specifically?
Antoine
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint
2026-02-06 15:04 ` Antoine Tenart
@ 2026-02-06 15:21 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2026-02-06 15:21 UTC (permalink / raw)
To: Antoine Tenart
Cc: Toke Høiland-Jørgensen, Jakub Kicinski, Eric Dumazet,
netdev, David S. Miller, Paolo Abeni, bpf, horms, jiri, edumazet,
xiyou.wangcong, jhs, carges, kernel-team
On 06/02/2026 16.04, Antoine Tenart wrote:
> On Fri, Feb 06, 2026 at 03:41:23PM +0100, Jesper Dangaard Brouer wrote:
>> On 05/02/2026 17.24, Antoine Tenart wrote:
>>
>> Can you point me at the OVS userspace code that consume these subsys
>> drop_reason's?
>
> I think this was mostly added for debugging utilities and not consumed
> by OvS directly but I don't know the OvS userspace code tbf. Anything
> you're looking for specifically?
>
I'm just trying to making sure that I/we don't break any userspace code.
Yes, looks like debugging tools are the consumer. E.g. The Red Hat
NetObserv[1] tool mentions OVS decoding drop reason.
[1]
https://www.redhat.com/en/blog/network-observability-real-time-per-flow-packets-drop
--Jesper
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-02-06 15:21 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-04 17:02 [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jesper Dangaard Brouer
2026-02-04 17:02 ` [PATCH net-next RFC v1 1/3] net: sched: introduce qdisc-specific drop reason tracing Jesper Dangaard Brouer
2026-02-04 17:03 ` [PATCH net-next RFC v1 2/3] net: sched: sfq: convert to qdisc drop reasons Jesper Dangaard Brouer
2026-02-04 17:03 ` [PATCH net-next RFC v1 3/3] net: sched: sch_cake: use enum qdisc_drop_reason for cobalt_should_drop Jesper Dangaard Brouer
2026-02-05 3:04 ` [PATCH net-next RFC v1 0/3] net: sched: refactor qdisc drop reasons into dedicated tracepoint Jakub Kicinski
2026-02-05 8:48 ` Jesper Dangaard Brouer
2026-02-05 11:34 ` Toke Høiland-Jørgensen
2026-02-05 14:15 ` Antoine Tenart
2026-02-05 14:46 ` Toke Høiland-Jørgensen
2026-02-05 16:24 ` Antoine Tenart
2026-02-06 14:41 ` Jesper Dangaard Brouer
2026-02-06 15:04 ` Antoine Tenart
2026-02-06 15:21 ` Jesper Dangaard Brouer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox