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
| ||
|
Date: Thu, 30 Apr 2020 14:25:51 +0200 From: Michal Kubecek <mkubecek@...e.cz> To: Oleksij Rempel <o.rempel@...gutronix.de> Cc: Marek Vasut <marex@...x.de>, Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Jonathan Corbet <corbet@....net>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Russell King <linux@...linux.org.uk>, mkl@...gutronix.de, kernel@...gutronix.de, David Jander <david@...tonic.nl>, Jakub Kicinski <kuba@...nel.org>, Christian Herber <christian.herber@....com>, "David S. Miller" <davem@...emloft.net>, Heiner Kallweit <hkallweit1@...il.com> Subject: Re: [PATCH net-next v3 1/2] ethtool: provide UAPI for PHY master/slave configuration. On Thu, Apr 30, 2020 at 02:09:45PM +0200, Oleksij Rempel wrote: > > > > > @@ -119,7 +123,12 @@ static int linkmodes_fill_reply(struct sk_buff *skb, > > > > > } > > > > > > > > > > if (nla_put_u32(skb, ETHTOOL_A_LINKMODES_SPEED, lsettings->speed) || > > > > > - nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, lsettings->duplex)) > > > > > + nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, lsettings->duplex) || > > > > > + nla_put_u8(skb, ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG, > > > > > + lsettings->master_slave_cfg) || > > > > > + nla_put_u8(skb, ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE, > > > > > + lsettings->master_slave_state)) > > > > > + > > > > > return -EMSGSIZE; > > > > > > > > From the two handlers you introduced, it seems we only get CFG_UNKNOWN > > > > or STATE_UNKNOWN if driver or device does not support the feature at all > > > > so it would be IMHO more appropriate to omit the attribute in such case. > > > > > > STATE_UNKNOWN is returned if link is not active. > > > > How about distinguishing the two cases then? Omitting both if CFG is > > CFG_UNKNOWN (i.e. driver does not support the feature) and sending > > STATE=STATE_UNKNOWN to userspace only if we know it's a meaningful value > > actually reported by the driver? > > Hm... after thinking about it, we should keep state and config > separately. The TJA1102 PHY do not provide actual state. Even no error > related to Master/Master conflict. I reworked the code to have > unsupported and unknown values. In case we know, that state or config > is not supported, it will not be exported to the user space. Sounds good to me, splitting "unsupported" and "unknown" states will be probably the best option. Michal
Powered by blists - more mailing lists