All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Josua Mayer <josua@solid-run.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH] net: sfp: handle 100G/25G active optical cables in sfp_parse_support
Date: Thu, 10 Aug 2023 13:05:26 +0100	[thread overview]
Message-ID: <ZNTShohLvCQR5AlU@shell.armlinux.org.uk> (raw)
In-Reply-To: <62adb14a-103d-4d29-9ecc-96203468e447@solid-run.com>

On Thu, Aug 10, 2023 at 01:38:13PM +0200, Josua Mayer wrote:
> Hi Russell,
> 
> Am 10.08.23 um 12:39 schrieb Russell King (Oracle):
> > On Thu, Aug 10, 2023 at 11:48:17AM +0200, Josua Mayer wrote:
> > > Handle extended compliance code 0x1 (SFF8024_ECC_100G_25GAUI_C2M_AOC)
> > > for active optical cables supporting 25G and 100G speeds.
> > Thanks. I think I would like one extra change:
> > 
> > > +	case SFF8024_ECC_100G_25GAUI_C2M_AOC:
> > >   	case SFF8024_ECC_100GBASE_SR4_25GBASE_SR:
> > >   		phylink_set(modes, 100000baseSR4_Full);
> > Since SFPs are single lane, SR4 doesn't make sense (which requires
> > four lanes), and I shouldn't have added it when adding these modes.
> > It would be a good idea to drop that, or at least for the
> > addition of the SFF8024_ECC_100G_25GAUI_C2M_AOC case.
> > 
> Would it be okay changing 100000baseSR4 to 100000baseSR dropping the "4"?

Not for SFF8024_ECC_100GBASE_SR4_25GBASE_SR. SFF-8024 states for this
code:

         02h        100GBASE-SR4 or 25GBASE-SR

100GBASE-SR4: IEEE 802.3 Physical Layer specification for 100 Gb/s using
100GBASE-R encoding over four lanes of multimode fiber, with reach
up to at least 100 m. (See IEEE Std 802.3, Clause 95.)

100GBASE-R encoding: The physical coding sublayer encoding defined in
Clause 82 for 100 Gb/s operation. (See IEEE Std 802.3, Clause 82.)

25GBASE-SR: IEEE 802.3 Physical Layer specification for 25 Gb/s using
25GBASE-R encoding over multimode fiber. (See IEEE Std 802.3, Clause 112.)

IEEE 802.3-2018 doesn't define 100GBASE-SR, so I assume that's a later
development, which would be 100GBASE-R encoding over one lane of fiber.

So, 100GBASE-SR and 100GBASE-SR4 are not equivalent, and since
SFF8024_ECC_100GBASE_SR4_25GBASE_SR specifies 100GBASE-SR4, that
being _four_ lanes of fiber, and SFP form-factor modules only being
capable of carrying a single lane, and sfp-bus.c only being for SFP
modules, 100GBASE-SR4 is just not relevant for our purposes in
sfp-bus.c - and it makes no sense to switch to 100GBASE-SR because
that is not what this code tells us.


For the SFF8024_ECC_100G_25GAUI_C2M_AOC in a SFP28 module, the SFP28
form factor only supports up to 28Gb/s, so that means the module is
definitely 25GBASE-R ethernet. So that also excludes 100G operation.

So, until we see a module in the SFP form factor (implying a single
lane) that does operate at 100G speeds, I think we should omit it.

I'm also wondering whether we should check br_nom/br_max/br_min now,
so that if we have to check that in the future, we don't start causing
regressions. Knowing how module EEPROMs are randomly wrong, it would
be a good idea to start with something sensible and see whether any
fail. Bear in mind that br_nom doesn't always get set to the correct
value - for example, 1G operates at 1250Mbps, and the SFP MSA specifies
that br_nom should be 1300 for 1G ethernet, but some modules use 1200.
I guess start at the correct value and then adjust to allow a range
as we see more modules.

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

  reply	other threads:[~2023-08-10 12:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-10  9:48 [PATCH] net: sfp: handle 100G/25G active optical cables in sfp_parse_support Josua Mayer
2023-08-10 10:39 ` Russell King (Oracle)
2023-08-10 11:38   ` Josua Mayer
2023-08-10 12:05     ` Russell King (Oracle) [this message]
     [not found]       ` <33a1c7b9-728c-46ac-840e-7aac0a725b7e@solid-run.com>
2023-08-14 12:22         ` Russell King (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZNTShohLvCQR5AlU@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=josua@solid-run.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.