netdev - Re: [PATCH net-next 7/7] net: sfp: add support for rate selection

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: <255a933c-95d8-4872-b394-b948a7f68a92@lunn.ch>
Date: Wed, 17 May 2023 17:52:11 +0200
From: Andrew Lunn <andrew@...n.ch>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Marek Behún <kabel@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 7/7] net: sfp: add support for rate selection

> +static void sfp_module_parse_rate_select(struct sfp *sfp)
> +{
> +	u8 rate_id;
> +
> +	sfp->rs_threshold_kbd = 0;
> +	sfp->rs_state_mask = 0;
> +
> +	if (!(sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_RATE_SELECT)))
> +		/* No support for RateSelect */
> +		return;
> +
> +	/* Default to INF-8074 RateSelect operation. The signalling threshold
> +	 * rate is not well specified, so always select "Full Bandwidth", but
> +	 * SFF-8079 reveals that it is understood that RS0 will be low for
> +	 * 1.0625Gb/s and high for 2.125Gb/s. Choose a value half-way between.
> +	 * This method exists prior to SFF-8472.
> +	 */
> +	sfp->rs_state_mask = SFP_F_RS0;
> +	sfp->rs_threshold_kbd = 1594;
> +
> +	/* Parse the rate identifier, which is complicated due to history:
> +	 * SFF-8472 rev 9.5 marks this field as reserved.
> +	 * SFF-8079 references SFF-8472 rev 9.5 and defines bit 0. SFF-8472
> +	 *  compliance is not required.
> +	 * SFF-8472 rev 10.2 defines this field using values 0..4
> +	 * SFF-8472 rev 11.0 redefines this field with bit 0 for SFF-8079
> +	 * and even values.
> +	 */
> +	rate_id = sfp->id.base.rate_id;
> +	if (rate_id == 0)
> +		/* Unspecified */
> +		return;
> +
> +	/* SFF-8472 rev 10.0..10.4 did not account for SFF-8079 using bit 0,
> +	 * and allocated value 3 to SFF-8431 independent tx/rx rate select.
> +	 * Convert this to a SFF-8472 rev 11.0 rate identifier.
> +	 */
> +	if (sfp->id.ext.sff8472_compliance >= SFP_SFF8472_COMPLIANCE_REV10_2 &&
> +	    sfp->id.ext.sff8472_compliance < SFP_SFF8472_COMPLIANCE_REV11_0 &&
> +	    rate_id == 3)
> +		rate_id = SFF_RID_8431;
> +
> +	if (rate_id & SFF_RID_8079) {
> +		/* SFF-8079 RateSelect / Application Select in conjunction with
> +		 * SFF-8472 rev 9.5. SFF-8079 defines rate_id as a bitfield
> +		 * with only bit 0 used, which takes precedence over SFF-8472.
> +		 */
> +		if (!(sfp->id.ext.enhopts & SFP_ENHOPTS_APP_SELECT_SFF8079)) {
> +			/* SFF-8079 Part 1 - rate selection between Fibre
> +			 * Channel 1.0625/2.125/4.25 Gbd modes. Note that RS0
> +			 * is high for 2125, so we have to subtract 1 to
> +			 * include it.
> +			 */
> +			sfp->rs_threshold_kbd = 2125 - 1;
> +			sfp->rs_state_mask = SFP_F_RS0;
> +		}
> +		return;
> +	}
> +
> +	/* SFF-8472 rev 9.5 does not define the rate identifier */
> +	if (sfp->id.ext.sff8472_compliance <= SFP_SFF8472_COMPLIANCE_REV9_5)
> +		return;
> +
> +	/* SFF-8472 rev 11.0 defines rate_id as a numerical value which will
> +	 * always have bit 0 clear due to SFF-8079's bitfield usage of rate_id.
> +	 */
> +	switch (rate_id) {
> +	case SFF_RID_8431_RX_ONLY:
> +		sfp->rs_threshold_kbd = 4250;
> +		sfp->rs_state_mask = SFP_F_RS0;
> +		break;
> +
> +	case SFF_RID_8431_TX_ONLY:
> +		sfp->rs_threshold_kbd = 4250;
> +		sfp->rs_state_mask = SFP_F_RS1;
> +		break;
> +
> +	case SFF_RID_8431:
> +		sfp->rs_threshold_kbd = 4250;
> +		sfp->rs_state_mask = SFP_F_RS0 | SFP_F_RS1;
> +		break;
> +
> +	case SFF_RID_10G8G:
> +		sfp->rs_threshold_kbd = 9000;
> +		sfp->rs_state_mask = SFP_F_RS0 | SFP_F_RS1;
> +		break;
> +	}
> +}

Having read all that, you can kind of understand why few vendors get
SFP EEPROMs correct.

Reviewed-by: Andrew Lunn <andrew@...n.ch>

    Andrew