pkt_sched: Fix qdisc len in qdisc_peek_dequeued() [61c9eaf9008] - question

pkt_sched: Fix qdisc len in qdisc_peek_dequeued() [61c9eaf9008] - question

[Michal Soltys]

Hi,

I was wondering (probably missing some subtleties) about that particular 
patch, namely:

61c9eaf90081cbe6dc4f389e0056bff76eca19ec

Why would that qlen++ change be necessary ? As far as peeked qdisc sees 
things, the packet is already deqeued and gone - so not really part of 
the queue anymore in this context (not ever requeued either). More 
advanced qdiscs such as say fq_codel - if for example they decide to 
drop head during further enqueue operation - obviously won't even 
consider the peeked packet.

Increasing qlen from what I can see artificially shortens queue by 1. 
For some classful schedulers (say like hfsc that instantly peeks next 
packet length after dequeuing), child qdiscs will be almost all the time 
formally operating at queue size decreased by 1.

Re: pkt_sched: Fix qdisc len in qdisc_peek_dequeued() [61c9eaf9008] - question

[Eric Dumazet]

On Sun, 2014-11-16 at 14:04 +0100, Michal Soltys wrote:
> Hi,
> 
> I was wondering (probably missing some subtleties) about that particular 
> patch, namely:
> 
> 61c9eaf90081cbe6dc4f389e0056bff76eca19ec
> 
> Why would that qlen++ change be necessary ? As far as peeked qdisc sees 
> things, the packet is already deqeued and gone - so not really part of 
> the queue anymore in this context (not ever requeued either). More 
> advanced qdiscs such as say fq_codel - if for example they decide to 
> drop head during further enqueue operation - obviously won't even 
> consider the peeked packet.

peeked packet has not to be considered by qdisc : Their dequeue()
methods is called only when there is no more peeked packet.

> 
> Increasing qlen from what I can see artificially shortens queue by 1. 
> For some classful schedulers (say like hfsc that instantly peeks next 
> packet length after dequeuing), child qdiscs will be almost all the time 
> formally operating at queue size decreased by 1.

Well, the peeked packet is part of the queue, as it is not yet
transmitted.

Re: pkt_sched: Fix qdisc len in qdisc_peek_dequeued() [61c9eaf9008] - question

[Michal Soltys]

Actually, now that I analyzed it more carefully - not having that patch 
would cause obvious issues (e.g. hfsc trying to activate already active 
leaf in certain scenarios).

So, sorry for the noise.

PS.
Removed Jarek from CC, as that email is no longer valid.