netdev - RE: [ethtool] ethtool: check mac type from ethtool

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]

Date:	Tue, 30 Apr 2013 18:12:22 +0000
From:	"Tantilov, Emil S" <emil.s.tantilov@...el.com>
To:	Ben Hutchings <bhutchings@...arflare.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [ethtool] ethtool: check mac type from ethtool_regs.version

>-----Original Message-----
>From: Ben Hutchings [mailto:bhutchings@...arflare.com]
>Sent: Tuesday, April 30, 2013 8:24 AM
>To: Kirsher, Jeffrey T; Tantilov, Emil S
>Cc: netdev@...r.kernel.org; gospo@...hat.com; sassmann@...hat.com
>Subject: Re: [ethtool] ethtool: check mac type from ethtool_regs.version
>
>On Thu, 2013-04-25 at 22:12 -0700, Jeff Kirsher wrote:
>> From: Emil Tantilov <emil.s.tantilov@...el.com>
>>
>> This patch cleans up the mac type checks by using ethtool_regs.version
>> provided by the driver. This change eliminates the need to add device IDs
>> every time they are added to the driver.
>>
>> This patch depends on a driver change that will add the mac_type to the
>> upper 8 bits of ethtool_regs.version.
>>
>> Note that when using ethtool with this patch with a version of ixgbe that
>> does not provide the mac_type in ethtool_regs.version the register dump
>> may be incomplete. However this issue would've existed previously for
>> device IDs that were not added to ethtool.
>[...]
>
>I don't think this is acceptable; ethtool should remain backward
>compatible if at all possible.
>
>It seems like this would work with both old and new drivers:
>
>diff --git a/ixgbe.c b/ixgbe.c
>index dae11d4..9b005f2 100644
>--- a/ixgbe.c
>+++ b/ixgbe.c
>@@ -133,10 +133,13 @@ ixgbe_dump_regs(struct ethtool_drvinfo *info, struct
>ethtool_regs *regs)
> 	u8 i;
> 	enum ixgbe_mac_type mac_type;
>
>-	if (version != 1)
>+	if (version == 0)
> 		return -1;
>
>-	mac_type = ixgbe_get_mac_type(hw_device_id);
>+	/* The current driver reports the MAC type, but older versions
>+	 * only report the device ID so we have to infer the MAC type.
>+	 */
>+	mac_type = version > 1 ? version : ixgbe_get_mac_type(hw_device_id);
>
> 	reg = regs_buff[1065];
> 	fprintf(stdout,

Yeah, this seems like a better way to go.

Thanks a lot,
Emil

>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.