On Fri, May 19, 2017 at 09:05:38AM +0200, Theo Buehler wrote:
> On Mon, May 15, 2017 at 03:49:55PM +0200, Mike Belopuhov wrote:
> > On Sun, May 07, 2017 at 18:59 -0500, Matthew Martin wrote:
> > > RFC 4861 specifies ReachableTime "should be a uniformly distributed
> > > random value between MIN_RANDOM_FACTOR and MAX_RANDOM_FACTOR times
> > > BaseReachableTime milliseconds." I think the author intended to do the
> > > multiplication by (x>>10) outside the mask, but it's still missing a -1.
> > >
> > > - Matthew Martin
> > >
> > >
> > >
> > > diff --git nd6.h nd6.h
> > > index 4274cd4dd07..8eea089a40c 100644
> > > --- nd6.h
> > > +++ nd6.h
> > > @@ -162,8 +162,8 @@ struct llinfo_nd6 {
> > > #define MIN_RANDOM_FACTOR 512 /* 1024 * 0.5 */
> > > #define MAX_RANDOM_FACTOR 1536 /* 1024 * 1.5 */
> > > #define ND_COMPUTE_RTIME(x) \
> > > - (((MIN_RANDOM_FACTOR * (x >> 10)) + (arc4random() & \
> > > - ((MAX_RANDOM_FACTOR - MIN_RANDOM_FACTOR) * (x >> 10)))) /1000)
> > > + (((arc4random() & (MAX_RANDOM_FACTOR - MIN_RANDOM_FACTOR - 1)) \
> > > + + MIN_RANDOM_FACTOR) * (x >> 10) / 1000)
> > >
> > > TAILQ_HEAD(nd_drhead, nd_defrouter);
> > > struct nd_defrouter {
> > >
> >
> > Hmm, how about we use an arc4random_uniform here? It will generate
> > numbers less than the upper bound specified as an argument so I don't
> > need to -1 there.
> >
> > I don't think they wanted to do a multiplication by (x>>10) outside
> > the mask and in this case as well we want arc4random_uniform to operate
> > on an extended range.
> >
> > Does this look good?
>
> Your version is better than what's currently there and would seem to
> match the intention of the code as it was written. Sine there was no
> objection:
>
> ok tb
Are you going to commit this or should I?
>
>
> However, for whatever that's worth, I initially read the cited passage
> the same way as Matthew, but the more I look at it, the more unclear it
> becomes what is actually meant. Your interpretation is definitely
> possible.
>
> DragonFly, FreeBSD and NetBSD all have versions of what we currently
> have, while Linux does something that looks suspiciously like the analog of
>
> arc4random() % (x >> 10) + (x >> 10).
>
