netdev - Re: [PATCH net-next v5 1/5] 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]

Date: Sun, 22 Oct 2023 18:47:02 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: miguel.ojeda.sandonis@...il.com
Cc: andrew@...n.ch, benno.lossin@...ton.me, fujita.tomonori@...il.com,
 netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu,
 boqun.feng@...il.com, wedsonaf@...il.com, greg@...ah.com
Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network
 PHY drivers

On Sat, 21 Oct 2023 14:47:04 +0200
Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:

> On Fri, Oct 20, 2023 at 8:42 PM Andrew Lunn <andrew@...n.ch> wrote:
>>
>> We don't want to hide phy_device too much, since at the moment, the
>> abstraction is very minimal. Anybody writing a driver is going to need
>> a good understanding of the C code in order to find the helpers they
>> need, and then add them to the abstraction. So i would say we need to
>> explain the relationship between the C structure and the Rust
>> structure, to aid developers.
> 
> I don't see how exposing `phy_device` in the documentation (note: not
> the implementation) helps with that. If someone has to add things to
> the abstraction, then at that point they need to be reading the
> implementation of the abstraction, and thus they should read the
> comments.
> 
> That is why the helpers should in general not be mentioned in the
> documentation, i.e. a Rust API user should not care / need to know
> about them.
> 
> If we mix things up in the docs, then it actually becomes harder later
> on to properly split it; and in the Rust side we want to maintain this
>  "API documentation" vs. "implementation comments" split. Thus it is
> important to do it right in the first examples we will have in-tree.
> 
> And if an API is not abstracted yet, it should not be documented. APIs
> and their docs should be added together, in the same patch, wherever
> possible. Of course, implementation comments are different, and
> possibly a designer of an abstraction may establish some rules or
> guidelines for future APIs added -- that is fine, but if the user does
> not need to know, it should not be in the docs, even if it is added
> early.
> 
> Regarding this, part of the `phy` module documentation (i.e. the three
> paragraphs) in this patch currently sounds more like an implementation
> comment to me. It should probably be rewritten/split properly in docs
> vs. comments.

Agreed that the first three paragraphs at the top of the file are
implementation comments. Are there any other comments in the file,
which look implementation comments to you? To me, the rest look the
docs for Rust API users.

I'm not sure that a comment on the relationship between C and Rust
structures like "Wraps the kernel's `struct phy_driver`" is API docs
but the in-tree files like mutex.rs have the similar so I assume it's
fine.

Where the implementation comments are supposed to be placed?
Documentation/networking?