Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 7 Sep 2013 19:52:22 +0100
From:      Mark R V Murray <mark@grondar.org>
To:        =?iso-8859-1?Q?Dag-Erling_Sm=F8rgrav?= <des@des.no>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mark Murray <markm@FreeBSD.org>
Subject:   Re: svn commit: r255362 - in head: share/examples/kld share/examples/kld/random_adaptor sys/conf sys/dev/glxsb sys/dev/hifn sys/dev/random sys/dev/rndtest sys/dev/safe sys/dev/ubsec sys/kern sys/mips/c...
Message-ID:  <C0253B44-6D0B-4234-8448-DA08468CC663@grondar.org>
In-Reply-To: <86bo44iipx.fsf@nine.des.no>
References:  <201309071415.r87EFDMv025499@svn.freebsd.org> <86bo44iipx.fsf@nine.des.no>

next in thread | previous in thread | raw e-mail | index | archive | help

--Apple-Mail=_3CD6B1A5-89CE-46A0-A6F9-909AFB43C969
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=iso-8859-1


On 7 Sep 2013, at 19:34, Dag-Erling Sm=F8rgrav <des@des.no> wrote:

> I didn't see anything that deviated from the plan we agreed upon in
> Cambridge.

That was the idea! :-D

> Several random_harvest() calls have been changed to reduce the entropy
> estimate - that's a good thing (as long as we don't reduce it to an
> unusable level, which I don't think is the case).  I also see that the
> network entropy harvesting bug we talked about has been fixed, which =
is
> also a good thing.  As far as I can tell, these are the only changes
> which affect the quality of the output.

Indeed.

> The renaming part made the patch hard to read - IWBNI it had been
> committed separately, but it didn't kill me.  Another factor that
> reduces readability is that the patch needlessly unfolds
> previously wrapped lines, e.g.
>=20
> -			if (++random_state.outputblocks >=3D
> -				random_state.gengateinterval) {
> +			if (++random_state.outputblocks >=3D =
random_state.gengateinterval) {
>=20
> which doesn't actually change anything but introduces a style bug and
> increases the reviewers' workload.  In fact, the patch introduces =
quite
> a few style bugs (including some that I personally aprove of but bde@
> may complain about, such as s/u_char/uint8_t/), but that can be
> addressed when we get a fresh batch of round tuts.

Apologies for making your life hard; that was unintended. I'll be =
working on
this code some more; I'll fix those long lines then.

> (joking aside - barring an overriding reason, we should strive to =
always
> conform to style(9))

Indeed; while this may not be a perfect lurch in that direction, the =
uintN_t
changes were a style(9) nod.

> In Yarrow, buffer sizes are now consistently referred to by BLOCKSIZE
> rather than a mix of BLOCKSIZE and (int)sizeof(whatever), which =
improves
> code readability at the cost of patch readability.  However, it =
appears
> that the *meaning* of BLOCKSIZE has changed from bits to bytes, and if =
I
> read the code correctly, it used to be 256 bits but is now 128.

Correct; it was a mess and I tidied it up. Having 256-bit blocks bought
us nothing; the intent is to use the natural key and block sizes of the
AES and SHA256 building-blocks.

> I dislike the use of "pseudo" in sys/dev/random/pseudo_rng.c since it =
is
> easily confused with the P in PRNG and pseudo_rng.c is actually not a
> PRNG but rather a collection of fake or dummy RNGs for testing =
purposes.
> Perhaps s/pseudo/dummy/g would be in order.

Good point. I'll do that.

> So, this is a provisional OK from my part.  *However*, I did not =
review
> the new harvesting queue in detail, and a bug there could, in the =
worst
> case, result in all the harvested entropy being discarded and Yarrow
> receiving kilobyte upon kilobyte of zeroes; so I'd like to get a =
second
> opinion.

Of course!

M
--=20
Mark R V Murray


--Apple-Mail=_3CD6B1A5-89CE-46A0-A6F9-909AFB43C969
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP using GPGMail

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.20 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQCVAwUBUit15t58vKOKE6LNAQpZDgP9HYhprygJEJAW1vLDcW3HXaa/xBgtwdwv
nqUK10AAm9ak9DnrxLWFlNzG91Wh+npq71aNHMo2nCyTv+Q8ytVWttniDPYSNTw5
orpi7ShdVd2+RzKiK/JRyYpQ3FWBIiMuLsIyL66/SjIyMwK7z53SlCLsEVxPds4R
z8wPryWtmA4=
=HoEg
-----END PGP SIGNATURE-----

--Apple-Mail=_3CD6B1A5-89CE-46A0-A6F9-909AFB43C969--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?C0253B44-6D0B-4234-8448-DA08468CC663>