netdev - Re: [PATCH net-next v3 2/3] ethtool: Add generic options for tunables

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: <alpine.LNX.2.03.1408270247280.28873@ws.cisco>
Date:	Wed, 27 Aug 2014 03:03:23 +0530 (IST)
From:	Govindarajulu Varadarajan <_govind@....com>
To:	Ben Hutchings <ben@...adent.org.uk>
cc:	Govindarajulu Varadarajan <_govind@....com>, davem@...emloft.net,
	netdev@...r.kernel.org, stephen@...workplumber.org,
	ssujith@...co.com, benve@...co.com
Subject: Re: [PATCH net-next v3 2/3] ethtool: Add generic options for
 tunables

On Fri, 22 Aug 2014, Ben Hutchings wrote:
> On Thu, 2014-08-14 at 14:59 +0530, Govindarajulu Varadarajan wrote:
>> @@ -257,6 +262,7 @@ struct ethtool_ops {
>>  				     struct ethtool_eeprom *, u8 *);
>>  	int	(*get_eee)(struct net_device *, struct ethtool_eee *);
>>  	int	(*set_eee)(struct net_device *, struct ethtool_eee *);
>> +	struct ethtool_tunable_ops tunable_ops[ETHTOOL_TUNABLE_MAX];
>
> This is OK but if we add a lot of tunables then it bloats up each driver
> with a (probably quite sparse) array of function pointers.
>

Will fix with David's suggestion.

>> +struct ethtool_tunable {
>> +	u32 cmd;
>> +	u32 tcmd;
>> +	u32 len;
>> +	union {
>> +		u32 rx_copybreak;
>> +		u32 data[48];
>> +	} data;
> [...]
>
> This is not at all generic.  If tunables don't all have the same type,
> then the type - not just the length - should be explicit.
>
> I also think that if the length of a value can vary then we should not
> declare a data member at all.  So we would have something like:
>
> struct ethtool_tunable {
> 	__u32	cmd;
> 	__u32	id;
> 	__u32	type_id;
> 	__u32	len;
> 	__u8	data[0];
> };
>
> Then the value buffer would be passed to the driver functions separately
> (as for other variable-length command structures).
>

OK. id show be the tunable command type right? Like RX_COPYBREAK, TX_COPYBREAK
etc. What is the type_id?

> By the way, you must use double-underscore prefixes on fixed-width
> integer type names in UAPI headers.
>

will fix it.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html