netdev - Re: [PATCH net-next v3 1/3] rust: core abstractions for network PHY drivers

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: <20231014.001514.876461873397203589.fujita.tomonori@gmail.com>
Date: Sat, 14 Oct 2023 00:15:14 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: miguel.ojeda.sandonis@...il.com, andrew@...n.ch
Cc: fujita.tomonori@...il.com, gregkh@...uxfoundation.org,
 netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu,
 wedsonaf@...il.com
Subject: Re: [PATCH net-next v3 1/3] rust: core abstractions for network
 PHY drivers

On Fri, 13 Oct 2023 13:59:12 +0200
Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:

> On Thu, Oct 12, 2023 at 1:18 AM FUJITA Tomonori
> <fujita.tomonori@...il.com> wrote:
>>
>> IIRC, Andrew prefers to avoid creating a temporary rust variant (Greg
>> does too, I understand). I guess that only solusion that both Rust and
>> C devs would be happy with is generating safe Rust code from C. The
> 
> As far as I understand, the workaround I just suggested in the
> previous reply was not discussed so far. I am not sure which of the
> alternatives you mean by the "temporary rust variant", so I may be
> misunderstanding your message.

I meant that defining Rust's enum corresponding to the kernel's enum
phy_state like.

+pub enum DeviceState {
+    /// PHY device and driver are not ready for anything.
+    Down,
+    /// PHY is ready to send and receive packets.
+    Ready,
+    /// PHY is up, but no polling or interrupts are done.
+    Halted,
+    /// PHY is up, but is in an error state.
+    Error,
+    /// PHY and attached device are ready to do work.
+    Up,
+    /// PHY is currently running.
+    Running,
+    /// PHY is up, but not currently plugged in.
+    NoLink,
+    /// PHY is performing a cable test.
+    CableTest,
+}

Then write match code by hand.

I'll leave it to PHYLIB maintainers. The subsystem maintainers decide
whether they merges the code.

Andrew, what do you think about the status of the abstraction patchset?


> Having said that, to try to unblock things, I spent some time
> prototyping the workaround I suggested, see below [1]. That catches
> the "new C variant added" desync between Rust and C.

Thanks, but we have to maintain the following code by hand? if so,
the maintanace nightmare problem isn't solved?


btw, I can't apply the patch, line wrapping?

> new file mode 100644
> index 000000000000..7c62bab12ea1
> --- /dev/null
> +++ b/rust/bindings/bindings_enum_check.rs
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Bindings exhaustiveness enum check.
> +//!
> +//! Eventually, this should be replaced by a safe version of
> `--rustified-enum`, see
> +//! https://github.com/rust-lang/rust-bindgen/issues/2646.
> +
> +#![no_std]
> +#![allow(
> +    clippy::all,
> +    dead_code,
> +    missing_docs,
> +    non_camel_case_types,
> +    non_upper_case_globals,
> +    non_snake_case,
> +    improper_ctypes,
> +    unreachable_pub,
> +    unsafe_op_in_unsafe_fn
> +)]
> +
> +include!(concat!(
> +    env!("OBJTREE"),
> +    "/rust/bindings/bindings_generated_enum_check.rs"
> +));
> +
> +fn check_phy_state(
> +    (phy_state::PHY_DOWN
> +    | phy_state::PHY_READY
> +    | phy_state::PHY_HALTED
> +    | phy_state::PHY_ERROR
> +    | phy_state::PHY_UP
> +    | phy_state::PHY_RUNNING
> +    | phy_state::PHY_NOLINK
> +    | phy_state::PHY_CABLETEST): phy_state,
> +) {
> +}
>