From: Jesper Dangaard Brouer <hawk@kernel.org>
To: netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>
Cc: "Jesper Dangaard Brouer" <hawk@kernel.org>,
bpf@vger.kernel.org, tom@herbertland.com,
"Eric Dumazet" <eric.dumazet@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"Paolo Abeni" <pabeni@redhat.com>,
"Toke Høiland-Jørgensen" <toke@toke.dk>,
dsahern@kernel.org, makita.toshiaki@lab.ntt.co.jp,
kernel-team@cloudflare.com
Subject: [PATCH net-next RFC V3 1/2] net: sched: generalize check for no-op qdisc on TX queue
Date: Mon, 14 Apr 2025 17:45:50 +0200 [thread overview]
Message-ID: <174464555063.20396.9545196538212416415.stgit@firesoul> (raw)
In-Reply-To: <174464549885.20396.6987653753122223942.stgit@firesoul>
WARNING: Testing show this is NOT correct!
- I need help figuring out why this patch is incorrect.
- Testing against &noop_qdisc is not the same as q->enqueue == NULL
- I copied test (txq->qdisc == &noop_qdisc) from qdisc_tx_is_noop
- Q: is qdisc_tx_is_noop() function incorrect?
The vrf driver includes an open-coded check to determine whether a TX
queue (netdev_queue) has a real qdisc attached. This is done by testing
whether qdisc->enqueue is NULL, which is functionally equivalent to
checking whether the qdisc is &noop_qdisc.
This equivalence stems from noqueue_init(), which explicitly clears the
enqueue pointer to signal no-op behavior to __dev_queue_xmit().
This patch introduces a shared helper, qdisc_txq_is_noop(), to clarify
intent and make this logic reusable. The vrf driver is updated to use
this new helper.
Subsequent patches will make further use of this helper in other drivers,
such as veth.
This is a non-functional change.
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
---
drivers/net/vrf.c | 4 +---
include/net/sch_generic.h | 7 ++++++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 7168b33adadb..f0a24fc85945 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -343,15 +343,13 @@ static int vrf_ifindex_lookup_by_table_id(struct net *net, u32 table_id)
static bool qdisc_tx_is_default(const struct net_device *dev)
{
struct netdev_queue *txq;
- struct Qdisc *qdisc;
if (dev->num_tx_queues > 1)
return false;
txq = netdev_get_tx_queue(dev, 0);
- qdisc = rcu_access_pointer(txq->qdisc);
- return !qdisc->enqueue;
+ return qdisc_txq_is_noop(txq);
}
/* Local traffic destined to local address. Reinsert the packet to rx
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index d48c657191cd..a1f5560350a6 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -803,6 +803,11 @@ static inline bool qdisc_tx_changing(const struct net_device *dev)
return false;
}
+static inline bool qdisc_txq_is_noop(const struct netdev_queue *txq)
+{
+ return rcu_access_pointer(txq->qdisc) == &noop_qdisc;
+}
+
/* Is the device using the noop qdisc on all queues? */
static inline bool qdisc_tx_is_noop(const struct net_device *dev)
{
@@ -810,7 +815,7 @@ static inline bool qdisc_tx_is_noop(const struct net_device *dev)
for (i = 0; i < dev->num_tx_queues; i++) {
struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
- if (rcu_access_pointer(txq->qdisc) != &noop_qdisc)
+ if (!qdisc_txq_is_noop(txq))
return false;
}
return true;
next prev parent reply other threads:[~2025-04-14 15:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-14 15:45 [PATCH net-next RFC V3 0/2] veth: qdisc backpressure and qdisc check refactor Jesper Dangaard Brouer
2025-04-14 15:45 ` Jesper Dangaard Brouer [this message]
2025-04-14 15:45 ` [PATCH net-next RFC V3 2/2] veth: apply qdisc backpressure on full ptr_ring to reduce TX drops Jesper Dangaard Brouer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=174464555063.20396.9545196538212416415.stgit@firesoul \
--to=hawk@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=eric.dumazet@gmail.com \
--cc=kernel-team@cloudflare.com \
--cc=kuba@kernel.org \
--cc=makita.toshiaki@lab.ntt.co.jp \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=toke@toke.dk \
--cc=tom@herbertland.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox