netdev - Re: [PATCH net,stable] phy: sfp: fix high power modules without diag mode

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: <YathNbNBob2kHxrH@shell.armlinux.org.uk>
Date:   Sat, 4 Dec 2021 12:38:13 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Bjørn Mork <bjorn@...k.no>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        照山周一郎 <teruyama@...ingboard-inc.jp>
Subject: Re: [PATCH net,stable] phy: sfp: fix high power modules without diag
 mode

On Fri, Dec 03, 2021 at 02:55:17PM +0100, Bjørn Mork wrote:
> "Russell King (Oracle)" <linux@...linux.org.uk> writes:
> 
> > Thinking a little more, how about this:
> >
> >  drivers/net/phy/sfp.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > index 51a1da50c608..4c900d063b19 100644
> > --- a/drivers/net/phy/sfp.c
> > +++ b/drivers/net/phy/sfp.c
> > @@ -1752,17 +1752,20 @@ static int sfp_sm_probe_for_phy(struct sfp *sfp)
> >  static int sfp_module_parse_power(struct sfp *sfp)
> >  {
> >  	u32 power_mW = 1000;
> > +	bool supports_a2;
> >  
> >  	if (sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_POWER_DECL))
> >  		power_mW = 1500;
> >  	if (sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_HIGH_POWER_LEVEL))
> >  		power_mW = 2000;
> >  
> > +	supports_a2 = sfp->id.ext.sff8472_compliance !=
> > +				SFP_SFF8472_COMPLIANCE_NONE ||
> > +		      sfp->id.ext.diagmon & SFP_DIAGMON_DDM;
> > +
> >  	if (power_mW > sfp->max_power_mW) {
> >  		/* Module power specification exceeds the allowed maximum. */
> > -		if (sfp->id.ext.sff8472_compliance ==
> > -			SFP_SFF8472_COMPLIANCE_NONE &&
> > -		    !(sfp->id.ext.diagmon & SFP_DIAGMON_DDM)) {
> > +		if (!supports_a2) {
> >  			/* The module appears not to implement bus address
> >  			 * 0xa2, so assume that the module powers up in the
> >  			 * indicated mode.
> > @@ -1779,11 +1782,24 @@ static int sfp_module_parse_power(struct sfp *sfp)
> >  		}
> >  	}
> >  
> > +	if (power_mW <= 1000) {
> > +		/* Modules below 1W do not require a power change sequence */
> > +		return 0;
> > +	}
> > +
> > +	if (!supports_a2) {
> > +		/* The module power level is below the host maximum and the
> > +		 * module appears not to implement bus address 0xa2, so assume
> > +		 * that the module powers up in the indicated mode.
> > +		 */
> > +		return 0;
> > +	}
> > +
> >  	/* If the module requires a higher power mode, but also requires
> >  	 * an address change sequence, warn the user that the module may
> >  	 * not be functional.
> >  	 */
> > -	if (sfp->id.ext.diagmon & SFP_DIAGMON_ADDRMODE && power_mW > 1000) {
> > +	if (sfp->id.ext.diagmon & SFP_DIAGMON_ADDRMODE) {
> >  		dev_warn(sfp->dev,
> >  			 "Address Change Sequence not supported but module requires %u.%uW, module may not be functional\n",
> >  			 power_mW / 1000, (power_mW / 100) % 10);
> 
> Looks nice to me at least.  But I don't have the hardware to test it.

I don't have the hardware either, so I can't test it - but it does need
testing. I assume as you've reported it and sent a patch, you know
someone who has run into this issue? It would be great if you could ask
them to test it and let us know if it solves the problem.

Thanks.

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