netdev - Re: [PATCH net-next 3/3] net: atlantic: implement media detect feature via phy tunables

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]

Date:   Tue, 29 Sep 2020 19:18:15 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Igor Russkikh <irusskikh@...vell.com>
Cc:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next 3/3] net: atlantic: implement media detect
 feature via phy tunables

> @@ -923,6 +923,12 @@ static int aq_ethtool_get_phy_tunable(struct net_device *ndev,
>  	struct aq_nic_s *aq_nic = netdev_priv(ndev);
>  
>  	switch (tuna->id) {
> +	case ETHTOOL_PHY_EDPD: {
> +		u16 *val = data;
> +
> +		*val = (u16)aq_nic->aq_nic_cfg.is_media_detect;
> +		break;
> +	}
>  	case ETHTOOL_PHY_DOWNSHIFT: {
>  		u8 *val = data;
>  
> @@ -943,6 +949,14 @@ static int aq_ethtool_set_phy_tunable(struct net_device *ndev,
>  	struct aq_nic_s *aq_nic = netdev_priv(ndev);
>  
>  	switch (tuna->id) {
> +	case ETHTOOL_PHY_EDPD: {
> +		const u16 *val = data;
> +
> +		/* msecs plays no role - configuration is always fixed in PHY */
> +		aq_nic->aq_nic_cfg.is_media_detect = *val ? 1 : 0;

This is the wrong usage of the API:

include/uapi/linux/ethtool.h:

* The interval units for TX wake-up are in milliseconds, since this should
 * cover a reasonable range of intervals:
 *  - from 1 millisecond, which does not sound like much of a power-saver
 *  - to ~65 seconds which is quite a lot to wait for a link to come up when
 *    plugging a cable
 */

I guess your PHY is not hard coded to 1 millisecond? Please return the
real value. And the set call should really only allow 0, or the value
the PHY is using.

    Andrew