From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0F9EBA0350; Mon, 29 Jun 2020 19:58:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BAFB31BF81; Mon, 29 Jun 2020 19:58:13 +0200 (CEST) Received: from mail-io1-f68.google.com (mail-io1-f68.google.com [209.85.166.68]) by dpdk.org (Postfix) with ESMTP id 702481BF80 for ; Mon, 29 Jun 2020 19:58:12 +0200 (CEST) Received: by mail-io1-f68.google.com with SMTP id m81so18163633ioa.1 for ; Mon, 29 Jun 2020 10:58:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Y1hv/SZ5pLqjU9nBBBWokZldDqM/HuQ6hAExCQvWwxs=; b=ITZAOEnz2YCHymtc1SwjUWouHxssJgpuaUMApN4xctTNeorDS7ogZU81cKIPBHCCaf UdR3ASgISpcDuHZAxG6CaMHW0vWdjosSUEjDO0kt3DNRpfRq9U7v/0FsxXWmMCA7QaD+ RRkm02why9eb9JZTTw2/YIVEO+o3RNALJ4wDy/u7f7K/g26oUgL5zLFWmBIf80Qfj7oZ MAmWkeVwtlXB+fHHDJRvufsBImFjc+wcVOj7ILQL5dbvypu5n2aLNlrduw94zIsU/2d7 E+CNKD4pAgIQcdbmcW+AvZT2AoWwXF3mzUxIVHTLKfoTl3NpDeqs948b4UIWW9DNBasu j8Rg== X-Gm-Message-State: AOAM5300xb2/dsSpjy2M0EFxJBXShyoEW5aHRnBG0z+Kwfwa6q34iEdC a+BVOh0EHVCo0UBgpG0ZJm8kFx6eiTQkilCBRvM= X-Google-Smtp-Source: ABdhPJyWyCVoMfCTNvBZY3IFvFIDt41ck3HIdNuyV3Hv6P4diE5q5QNEd1iADQeZNvMS2LlC/lG0A+Xv0U5dSYary6Q= X-Received: by 2002:a05:6638:14d3:: with SMTP id l19mr18967396jak.25.1593453491617; Mon, 29 Jun 2020 10:58:11 -0700 (PDT) MIME-Version: 1.0 References: <20200421195446.1730-1-dg@adax.com> <20200422234255.7066-1-dg@adax.com> <20200422234255.7066-3-dg@adax.com> In-Reply-To: From: Dan Gora Date: Mon, 29 Jun 2020 14:57:35 -0300 Message-ID: To: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= Cc: "dev@dpdk.org" , David Marchand , Jerin Jacob Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v4 2/2] eal: emulate glibc getentropy for initial random seed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Jun 29, 2020 at 6:30 AM Mattias R=C3=B6nnblom wrote: > > On 2020-04-23 01:42, 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() to emulate the > > glibc getentropy() function by reading from /dev/urandom to remove > > this dependency on the glibc version. > > > > Since __rte_genentropy() should never fail, the rdseed method is > > tried first. > > > > Signed-off-by: Dan Gora > > --- > > lib/librte_eal/common/rte_random.c | 62 ++++++++++++++++++++++++++---= - > > lib/librte_eal/meson.build | 3 -- > > 2 files changed, 54 insertions(+), 11 deletions(-) > > > > diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common= /rte_random.c > > index 2c84c8527..f043adf03 100644 > > --- a/lib/librte_eal/common/rte_random.c > > +++ b/lib/librte_eal/common/rte_random.c > > @@ -7,6 +7,7 @@ > > #endif > > #include > > #include > > +#include > > > > #include > > #include > > @@ -176,20 +177,61 @@ rte_rand_max(uint64_t upper_bound) > > return res; > > } > > > > +/* Emulate glibc getentropy() using /dev/urandom */ > > +static int > > +__rte_getentropy(void *buffer, size_t length) > > +{ > > + uint8_t *start =3D buffer; > > + uint8_t *end; > > + ssize_t bytes; > > + int fd; > > + int rc =3D -1; > > + > > + if (length > 256) { > > + errno =3D EIO; > > > First of all; only the return code is needed, so why bother with errno? > If you would, I suspect it should be rte_errno and not errno (which is > already set). Because, as I thought that I clearly explained in the previous email in this thread: https://www.mail-archive.com/dev@dpdk.org/msg164646.html this function is emulating the getentropy() system call. Since we want it to have to the same semantics as getentropy() and since getentropy() is a system call, it clears and sets errno, just like getentropy(): https://sourceware.org/git/?p=3Dglibc.git;a=3Dblob;f=3Dsysdeps/unix/sysv/li= nux/getentropy.c;h=3D1778632ff1f1fd77019401c3fbaa164c167248b0;hb=3D92dcaa3e= 2f7bf0f7f1c04cd2fb6a317df1a4e225 > > > > + return -1; > > + } > > + > > + fd =3D open("/dev/urandom", O_RDONLY); > > + if (fd < 0) { > > + errno =3D ENODEV; > > > See above. > > > > + return -1; > > + } > > + > > + end =3D start + length; > > + while (start < end) { > > + bytes =3D read(fd, start, end - start); > > + if (bytes < 0) { > > + if (errno =3D=3D EINTR) > > + /* Supposedly cannot be interrupted by > > + * a signal, but just in case... > > + */ > > + continue; > > + else > > + goto out; > > + } > > + if (bytes =3D=3D 0) { > > + /* no more bytes available, should not happen und= er > > + * normal circumstances. > > + */ > > + errno =3D EIO; > > + goto out; > > + } > > + start +=3D bytes; > > + } > > > There's no need for this loop. A /dev/urandom read() is guaranteed to > return as many bytes as requested, up to 256 bytes. See random(4) for > details. It can't be interrupted by a signal? Are you _sure_ that it cannot return less than the requested number of bytes and has been that was forever and always? Why does getentropy() check this then? In the case where it does not fail this error checking makes no difference other than a couple extra instructions. In the case that it does, it saves your bacon. > > > > + rc =3D 0; > > + errno =3D 0; > > > Why are you changing errno? You should never touch errno on success. Because getentropy() does and we are emulating getentropy() and want to have the same semantics: https://sourceware.org/git/?p=3Dglibc.git;a=3Dblob;f=3Dsysdeps/unix/sysv/li= nux/getentropy.c;h=3D1778632ff1f1fd77019401c3fbaa164c167248b0;hb=3D92dcaa3e= 2f7bf0f7f1c04cd2fb6a317df1a4e225 > > > > +out: > > + close(fd); > > + return rc; > > +} > > + > > static uint64_t > > __rte_random_initial_seed(void) > > { > > -#ifdef RTE_LIBEAL_USE_GETENTROPY > > - int ge_rc; > > uint64_t ge_seed; > > > > - ge_rc =3D getentropy(&ge_seed, sizeof(ge_seed)); > > - > > - if (ge_rc =3D=3D 0) > > - return ge_seed; > > -#endif > > #if defined(RTE_ARCH_X86) > > - /* first fallback: rdseed instruction, if available */ > > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED)) { > > unsigned int rdseed_low; > > unsigned int rdseed_high; > > @@ -200,6 +242,10 @@ __rte_random_initial_seed(void) > > ((uint64_t)rdseed_high << 32); > > } > > #endif > > + /* first fallback: read from /dev/urandom.. */ > > > Remove "..". *sigh*..... thanks dan