netdev - Re: [PATCH net-next V4 1/2] net: sched: generalize check for no-queue qdisc on TX queue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f448fcb8-6330-4517-863f-4bf0a2242313@kernel.org>
Date: Tue, 15 Apr 2025 08:43:27 -0700
From: David Ahern <dsahern@...nel.org>
To: Jesper Dangaard Brouer <hawk@...nel.org>, netdev@...r.kernel.org,
 Jakub Kicinski <kuba@...nel.org>
Cc: bpf@...r.kernel.org, tom@...bertland.com,
 Eric Dumazet <eric.dumazet@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>,
 Toke Høiland-Jørgensen <toke@...e.dk>,
 makita.toshiaki@....ntt.co.jp, kernel-team@...udflare.com, phil@....cc
Subject: Re: [PATCH net-next V4 1/2] net: sched: generalize check for no-queue
 qdisc on TX queue

On 4/15/25 7:44 AM, Jesper Dangaard Brouer wrote:
> The "noqueue" qdisc can either be directly attached, or get default
> attached if net_device priv_flags has IFF_NO_QUEUE. In both cases, the
> allocated Qdisc structure gets it's enqueue function pointer reset to
> NULL by noqueue_init() via noqueue_qdisc_ops.
> 
> This is a common case for software virtual net_devices. For these devices
> with no-queue, the transmission path in __dev_queue_xmit() will bypass
> the qdisc layer. Directly invoking device drivers ndo_start_xmit (via
> dev_hard_start_xmit).  In this mode the device driver is not allowed to
> ask for packets to be queued (either via returning NETDEV_TX_BUSY or
> stopping the TXQ).
> 
> The simplest and most reliable way to identify this no-queue case is by
> checking if enqueue == NULL.
> 
> The vrf driver currently open-codes this check (!qdisc->enqueue). While
> functionally correct, this low-level detail is better encapsulated in a
> dedicated helper for clarity and long-term maintainability.
> 
> To make this behavior more explicit and reusable, this patch introduce a
> new helper: qdisc_txq_has_no_queue(). Helper will also be used by the
> veth driver in the next patch, which introduces optional qdisc-based
> backpressure.
> 
> This is a non-functional change.
> 
> Signed-off-by: Jesper Dangaard Brouer <hawk@...nel.org>
> ---
>  drivers/net/vrf.c         |    4 +---
>  include/net/sch_generic.h |    8 ++++++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 


>  /* 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..b6c177f7141c 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -803,6 +803,14 @@ static inline bool qdisc_tx_changing(const struct net_device *dev)
>  	return false;
>  }
>  
> +/* "noqueue" qdisc identified by not having any enqueue, see noqueue_init() */
> +static inline bool qdisc_txq_has_no_queue(const struct netdev_queue *txq)
> +{
> +	struct Qdisc *qdisc = rcu_access_pointer(txq->qdisc);
> +
> +	return qdisc->enqueue == NULL;

Did checkpatch not complain that this should be '!qdisc->enqueue' ?


Reviewed-by: David Ahern <dsahern@...nel.org>