netdev - Re: [PATCH 1/3] ethtool: correctly interpret bitrate of 255

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: <20191021080944.GL25745@shell.armlinux.org.uk>
Date:   Mon, 21 Oct 2019 09:09:44 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Simon Horman <simon.horman@...ronome.com>
Cc:     netdev@...r.kernel.org, linville@...driver.com, andrew@...n.ch,
        f.fainelli@...il.com
Subject: Re: [PATCH 1/3] ethtool: correctly interpret bitrate of 255

On Mon, Oct 21, 2019 at 09:40:31AM +0200, Simon Horman wrote:
> On Fri, Oct 18, 2019 at 09:31:13PM +0100, Russell King wrote:
> > From: Russell King <rmk+kernel@...linux.org.uk>
> > 
> > A bitrate of 255 is special, it means the bitrate is encoded in
> > byte 66 in units of 250MBaud.  Add support for parsing these bit
> > rates.
> 
> Hi Russell,
> 
> it seems from the code either that 0 is also special or its
> handling has been optimised. Perhaps that would be worth mentioning
> in the changelog too.

The text of SFF8472 rev 12.2:

5.7     BR, nominal [Address A0h, Byte 12]
The nominal bit (signaling) rate (BR, nominal) is specified in units of
100MBd, rounded off to the nearest 100MBd. The bit rate includes those
bits necessary to encode and delimit the signal as well as those bits
carrying data information. A value of FFh indicates the bit rate is
greater than 25.0Gb/s and addresses 66 and 67 are used to determine
bit rate. A value of 0 indicates that the bit rate is not specified and
must be determined from the transceiver technology. The actual
information transfer rate will depend on the encoding of the data, as
defined by the encoding value.

8.4    BR, max [Address A0h, Byte 66]
If address 12 is not set to FFh, the upper bit rate limit at which the
transceiver will still meet its specifications (BR, max) is specified
in units of 1% above the nominal bit rate. If address 12 is set to FFh,
the nominal bit (signaling) rate (BR, nominal) is specified in units of
250 MBd, rounded off to the nearest 250 MBd. A value of 00h indicates
that this field is not specified.

8.5    BR, min [Address A0h, Byte 67]
If address 12 is not set to FFh, the lower bit rate limit at which the
transceiver will still meet its specifications (BR, min) is specified in
units of 1% below the nominal bit rate. If address 12 is set to FFh, the
limit range of bit rates specified in units of +/- 1% around the nominal
signaling rate. A value of zero indicates that this field is not
specified.

So I guess you could have a br_nom == 0 (meaning it should be derived
from other information) but max/min != 0 - which would be complex to
implement, and means that we're doing significant interpretation of
the contents.

> 
> > 
> > Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
> > ---
> >  sfpid.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sfpid.c b/sfpid.c
> > index a1753d3a535f..71f0939c6282 100644
> > --- a/sfpid.c
> > +++ b/sfpid.c
> > @@ -328,11 +328,24 @@ void sff8079_show_all(const __u8 *id)
> >  {
> >  	sff8079_show_identifier(id);
> >  	if (((id[0] == 0x02) || (id[0] == 0x03)) && (id[1] == 0x04)) {
> > +		unsigned int br_nom, br_min, br_max;
> > +
> > +		if (id[12] == 0) {
> > +			br_nom = br_min = br_max = 0;
> > +		} else if (id[12] == 255) {
> > +			br_nom = id[66] * 250;
> > +			br_max = id[67];
> > +			br_min = id[67];
> > +		} else {
> > +			br_nom = id[12] * 100;
> > +			br_max = id[66];
> > +			br_min = id[67];
> > +		}
> >  		sff8079_show_ext_identifier(id);
> >  		sff8079_show_connector(id);
> >  		sff8079_show_transceiver(id);
> >  		sff8079_show_encoding(id);
> > -		sff8079_show_value_with_unit(id, 12, "BR, Nominal", 100, "MBd");
> > +		printf("\t%-41s : %u%s\n", "BR, Nominal", br_nom, "MBd");
> >  		sff8079_show_rate_identifier(id);
> >  		sff8079_show_value_with_unit(id, 14,
> >  					     "Length (SMF,km)", 1, "km");
> > @@ -348,8 +361,8 @@ void sff8079_show_all(const __u8 *id)
> >  		sff8079_show_ascii(id, 40, 55, "Vendor PN");
> >  		sff8079_show_ascii(id, 56, 59, "Vendor rev");
> >  		sff8079_show_options(id);
> > -		sff8079_show_value_with_unit(id, 66, "BR margin, max", 1, "%");
> > -		sff8079_show_value_with_unit(id, 67, "BR margin, min", 1, "%");
> > +		printf("\t%-41s : %u%s\n", "BR margin, max", br_max, "%");
> > +		printf("\t%-41s : %u%s\n", "BR margin, min", br_min, "%");
> >  		sff8079_show_ascii(id, 68, 83, "Vendor SN");
> >  		sff8079_show_ascii(id, 84, 91, "Date code");
> >  	}
> > -- 
> > 2.7.4
> > 
> 
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up