DPDK patches and discussions
 help / color / mirror / Atom feed
From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
To: Dan Gora <dg@adax.com>
Cc: "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, 23 Apr 2020 12:36:29 +0000
Message-ID: <4afd42ec-3f0e-0ee3-d05f-0adf5a4a80a0@ericsson.com> (raw)
In-Reply-To: <CAGyogRb=uGbBsVgMycgLy93EzLafP78xjOX910Yct_1AoxZORg@mail.gmail.com>

On 2020-04-22 22:35, Dan Gora wrote:
> On Wed, Apr 22, 2020 at 5:14 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-22 19:44, Dan Gora wrote:
>>> On Wed, Apr 22, 2020 at 5:28 AM Mattias Rönnblom
>>> <mattias.ronnblom@ericsson.com> wrote:
>>>> On 2020-04-21 21:54, Dan Gora wrote:
>>>>> The getentropy() function was introduced into glibc v2.25 and so is
>>>>> not available on all supported platforms.  Previously, if DPDK was
>>>>> compiled (using meson) on a system which has getentropy(), it would
>>>>> introduce a dependency on glibc v2.25 which would prevent that binary
>>>>> from running on a system with an older glibc.  Similarly if DPDK was
>>>>> compiled on a system which did not have getentropy(), getentropy()
>>>>> could not be used even if the execution system supported it.
>>>>> Introduce a new static function, __rte_getentropy() which will try to
>>>>> resolve the getentropy() function dynamically using dlopen()/dlsym(),
>>>>> returning failure if the getentropy() function cannot be resolved or
>>>>> if it fails.
>>>> Two other options: providing a DPDK-native syscall wrapper for
>>>> getrandom(), or falling back to reading /dev/urandom. Have you
>>>> considered any of those two options? If so, why do you prefer
>>>> dlopen()/dlsym()?
>>> I didn't give any thought at all to using /dev/urandom.  The goal was
>>> not really to change how the thing worked, just to remove the
>>> dependency on glibc 2.25.
>> /dev/urandom is basically only a different interface to the same
>> underlying mechanism.
>> Such an alternative would look something like:
>> static int
>> getentropy(void *buffer, size_t length)
>> {
>>           int rc = -1;
>>           int old_errno = errno;
>>           int fd;
>>           fd = open("/dev/urandom", O_RDONLY);
>>           if (fd < 0)
>>                   goto out;
>>           if (read(fd, buffer, length) != length)
>>                   goto out_close;
>>           rc = 0;
>> out_close:
>>           close(fd);
>> out:
>>           errno = old_errno;
>>           return rc;
>> }
> That's fine with me, but like I said I wasn't trying to change how any
> of this worked, just work around glibc dependencies.  There seems to
> be some subtle difference between /dev/urandom and /dev/random, but...
> https://protect2.fireeye.com/v1/url?k=1705be57-4b8f6b41-1705fecc-862f14a9365e-bb983def357fdfad&q=1&e=10fec9c1-51b3-4bc3-b77d-7eb39787d007&u=https%3A%2F%2Fpatches-gcc.linaro.org%2Fcomment%2F14484%2F
>>>> Failure to run on old libc seems like a non-issue to me.
>>> Well, again, it's a new dependency that didn't exist before.. We sell
>>> to telco customers, so we have to support 10s of different target
>>> platforms of various ages.  If they update their system, we'd have to
>>> recompile our code to be able to use getentropy().  Similarly, if we
>>> compiled on a system which has getentropy(), but the target system
>>> doesn't, then they cannot run our binary because of the glibc 2.25
>>> dependency.  That means that we have to have separate versions with
>>> and without getentropy().  It's a maintenance headache for no real
>>> benefit.
>> I'm not sure I follow. Why would you need to recompile DPDK in case they
>> upgrade their system? It sounds like you care about initial seeding,
>> since you want getentropy() if it exists, but then in the next paragraph
>> you want to throw it out, so I'm a little confused.
> Well  _I_ wouldn't but maybe someone wants getentropy() for the
> initial seed.. I assume that's why it was added in the first place..
> For my application we don't care at all.  I just want to get rid of
> this dependency on glibc 2.25 and have the behavior be the same on
> meson and Makefile builds on the same complication system.

The reason for trying to avoid a wall time-based seed as the default is 
that application instances started at the roughly the same time might 
end up having a the same seed, which in turn might impact their behavior 
in an adverse way. For example, random back-off timers may be the same. 
On x86_64, TSC has a high resolution, but on other platforms its 
equivalent the clock rate is much lower.

>> Why doesn't the standard practice of compiling against the oldest
>> supported libc work for you?
> I guess I didn't realize that was "standard practice" but even so it
> still adds an unnecessary restriction on the complication platform.

If DPDK has the policy of attempting to allow DPDK applications compiled 
against one glibc version to run against another, older, version, we can 
go ahead and discuss the details further. That would be up to the tech 
board to decide. I would vote against it.

If the fix was simple, that's one thing. dlopen()/dlsym() doesn't 
qualify as such, nor does a syscall wrapper, as you pointed out.

>>> To my mind, since getentropy() can block it seems like it would
>>> probably be better to just remove it entirely, but I suppose that's up
>>> to the person(s) who put it in in the first place.
>> Maybe I'm wrong, but I found it unlikely that a DPDK application would
>> start before the entropy pool was initialized. After this point,
>> getentropy() will not block. Do you consider this a real problem?
> Not really.. I don't know the original motivations for adding
> getentropy() in the first place.  I assumed that there was some good
> reason.  If there's not we could just back out all of these
> complications and revert commit faf8fd252785ee8b1 (et al...)
> Again, I don't have any dog in the fight about how to get the seed, I
> just wanted to remove the glibc dependency and the different behavior
> for Makefile and meson builds on the same complication system.
> For me, using dlsym() or reading from /dev/[u]random is 6 for half a
> dozen.  Both have precedents in other places in the DPDK code base.  I
> can fix it either way.
> thanks
> dan

  parent reply	other threads:[~2020-04-23 12:36 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
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 [this message]
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:

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

  git send-email \
    --in-reply-to=4afd42ec-3f0e-0ee3-d05f-0adf5a4a80a0@ericsson.com \
    --to=mattias.ronnblom@ericsson.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=dg@adax.com \
    --cc=jerinjacobk@gmail.com \


* 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 \
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:

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