Thread (99 messages) 99 messages, 19 authors, 2022-01-11

Re: [PATCH v43 01/15] Linux Random Number Generator

From: Willy Tarreau <w@1wt.eu>
Date: 2021-12-11 08:58:17
Also in: lkml

Hi Stephan,

On Sat, Dec 11, 2021 at 09:09:55AM +0100, Stephan Müller wrote:
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help