netdev - RE: [RFC PATCH net-next 1/9] ethtool: Add ethtool operation to write to a transceiver module EEPROM

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:
 <DM6PR12MB4516A145E4343BADECE13C1ED87B2@DM6PR12MB4516.namprd12.prod.outlook.com>
Date: Wed, 24 Jan 2024 13:10:45 +0000
From: Danielle Ratson <danieller@...dia.com>
To: Russell King <linux@...linux.org.uk>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"corbet@....net" <corbet@....net>, "sdf@...gle.com" <sdf@...gle.com>,
	"kory.maincent@...tlin.com" <kory.maincent@...tlin.com>,
	"maxime.chevallier@...tlin.com" <maxime.chevallier@...tlin.com>,
	"vladimir.oltean@....com" <vladimir.oltean@....com>,
	"przemyslaw.kitszel@...el.com" <przemyslaw.kitszel@...el.com>,
	"ahmed.zaki@...el.com" <ahmed.zaki@...el.com>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "shayagr@...zon.com" <shayagr@...zon.com>,
	"paul.greenwalt@...el.com" <paul.greenwalt@...el.com>, "jiri@...nulli.us"
	<jiri@...nulli.us>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, mlxsw
	<mlxsw@...dia.com>, Petr Machata <petrm@...dia.com>, Ido Schimmel
	<idosch@...dia.com>
Subject: RE: [RFC PATCH net-next 1/9] ethtool: Add ethtool operation to write
 to a transceiver module EEPROM

> >  /**
> > - * struct ethtool_module_eeprom - EEPROM dump from specified page
> > - * @offset: Offset within the specified EEPROM page to begin read, in
> bytes.
> > - * @length: Number of bytes to read.
> > - * @page: Page number to read from.
> > - * @bank: Page bank number to read from, if applicable by EEPROM spec.
> > + * struct ethtool_module_eeprom - plug-in module EEPROM read / write
> > + parameters
> > + * @offset: Offset within the specified page, in bytes.
> > + * @length: Number of bytes to read / write.
> > + * @page: Page number.
> > + * @bank: Bank number, if supported by EEPROM spec.
> 
> I suppose I should have reviewed the addition of this (I can't recall whether I
> got the original or not.)
> 
> If one looks at SFF-8472, then the first 128 bytes of the EEPROM at
> 0x50 (0xA0 on the wire) are not paged. Whereas bytes 128..255 are the paged
> bytes. Therefore, "offset within the specified page" can sensibly be
> interpreted as referring to the EEPROM at 0x50, at an offset of
> 128 + offset.
> 
> Meanwhile, the actual implementation doesn't do that - the offset is the
> offset from the beginning of the EEPROM, and offsets >= 128 access the
> paged area.
> 
> What this means is that the parameter description here is basically wrong,
> both before and after your change.
> 
> This really ought to be fixed so that we describe things correctly rather than
> misleading people who read documentation. Otherwise, it's a recipe for
> broken implementations... and it's also completely pointless documenting it if
> the documentation is wrong.
> 

Will rephrase.

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!