Maxime Chevallier <[email protected]> writes: > Hi again Björn, > > First, thanks for iterating so quick !
Thank *you* for helping me navigating the lower levels of the stack! I'm trying to be like the Iron Maiden tune: Be quick or be... ? :-P >> diff --git a/Documentation/netlink/specs/ethtool.yaml >> b/Documentation/netlink/specs/ethtool.yaml >> index 4707063af3b4..8bd14a3c946a 100644 >> --- a/Documentation/netlink/specs/ethtool.yaml >> +++ b/Documentation/netlink/specs/ethtool.yaml >> @@ -211,6 +211,49 @@ definitions: >> name: discard >> value: 31 >> >> + - >> + name: loopback-component >> + type: enum >> + doc: | >> + Loopback component. Identifies where in the network path the >> + loopback is applied. >> + entries: >> + - >> + name: mac >> + doc: MAC loopback. Loops traffic at the MAC block. >> + - >> + name: pcs >> + doc: | >> + PCS loopback. Loops traffic at the PCS sublayer between the >> + MAC and the PHY. >> + - >> + name: phy >> + doc: | >> + Ethernet PHY loopback. This refers to the Ethernet PHY managed >> + by phylib, not generic PHY drivers. A Base-T SFP module >> + containing an Ethernet PHY driven by Linux should report >> + loopback under this component, not module. >> + - >> + name: module >> + doc: | >> + Pluggable module (e.g. CMIS (Q)SFP) loopback. Covers loopback >> + modes controlled via module firmware or EEPROM registers. When >> + Linux drives an Ethernet PHY inside the module via phylib, use >> + the phy component instead. > > So to get back on Andrew's remarks, let's see if we can get something > closer to 802.3. > > Here, we have loopback at various locations, which all depends on the > Ethernet standard you use. > > It's usually in the PCS, PMA or PMD components. Thing is, we may have > these in multiple places in our link. > > If we take an example with a 10G PHY, we may have : > > +----SoC-----+ > | | > | MAC |- drivers/net/ethernet > | | | > | Base-R PCS |- could be in drivers/net/pcs, or directly > | | | in the MAC driver > | | | > | SerDes |- May be in drivers/phy, maybe handled by firmware, > | | | maybe by the MAC driver, maybe by the PCS driver ? > +---|--------+ > | > | 10GBase-R > | > +---|-PHY+ > | | | > | SerDes | \ > | | | | > | PCS | | > | | | > All of that handled by the drivers/net/phy PHY driver > | PMA | | > | | | | > | PMD | / > +---|----+ > | > v 10GBaseT > > So even the "PCS" loopback component is a bit ambiguous, are we talking > about the PHY PCS or the MAC PCS ? > > Another thing to consider is that there may be multiple PCSs in the SoC > (e.g. a BaseX and a BaseR PCS like we have in mvpp2), the one in use > depends on the current interface between the MAC and the PHY. > > Another open question is, do we deal with loopbacks that may affect > multi-netdev links ? Like the multi-lane modes we discussed with fbnic, > or even for embedded, interfaces such as QSGMII ? Hmm, TBH punt on it for now. The current design is per-netdev, and drivers should only expose loopbacks they can scope to a single netdev. Multi-netdev loopbacks can be addressed later if a real use case arises. That keeps the series focused and avoids designing for hypotheticals. > As for the SerDes on the MAC side (say, the comphy on Marvell devices), > can we say it's a PMA for 10GBase-KR ? Or is it something that's simply > out of spec ? > > So I'd say, maybe we should not have a PCS loopback component at all, > but instead loopback at the well-defined components on our link, that is: > > - MAC => MAC loopack, PCS on the MAC side, SerDes on the SoC, etc. > - PHY => Loopbacks on the PCS/PHY/PMA withing the PHY device > - Module => For non-PHY (Q)SFPs Less is more! I like that! So, the component maps to the Linux driver boundary (who owns the loopback), and the name is the 802.3 sublayer within that device? > The important part would therefore to get the "name" part right, making > sure we don't fall into driver specific names. > > We can name that 'pcs', 'pma', 'pmd', or maybe even 'mii' ? Let's see : > > +----SoC-----+ > | | > | MAC |- component = MAC, name = 'mac' > | | | > | Base-R PCS |- component = MAC, name = 'pcs' > | | | > | | | > | SerDes |- component = MAC, name = 'mii' ? > | | | > +---|--------+ > | > | 10GBase-R > | > +---|-PHY+ > | | | > | SerDes | - component = PHY, name = 'mii' ? > | | | > | PCS | - component = PHY, name = 'pcs' > | | | > | PMA | - component = PHY, name = 'pma' > | | | > | PMD |- component = PHY, name = 'pmd' or 'mdi' ? > +---|----+ > | > v 10GBaseT > > Sorry that's a lot of questions and I don't expect you to have the > answer, but as what you've come-up with is taking a good shape, it's > important to decide on the overall design and draw some lines about > what do we support, and how :( (Again, this is why input from folks like you/Andrew/Naveen is excellent! (Hey, I just wanted the CMIS loopback to start with! ;-))) I like this. The nice thing is that since "name" is a string, we're not locked into an enum -- drivers report what they have using 802.3 vocabulary, and we document the recommended names (pcs, pma, pmd, mii) with references? That way it's unambiguous, but not too constrained. For the next spin, I'll drop the pcs component entirely and keep only mac, phy, and module. I'll also expand the component docs to explain that the sublayer granularity lives in the name attribute using 802.3 terminology. How does that sound? >> + - >> + name: loopback-direction >> + type: flags >> + doc: | >> + Loopback direction flags. Used as a bitmask in supported, and as >> + a single value in direction. >> + entries: >> + - >> + name: near-end >> + doc: Near-end loopback; host-loop-host >> + - >> + name: far-end >> + doc: Far-end loopback; line-loop-line > > I was browsing 802.3, it uses the terminlogy of "local loopback" vs > "remote loopback", I suggest we use those. Sounds good! Thanks for taking the time to think through the layering -- this is much cleaner. Björn
