Re: [PATCH 2/2] net: netem: always adjust now/delay when not reordering

[Ferry Huberts <mailings@hupie.com>]

On 21/08/13 18:14, Ferry Huberts wrote:
> 
> 
> On 21/08/13 17:39, Eric Dumazet wrote:
>> On Wed, 2013-08-21 at 17:17 +0200, Johannes Naab wrote:
>>> On 08/20/2013 05:11 PM, Ferry Huberts wrote:
>>>> From: Ferry Huberts <ferry.huberts@pelagic.nl>
>>>>
>>>> Not doing this (current behaviour) introduces reordering.
>>>>
>>>> The packet_len_2_sched_time call is the only thing that logically
>>>> depends on q->rate, so move the now/delay adjustment out of the if.
>>>>
>>>> Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
>>>> ---
>>>
>>> Hi,
>>>
>>> The documentation for netem does explicitly mention the reordering with
>>> jitter, and gives instructions on how to avoid it. (I have not tested if
>>> it works as intended).
>>
>>
>> Yes.
>>
>> The user specifically adds a random delay of 0 to 510 ms to packets,
>> and expect netem to not reorder packets sent every 100ms.
>>
>> They see netem as a single medium between two endpoints with a guarantee
>> of no reordering, and cumulative delays.
> 
> Well no. We expected no reordering because reordering is not enabled.
> 
> The documentation is very confusing if you compare it to the source
> code, and even incorrect.
> 
> What the code does is (when reordering is disabled):
> - reorders if the rate is NOT set
> - does NOT reorder if the rate is set
> That is quite different, the documentation doesn't even mention the rate
> nor the reordering setting in this context.
> 
> I'm confused on how to proceed now, so CC-ing Hagen Paul Pfeifer
> 
> I'll also discuss this with Teco, who asked me to write up a patch.
> 

I discussed this issue with Teco and we decided to drop thes patches.

We still feel that the behaviour is unexpected and that (at least) the
documentation should be updated to reflect the actual behaviour.

We're going with the fifo approach for now.

So thanks for the feedback and discussions.
And apologies for any disturbances ;-)


Ferry