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
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).
