linux-kernel - Re: [PATCH v2 1/2] leds: trigger: netdev: display only supported link speed attribute

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: <20231213163757.1d576bb7@dellmb>
Date:   Wed, 13 Dec 2023 16:37:57 +0100
From:   Marek Behún <kabel@...nel.org>
To:     Christian Marangi <ansuelsmth@...il.com>
Cc:     Pavel Machek <pavel@....cz>, Lee Jones <lee@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Daniel Golle <daniel@...rotopia.org>,
        "David S. Miller" <davem@...emloft.net>,
        Li Zetao <lizetao1@...wei.com>, linux-kernel@...r.kernel.org,
        linux-leds@...r.kernel.org
Subject: Re: [PATCH v2 1/2] leds: trigger: netdev: display only supported
 link speed attribute

> +	/* Display only supported entry */
> +	if (attr == &dev_attr_link_10.attr &&
> +	    (test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported_link_speed) ||
> +	     test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported_link_speed)))
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_link_100.attr &&
> +	    (test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported_link_speed) ||
> +	     test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported_link_speed)))
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_link_1000.attr &&
> +	    (test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported_link_speed) ||
> +	     test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported_link_speed)))
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_link_2500.attr &&
> +	    test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported_link_speed))
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_link_5000.attr &&
> +	    test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, supported_link_speed))
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_link_10000.attr &&
> +	    test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported_link_speed))
> +		return attr->mode;

Why only the T modes? There are much more ethtool modes for these
speeds, for example at least 5 modes for 1000 mbps speed:
  1000baseT_Half
  1000baseT_Full
  1000baseKX_Full
  1000baseX_Full
  1000baseT1_Full

There are also 2 possible modes for 2500 mbps
  2500baseT
  2500baseX

Ditto for 10 mbps and 100 mbps.

So if you're doing this, why not do it properly?

There is an aarray
  static const struct phy_setting settings[]
in
  drivers/net/phy/phy-core.c
and a function phy_speeds() which will tell you which speeds are
supported for a given ethtool mode bitmap.

Or you can add another function there,
  bool phy_speed_supported(unsigned long mask, unsigned int speed)
and use that one.

Marek