| 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, 5 Oct 2023 01:25:23 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Cc: netdev@...r.kernel.org
Subject: What is the purpose of the first phylink_validate() call from
phylink_create()?
Hi Russell,
In phylink_create() we have this code which populates pl->supported with
a maximal link mode configuration and then makes a best-effort attempt
to reduce it to what the physical port actually supports:
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3951e5af8cb5..1e89634ec8ae 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1677,10 +1677,6 @@ struct phylink *phylink_create(struct phylink_config *config,
__set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
timer_setup(&pl->link_poll, phylink_fixed_poll, 0);
- bitmap_fill(pl->supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
- linkmode_copy(pl->link_config.advertising, pl->supported);
- phylink_validate(pl, pl->supported, &pl->link_config);
-
ret = phylink_parse_mode(pl, fwnode);
if (ret < 0) {
kfree(pl);
However:
- in MLO_AN_FIXED mode, the later call to phylink_parse_fixedlink() will
overwrite this pl->supported and pl->link_config.advertising with
another set
- in MLO_AN_INBAND mode, the later call to phylink_parse_mode() will
also overwrite pl->supported and pl->link_config.advertising
- with a PHY (either in MLO_AN_INBAND or MLO_AN_PHY modes),
phylink_bringup_phy() will overwrite pl->supported and
pl->link_config.advertising with stuff from the PHY
Of these 3 cases, phylink_bringup_phy() is the only one which
potentially does not come immediately after phylink_create().
So, the effect of the phylink_validate() from phylink_create() will be
visible only when it's not overwritten, for example when phylink_connect_phy()
(or one of variants) isn't called at probe time but is delayed until
ndo_open().
Since mvneta calls phylink_of_phy_connect() from mvneta_open() and I can
test that, I'm comparing the "ethtool" output produced before running
"ip link set dev eth0 up", in 2 cases:
- With the phylink_validate() from phylink_create() kept in place:
$ ethtool eth0
Settings for eth0:
Supported ports: [ TP AUI MII FIBRE BNC Backplane ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
1000baseKX/Full
1000baseX/Full
100baseT1/Full
1000baseT1/Full
100baseFX/Half 100baseFX/Full
10baseT1L/Full
10baseT1S/Full
10baseT1S/Half
10baseT1S_P2MP/Half
Supported pause frame use: Symmetric
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Advertised FEC modes: Not reported
Speed: Unknown!
Duplex: Half
Auto-negotiation: off
Port: MII
PHYAD: 0
Transceiver: internal
Supports Wake-on: d
Wake-on: d
Link detected: no
- And with it removed (the diff from the beginning):
$ ethtool eth0
Settings for eth0:
Supported ports: [ ]
Supported link modes: Not reported
Supported pause frame use: No
Supports auto-negotiation: No
Supported FEC modes: Not reported
Advertised link modes: Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Advertised FEC modes: Not reported
Speed: Unknown!
Duplex: Half
Auto-negotiation: off
Port: MII
PHYAD: 0
Transceiver: internal
Supports Wake-on: d
Wake-on: d
Link detected: no
But I'm not sure that this ethtool output is very valuable to user space?
At this stage it is essentially just the output of phylink_generic_validate()
for the MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD capabilities using a
gigabit phy_interface_t. It is subject to change as more link modes get
introduced. The output with no link modes reported until the PHY connects
seems at least equally reasonable, given that the PHY dictates the link modes.
So what is the purpose of the early phylink_validate() call and the
associated population of pl->supported? Is it just to report some link
modes until we have a PHY and we're not in-band, or am I missing something?
On a visual inspection, this code structure exists since commit
9525ae83959b ("phylink: add phylink infrastructure"), but I cannot test
as far back as that to be absolutely sure.
For reference, here is the ethtool output when the port has been brought up:
$ ethtool eth0
Settings for eth0:
Supported ports: [ TP MII FIBRE ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Supported pause frame use: Symmetric
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Advertised pause frame use: Symmetric
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Auto-negotiation: on
Port: Twisted Pair
PHYAD: 1
Transceiver: external
MDI-X: Unknown
Supports Wake-on: pg
Wake-on: d
Link detected: yes
Powered by blists - more mailing lists