From: FUJITA Tomonori <fujita.tomonori@gmail.com>
To: benno.lossin@proton.me
Cc: fujita.tomonori@gmail.com, boqun.feng@gmail.com,
tmgross@umich.edu, netdev@vger.kernel.org,
rust-for-linux@vger.kernel.org, andrew@lunn.ch,
miguel.ojeda.sandonis@gmail.com, greg@kroah.com
Subject: Re: [PATCH net-next v3 1/3] rust: core abstractions for network PHY drivers
Date: Fri, 13 Oct 2023 18:53:48 +0900 (JST) [thread overview]
Message-ID: <20231013.185348.94552909652217598.fujita.tomonori@gmail.com> (raw)
In-Reply-To: <1da8acc8-ca48-49ae-8293-5e2a7ed86653@proton.me>
On Fri, 13 Oct 2023 07:56:07 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
>> btw, what's the purpose of using Rust in linux kernel? Creating sound
>> Rust abstractions? Making linux kernel more reliable, or something
>> else? For me, making linux kernel more reliable is the whole
>> point. Thus I still can't understand the slogan that Rust abstractions
>> can't trust subsystems.
>
> For me it is making the Linux kernel more reliable. The Rust abstractions
> are just a tool for that goal: we offload the difficult task of handling
> the C <-> Rust interactions and other `unsafe` features into those
> abstractions. Then driver authors do not need to concern themselves with
> that and can freely write drivers in safe Rust. Since there will be a lot
> more drivers than abstractions, this will pay off in the end, since we will
> have a lot less `unsafe` code than safe code.
>
> Concentrating the difficult/`unsafe` code in the abstractions make it
> easier to review (compared to `unsafe` code in every driver) and easier to
> maintain, if we find a soundness issue, we only have to fix it in the
> abstractions.
Agreed.
>> Rust abstractions always must check the validity of values that
>> subsysmtes give because subsysmtes might give an invalid value. Like
>> the enum state issue, if PHYLIB has a bug then give a random value, so
>> the abstraction have to prevent the invalid value in Rust with
>> validity checking. But with such critical bug, likely the system
>> cannot continue to run anyway. Preventing the invalid state in Rust
>> aren't useful much for system reliability.
>
> It's not that we do not trust the subsystems, for example when we register
> a callback `foo` and the C side documents that it is ok to sleep within
> `foo`, then we will assume so. If we would not trust the C side, then we
> would have to disallow sleeping there, since sleeping while holding a
> spinlock is UB (and the C side could accidentally be holding a spinlock).
>
> But there are certain things where we do not trust the subsystems, these
> are mainly things where we can afford it from a performance and usability
> perspective (in the example above we could not afford it from a usability
> perspective).
You need maintenance cost too here. That's exactly the discussion
point during reviewing the enum code, the kinda cut-and-paste from C
code and match code that Andrew and Grek want to avoid.
> In the enum case it would also be incredibly simple for the C side to just
> make a slight mistake and set the integer to a value outside of the
> specified range. This strengthens the case for checking validity here.
> When an invalid value is given to Rust we have immediate UB. In Rust UB
> always means that anything can happen so we must avoid it at all costs.
I'm not sure the general rules in Rust can be applied to linux kernel.
If the C side (PHYLIB) to set in an invalid value to the state,
probably the network doesn't work; already anything can happen in the
system at this point. Then the Rust abstractions get the invalid value
from the C side and detect an error with a check. The abstractions
return an error to a Rust PHY driver. Next what can the Rust PHY
driver do? Stop working? Calling dev_err() to print something and then
selects the state randomly and continue?
What's the practical benefit from the check?
> In this case having a check would not really hurt performance and in terms
> of usability it also seems reasonable. If it would be bad for performance,
> let us know.
Bad for maintenance cost. Please read the discussion in the review on rfc v1.
next prev parent reply other threads:[~2023-10-13 9:53 UTC|newest]
Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-09 1:39 [PATCH net-next v3 0/3] Rust abstractions for network PHY drivers FUJITA Tomonori
2023-10-09 1:39 ` [PATCH net-next v3 1/3] rust: core " FUJITA Tomonori
2023-10-09 3:17 ` Trevor Gross
2023-10-09 12:19 ` Benno Lossin
2023-10-09 13:02 ` Andrew Lunn
2023-10-09 13:56 ` Benno Lossin
2023-10-09 14:13 ` Andrew Lunn
2023-10-11 14:16 ` FUJITA Tomonori
2023-10-09 12:59 ` Miguel Ojeda
2023-10-09 13:49 ` FUJITA Tomonori
2023-10-09 14:32 ` Miguel Ojeda
2023-10-09 15:15 ` FUJITA Tomonori
2023-10-09 15:19 ` Miguel Ojeda
2023-10-09 15:11 ` Greg KH
2023-10-09 15:24 ` FUJITA Tomonori
2023-10-09 15:39 ` Miguel Ojeda
2023-10-09 15:50 ` FUJITA Tomonori
2023-10-11 9:59 ` Miguel Ojeda
2023-10-11 23:18 ` FUJITA Tomonori
2023-10-13 11:59 ` Miguel Ojeda
2023-10-13 15:15 ` FUJITA Tomonori
2023-10-13 18:33 ` Miguel Ojeda
2023-10-14 12:31 ` FUJITA Tomonori
2023-10-14 16:19 ` Miguel Ojeda
2023-10-12 0:29 ` FUJITA Tomonori
2023-10-09 21:07 ` Trevor Gross
2023-10-09 21:21 ` Andrew Lunn
2023-10-11 7:04 ` FUJITA Tomonori
2023-10-09 13:54 ` Andrew Lunn
2023-10-09 14:48 ` Miguel Ojeda
2023-10-09 17:04 ` Andrew Lunn
2023-10-12 3:59 ` FUJITA Tomonori
2023-10-12 4:43 ` Trevor Gross
2023-10-12 7:09 ` FUJITA Tomonori
2023-10-11 18:29 ` Boqun Feng
2023-10-12 5:58 ` FUJITA Tomonori
2023-10-12 6:34 ` Boqun Feng
2023-10-12 6:44 ` FUJITA Tomonori
2023-10-12 7:02 ` FUJITA Tomonori
2023-10-12 7:13 ` Boqun Feng
2023-10-12 7:32 ` Trevor Gross
2023-10-12 7:58 ` FUJITA Tomonori
2023-10-12 9:10 ` Benno Lossin
2023-10-13 4:17 ` Boqun Feng
2023-10-13 5:45 ` FUJITA Tomonori
2023-10-13 7:56 ` Benno Lossin
2023-10-13 9:53 ` FUJITA Tomonori [this message]
2023-10-13 10:03 ` Benno Lossin
2023-10-13 10:53 ` FUJITA Tomonori
2023-10-14 7:47 ` Benno Lossin
2023-10-14 21:55 ` Andrew Lunn
2023-10-14 22:18 ` Benno Lossin
2023-10-14 22:33 ` Andrew Lunn
2023-10-14 4:11 ` Boqun Feng
2023-10-14 11:59 ` Miguel Ojeda
2023-10-12 7:07 ` Boqun Feng
2023-10-09 1:39 ` [PATCH net-next v3 2/3] MAINTAINERS: add Rust PHY abstractions to the ETHERNET PHY LIBRARY FUJITA Tomonori
2023-10-09 1:39 ` [PATCH net-next v3 3/3] net: phy: add Rust Asix PHY driver FUJITA Tomonori
2023-10-09 3:22 ` Trevor Gross
2023-10-09 7:23 ` Jiri Pirko
2023-10-09 10:58 ` Miguel Ojeda
2023-10-09 11:41 ` FUJITA Tomonori
2023-10-09 12:32 ` Andrew Lunn
2023-10-09 14:01 ` Miguel Ojeda
2023-10-09 14:31 ` Andrew Lunn
2023-10-09 15:27 ` Miguel Ojeda
2023-10-09 15:35 ` Miguel Ojeda
2023-10-09 16:09 ` Andrew Lunn
2023-10-09 10:10 ` Greg KH
2023-10-12 11:57 ` FUJITA Tomonori
2023-10-09 12:42 ` Benno Lossin
2023-10-09 13:15 ` Andrew Lunn
2023-10-09 13:45 ` Benno Lossin
2023-10-09 12:48 ` [PATCH net-next v3 0/3] Rust abstractions for network PHY drivers Andrew Lunn
2023-10-09 12:53 ` Miguel Ojeda
2023-10-09 13:06 ` Greg KH
2023-10-09 14:13 ` Miguel Ojeda
2023-10-09 14:52 ` Greg KH
2023-10-09 15:06 ` Miguel Ojeda
2023-10-09 15:14 ` Greg KH
2023-10-09 15:15 ` Miguel Ojeda
2023-10-09 13:24 ` Andrew Lunn
2023-10-09 13:36 ` Miguel Ojeda
2023-10-09 14:21 ` Andrea Righi
2023-10-09 14:22 ` Miguel Ojeda
2023-10-09 14:56 ` Andrew Lunn
2023-10-09 15:04 ` Greg KH
2023-10-09 15:10 ` Miguel Ojeda
2023-10-09 15:15 ` Miguel Ojeda
2023-10-09 14:56 ` Greg KH
2023-10-09 15:09 ` Andrea Righi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231013.185348.94552909652217598.fujita.tomonori@gmail.com \
--to=fujita.tomonori@gmail.com \
--cc=andrew@lunn.ch \
--cc=benno.lossin@proton.me \
--cc=boqun.feng@gmail.com \
--cc=greg@kroah.com \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).