netdev - Re: [PATCH RFC russell-king 2/4] net: phy: sfp: add support for multigig RollBall modules

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:   Wed, 12 Aug 2020 16:42:01 +0200
From:   Marek Behún <marek.behun@....cz>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Maxime Chevallier <maxime.chevallier@...tlin.com>,
        Baruch Siach <baruch@...s.co.il>,
        Chris Healy <cphealy@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH RFC russell-king 2/4] net: phy: sfp: add support for
 multigig RollBall modules

On Wed, 12 Aug 2020 15:33:04 +0100
Russell King - ARM Linux admin <linux@...linux.org.uk> wrote:

> On Wed, Aug 12, 2020 at 03:33:26PM +0200, Marek Behún wrote:
> > On Tue, 11 Aug 2020 16:15:53 +0100
> > Russell King - ARM Linux admin <linux@...linux.org.uk> wrote:
> >   
> > > > +	if (rollball) {
> > > > +		/* TODO: try to write this to EEPROM */
> > > > +		id.base.extended_cc =
> > > > SFF8024_ECC_10GBASE_T_SFI;    
> > > 
> > > Should we really be "fixing" vendors EEPROMs for them?
> > >   
> > 
> > Are you reffering to the TODO comment or the id.base.extended_cc
> > assignment?
> > If the comment, well, your code does it for cotsworks modules, but
> > I am actually indifferent.  
> 
> No, that's Chris' code, and there's quite a bit of history there:
> It appears Cotsworks programmed things like the serial number into
> the EEPROM and did not update the checksums.  After quite some time,
> it seems Cotsworks have seen sense, and have fixed their production
> line to properly program the EEPROM, but that leaves a whole bunch
> of modules with bad checksums.
> 
> I'm more than happy that we should continue issuing the warning, but
> Chris has decided to fix them up.  I'm not particularly happy with
> that idea, but I didn't get the chance to express it before David
> picked up the patch.  So, it's now in mainline.
> 
> Fixing the checksum for a module that is known to suffer bad checksums
> is one thing - it's a single byte write, and as the checksum is wrong,
> it's likely other systems that know about the issue will ignore it.
> 
> However, changing the module description to be "correct" is a
> completely different level - there are many modules that do not
> report "correct" data, and, if we start fixing these up, it's likely
> that fixups that other SFP cage implementations have could stop
> working since they may not recognise the module.
> 
> Remember, things like the extended CC codes are dependent on the SFF
> spec revisions, so if we start changing the extended CC code in byte
> 36, should we also change the SFF8472 compliance code as well (to
> be > rev 11.9)?  Since SFF8472 rev 11.9 changed the definition of this
> byte.
> 

Thank you Russell for this explanation.