netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2] tc: util: constrain percentage in 0-100 interval
@ 2019-07-13  9:44 Andrea Claudi
  2019-07-15 20:48 ` Stephen Hemminger
  0 siblings, 1 reply; 2+ messages in thread
From: Andrea Claudi @ 2019-07-13  9:44 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern

parse_percent() currently allows to specify negative percentages
or value above 100%. However this does not seems to make sense,
as the function is used for probabilities or bandiwidth rates.

Moreover, using negative values leads to erroneous results
(using Bernoulli loss model as example):

$ ip link add test type dummy
$ ip link set test up
$ tc qdisc add dev test root netem loss gemodel -10% limit 10
$ tc qdisc show dev test
qdisc netem 800c: root refcnt 2 limit 10 loss gemodel p 90% r 10% 1-h 100% 1-k 0%

Using values above 100% we have instead:

$ ip link add test type dummy
$ ip link set test up
$ tc qdisc add dev test root netem loss gemodel 140% limit 10
$ tc qdisc show dev test
qdisc netem 800f: root refcnt 2 limit 10 loss gemodel p 40% r 60% 1-h 100% 1-k 0%

This commit changes parse_percent() with a check to ensure
percentage values stay between 1.0 and 0.0.
parse_percent_rate() function, which already employs a similar
check, is adjusted accordingly.

With this check in place, we have:

$ ip link add test type dummy
$ ip link set test up
$ tc qdisc add dev test root netem loss gemodel -10% limit 10
Illegal "loss gemodel p"

Fixes: 927e3cfb52b58 ("tc: B.W limits can now be specified in %.")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 tc/tc_util.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 53d15e08e9734..b90d256c33a4a 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -198,7 +198,7 @@ int parse_percent(double *val, const char *str)
 	char *p;
 
 	*val = strtod(str, &p) / 100.;
-	if (*val == HUGE_VALF || *val == HUGE_VALL)
+	if (*val > 1.0 || *val < 0.0)
 		return 1;
 	if (*p && strcmp(p, "%"))
 		return -1;
@@ -226,16 +226,16 @@ static int parse_percent_rate(char *rate, size_t len,
 	if (ret != 1)
 		goto malf;
 
-	if (parse_percent(&perc, str_perc))
+	ret = parse_percent(&perc, str_perc);
+	if (ret == 1) {
+		fprintf(stderr, "Invalid rate specified; should be between [0,100]%% but is %s\n", str);
+		goto err;
+	} else if (ret == -1) {
 		goto malf;
+	}
 
 	free(str_perc);
 
-	if (perc > 1.0 || perc < 0.0) {
-		fprintf(stderr, "Invalid rate specified; should be between [0,100]%% but is %s\n", str);
-		return -1;
-	}
-
 	rate_bit = perc * dev_mbit * 1000 * 1000;
 
 	ret = snprintf(rate, len, "%lf", rate_bit);
@@ -247,8 +247,9 @@ static int parse_percent_rate(char *rate, size_t len,
 	return 0;
 
 malf:
-	free(str_perc);
 	fprintf(stderr, "Specified rate value could not be read or is malformed\n");
+err:
+	free(str_perc);
 	return -1;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH iproute2] tc: util: constrain percentage in 0-100 interval
  2019-07-13  9:44 [PATCH iproute2] tc: util: constrain percentage in 0-100 interval Andrea Claudi
@ 2019-07-15 20:48 ` Stephen Hemminger
  0 siblings, 0 replies; 2+ messages in thread
From: Stephen Hemminger @ 2019-07-15 20:48 UTC (permalink / raw)
  To: Andrea Claudi; +Cc: netdev, dsahern

On Sat, 13 Jul 2019 11:44:07 +0200
Andrea Claudi <aclaudi@redhat.com> wrote:

> parse_percent() currently allows to specify negative percentages
> or value above 100%. However this does not seems to make sense,
> as the function is used for probabilities or bandiwidth rates.
> 
> Moreover, using negative values leads to erroneous results
> (using Bernoulli loss model as example):
> 
> $ ip link add test type dummy
> $ ip link set test up
> $ tc qdisc add dev test root netem loss gemodel -10% limit 10
> $ tc qdisc show dev test
> qdisc netem 800c: root refcnt 2 limit 10 loss gemodel p 90% r 10% 1-h 100% 1-k 0%
> 
> Using values above 100% we have instead:
> 
> $ ip link add test type dummy
> $ ip link set test up
> $ tc qdisc add dev test root netem loss gemodel 140% limit 10
> $ tc qdisc show dev test
> qdisc netem 800f: root refcnt 2 limit 10 loss gemodel p 40% r 60% 1-h 100% 1-k 0%
> 
> This commit changes parse_percent() with a check to ensure
> percentage values stay between 1.0 and 0.0.
> parse_percent_rate() function, which already employs a similar
> check, is adjusted accordingly.
> 
> With this check in place, we have:
> 
> $ ip link add test type dummy
> $ ip link set test up
> $ tc qdisc add dev test root netem loss gemodel -10% limit 10
> Illegal "loss gemodel p"
> 
> Fixes: 927e3cfb52b58 ("tc: B.W limits can now be specified in %.")
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>

Looks good. Applied

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-07-15 20:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-13  9:44 [PATCH iproute2] tc: util: constrain percentage in 0-100 interval Andrea Claudi
2019-07-15 20:48 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).