netdev - Re: [PATCH net-next v7 0/5] Rust 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: Fri, 27 Oct 2023 12:21:33 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, tmgross@...ch.edu, benno.lossin@...ton.me, 
	wedsonaf@...il.com
Subject: Re: [PATCH net-next v7 0/5] Rust abstractions for network PHY drivers

On Fri, Oct 27, 2023 at 1:48 AM Andrew Lunn <andrew@...n.ch> wrote:
>
> That is actually about right for netdev. As i said, netdev moves fast,
> review comments are expected within about 3 days. We also say don't
> post a new version within 24 hours. So that gives you an idea of the
> min and max.

And as I said when you told us that, that may be the right policy for
C netdev, which has been around for a long time, has a well supported
infrastructure, the code base is well-known, etc.

For Rust, it is not, for multiple reasons. For starters, we are not
talking here about just another patch to your subsystem. This is a
whole new subsystem API, in a new language, that needs careful design.
Moreover, Rust abstractions are supposed to be "sound" (a property
that C APIs do not need).

On top of that, two subsystems are reviewing it, and on our side we
simply do not have the resources to commit to netdev review pace, as
we told you privately. I am aware of at least 3 reviewers who have not
had the time to look into the more recent versions yet, and it is
unlikely they will have time before LPC. I would really recommend they
are given the chance to give feedback.

So, if you appreciate/want/need our feedback, you will need to be a
bit more patient.

By the way, your docs say patches are triaged in less than 48 hours as
well as "Generally speaking". From that wording, I wouldn't expect
every single patch to take 48 hours to be fully reviewed (not just
triaged), and I would say the "very first Rust abstractions code" is
not a common situation.

> It is however good to let discussion reach some sort of conclusion,
> but that also requires prompt discussion.

I don't see why that would require "prompt discussion".

> And if that discussion is
> not prompt, posting a new version is a way to kick reviewers into
> action.

No, sorry, posting a new version in order to push reviewers is not the
right thing to do. The patches were not being ignored.

>From your own (netdev) docs -- not even the general kernel ones:

    "Do not post a new version of the code if the discussion about the
previous version is still ongoing, unless directly instructed by a
reviewer."

    "Asking the maintainer for status updates on your patch is a good
way to ensure your patch is ignored or pushed to the bottom of the
priority list."

    "Make sure you address all the feedback in your new posting."

Cheers,
Miguel