diff mbox series

[v2] package/haveged: set write_wakeup_threshold to 2048

Message ID 20210412164733.17098-1-matthew.weber@rockwellcollins.com
State Rejected
Headers show
Series [v2] package/haveged: set write_wakeup_threshold to 2048 | expand

Commit Message

Matt Weber April 12, 2021, 4:47 p.m. UTC
Adjust the low water mark to wake-up the haveged daemon at the
same time that rngd would wake-up when a hardware RNG is present.

This supports the theory that rngd then can't dominate the entropy
pool. Instead haveged and rngd would complete to fill the pool.
https://tails.boum.org/contribute/design/random/#index5h2

If rngd isn't enabled or there isn't a hardware RNG, this change
should not impact the behavior of the entropy pool other then
the pool should be less likely to deplete and cause an entropy
starve with haveged filling it earlier.

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
Changes
v1 -> v2
 - Rebased on master
 - verified no CPE ids match
---
 package/haveged/S21haveged | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Yann E. MORIN April 24, 2021, 9:01 a.m. UTC | #1
Matt, All,

On 2021-04-12 11:47 -0500, Matt Weber spake thusly:
> Adjust the low water mark to wake-up the haveged daemon at the
> same time that rngd would wake-up when a hardware RNG is present.

2048 is indeed the default for rngd, but then don't we want to enforce
that the two have the same setting in Buildroot, and let the user
override that even for haveged (it's already possible for rng-tools),
e.g.:

    diff --git a/package/haveged/S21haveged b/package/haveged/S21haveged
    index 4b6e6ced4e..1dce16aa73 100644
    --- a/package/haveged/S21haveged
    +++ b/package/haveged/S21haveged
    @@ -1,9 +1,18 @@
     #!/bin/sh
     
    +NAME="haveged"
    +DAEMON="/usr/sbin/${NAME}"
    +DAEMON_ARGS=""
    +CFG_FILE="/etc/default/${NAME}"
    +PID_FILE="/var/run/${NAME}.pid"
    +
    +# Read configuration variable file if it is present
    +[ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
    +
     case "$1" in
     	start)
     		printf "Starting haveged: "
    -		start-stop-daemon -S -x /usr/sbin/haveged -- -w 1024 -r 0
    +		start-stop-daemon -S -x /usr/sbin/haveged -- -w 2048 -r 0 ${DAEMON_ARGS}
     		[ $? = 0 ] && echo "OK" || echo "FAIL"
     		;;
     	stop)
    diff --git a/package/rng-tools/S21rngd b/package/rng-tools/S21rngd
    index b38be5a507..77cccc790f 100644
    --- a/package/rng-tools/S21rngd
    +++ b/package/rng-tools/S21rngd
    @@ -12,7 +12,7 @@ PID_FILE="/var/run/${NAME}.pid"
     start()
     {
     	printf "Starting ${NAME}: "
    -	start-stop-daemon -S -q -x "${DAEMON}" -- ${DAEMON_ARGS}
    +	start-stop-daemon -S -q -x "${DAEMON}" -- -W 2048 ${DAEMON_ARGS}
     	[ $? = 0 ] && echo "OK" || echo "FAIL"
     }
     

Otherwise, we woud depend on the fact that the default does not change
eveytime we update the rng-tools version.

> This supports the theory that rngd then can't dominate the entropy
> pool. Instead haveged and rngd would complete to fill the pool.

*compete

> https://tails.boum.org/contribute/design/random/#index5h2
> 
> If rngd isn't enabled or there isn't a hardware RNG, this change
> should not impact the behavior of the entropy pool other then

*other than

Regards,
Yann E. MORIN.

> the pool should be less likely to deplete and cause an entropy
> starve with haveged filling it earlier.
> 
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
> Changes
> v1 -> v2
>  - Rebased on master
>  - verified no CPE ids match
> ---
>  package/haveged/S21haveged | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/haveged/S21haveged b/package/haveged/S21haveged
> index 4b6e6ced4e..22b84e4f27 100644
> --- a/package/haveged/S21haveged
> +++ b/package/haveged/S21haveged
> @@ -3,7 +3,7 @@
>  case "$1" in
>  	start)
>  		printf "Starting haveged: "
> -		start-stop-daemon -S -x /usr/sbin/haveged -- -w 1024 -r 0
> +		start-stop-daemon -S -x /usr/sbin/haveged -- -w 2048 -r 0
>  		[ $? = 0 ] && echo "OK" || echo "FAIL"
>  		;;
>  	stop)
> -- 
> 2.17.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni July 25, 2022, 8:58 a.m. UTC | #2
Hello Matt,

On Mon, 12 Apr 2021 11:47:33 -0500
Matt Weber <matthew.weber@rockwellcollins.com> wrote:

> Adjust the low water mark to wake-up the haveged daemon at the
> same time that rngd would wake-up when a hardware RNG is present.
> 
> This supports the theory that rngd then can't dominate the entropy
> pool. Instead haveged and rngd would complete to fill the pool.
> https://tails.boum.org/contribute/design/random/#index5h2

If I read this link correctly, it doesn't really say anything about
"aligning" the low water mark between haveged and rngd.

While not being random number experts, Arnout and I took advantage of
being in the same room today to have some (random?) discussion about
this patch. Our reasoning is that it is actually desirable to have a
lower low water mark for haveged than rngd.

If you have rngd and a hardware random number generator, using in
priority the hardware random number generator over haveged seems like a
good idea: it provides better random numbers, at less CPU cost. So it's
only if the hardware RNG is too slow that you may want haveged to be
involved and contribute to refilling the entropy pool. But if the
hardware RNG is fast enough compared to the "consumption" of random
numbers by the system, we don't really see why haveged should be
involved. It produces random numbers that are less "good", at at higher
CPU cost.

So overall the default of a low water mark of 2048 for rngd and 1024
for haveged seems to implement exactly what is desirable.

So for now I've marked the patch as Rejected. However should you have
other arguments to back your theory, we would be interested to hear
them and we can always revisit that decision.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/haveged/S21haveged b/package/haveged/S21haveged
index 4b6e6ced4e..22b84e4f27 100644
--- a/package/haveged/S21haveged
+++ b/package/haveged/S21haveged
@@ -3,7 +3,7 @@ 
 case "$1" in
 	start)
 		printf "Starting haveged: "
-		start-stop-daemon -S -x /usr/sbin/haveged -- -w 1024 -r 0
+		start-stop-daemon -S -x /usr/sbin/haveged -- -w 2048 -r 0
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)