DPDK patches and discussions
 help / color / mirror / Atom feed
From: Dan Gora <dg@adax.com>
To: Luca Boccassi <bluca@debian.org>
Cc: Mattias Rönnblom <mattias.ronnblom@ericsson.com>,
	 "dev@dpdk.org" <dev@dpdk.org>,
	David Marchand <david.marchand@redhat.com>,
	 Jerin Jacob <jerinjacobk@gmail.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] eal: resolve getentropy at run time for random seed
Date: Thu, 30 Apr 2020 17:43:51 -0300
Message-ID: <CAGyogRZfrmGNNHLiaR6njBep8JxDx465=WB1eRxHMWervRjP+g@mail.gmail.com> (raw)
In-Reply-To: <f1f83e76a08d238915a2612222f8c93000fae296.camel@debian.org>

On Thu, Apr 30, 2020 at 5:29 PM Luca Boccassi <bluca@debian.org> wrote:

> > > Adding a new dependecy happens only when building with the new version
> > > of the library. If it's not available, then there's no new dependency.
> >
> > But you also do not get to use the new getentropy() if you happen to
> > compile on a system which does not have the latest glibc, or if you
> > use the makefile system..
>
> And that's perfectly fine - backward compatibility workarounds are not
> a problem to me.
>
> > > It sounds to me like you are trying to add workarounds for issues in
> > > your downstream build/deployment model, where your build dependencies
> > > are newer than your runtime dependencies. This in general is rarely
> > > well supported.
> >
> > I am fully aware of that.  I am not adding "workarounds", I am
> > eliminating unnecessary dependencies which probably never should have
> > been introduced in the first place.
>
> It's not unnecessary. It's a better interface, and worth using if
> available.

"if available" is the key phrase here.. It not only has to be
available on  the execution machine, it has to be available on the
compilation machine as well...
>
> > > Now I'm fine with adding workarounds as _fallbacks_ - what I am
> > > explicitly NACKing is forcibly restricting to the least common
> > > denominator because of issues in a third party build/deployment system
> > > that doesn't follow the common norm.
> >
> > ugh.. this is the exact _opposite_ of what this patch does.  It is not
> > restricting anything to a least common denominator.  It is allowing
> > the DPDK to use the "best" available function, regardless of the build
> > system.
> >
> > Restricting to the least common denominator is what the original patch did...
>
> This is restricting to the least common denominator of /dev/urandom,
> which is a bad interface, frail with issues that everybody is moving
> away from, in favour of the programmatic API that this patch is
> removing, in order to fix a corner case with a non-standard, third-
> party build system that downgrades dependencies at runtime vs build
> time.

Well, no, because rdseed is used first if available and /dev/urandom
is used next..

And this is not a corner case at all.. There are lots of linux
distributions which DPDK claims to support which do not support
getentropy().  This is hardly a non-standard build system.  You really
compile and support a separate binary for every possible system that
your customers will use?

And what is this about third parties?  Last I checked DPDK was an API
framework, not a proprietary standalone application.  It is designed,
by definition, to be used by "third parties".  Or does it only have to
work in Intel's toolchains?

>
> > > This is especially true when dealing with RNG APIs, where the tiniest
> > > and most innocent-looking mistake could have disastrous consequences.
> >
> > This does not change the functionality of the RNG at all.  It just
> > makes it work in the way that it was intended.  These changes were
> > only introduced into 19.08, so they are not historical artifacts or
> > anything.
>
> It's reimplementing a syscall using a different interface which has
> different semantics. A small mistake there is going to cost us dearly.

The code was copied almost verbatim from the getentropy() function in
glibc, it's hard to see it going that wrong there.  The code can be
tested using the same test cases that the original patch used.  I
don't see how there is a difference in test coverage here.

If it's such a big deal to use /dev/urandom, then what about my v3
patch which used dlopen()/dlsym() to try to find getentropy()?  Is
that not then acceptable?   dlopen/dlsym() are used in several placed
in DPDK.

thanks
dan

  reply	other threads:[~2020-04-30 20:44 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-21 19:54 [dpdk-dev] [PATCH 0/2] eal: choose initial PRNG seed source at runtime Dan Gora
2020-04-21 19:54 ` [dpdk-dev] [PATCH 1/2] eal: check for rdseed at run time for random seed Dan Gora
2020-04-22  8:22   ` Mattias Rönnblom
2020-04-21 19:54 ` [dpdk-dev] [PATCH 2/2] eal: resolve getentropy " Dan Gora
2020-04-21 21:03   ` Stephen Hemminger
2020-04-21 21:08     ` Dan Gora
2020-04-22  8:28   ` Mattias Rönnblom
2020-04-22 17:44     ` Dan Gora
2020-04-22 20:14       ` Mattias Rönnblom
2020-04-22 20:35         ` Dan Gora
2020-04-23 10:04           ` Luca Boccassi
2020-04-23 17:38             ` Dan Gora
2020-04-27 12:44               ` Luca Boccassi
2020-04-27 16:57                 ` Dan Gora
2020-04-30  8:41                   ` Luca Boccassi
2020-04-30 20:43                     ` Dan Gora [this message]
2020-05-01 10:33                       ` Luca Boccassi
2020-05-01 21:05                         ` Dan Gora
2020-05-04  8:04                           ` Mattias Rönnblom
2020-05-04 14:13                             ` Dan Gora
2020-05-04 14:19                               ` Dan Gora
2020-06-02  5:10                                 ` Dan Gora
2020-06-09 15:37                                   ` Dan Gora
2020-06-10  8:15                                     ` Thomas Monjalon
2020-06-10  8:33                                       ` Luca Boccassi
2020-06-10  8:07                               ` Thomas Monjalon
2020-04-23 12:36           ` Mattias Rönnblom
2020-04-23 17:27             ` Dan Gora
2020-04-21 20:41 ` [dpdk-dev] [PATCH v2 0/2] eal: choose initial PRNG seed source at runtime Dan Gora
2020-04-21 20:41   ` [dpdk-dev] [PATCH v2 1/2] eal: check for rdseed at run time for random seed Dan Gora
2020-04-21 20:41   ` [dpdk-dev] [PATCH v2 2/2] eal: resolve getentropy " Dan Gora
2020-04-22 18:15 ` [dpdk-dev] [PATCH v3 0/2] eal: choose initial PRNG seed source at runtime Dan Gora
2020-04-22 18:15   ` [dpdk-dev] [PATCH v3 1/2] eal: check for rdseed at run time for random seed Dan Gora
2020-04-22 18:15   ` [dpdk-dev] [PATCH v3 2/2] eal: resolve getentropy " Dan Gora
2020-04-22 23:42 ` [dpdk-dev] [PATCH v4 0/2] eal: choose initial PRNG seed source at runtime Dan Gora
2020-04-22 23:42   ` [dpdk-dev] [PATCH v4 1/2] eal: check for rdseed at run time for random seed Dan Gora
2020-04-22 23:42   ` [dpdk-dev] [PATCH v4 2/2] eal: emulate glibc getentropy for initial " Dan Gora
2020-04-23  2:39     ` Stephen Hemminger
2020-04-23 17:42       ` Dan Gora
2020-06-29  9:30     ` Mattias Rönnblom
2020-06-29 17:57       ` Dan Gora
2020-06-29 20:57         ` Mattias Rönnblom
2020-06-29  9:32   ` [dpdk-dev] [PATCH v4 0/2] eal: choose initial PRNG seed source at runtime Mattias Rönnblom
2020-06-29 18:01     ` Dan Gora
2020-06-29 18:04       ` Dan Gora
2020-06-29 21:05       ` Mattias Rönnblom
2020-06-29 21:14         ` Dan Gora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGyogRZfrmGNNHLiaR6njBep8JxDx465=WB1eRxHMWervRjP+g@mail.gmail.com' \
    --to=dg@adax.com \
    --cc=bluca@debian.org \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jerinjacobk@gmail.com \
    --cc=mattias.ronnblom@ericsson.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git