Re: [dpdk-dev] [PATCH v4 2/2] eal: emulate glibc getentropy for initial random seed

[Dan Gora <dg@adax.com>]

On Wed, Apr 22, 2020 at 11:39 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed, 22 Apr 2020 20:42:54 -0300
> Dan Gora <dg@adax.com> wrote:
>
> > +     fd = open("/dev/urandom", O_RDONLY);
> > +     if (fd < 0) {
> > +             errno = ENODEV;
> > +             return -1;
> > +     }
> > +
> > +     end = start + length;
> > +     while (start < end) {
> > +             bytes = read(fd, start, end - start);
> > +             if (bytes < 0) {
>
> You are overdoing the complexity here. More error handling is not better.

I've definitely never heard that expression before!

> 1. This should only be called once at startup EINTR is not an issue then
> 2. The amount requested is always returned when using urandom (see man page for random(4))
>
>        The  O_NONBLOCK  flag  has  no effect when opening /dev/urandom.  When calling
>        read(2) for the device /dev/urandom, reads of up to 256 bytes will  return  as
>        many  bytes  as are requested and will not be interrupted by a signal handler.
>        Reads with a buffer over this limit may return less than the requested  number
>        of bytes or fail with the error EINTR, if interrupted by a signal handler.

I didn't just make this up out of whole cloth... This code was lifted,
almost verbatim, from the glibc implementation of getentropy(), which
is the function that we are trying to emulate:

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getentropy.c;h=1778632ff1f1fd77019401c3fbaa164c167248b0;hb=92dcaa3e2f7bf0f7f1c04cd2fb6a317df1a4e225

I assumed that they added this error handling for a reason.

Yes, since this function is only called once at startup EINTR should
not be an issue, but if we need to add __rte_getentropy() as a
generic, exported interface later, that error case would already be
taken care of.

thanks
dan