From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
Subject: Re: [PATCH v3 iproute2 2/3] tc: Add support for the ETF Qdisc
Date: Fri, 6 Jul 2018 14:54:06 -0700
Message-ID: <7e0f803d-b44d-6a62-ceb4-6f3ca881297e@intel.com>
References: <20180705224227.22843-1-jesus.sanchez-palencia@intel.com>
 <20180705224227.22843-3-jesus.sanchez-palencia@intel.com>
 <20180706143237.60807e16@xeon-e3>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, jhs@mojatatu.com, xiyou.wangcong@gmail.com,
        jiri@resnulli.us, Vinicius Costa Gomes <vinicius.gomes@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mga11.intel.com ([192.55.52.93]:24709 "EHLO mga11.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1754215AbeGFV7J (ORCPT <rfc822;netdev@vger.kernel.org>);
        Fri, 6 Jul 2018 17:59:09 -0400
In-Reply-To: <20180706143237.60807e16@xeon-e3>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Stephen,


On 07/06/2018 02:32 PM, Stephen Hemminger wrote:
> 
>> diff --git a/tc/q_etf.c b/tc/q_etf.c
>> new file mode 100644
>> index 00000000..5db1dd6f
>> --- /dev/null
>> +++ b/tc/q_etf.c
>> @@ -0,0 +1,168 @@
>> +/*
>> + * q_etf.c		Earliest TxTime First (ETF).
>> + *
>> + *		This program is free software; you can redistribute it and/or
>> + *		modify it under the terms of the GNU General Public License
>> + *		as published by the Free Software Foundation; either version
>> + *		2 of the License, or (at your option) any later version.
>> + *
>> + * Authors:	Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> + *		Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
>> + *
>> + */
> 
> 
> Please use SPDX tag rather than GPL boilerplate when adding new code.

Sure, will do for v4.

> 
>> +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) {
>> +			*val = c->clockid;
>> +
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	return -1;
>> +}
> 
> Internally, kernel must use ktime. For the userspace part the TC standard
> is to use USER HZ of 100.
> 
> Please change user kernel API of this module to match other existing modules.
> Doing something unique for this module is not necessary.


I don't follow you on the above, sorry.

The qdisc must be configured with a valid clockid_t. This type is used by both
userspace and kernel.

As requested before, we made the tc etf command line interface more
user-friendly and allowed for users to input the clock name as a string. The
code above is just lookup table converting the string into a a valid clockid_t
so we can then pass it to the kernel.

There is no ktime or any other timestamp type above, only clockid_t.

Can you please clarify what your request is?


Thanks,
Jesus