[PATCH] iproute2: Avoid rounding errors for 100%.

[PATCH] iproute2: Avoid rounding errors for 100%.

[hannemann]

From: Arnd Hannemann <hannemann@nets.rwth-aachen.de>

We noticed that a netem reorder percentage of 100% will still get packets reordered.
This patch fixes that.

Signed-off-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
---
 tc/tc_util.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index fe2c7eb..2641f2e 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -363,8 +363,10 @@ int get_percent(__u32 *percent, const char *str)
 		return -1;
 	if (*p && strcmp(p, "%"))
 		return -1;
-
-	*percent = (unsigned) rint(per * max_percent_value);
+	if (per == 1.)
+		*percent = max_percent_value;
+	else
+		*percent = (unsigned) rint(per * max_percent_value);
 	return 0;
 }
 
-- 
1.6.3.3

Re: [PATCH] iproute2: Avoid rounding errors for 100%.

[Arnd Hannemann]

Although the patch makes sense,
it does not fix the bug/effect we were seeing.
A netem reorder percentage of 100% will still get packets reordered.
(if the netem queue is not empty)


hannemann@nets.rwth-aachen.de wrote:
> From: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> 
> We noticed that a netem reorder percentage of 100% will still get packets reordered.
> This patch fixes that.
> 
> Signed-off-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> ---
>  tc/tc_util.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index fe2c7eb..2641f2e 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -363,8 +363,10 @@ int get_percent(__u32 *percent, const char *str)
>  		return -1;
>  	if (*p && strcmp(p, "%"))
>  		return -1;
> -
> -	*percent = (unsigned) rint(per * max_percent_value);
> +	if (per == 1.)
> +		*percent = max_percent_value;
> +	else
> +		*percent = (unsigned) rint(per * max_percent_value);
>  	return 0;
>  }
>  


-- 
Dipl.-Inform. Arnd Hannemann
RWTH Aachen University
Dept. of Computer Science, Informatik 4
Ahornstr. 55, D-52074 Aachen, Germany
Phone: (+49 241) 80-21423 Fax: (+49 241) 80-22220

Re: [PATCH] iproute2: Avoid rounding errors for 100%.

[Stephen Hemminger]

On Tue, 03 Nov 2009 17:19:45 +0100
Arnd Hannemann <hannemann@nets.rwth-aachen.de> wrote:

> Although the patch makes sense,
> it does not fix the bug/effect we were seeing.
> A netem reorder percentage of 100% will still get packets reordered.
> (if the netem queue is not empty)
> 
> 
> hannemann@nets.rwth-aachen.de wrote:
> > From: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> > 
> > We noticed that a netem reorder percentage of 100% will still get packets reordered.
> > This patch fixes that.
> > 
> > Signed-off-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> > ---
> >  tc/tc_util.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tc/tc_util.c b/tc/tc_util.c
> > index fe2c7eb..2641f2e 100644
> > --- a/tc/tc_util.c
> > +++ b/tc/tc_util.c
> > @@ -363,8 +363,10 @@ int get_percent(__u32 *percent, const char *str)
> >  		return -1;
> >  	if (*p && strcmp(p, "%"))
> >  		return -1;
> > -
> > -	*percent = (unsigned) rint(per * max_percent_value);
> > +	if (per == 1.)
> > +		*percent = max_percent_value;
> > +	else
> > +		*percent = (unsigned) rint(per * max_percent_value);
> >  	return 0;
> >  }
> >  
> 
> 

If you don't want reordering, don't specify reordering?


-- 

Re: [PATCH] iproute2: Avoid rounding errors for 100%.

[Arnd Hannemann]

Stephen Hemminger wrote:
> On Tue, 03 Nov 2009 17:19:45 +0100
> Arnd Hannemann <hannemann@nets.rwth-aachen.de> wrote:
> 
>> Although the patch makes sense,
>> it does not fix the bug/effect we were seeing.
>> A netem reorder percentage of 100% will still get packets reordered.
>> (if the netem queue is not empty)
>>
>>
>> hannemann@nets.rwth-aachen.de wrote:
>>> From: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
>>>
>>> We noticed that a netem reorder percentage of 100% will still get packets reordered.
>>> This patch fixes that.
>>>
>>> Signed-off-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
>>> ---
>>>  tc/tc_util.c |    6 ++++--
>>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>>> index fe2c7eb..2641f2e 100644
>>> --- a/tc/tc_util.c
>>> +++ b/tc/tc_util.c
>>> @@ -363,8 +363,10 @@ int get_percent(__u32 *percent, const char *str)
>>>  		return -1;
>>>  	if (*p && strcmp(p, "%"))
>>>  		return -1;
>>> -
>>> -	*percent = (unsigned) rint(per * max_percent_value);
>>> +	if (per == 1.)
>>> +		*percent = max_percent_value;
>>> +	else
>>> +		*percent = (unsigned) rint(per * max_percent_value);
>>>  	return 0;
>>>  }
>>>  
>>
> 
> If you don't want reordering, don't specify reordering?

Are you arguing against the correctness of my patch?

Regarding, the reordering thingy/bug/effect whatever:
We would like to specify reordering with something like this:
Please delay 1% of the packets with 5ms.

We thought that would be possible with
tc qdisc add dev eth0 root netem delay 5m reorder 99%

But unfortunately it is not. Any better idea?

Best regards,
Arnd

Re: [PATCH] iproute2: Avoid rounding errors for 100%.

[Stephen Hemminger]

On Tue, 03 Nov 2009 18:19:19 +0100
Arnd Hannemann <hannemann@nets.rwth-aachen.de> wrote:

> Stephen Hemminger wrote:
> > On Tue, 03 Nov 2009 17:19:45 +0100
> > Arnd Hannemann <hannemann@nets.rwth-aachen.de> wrote:
> > 
> >> Although the patch makes sense,
> >> it does not fix the bug/effect we were seeing.
> >> A netem reorder percentage of 100% will still get packets reordered.
> >> (if the netem queue is not empty)
> >>
> >>
> >> hannemann@nets.rwth-aachen.de wrote:
> >>> From: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> >>>
> >>> We noticed that a netem reorder percentage of 100% will still get packets reordered.
> >>> This patch fixes that.
> >>>
> >>> Signed-off-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> >>> ---
> >>>  tc/tc_util.c |    6 ++++--
> >>>  1 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/tc/tc_util.c b/tc/tc_util.c
> >>> index fe2c7eb..2641f2e 100644
> >>> --- a/tc/tc_util.c
> >>> +++ b/tc/tc_util.c
> >>> @@ -363,8 +363,10 @@ int get_percent(__u32 *percent, const char *str)
> >>>  		return -1;
> >>>  	if (*p && strcmp(p, "%"))
> >>>  		return -1;
> >>> -
> >>> -	*percent = (unsigned) rint(per * max_percent_value);
> >>> +	if (per == 1.)
> >>> +		*percent = max_percent_value;
> >>> +	else
> >>> +		*percent = (unsigned) rint(per * max_percent_value);
> >>>  	return 0;
> >>>  }
> >>>  
> >>
> > 
> > If you don't want reordering, don't specify reordering?
> 
> Are you arguing against the correctness of my patch?
> 
> Regarding, the reordering thingy/bug/effect whatever:
> We would like to specify reordering with something like this:
> Please delay 1% of the packets with 5ms.
> 
> We thought that would be possible with
> tc qdisc add dev eth0 root netem delay 5m reorder 99%
> 
> But unfortunately it is not. Any better idea?
> 

I think the problem isn't in iproute utilities but in the code inside
netem kernel module. 

--