From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH v3 iproute2 2/3] tc: Add support for the ETF Qdisc
Date: Fri, 6 Jul 2018 09:58:08 -0600
Message-ID: <b2fda267-bc71-4280-9622-f2c949faab56@gmail.com>
References: <20180705224227.22843-1-jesus.sanchez-palencia@intel.com>
 <20180705224227.22843-3-jesus.sanchez-palencia@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
        Vinicius Costa Gomes <vinicius.gomes@intel.com>
To: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>,
        netdev@vger.kernel.org
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf0-f196.google.com ([209.85.192.196]:44880 "EHLO
        mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S932851AbeGFP5g (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 6 Jul 2018 11:57:36 -0400
Received: by mail-pf0-f196.google.com with SMTP id j3-v6so8777868pfh.11
        for <netdev@vger.kernel.org>; Fri, 06 Jul 2018 08:57:36 -0700 (PDT)
In-Reply-To: <20180705224227.22843-3-jesus.sanchez-palencia@intel.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 7/5/18 4:42 PM, Jesus Sanchez-Palencia wrote:

> +static int get_clockid(__s32 *val, const char *arg)
> +{
> +	const struct static_clockid {
> +		const char *name;
> +		clockid_t clockid;
> +	} clockids_sysv[] = {
> +		{ "CLOCK_REALTIME", CLOCK_REALTIME },
> +		{ "CLOCK_TAI", CLOCK_TAI },
> +		{ "CLOCK_BOOTTIME", CLOCK_BOOTTIME },
> +		{ "CLOCK_MONOTONIC", CLOCK_MONOTONIC },
> +		{ NULL }
> +	};
> +
> +	const struct static_clockid *c;
> +
> +	for (c = clockids_sysv; c->name; c++) {
> +		if (strncasecmp(c->name, arg, 25) == 0) {

Why 25?

be nice to allow shortcuts -- e.g., just REALTIME or realtime.

> +			*val = c->clockid;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -1;
> +}
> +
> +
> +static int etf_parse_opt(struct qdisc_util *qu, int argc,
> +			 char **argv, struct nlmsghdr *n, const char *dev)
> +{
> +	struct tc_etf_qopt opt = {
> +		.clockid = CLOCKID_INVALID,
> +	};
> +	struct rtattr *tail;
> +
> +	while (argc > 0) {
> +		if (matches(*argv, "offload") == 0) {
> +			if (opt.flags & TC_ETF_OFFLOAD_ON) {
> +				fprintf(stderr, "etf: duplicate \"offload\" specification\n");
> +				return -1;
> +			}
> +
> +			opt.flags |= TC_ETF_OFFLOAD_ON;
> +		} else if (matches(*argv, "deadline_mode") == 0) {
> +			if (opt.flags & TC_ETF_DEADLINE_MODE_ON) {
> +				fprintf(stderr, "etf: duplicate \"deadline_mode\" specification\n");
> +				return -1;
> +			}
> +
> +			opt.flags |= TC_ETF_DEADLINE_MODE_ON;
> +		} else if (matches(*argv, "delta") == 0) {
> +			NEXT_ARG();
> +			if (opt.delta) {
> +				fprintf(stderr, "etf: duplicate \"delta\" specification\n");
> +				return -1;
> +			}
> +			if (get_s32(&opt.delta, *argv, 0)) {
> +				explain1("delta", *argv);
> +				return -1;
> +			}
> +		} else if (matches(*argv, "clockid") == 0) {
> +			NEXT_ARG();
> +			if (opt.clockid != CLOCKID_INVALID) {
> +				fprintf(stderr, "etf: duplicate \"clockid\" specification\n");
> +				return -1;
> +			}
> +			if (get_clockid(&opt.clockid, *argv)) {
> +				explain_clockid(*argv);
> +				return -1;
> +			}
> +		} else if (strcmp(*argv, "help") == 0) {
> +			explain();
> +			return -1;
> +		} else {
> +			fprintf(stderr, "etf: unknown parameter \"%s\"\n", *argv);
> +			explain();
> +			return -1;
> +		}
> +		argc--; argv++;
> +	}
> +
> +	tail = NLMSG_TAIL(n);
> +	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
> +	addattr_l(n, 2024, TCA_ETF_PARMS, &opt, sizeof(opt));
> +	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
> +	return 0;
> +}
> +
> +static int etf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> +{
> +	struct rtattr *tb[TCA_ETF_MAX+1];
> +	struct tc_etf_qopt *qopt;
> +
> +	if (opt == NULL)
> +		return 0;
> +
> +	parse_rtattr_nested(tb, TCA_ETF_MAX, opt);
> +
> +	if (tb[TCA_ETF_PARMS] == NULL)
> +		return -1;
> +
> +	qopt = RTA_DATA(tb[TCA_ETF_PARMS]);
> +	if (RTA_PAYLOAD(tb[TCA_ETF_PARMS])  < sizeof(*qopt))
> +		return -1;
> +
> +	if (qopt->clockid == CLOCKID_INVALID)
> +		print_string(PRINT_ANY, "clockid", "clockid %s ", "invalid");
> +	else
> +		print_uint(PRINT_ANY, "clockid", "clockid %d ", qopt->clockid);

If you allow the user to input a string, then you should return it here too.