Message-ID: <20211211085704.GB3083@1wt.eu>
Date:   Sat, 11 Dec 2021 09:57:04 +0100
From:   Willy Tarreau <w@....eu>
To:     Stephan Müller <smueller@...onox.de>
Cc:     Simo Sorce <simo@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Marcelo Henrique Cerri <marcelo.cerri@...onical.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Jeffrey Walton <noloader@...il.com>, Tso Ted <tytso@....edu>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Nicolai Stange <nstange@...e.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        "Alexander E. Patrakov" <patrakov@...il.com>,
        "Ahmed S. Darwish" <darwish.07@...il.com>,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Vito Caputo <vcaputo@...garu.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Jan Kara <jack@...e.cz>, Ray Strode <rstrode@...hat.com>,
        William Jon McCann <mccann@....edu>,
        zhangjs <zachary@...shancloud.com>,
        Andy Lutomirski <luto@...nel.org>,
        Florian Weimer <fweimer@...hat.com>,
        Lennart Poettering <mzxreary@...inter.de>,
        Peter Matthias <matthias.peter@....bund.de>,
        Eric Biggers <ebiggers@...nel.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Julia Lawall <julia.lawall@...ia.fr>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Andy Lavr <andy.lavr@...il.com>,
        Petr Tesarik <ptesarik@...e.cz>,
        John Haxby <john.haxby@...cle.com>,
        Alexander Lobakin <alobakin@...lbox.org>,
        Jirka Hladky <jhladky@...hat.com>
Subject: Re: [PATCH v43 01/15] Linux Random Number Generator

Hi Stephan,

On Sat, Dec 11, 2021 at 09:09:55AM +0100, Stephan Müller wrote:
> > It is obviously natural to think this way, but you can also understand
> > that reviewing patches is extremely time consuming. And it's extremely
> > difficult to review a patch series which says "replace all that
> > infrastructure with a new one", especially when the motivations are
> > "comply with this or that standard" without the benefits being obvious
> > at all for those having to review those patches.
> 
> I am so surprised by such statements. Patch 00/15 lists in a bullet list the 
> significant benefits of the LRNG. But seemingly nobody reads the introduction 
> with its concise bullet list or the documentation. The FIPS bits are a tiny 
> aspect of the whole effort (which even can be completely compiled out based on 
> config options), the more significant aspects that have nothing to do with 
> FIPS and benefit all are testability, performance, use of contemporary 
> cryptography, and flexibility.

But this is where the problem is. You're not proposing to improve the
current one but to replace it. Who has enough energy to review some new
code that claims to be compatible with older one ? It requires to perform
a mental "diff" which is extremely complicated. There are possibly corner
cases in the current code that nobody knows about and that some code
currently relies on. Who wlil detect that you're not going to break them
with a fresh new implementation ?

Incremental changes allow to focus on the changes. You don't need to
know how everything else works, just that the modifications do not
break the part they are inserted into. This makes a huge difference,
and this is why everyone constantly insists on seeing small incremental
changes. Sometimes you'll notice that you can even see review from
different people for very close parts in the same file.

> > But this does mean that a list of incremental changes/additions has to
> > be established on the submitter's side, not a list of replacements.
> 
> Before I started the endeavor of the stand-alone patch of the LRNG, I 
> developed cleanup patches to random.c in 2014 and 2015. I got massively 
> discouraged to continue working on random.c as I did not get feedback from the 
> maintainer. Some patches were taken, some were not without a comment... 

We all know that this is extremely irritating. It happens everywhere and
in every project. Sometimes lack of time, lost messages, flipped priorities,
or lack of interest, or a combination of all of this. I do it myself from
time to time by accident and I really feel bad when this happens. That's
not much different than when you're reminding a coworker that you need
their help and they forget because of other priorities. And this must
never discourage one from pinging again and asking why there's no
response. But one thing is certain to me, it's that if a maintainer,
for any reason, doesn't respond to tiny patches to their code, there's
hardly any chance to see a response to a whole replacement.

Here Jason offered to invest time reviewing changes. If you have changes
to propose, why not try ? And even if it takes one year to get everything
done, who cares ? You seem to have been working on this for 7 years
already, it might be worth trying another approach.

Regards,
Willy

Powered by blists - more mailing lists