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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Nov 2023 15:19:39 +0900 (JST) From: FUJITA Tomonori <fujita.tomonori@...il.com> To: andrew@...n.ch Cc: fujita.tomonori@...il.com, aliceryhl@...gle.com, benno.lossin@...ton.me, miguel.ojeda.sandonis@...il.com, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu, wedsonaf@...il.com Subject: Re: [PATCH net-next v7 5/5] net: phy: add Rust Asix PHY driver On Sun, 19 Nov 2023 17:03:30 +0100 Andrew Lunn <andrew@...n.ch> wrote: >> >> + let _ = dev.init_hw(); >> >> + let _ = dev.start_aneg(); >> > >> > Just to confirm: You want to call `start_aneg` even if `init_hw` returns >> > failure? And you want to ignore both errors? >> >> Yeah, I tried to implement the exact same behavior in the original C driver. > > You probably could check the return values, and it would not make a > difference. Also, link_change_notify() is a void function, so you > cannot return the error anyway. > > These low level functions basically only fail if the hardware is > `dead`. You get an -EIO or maybe -TIMEDOUT back. And there is no real > recovery. You tend to get such errors during probe and fail the > probe. Or maybe if power management is wrong and it has turned a > critical clock off. But that is unlikely in this case, we are calling > link_change_notify because the PHY has told us something changed > recently, so it probably is alive. > > I would say part of not checking the return code is also that C does > not have the nice feature that Rust has of making very simple to check > the return code. That combined with it being mostly pointless for PHY > drivers. Understood. I'll check the first return value if you prefer. I might add WARN_ON_ONCE after Rust supports it. diff --git a/drivers/net/phy/ax88796b_rust.rs b/drivers/net/phy/ax88796b_rust.rs index ce6640ff523f..7522070a6acd 100644 --- a/drivers/net/phy/ax88796b_rust.rs +++ b/drivers/net/phy/ax88796b_rust.rs @@ -95,7 +95,9 @@ fn link_change_notify(dev: &mut phy::Device) { // Reset PHY, otherwise MII_LPA will provide outdated information. // This issue is reproducible only with some link partner PHYs. if dev.state() == phy::DeviceState::NoLink { - let _ = dev.init_hw(); + if let Err(_) = dev.init_hw() { + return; + } let _ = dev.start_aneg(); } }
Powered by blists - more mailing lists