netdev - Re: [PATCH net-next 4/4] driver/ncn26000: add PLCA support

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] [day] [month] [year] [list]

Date:   Sun, 4 Dec 2022 21:33:51 +0100
From:   Piergiorgio Beruto <piergiorgio.beruto@...il.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, Oleksij Rempel <o.rempel@...gutronix.de>
Subject: Re: [PATCH net-next 4/4] driver/ncn26000: add PLCA support

On Sun, Dec 04, 2022 at 08:22:40PM +0000, Russell King (Oracle) wrote:
> On Sun, Dec 04, 2022 at 09:09:08PM +0100, Piergiorgio Beruto wrote:
> > On Sun, Dec 04, 2022 at 07:48:24PM +0100, Andrew Lunn wrote:
> > > On Sun, Dec 04, 2022 at 05:06:50PM +0000, Russell King (Oracle) wrote:
> > > > On Sun, Dec 04, 2022 at 03:32:06AM +0100, Piergiorgio Beruto wrote:
> > > > > --- a/include/uapi/linux/mdio.h
> > > > > +++ b/include/uapi/linux/mdio.h
> > > > > @@ -26,6 +26,7 @@
> > > > >  #define MDIO_MMD_C22EXT		29	/* Clause 22 extension */
> > > > >  #define MDIO_MMD_VEND1		30	/* Vendor specific 1 */
> > > > >  #define MDIO_MMD_VEND2		31	/* Vendor specific 2 */
> > > > > +#define MDIO_MMD_OATC14		MDIO_MMD_VEND2
> > > > 
> > > > If this is in the vendor 2 register set, I doubt that this is a feature
> > > > described by IEEE 802.3, since they allocated the entirety of this MMD
> > > > over to manufacturers to do whatever they please with this space.
> > > > 
> > > > If this is correct, then these definitions have no place being in this
> > > > generic header file, since they are likely specific to the vendors PHY.
> > > 
> > > Piergiorgio can give you the full details.
> > > 
> > > As i understand it, IEEE 802.3 defines the basic functionality, but
> > > did not extend the standard to define the registers.
> > > 
> > > The Open Alliance member got together and added the missing parts, and
> > > published an Open Alliance document.
> > > 
> > > Piergiorgio, i suggest you add a header file for these defines, named
> > > to reflect that the Open Alliance defined them. And put in a comment,
> > > explaining their origin, maybe a link to the standard. I also don't
> > > think this needs to be a uapi header, they are not needed outside of
> > > the kernel.
> > > 
> > > I also would not use MDIO_MMD_OATC14, but rather MDIO_MMD_VEND2. There
> > > is no guarantee they are not being used for other things, and
> > > MDIO_MMD_VEND2 gives a gentle warning about this.
> > Thanks Andrew for commenting on this one. This is right, in the IEEE
> > 802.3cg group we could not allocate an MMD for the PLCA reconciliation
> > sublayer because of an 'unfriendly' wording in Clause 45 ruling out
> > Reconciliation Sublayers from what can be configured via registers.
> > Clause 45 says you can have registers for the PHY, while it should have
> > said 'Physical Layer" and there is a subtle difference between the two
> > words. PLCA, for example, is part of the Physical Layer but not of the
> > PHY. Since we could not change that wording, we had to define
> > configuration parameters in Clause 30, and let organizations outside the
> > IEEE define memory maps for PHYs that integrate PLCA.
> > 
> > The OPEN Alliance SIG (see the reference in the patches) defined
> > registers for the PLCA RS in MMD31, which is in fact vendor-specific
> > from an IEEE perspective, but part of it is now standardized in the OPEN
> > Alliance. So unfortunately we have to live with this somehow.
> > 
> > So ok, I can separate these definitions into a different non-UAPI header
> > as Andrew is suggesting. I'll do this in the next patchset.
> 
> Sounds like yet another clause 45 mess :(
I'm really sorry for this, I can assure you I personally pushed very
hard to get this through, but eventually we got an hard stop. The IEEE
has very strict formal rules too, and we were not allowed to change that
portion of the specs. To get permission we should have delayed the
standard by one year, and that would have upset many people from
different industries...

Thanks for your understanding.
Piergiorgio