PTAL. I did not yet delete the stuff support TCP, I can do that in a CL
that will change tcp and random.

I think I fixed all the issues.

On Thu, Feb 4, 2016 at 8:01 AM Barret Rhoden <[email protected]> wrote:

> Hi -
>
> Looks nice, minor comments below.
>
> On 2016-02-04 at 04:01 ron minnich <[email protected]> wrote:
> > This is a series starting with import the code, formatting it,
> > getting it to build, and changing the device.
> >
> >
> > The following changes since commit
> > 47341e4900dead535ab786d887607f8f8e2ea194:
> >
> >   New and easy strace framework. (2016-02-03 18:31:54 -0500)
> >
> > are available in the git repository at:
> >
> >   [email protected]:rminnich/akaros newrandom
>
> > From 3666b7600e0a986e64a7f961674d32aa2fae1cb0 Mon Sep 17 00:00:00 2001
> > From: "Ronald G. Minnich" <[email protected]>
> > Date: Wed, 3 Feb 2016 19:09:02 -0800
> > Subject: Add a random device; remove old genrandom junk; remove random
> from
> >  #cons
>
> > diff --git a/kern/drivers/dev/random.c b/kern/drivers/dev/random.c
> > new file mode 100644
> > index 000000000000..230b7a57495d
> > --- /dev/null
> > +++ b/kern/drivers/dev/random.c
> > @@ -0,0 +1,241 @@
> > +/*
> > + * copyright here.
> > + */
>
> This needs fixed.  AFAIK, urandomread looks like it's from another
> project and the other random bits at the end are from the original
> random.c file.  Though I think we can delete those.
>
> > +#include <vfs.h>
> > +#include <kfs.h>
> > +#include <slab.h>
> > +#include <kmalloc.h>
> > +#include <kref.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <assert.h>
> > +#include <error.h>
> > +#include <cpio.h>
> > +#include <pmap.h>
> > +#include <smp.h>
> > +#include <ip.h>
> > +#include <random/fortuna.h>
> > +
> > +static qlock_t rl;
> > +static spinlock_t tap_lock;
> > +static int len;
> > +
> > +/* While the rule *used to be* that urandom did not block, this rng is
> fast enough
> > + * that we don't think we will need that distinction. I'm not sure
> anything will
> > + * every really block. But some things may want a select-like interface
> anyway. */
>
> checkpatch complained about this:
>
> WARNING: line over 80 characters
> #156: FILE: kern/drivers/dev/random.c:24:
> +/* While the rule *used to be* that urandom did not block, this rng is
> fast enough
>
> WARNING: line over 80 characters
> #158: FILE: kern/drivers/dev/random.c:26:
> + * every really block. But some things may want a select-like interface
> anyway. */
>
> ERROR: trailing whitespace
> #163: FILE: kern/drivers/dev/random.c:31:
> + * hardware entropy source. $
>
>
> > +struct fdtap_slist data_tap;
> > +
> > +/*
> > + * Add entropy. This is not currently used but we might want to hook it
> into a
> > + * hardware entropy source.
> > + */
> > +void random_add(void *xp)
> > +{
> > +     ERRSTACK(1);
> > +
> > +     if (waserror()) {
> > +             qunlock(&rl);
> > +             nexterror();
> > +     }
> > +
> > +     qlock(&rl);
>
> For these funcs, do we want to qlock before the waserror?  With the
> understanding that waserror undoes things done before it (in the simpler
> cases)?  waserror() can handle either style, so I'm fine either way.
>
> > +     fortuna_add_entropy(xp, sizeof(xp));
> > +     qunlock(&rl);
> > +
> > +     poperror();
> > +}
> > +
> > +/*
> > + *  consume random bytes
> > + */
> > +static uint32_t _randomread(void *xp, uint32_t n)
> > +{
> > +     ERRSTACK(1);
> > +
> > +     if (waserror()) {
> > +             qunlock(&rl);
> > +             nexterror();
> > +     }
> > +
> > +     qlock(&rl);
> > +     fortuna_get_bytes(n, xp);
> > +     qunlock(&rl);
> > +
> > +     poperror();
> > +
> > +     return n;
> > +}
> > +
> > +/**
> > + * Fast random generator
> > + **/
> > +uint32_t urandomread(void *xp, uint32_t n)
> > +{
> > +     ERRSTACK(1);
> > +     uint64_t seed[16];
> > +     uint8_t *e, *p;
> > +     uint32_t x = 0;
> > +     uint64_t s0;
> > +     uint64_t s1;
> > +
> > +     if (waserror())
> > +             nexterror();
>
> I think we don't want a waserror() at all here (and thus the poperror
> below).  If all we do is nexterror, we didn't really do anything.
>
> > +
> > +     // The initial seed is from a good random pool.
> > +     _randomread(seed, sizeof(seed));
> > +     p = xp;
> > +     for (e = p + n; p < e;) {
> > +             s0 = seed[x];
> > +             s1 = seed[x = (x + 1) & 15];
> > +             s1 ^= s1 << 31;
> > +             s1 ^= s1 >> 11;
> > +             s0 ^= s0 >> 30;
> > +             *p++ = (seed[x] = s0 ^ s1) * 1181783497276652981LL;
> > +     }
> > +     poperror();
> > +     return n;
> > +}
> > +
>
> > +static struct walkqid *randomwalk(struct chan *c, struct chan *nc, char
> **name,
> > +                                                             int nname)
> > +{
> > +     return devwalk(c, nc, name, nname, randomdir,
> ARRAY_SIZE(randomdir), devgen);
>
> Checkpatch complaint:
>
> WARNING: line over 80 characters
> #270: FILE: kern/drivers/dev/random.c:138:
> +       return devwalk(c, nc, name, nname, randomdir,
> ARRAY_SIZE(randomdir),
> devgen);
>
>
> > +static long randomread(struct chan *c, void *va, long n, int64_t
> ignored)
> > +{
> > +     switch (c->qid.path) {
> > +             case Qdir:
> > +                     return devdirread(c, va, n, randomdir,
> > +                                       ARRAY_SIZE(randomdir), devgen);
> > +             case Qrandom:
> > +                     return _randomread(va, n);
> > +             case Qurandom:
> > +                     return _randomread(va, n);
>
> Should Qurandom be calling urandomread instead of randomread?
>
> > +             default:
> > +                     panic("randomread: qid %d is impossible",
> c->qid.path);
> > +     }
> > +     return -1;      /* not reached */
> > +}
>
> > +/* This is something used by the TCP stack.
> > + * I have no idea of why these numbers were chosen. */
> > +static uint32_t randn;
> > +
> > +static void seedrand(void)
> > +{
> > +     ERRSTACK(2);
> > +     if (!waserror()) {
> > +             _randomread((void *)&randn, sizeof(randn));
> > +             poperror();
> > +     }
> > +}
> > +
> > +int nrand(int n)
> > +{
> > +     if (randn == 0)
> > +             seedrand();
> > +     randn = randn * 1103515245 + 12345 + read_tsc();
>
> This is nasty crap.  I think I hacked in that read_tsc() to give us at
> least some variation in a busted RNG.
>
> I think nrand() is used as a way to get 16 bits of a random number.  We
> can probably change tcp to use urandom or something.  It looks like TCP
> is using nrand for SEQ numbers, so maybe we just give them a number.
>
> > +     return (randn >> 16) % n;
> > +}
> > +
> > +int rand(void)
> > +{
> > +     nrand(1);
> > +     return randn;
> > +}
>
> No one uses this other than a cheap test; I think we can remove all of
> this crap.
>
> > diff --git a/kern/lib/random/fortuna.c b/kern/lib/random/fortuna.c
> > index 868d59b55618..90c86cff0d04 100644
> > --- a/kern/lib/random/fortuna.c
> > +++ b/kern/lib/random/fortuna.c
> > @@ -31,9 +31,12 @@
> >
> >  #include <arch/arch.h>
> >  #include <time.h>
> > +
> > +#include <random/fortuna.h>
> >  #include <random/rijndael.h>
> >  #include <random/sha2.h>
> >
> > +
> >  /*
> >   * Why Fortuna-like: There does not seem to be any definitive reference
> >   * on Fortuna in the net.  Instead this implementation is based on
>
> I think this diff should have been squashed into d111786e69fb ("Get the
> basic random number generator functions to compile").  It's a good git
> exercise to do it. (I can help).
>
> --
> You received this message because you are subscribed to the Google Groups
> "Akaros" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To post to this group, send email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to