| Message ID | 20200205182608.22695-1-matthew.weber@rockwellcollins.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | package/rng-tools: make jitterentropy conditional | expand |
On Wed, 5 Feb 2020 12:26:08 -0600 Matt Weber <matthew.weber@rockwellcollins.com> wrote: > The update of rng-tools from 5 to 6.7 introduced a change where > the jitterentropy library was enabled by default instead of > returning a special 66 return code to hangle the case of no > hwrng. This patch reverts that change and allows a user to > select when to enable the jitterentropy source. The bug > documents an issue of when a hwrng is enabled with jitterentropy > there is a longer boot time. I don't understand this story of the 66 return code. Could you explain a bit more ? > index 11386d1e5d..c0bcffe59e 100644 > --- a/package/rng-tools/rngd.service > +++ b/package/rng-tools/rngd.service > @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon > > [Service] > ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS > +SuccessExitStatus=66 Will this work even if jitterentropy support is enabled ? Thomas
Thomas, On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Wed, 5 Feb 2020 12:26:08 -0600 > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > > The update of rng-tools from 5 to 6.7 introduced a change where > > the jitterentropy library was enabled by default instead of > > returning a special 66 return code to hangle the case of no > > hwrng. This patch reverts that change and allows a user to > > select when to enable the jitterentropy source. The bug > > documents an issue of when a hwrng is enabled with jitterentropy > > there is a longer boot time. > > I don't understand this story of the 66 return code. Could you explain > a bit more ? In Buildroot commit 22cb51e1 the systemd support addition included the successcode change based on the Fedora bugfix for ignoring if no hwrng is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178). > > > index 11386d1e5d..c0bcffe59e 100644 > > --- a/package/rng-tools/rngd.service > > +++ b/package/rng-tools/rngd.service > > @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon > > > > [Service] > > ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS > > +SuccessExitStatus=66 > > Will this work even if jitterentropy support is enabled ? My understanding is that you'd never hit this return case when jitterentropy support built in as it would always init the entropy pool using that library as a source. Thus the problem of the boot delay even when hwrng is present. Matt
Thomas, On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber <matthew.weber@rockwellcollins.com> wrote: > > Thomas, > > On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > > > On Wed, 5 Feb 2020 12:26:08 -0600 > > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > > > > The update of rng-tools from 5 to 6.7 introduced a change where > > > the jitterentropy library was enabled by default instead of > > > returning a special 66 return code to hangle the case of no > > > hwrng. This patch reverts that change and allows a user to > > > select when to enable the jitterentropy source. The bug > > > documents an issue of when a hwrng is enabled with jitterentropy > > > there is a longer boot time. > > > > I don't understand this story of the 66 return code. Could you explain > > a bit more ? > > In Buildroot commit 22cb51e1 the systemd support addition included the > successcode change based on the Fedora bugfix for ignoring if no hwrng > is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178). Ryan Barnett noticed that rng-tools dropped the special return code. I guess we could just drop support for this special case as well.... https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805 > > > > > > index 11386d1e5d..c0bcffe59e 100644 > > > --- a/package/rng-tools/rngd.service > > > +++ b/package/rng-tools/rngd.service > > > @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon > > > > > > [Service] > > > ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS > > > +SuccessExitStatus=66 > > > > Will this work even if jitterentropy support is enabled ? > > My understanding is that you'd never hit this return case when > jitterentropy support built in as it would always init the entropy > pool using that library as a source. Thus the problem of the boot > delay even when hwrng is present. > > Matt
>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: > Thomas, > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber > <matthew.weber@rockwellcollins.com> wrote: >> >> Thomas, >> >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni >> <thomas.petazzoni@bootlin.com> wrote: >> > >> > On Wed, 5 Feb 2020 12:26:08 -0600 >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote: >> > >> > > The update of rng-tools from 5 to 6.7 introduced a change where >> > > the jitterentropy library was enabled by default instead of >> > > returning a special 66 return code to hangle the case of no >> > > hwrng. This patch reverts that change and allows a user to >> > > select when to enable the jitterentropy source. The bug >> > > documents an issue of when a hwrng is enabled with jitterentropy >> > > there is a longer boot time. >> > >> > I don't understand this story of the 66 return code. Could you explain >> > a bit more ? >> >> In Buildroot commit 22cb51e1 the systemd support addition included the >> successcode change based on the Fedora bugfix for ignoring if no hwrng >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178). > Ryan Barnett noticed that rng-tools dropped the special return code. > I guess we could just drop support for this special case as well.... > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805 Has that Fedora patch ever been included upstream? I don't seem to find it in the history. I btw see there is a 6.9 release with more bugfixes, care to send a patch bumping the version?
Peter, On Sat, Feb 8, 2020 at 1:13 PM Peter Korsgaard <peter@korsgaard.com> wrote: > > >>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: > > > Thomas, > > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber > > <matthew.weber@rockwellcollins.com> wrote: > >> > >> Thomas, > >> > >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni > >> <thomas.petazzoni@bootlin.com> wrote: > >> > > >> > On Wed, 5 Feb 2020 12:26:08 -0600 > >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > >> > > >> > > The update of rng-tools from 5 to 6.7 introduced a change where > >> > > the jitterentropy library was enabled by default instead of > >> > > returning a special 66 return code to hangle the case of no > >> > > hwrng. This patch reverts that change and allows a user to > >> > > select when to enable the jitterentropy source. The bug > >> > > documents an issue of when a hwrng is enabled with jitterentropy > >> > > there is a longer boot time. > >> > > >> > I don't understand this story of the 66 return code. Could you explain > >> > a bit more ? > >> > >> In Buildroot commit 22cb51e1 the systemd support addition included the > >> successcode change based on the Fedora bugfix for ignoring if no hwrng > >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178). > > > Ryan Barnett noticed that rng-tools dropped the special return code. > > I guess we could just drop support for this special case as well.... > > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805 > > Has that Fedora patch ever been included upstream? I don't seem to find > it in the history. It doesn't look like it. I'm wondering if we should just drop this systemd unit return behavior and put a comment in the service file about enabling an entropy package if this service fails without a hardware rng present. Seems too complicated to try and cover this special case. > > I btw see there is a 6.9 release with more bugfixes, care to send a > patch bumping the version? I'll review the one that was just sent. I also ping'd the original bug report to see if they could test with this bump as there are a number of minor jitterentropy related fixes. http://patchwork.ozlabs.org/patch/1235396/ Matt
Hi Matt, On Tue, Feb 11, 2020 at 4:06 PM Matthew Weber <matthew.weber@rockwellcollins.com> wrote: > > Peter, > > > On Sat, Feb 8, 2020 at 1:13 PM Peter Korsgaard <peter@korsgaard.com> wrote: > > > > >>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: > > > > > Thomas, > > > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber > > > <matthew.weber@rockwellcollins.com> wrote: > > >> > > >> Thomas, > > >> > > >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni > > >> <thomas.petazzoni@bootlin.com> wrote: > > >> > > > >> > On Wed, 5 Feb 2020 12:26:08 -0600 > > >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > >> > > > >> > > The update of rng-tools from 5 to 6.7 introduced a change where > > >> > > the jitterentropy library was enabled by default instead of > > >> > > returning a special 66 return code to hangle the case of no > > >> > > hwrng. This patch reverts that change and allows a user to > > >> > > select when to enable the jitterentropy source. The bug > > >> > > documents an issue of when a hwrng is enabled with jitterentropy > > >> > > there is a longer boot time. > > >> > > > >> > I don't understand this story of the 66 return code. Could you explain > > >> > a bit more ? > > >> > > >> In Buildroot commit 22cb51e1 the systemd support addition included the > > >> successcode change based on the Fedora bugfix for ignoring if no hwrng > > >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178). > > > > > Ryan Barnett noticed that rng-tools dropped the special return code. > > > I guess we could just drop support for this special case as well.... > > > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805 > > > > Has that Fedora patch ever been included upstream? I don't seem to find > > it in the history. > > It doesn't look like it. I'm wondering if we should just drop this > systemd unit return behavior and put a comment in the service file > about enabling an entropy package if this service fails without a > hardware rng present. Seems too complicated to try and cover this > special case. > > > > > I btw see there is a 6.9 release with more bugfixes, care to send a > > patch bumping the version? > > I'll review the one that was just sent. I also ping'd the original > bug report to see if they could test with this bump as there are a > number of minor jitterentropy related fixes. > http://patchwork.ozlabs.org/patch/1235396/ We had similar issue today with am335x (kernel 5.4.x). Bumping rng-tools to v6.9 helped. The next bump will be more interesting as rng-tools moves to openssl instead of libgcrypt and this dependency is mandatory. Yegor
Yegor, On Tue, Feb 11, 2020 at 9:22 AM Yegor Yefremov <yegorslists@googlemail.com> wrote: > > Hi Matt, > > On Tue, Feb 11, 2020 at 4:06 PM Matthew Weber > <matthew.weber@rockwellcollins.com> wrote: > > > > Peter, > > > > > > On Sat, Feb 8, 2020 at 1:13 PM Peter Korsgaard <peter@korsgaard.com> wrote: > > > > > > >>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes: > > > > > > > Thomas, > > > > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber > > > > <matthew.weber@rockwellcollins.com> wrote: > > > >> > > > >> Thomas, > > > >> > > > >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni > > > >> <thomas.petazzoni@bootlin.com> wrote: > > > >> > > > > >> > On Wed, 5 Feb 2020 12:26:08 -0600 > > > >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > > >> > > > > >> > > The update of rng-tools from 5 to 6.7 introduced a change where > > > >> > > the jitterentropy library was enabled by default instead of > > > >> > > returning a special 66 return code to hangle the case of no > > > >> > > hwrng. This patch reverts that change and allows a user to > > > >> > > select when to enable the jitterentropy source. The bug > > > >> > > documents an issue of when a hwrng is enabled with jitterentropy > > > >> > > there is a longer boot time. > > > >> > > > > >> > I don't understand this story of the 66 return code. Could you explain > > > >> > a bit more ? > > > >> > > > >> In Buildroot commit 22cb51e1 the systemd support addition included the > > > >> successcode change based on the Fedora bugfix for ignoring if no hwrng > > > >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178). > > > > > > > Ryan Barnett noticed that rng-tools dropped the special return code. > > > > I guess we could just drop support for this special case as well.... > > > > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805 > > > > > > Has that Fedora patch ever been included upstream? I don't seem to find > > > it in the history. > > > > It doesn't look like it. I'm wondering if we should just drop this > > systemd unit return behavior and put a comment in the service file > > about enabling an entropy package if this service fails without a > > hardware rng present. Seems too complicated to try and cover this > > special case. > > > > > > > > I btw see there is a 6.9 release with more bugfixes, care to send a > > > patch bumping the version? > > > > I'll review the one that was just sent. I also ping'd the original > > bug report to see if they could test with this bump as there are a > > number of minor jitterentropy related fixes. > > http://patchwork.ozlabs.org/patch/1235396/ > > We had similar issue today with am335x (kernel 5.4.x). Bumping > rng-tools to v6.9 helped. > Thank you for that feedback, I'll make this patch is superseded as the bump resolved the bug. I'll add a note with this patchwork thread in the bug report as well. Matt
Hello Matt, On Thu, 13 Feb 2020 11:07:36 -0600 Matthew Weber <matthew.weber@rockwellcollins.com> wrote: > > We had similar issue today with am335x (kernel 5.4.x). Bumping > > rng-tools to v6.9 helped. > > Thank you for that feedback, I'll make this patch is superseded as the > bump resolved the bug. I'll add a note with this patchwork thread in > the bug report as well. As I was going through open bug reports, I looked again at https://bugs.busybox.net/show_bug.cgi?id=12511, and I disagree with the conclusion of discarding this patch "package/rng-tools: make jitterentropy conditional". If I understand correctly: - If you have a hardware RNG, the jitterentropy library is not necessary. - If you don't have a hardware RNG, rng-tools will error out, unless jitterentropy support is enabled. So, we really want to make the jitterentropy support in rng-tools optional, and leave it up to the user to enable it if (s)he has no hardware RNG available. And if there's no hardware RNG and no jitterentropy support, rng-tools will error out, the service will fail to start, and that's good. So the trick with the special 66 return code is not necessary. Do you agree ? Best regards, Thomas
Thomas, On Mon, May 18, 2020 at 2:49 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello Matt, > > On Thu, 13 Feb 2020 11:07:36 -0600 > Matthew Weber <matthew.weber@rockwellcollins.com> wrote: > > > > We had similar issue today with am335x (kernel 5.4.x). Bumping > > > rng-tools to v6.9 helped. > > > > Thank you for that feedback, I'll make this patch is superseded as the > > bump resolved the bug. I'll add a note with this patchwork thread in > > the bug report as well. > > As I was going through open bug reports, I looked again at > https://bugs.busybox.net/show_bug.cgi?id=12511, and I disagree with the > conclusion of discarding this patch "package/rng-tools: make > jitterentropy conditional". > > If I understand correctly: > > - If you have a hardware RNG, the jitterentropy library is not > necessary. > Right, jitterentropy is a fall back if there isn't a hardware RNG setup. The performance impacts of having it enabled when there is a hardware RNG were improved between versions and that was the motivation to drop this patch as it didn't seem to still be a bug (having it enabled when it isn't really needed). > - If you don't have a hardware RNG, rng-tools will error out, unless > jitterentropy support is enabled. Correct. > > So, we really want to make the jitterentropy support in rng-tools > optional, and leave it up to the user to enable it if (s)he has no > hardware RNG available. And if there's no hardware RNG and no > jitterentropy support, rng-tools will error out, the service will fail > to start, and that's good. So the trick with the special 66 return code > is not necessary. > > Do you agree ? I agree, we should drop that rngd.service change but still keep the option where a user could add the jitterentropy library if they need it. Regards, Matt
diff --git a/package/rng-tools/Config.in b/package/rng-tools/Config.in index 71514260e6..ddcc221d06 100644 --- a/package/rng-tools/Config.in +++ b/package/rng-tools/Config.in @@ -4,7 +4,6 @@ config BR2_PACKAGE_RNG_TOOLS # pthread_setaffinity_np depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL select BR2_PACKAGE_ARGP_STANDALONE if BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_MUSL - select BR2_PACKAGE_JITTERENTROPY_LIBRARY # For rdrand & darn ligcrypt is required and it's not obvious to users select BR2_PACKAGE_LIBGCRYPT if BR2_i386 || BR2_x86_64 || BR2_powerpc64le select BR2_PACKAGE_LIBSYSFS diff --git a/package/rng-tools/rng-tools.mk b/package/rng-tools/rng-tools.mk index 274079044c..e458fbb8eb 100644 --- a/package/rng-tools/rng-tools.mk +++ b/package/rng-tools/rng-tools.mk @@ -8,7 +8,7 @@ RNG_TOOLS_VERSION = 6.8 RNG_TOOLS_SITE = $(call github,nhorman,$(RNG_TOOLS_NAME),v$(RNG_TOOLS_VERSION)) RNG_TOOLS_LICENSE = GPL-2.0 RNG_TOOLS_LICENSE_FILES = COPYING -RNG_TOOLS_DEPENDENCIES = libsysfs jitterentropy-library host-pkgconf +RNG_TOOLS_DEPENDENCIES = libsysfs host-pkgconf # From git RNG_TOOLS_AUTORECONF = YES @@ -29,6 +29,13 @@ else RNG_TOOLS_CONF_OPTS += --without-libgcrypt endif +ifeq ($(BR2_PACKAGE_JITTERENTROPY_LIBRARY),y) +RNG_TOOLS_DEPENDENCIES += jitterentropy-library +RNG_TOOLS_CONF_OPTS += --enable-jitterentropy +else +RNG_TOOLS_CONF_OPTS += --disable-jitterentropy +endif + define RNG_TOOLS_INSTALL_INIT_SYSV $(INSTALL) -D -m 755 package/rng-tools/S21rngd \ $(TARGET_DIR)/etc/init.d/S21rngd diff --git a/package/rng-tools/rngd.service b/package/rng-tools/rngd.service index 11386d1e5d..c0bcffe59e 100644 --- a/package/rng-tools/rngd.service +++ b/package/rng-tools/rngd.service @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon [Service] ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS +SuccessExitStatus=66 EnvironmentFile=-/etc/default/rngd [Install]
The update of rng-tools from 5 to 6.7 introduced a change where the jitterentropy library was enabled by default instead of returning a special 66 return code to hangle the case of no hwrng. This patch reverts that change and allows a user to select when to enable the jitterentropy source. The bug documents an issue of when a hwrng is enabled with jitterentropy there is a longer boot time. Fixes: https://bugs.busybox.net/show_bug.cgi?id=12511 Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com> --- package/rng-tools/Config.in | 1 - package/rng-tools/rng-tools.mk | 9 ++++++++- package/rng-tools/rngd.service | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)