| Message ID | 20230810094817.29262-1-josua@solid-run.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: sfp: handle 100G/25G active optical cables in sfp_parse_support | expand |
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. > > Since the specification makes no statement about transmitter range, and > as the specific sfp module that had been tested features only 2m fiber - > short-range (SR) modes are selected. > > sfp_parse_support already handles SFF8024_ECC_100GBASE_SR4_25GBASE_SR > with compatible properties: 100000baseSR4; 25000baseSR; protocol 25gbase-r. > Add SFF8024_ECC_100G_25GAUI_C2M_AOC to the same case. > > Tested with fs.com S28-AO02 AOC SFP28 module. > > Signed-off-by: Josua Mayer <josua@solid-run.com> 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.
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"? - Josua Mayer
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.
On Mon, Aug 14, 2023 at 02:12:22PM +0200, Josua Mayer wrote: > Hi Russell, > > Am 10.08.23 um 14:05 schrieb Russell King (Oracle): > > 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. > Okay. So probably the simple correct solution is to make a seperate > case SFF8024_ECC_100G_25GAUI_C2M_AO that only sets 25gbase-r, and > 25000baseSR_Full. > > > > 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. > I don't fully understand how you would like to use br_nom. > I see e.g. in sfp-bus.c at the end of sfp_parse_support a mapping of bitrate > to 1000/2500 baseX modes. > Are you referring to this section? > > However there are no baseX modes for 25Gbps in ethtool.h - only SR/KR/CR. I'm thinking something like: case SFF8024_ECC_100G_25GAUI_C2M_AO: if (br_min <= 28000 && br_max >= 25000) { /* 25GBASE-R, possibly with FEC */ __set_bit(PHY_INTERFACE_MODE_25GBASER, interfaces); /* Re-use 25000baseSR as there is no 25Gbase- suffix * for this */ phylink_set(modes, 25000baseSR_Full); } break; I don't know what the actual numerical values should be though.
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c index e8dd47bffe43..db26e1d8a10d 100644 --- a/drivers/net/phy/sfp-bus.c +++ b/drivers/net/phy/sfp-bus.c @@ -258,6 +258,7 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id, switch (id->base.extended_cc) { case SFF8024_ECC_UNSPEC: break; + case SFF8024_ECC_100G_25GAUI_C2M_AOC: case SFF8024_ECC_100GBASE_SR4_25GBASE_SR: phylink_set(modes, 100000baseSR4_Full); phylink_set(modes, 25000baseSR_Full);
Handle extended compliance code 0x1 (SFF8024_ECC_100G_25GAUI_C2M_AOC) for active optical cables supporting 25G and 100G speeds. Since the specification makes no statement about transmitter range, and as the specific sfp module that had been tested features only 2m fiber - short-range (SR) modes are selected. sfp_parse_support already handles SFF8024_ECC_100GBASE_SR4_25GBASE_SR with compatible properties: 100000baseSR4; 25000baseSR; protocol 25gbase-r. Add SFF8024_ECC_100G_25GAUI_C2M_AOC to the same case. Tested with fs.com S28-AO02 AOC SFP28 module. Signed-off-by: Josua Mayer <josua@solid-run.com> --- drivers/net/phy/sfp-bus.c | 1 + 1 file changed, 1 insertion(+)