linux-kernel - [PATCH net-next v2] net: stmmac: drop the ethtool begin() callback

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-next>] [day] [month] [year] [list]

Message-Id: <20240829-stmmac-no-ethtool-begin-v2-1-a11b497a7074@redhat.com>
Date: Thu, 29 Aug 2024 15:48:44 -0500
From: Andrew Halaney <ahalaney@...hat.com>
To: Alexandre Torgue <alexandre.torgue@...s.st.com>, 
 Jose Abreu <joabreu@...opsys.com>, "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Maxime Coquelin <mcoquelin.stm32@...il.com>, 
 Abhishek Chauhan <quic_abchauha@...cinc.com>, 
 Sagar Cheluvegowda <quic_scheluve@...cinc.com>, 
 Dmitry Dolenko <d.dolenko@...rotek.ru>
Cc: netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com, 
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
 Andrew Halaney <ahalaney@...hat.com>
Subject: [PATCH net-next v2] net: stmmac: drop the ethtool begin() callback

This callback doesn't seem to serve much purpose, and prevents things
like:

    - systemd.link files from disabling autonegotiation
    - carrier detection in NetworkManager
    - any ethtool setting

prior to userspace bringing the link up.

The only fear I can think of is accessing unclocked resources due to
pm_runtime, but ethtool ioctls handle that as of commit
f32a21376573 ("ethtool: runtime-resume netdev parent before ethtool ioctl ops")

Reviewed-by: Dmitry Dolenko <d.dolenko@...rotek.ru>
Tested-by: Dmitry Dolenko <d.dolenko@...rotek.ru>
Signed-off-by: Andrew Halaney <ahalaney@...hat.com>
---
Changes in v2:
- Rebase on next-20240829
- Drop RFC/RFT tags, add Dmitry's Review/Test tag
- Link to v1: https://lore.kernel.org/r/20240429-stmmac-no-ethtool-begin-v1-1-04c629c1c142@redhat.com

I'd still like a few more folks to test this to feel more confident that
I'm not breaking anyone, but at least I've gotten one on list feedback
and one off list that its fine for them.
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 7008219fd88d..220c582904f4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -438,13 +438,6 @@ static void stmmac_ethtool_setmsglevel(struct net_device *dev, u32 level)

 }

-static int stmmac_check_if_running(struct net_device *dev)
-{
-	if (!netif_running(dev))
-		return -EBUSY;
-	return 0;
-}
-
 static int stmmac_ethtool_get_regs_len(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -1273,7 +1266,6 @@ static int stmmac_set_tunable(struct net_device *dev,
 static const struct ethtool_ops stmmac_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
-	.begin = stmmac_check_if_running,
 	.get_drvinfo = stmmac_ethtool_getdrvinfo,
 	.get_msglevel = stmmac_ethtool_getmsglevel,
 	.set_msglevel = stmmac_ethtool_setmsglevel,

---
base-commit: b18bbfc14a38b5234e09c2adcf713e38063a7e6e
change-id: 20240424-stmmac-no-ethtool-begin-f306f2f1f2f4

Best regards,
-- 
Andrew Halaney <ahalaney@...hat.com>