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: <YeV8BwzyXuuvxvBN@shell.armlinux.org.uk>
Date:   Mon, 17 Jan 2022 14:24:07 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Christian Lamparter <chunkeey@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Bjørn Mork <bjorn@...k.no>,
        netdev@...r.kernel.org,
        照山周一郎 <teruyama@...ingboard-inc.jp>
Subject: Re: [PATCH net,stable] phy: sfp: fix high power modules without diag
 mode

On Sat, Jan 15, 2022 at 05:58:43PM +0100, Christian Lamparter wrote:
> On 03/12/2021 13:58, Russell King (Oracle) wrote:
> > On Fri, Dec 03, 2021 at 11:54:57AM +0000, Russell King (Oracle) wrote:
> > [...]
> > 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);
> > 
> 
> The reporter has problems reaching you. But from what I can tell in his reply to his
> OpenWrt Github PR:
> <https://github.com/openwrt/openwrt/pull/4802#issuecomment-1013439827>
> 
> your approach is working perfectly. Could you spin this up as a fully-fledged patch (backports?)

There seems to be no problem - I received an email on the 30 December
complete with the test logs. However, that was during the holiday period
and has been buried, so thanks for the reminder.

However, I'm confused about who the reporter and testers actually are,
so I'm not sure who to put in the Reported-by and Tested-by fields.
>From what I can see, Bjørn Mork <bjorn@...k.no> reported it (at least
to mainline devs), and the fix was tested by 照山周一郎
<teruyama@...ingboard-inc.jp>.

Is that correct? Thanks.

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