From mboxrd@z Thu Jan 1 00:00:00 1970 From: mailings@hupie.com Subject: Re: [PATCH v2 2/2] net: netem: always adjust now/delay when not reordering Date: Wed, 21 Aug 2013 16:10:01 +0200 (CEST) Message-ID: <59093.80.101.237.101.1377094201.squirrel@hupie.dyndns.org> References: <1377030800.4226.89.camel@edumazet-glaptop> <1377064785-12629-1-git-send-email-mailings@hupie.com> <1377065647.4226.96.camel@edumazet-glaptop> <5214668E.504@hupie.com> <1377091824.4226.102.camel@edumazet-glaptop> <59065.80.101.237.101.1377093746.squirrel@hupie.dyndns.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT Cc: "Eric Dumazet" , "Ferry Huberts" , netdev@vger.kernel.org To: mailings@hupie.com Return-path: Received: from hupie.dyndns.org ([80.101.237.101]:55532 "EHLO hupie.dyndns.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751557Ab3HUOKD (ORCPT ); Wed, 21 Aug 2013 10:10:03 -0400 In-Reply-To: <59065.80.101.237.101.1377093746.squirrel@hupie.dyndns.org> Sender: netdev-owner@vger.kernel.org List-ID: >> On Wed, 2013-08-21 at 09:04 +0200, Ferry Huberts wrote: >>> On 21/08/13 08:14, Eric Dumazet wrote: >>> > On Wed, 2013-08-21 at 07:59 +0200, Ferry Huberts wrote: >>> >> From: Ferry Huberts >>> >> >>> >> 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. >>> >> >>> >> How to test: >>> >> ----------- >>> > >>> > I ask again : >>> > >>> > Did you test a config with both rate limiting and delay. >>> (sorry for missing that question) >>> Just did so and with rate limiting I get no reordering, which is > logical >>> looking at the code. >>> The thing is, the evaluation q->rate is within the 'no-reordering' > block >>> and in the current situation you can get reordering (with that > 'strange' >>> command). My patch makes sure that no reordering will occur, and > effectively 'clamps' the realised delay, which currently isn't done. >> >> >> OK, let me be very clear. >> >> I would like you post the results of regression tests, to make sure that > you do not add a new regression. >> > > Since I'm new to netdev development and therefore the processes attached > to it, let me ask where I can find such regression tests? > > I'm happy to run them once I know about them... ;-) > > >> It seems that you want _us_ to check all this for you. >> >> With "rate 1Mbits delay 100ms 10ms", and ping probes sent every 100ms, > the pong reply of _all_ probes should be between 90ms and 110ms >> >> Is it still the case after your patch ? >> > > Yes it is. > > In the script I described in the commit message I replaced the line > tc qdisc add dev "${fields[1]}" handle 1 root netem delay 10ms 500ms > with the line (1Mbit gave 'Illegal "rate"') > tc qdisc add dev "${fields[1]}" handle 1 root netem rate 1024000 delay > 100ms 10ms > > The output then was (tried a 10 times): > # ./netemreordering > PING 192.168.160.1 (192.168.160.1) 56(84) bytes of data. > 64 bytes from 192.168.160.1: icmp_seq=1 ttl=254 time=114 ms Wow. And of course Murphy would do this. This is the _only_ deviation there was and I copy&pasted it... I re-checked and it really is the only one. I re-ran 20 more times and all were ok. Must have been a busy router I guess. > 64 bytes from 192.168.160.1: icmp_seq=2 ttl=254 time=109 ms > 64 bytes from 192.168.160.1: icmp_seq=3 ttl=254 time=106 ms > 64 bytes from 192.168.160.1: icmp_seq=4 ttl=254 time=100 ms > 64 bytes from 192.168.160.1: icmp_seq=5 ttl=254 time=111 ms > 64 bytes from 192.168.160.1: icmp_seq=6 ttl=254 time=92.2 ms > 64 bytes from 192.168.160.1: icmp_seq=7 ttl=254 time=99.8 ms > 64 bytes from 192.168.160.1: icmp_seq=8 ttl=254 time=91.4 ms > 64 bytes from 192.168.160.1: icmp_seq=9 ttl=254 time=97.4 ms > 64 bytes from 192.168.160.1: icmp_seq=10 ttl=254 time=95.5 ms > > --- 192.168.160.1 ping statistics --- > 10 packets transmitted, 10 received, 0% packet loss, time 910ms > rtt min/avg/max/mdev = 91.413/101.879/114.534/7.723 ms, pipe 2 > > > Is this enough? > Please let me know, I'm happy to oblige. > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >