From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH iproute2/net-next v2]tc: B.W limits can now be specified
 in %.
Date: Wed, 15 Nov 2017 09:58:56 -0800
Message-ID: <20171115095856.591ee0a9@xeon-e3>
References: <20171115013618.GA19575@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, doucette@bu.edu, michel.machado@gmail.com
To: Nishanth Devarajan <ndev2021@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf0-f195.google.com ([209.85.192.195]:54439 "EHLO
        mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S933313AbdKOR7A (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 15 Nov 2017 12:59:00 -0500
Received: by mail-pf0-f195.google.com with SMTP id n89so17546033pfk.11
        for <netdev@vger.kernel.org>; Wed, 15 Nov 2017 09:58:59 -0800 (PST)
In-Reply-To: <20171115013618.GA19575@gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Wed, 15 Nov 2017 07:06:21 +0530
Nishanth Devarajan <ndev2021@gmail.com> 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;
> +}