Subject: Re: [PATCH iproute2/net-next v2]tc: B.W limits can now be specified in %.

From: Stephen Hemminger <[email-masked]>
Date: Wed, 15 Nov 2017 09:58:56 -0800

On Wed, 15 Nov 2017 07:06:21 +0530
Nishanth Devarajan <[email-masked]> wrote:

> int parse_percent_rate(char *rate, const char *str, char *dev)
You aren't modifyin dev so it should be const char *
> +{
> +	long max_rate_bits;
> +	int ret, saved_errno;
> +	double perc, rate_bits;
> +	char *str_perc, *p;
> +
> +	if (!dev[0]) {
> +		fprintf(stderr, "No device specified; specify device to rate limit by percentage\n");
> +		return -1;
> +	}
> +
> +	if (read_prop(dev, "speed", &max_rate_bits))
> +		return -1;

If speed is unknown, then many will devices will return -1.
You need to handle that.

Since speed reported by kernel is in mbits per second, it
would make sense to rename max_rate_bits to mbit.

> +
> +	ret = sscanf(str, "%m[0-9.%]", &str_perc);
> +	if (ret != 1)
> +		goto malf;
> +
> +	/* Make sure there's only one percent sign and it's at the end */
> +	perc = strtod(str_perc, &p);
> +	if (*p != '%' || *(p++) != '\0')
> +		goto malf;

There already is parse_percent in tc/q_netem.c.
Please move that to util and use that instead of coding here.

> +
> +	saved_errno = errno;
> +	free(str_perc);
> +
> +	if (perc > 100.0 || perc < 0.0 || saved_errno == ERANGE) {
> +		fprintf(stderr, "Invalid rate specified; should be between [0,100]%% but is %s\n", str);
> +		return -1;
> +	}
> +
> +	rate_bits = (perc * max_rate_bits) / 100.0;
> +
> +	ret = snprintf(rate, 20, "%lf", rate_bits);
> +	if (ret <= 0 || ret >= 20) {
> +		fprintf(stderr, "Unable to parse calculated rate\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +
> +malf:
> +	fprintf(stderr, "Specified rate value could not be read or is malformed\n");
> +	return -1;
> +}