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

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] [day] [month] [year] [list]

Date:	Fri, 23 Aug 2013 14:50:27 +0200
From:	Ferry Huberts <mailings@...ie.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Johannes Naab <jn@...sta.de>, netdev@...r.kernel.org,
	hagen@...u.net
Subject: Re: [PATCH 2/2] net: netem: always adjust now/delay when not reordering



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@...agic.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@...agic.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html