linux-kernel - Re: [PATCH v5 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS

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: <Y5CgIL+cu4Fv43vy@lunn.ch>
Date:   Wed, 7 Dec 2022 15:16:00 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Piergiorgio Beruto <piergiorgio.beruto@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, Oleksij Rempel <o.rempel@...gutronix.de>
Subject: Re: [PATCH v5 net-next 1/5] net/ethtool: add netlink interface for
 the PLCA RS

> > TBH I can't parse the "ETHTOOL_A_PLCA_VERSION is reported as 0Axx
> > where.." sentence. Specifically I'm confused about what the 0A is.
> How about this: "When this standard is supported, the upper byte of
> ``ETHTOOL_A_PLCA_VERSION`` shall be 0x0A (see Table A.1.0 — IDVER 
> bits assignment).

I think the 0x0A is pointless and should not be included here. If the
register does not contain 0x0A, the device does not follow the open
alliance standard, and hence the lower part of the register is
meaningless.

This is why i suggested -ENODEV should actually be returned on invalid
values in this register.

> > >   * struct ethtool_phy_ops - Optional PHY device options
> > >   * @get_sset_count: Get number of strings that @get_strings will write.
> > >   * @get_strings: Return a set of strings that describe the requested objects
> > >   * @get_stats: Return extended statistics about the PHY device.
> > > + * @get_plca_cfg: Return PLCA configuration.
> > > + * @set_plca_cfg: Set PLCA configuration.
> > 
> > missing get status in kdoc
> Fixed. Good catch.

Building with W=1 C=2 will tell you about kerneldoc issues. Ideally we
want all network code to be clean with these two options.

     Andrew