netdev - Re: [PATCH v3 iproute2 2/3] tc: Add support for the ETF Qdisc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7e0f803d-b44d-6a62-ceb4-6f3ca881297e@intel.com>
Date:   Fri, 6 Jul 2018 14:54:06 -0700
From:   Jesus Sanchez-Palencia <jesus.sanchez-palencia@...el.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
        jiri@...nulli.us, Vinicius Costa Gomes <vinicius.gomes@...el.com>
Subject: Re: [PATCH v3 iproute2 2/3] tc: Add support for the ETF Qdisc

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@...el.com>
>> + *		Jesus Sanchez-Palencia <jesus.sanchez-palencia@...el.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