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: <CANiq72=JQseA6JFy7g489Wwk8kc7-xk2GLVVJC8+T9eMNxvitw@mail.gmail.com>
Date: Fri, 13 Oct 2023 20:33:58 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: andrew@...n.ch, 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, Oct 13, 2023 at 5:15 PM FUJITA Tomonori
<fujita.tomonori@...il.com> wrote:
>
> 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.

Yes, but that alone is not enough -- that is what we do normally, but
we can still diverge with the C side. That is what the `bindgen`
proposal would solve (plus better maintenance). The workaround also
solves that, but with more maintenance effort. We could even go
further, but I don't think it is worth it given that we really want to
have it in `bindgen`.

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

Indeed, but the "no `--rustified-enum`" is a whole kernel policy we
want to keep, i.e. we are NAK'ing that small bit because we want to a
solution that does not introduce silent UB if a non-local mistake is
made.

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

That is correct, but that requires extra work on `bindgen`. What we
can ensure with he workaround is that it does not get our of sync (in
terms of the variants).

If Andrew prefers to wait for a proper `bindgen` solution, that is
fine with us; i.e. what we are only saying is "no, please" to the
`--rustified-enum` approach.

Also please note that there is still the question about the docs on
the generated `enum`, even with the current `bindgen` proposal in
place.

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

Yes, I just copy pasted it in Gmail to showcase the idea. I have
pushed it here in case you want to play with it:
https://github.com/ojeda/linux/tree/rust-bindgen-workaround

Cheers,
Miguel