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

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

On 15/04/2025 17.43, David Ahern wrote:
> 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@kernel.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' ?
> 

Nope:

./scripts/checkpatch.pl 
patches-veth_pushback_to_qdisc03/01-0001-net-sched-generalize-noop.patch
total: 0 errors, 0 warnings, 30 lines checked

patches-veth_pushback_to_qdisc03/01-0001-net-sched-generalize-noop.patch 
has no obvious style problems and is ready for submission.

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

Thx for review :-)

--Jesper