| Message ID | 1356315256-6572-29-git-send-email-akinobu.mita@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Dec 24, 2012 at 11:14:15AM +0900, Akinobu Mita wrote: > Use more preferable function name which implies using a pseudo-random > number generator. > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Jesse Gross <jesse@nicira.com> > Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com> > Cc: Vlad Yasevich <vyasevich@gmail.com> > Cc: Sridhar Samudrala <sri@us.ibm.com> > Cc: Neil Horman <nhorman@tuxdriver.com> > Cc: Steffen Klassert <steffen.klassert@secunet.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-sctp@vger.kernel.org > Cc: dev@openvswitch.org > Cc: netdev@vger.kernel.org > --- > include/net/red.h | 2 +- > net/802/garp.c | 2 +- > net/openvswitch/actions.c | 2 +- > net/rds/bind.c | 2 +- > net/sctp/socket.c | 2 +- > net/xfrm/xfrm_state.c | 2 +- > 6 files changed, 6 insertions(+), 6 deletions(-) > I'm largely indifferent to this patch, but I kind of feel like its just churn. Whats the real advantage in making this change? I grant that it clearly indicates the type of random number generator we're using at a given call site, But for those using net_random, you probably don't care too much about the source of your random bits. If you did really want true random vs. pseudo-random data, you need to explicitly use the right call. You're previous patch series did good cleanup on differentiating the different random calls, but this just seems like its removing what is otherwise useful indirection. Neil > diff --git a/include/net/red.h b/include/net/red.h > index ef46058..168bb2f 100644 > --- a/include/net/red.h > +++ b/include/net/red.h > @@ -303,7 +303,7 @@ static inline unsigned long red_calc_qavg(const struct red_parms *p, > > static inline u32 red_random(const struct red_parms *p) > { > - return reciprocal_divide(net_random(), p->max_P_reciprocal); > + return reciprocal_divide(prandom_u32(), p->max_P_reciprocal); > } > > static inline int red_mark_probability(const struct red_parms *p, > diff --git a/net/802/garp.c b/net/802/garp.c > index 8456f5d..cf7410d 100644 > --- a/net/802/garp.c > +++ b/net/802/garp.c > @@ -397,7 +397,7 @@ static void garp_join_timer_arm(struct garp_applicant *app) > { > unsigned long delay; > > - delay = (u64)msecs_to_jiffies(garp_join_time) * net_random() >> 32; > + delay = (u64)msecs_to_jiffies(garp_join_time) * prandom_u32() >> 32; > mod_timer(&app->join_timer, jiffies + delay); > } > > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c > index ac2defe..257bc36 100644 > --- a/net/openvswitch/actions.c > +++ b/net/openvswitch/actions.c > @@ -404,7 +404,7 @@ static int sample(struct datapath *dp, struct sk_buff *skb, > a = nla_next(a, &rem)) { > switch (nla_type(a)) { > case OVS_SAMPLE_ATTR_PROBABILITY: > - if (net_random() >= nla_get_u32(a)) > + if (prandom_u32() >= nla_get_u32(a)) > return 0; > break; > > diff --git a/net/rds/bind.c b/net/rds/bind.c > index 637bde5..7f95f4b 100644 > --- a/net/rds/bind.c > +++ b/net/rds/bind.c > @@ -118,7 +118,7 @@ static int rds_add_bound(struct rds_sock *rs, __be32 addr, __be16 *port) > rover = be16_to_cpu(*port); > last = rover; > } else { > - rover = max_t(u16, net_random(), 2); > + rover = max_t(u16, prandom_u32(), 2); > last = rover - 1; > } > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 9e65758..95860aa 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -5899,7 +5899,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr) > > inet_get_local_port_range(&low, &high); > remaining = (high - low) + 1; > - rover = net_random() % remaining + low; > + rover = prandom_u32() % remaining + low; > > do { > rover++; > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > index 3459692..35ddaab 100644 > --- a/net/xfrm/xfrm_state.c > +++ b/net/xfrm/xfrm_state.c > @@ -1546,7 +1546,7 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high) > } else { > u32 spi = 0; > for (h=0; h<high-low+1; h++) { > - spi = low + net_random()%(high-low+1); > + spi = low + prandom_u32() % (high - low + 1); > x0 = xfrm_state_lookup(net, mark, &x->id.daddr, htonl(spi), x->id.proto, x->props.family); > if (x0 == NULL) { > x->id.spi = htonl(spi); > -- > 1.7.11.7 > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2012/12/25 Neil Horman <nhorman@tuxdriver.com>: > On Mon, Dec 24, 2012 at 11:14:15AM +0900, Akinobu Mita wrote: >> Use more preferable function name which implies using a pseudo-random >> number generator. >> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> >> Cc: Jesse Gross <jesse@nicira.com> >> Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com> >> Cc: Vlad Yasevich <vyasevich@gmail.com> >> Cc: Sridhar Samudrala <sri@us.ibm.com> >> Cc: Neil Horman <nhorman@tuxdriver.com> >> Cc: Steffen Klassert <steffen.klassert@secunet.com> >> Cc: Herbert Xu <herbert@gondor.apana.org.au> >> Cc: "David S. Miller" <davem@davemloft.net> >> Cc: linux-sctp@vger.kernel.org >> Cc: dev@openvswitch.org >> Cc: netdev@vger.kernel.org >> --- >> include/net/red.h | 2 +- >> net/802/garp.c | 2 +- >> net/openvswitch/actions.c | 2 +- >> net/rds/bind.c | 2 +- >> net/sctp/socket.c | 2 +- >> net/xfrm/xfrm_state.c | 2 +- >> 6 files changed, 6 insertions(+), 6 deletions(-) >> > I'm largely indifferent to this patch, but I kind of feel like its just churn. > Whats the real advantage in making this change? I grant that it clearly > indicates the type of random number generator we're using at a given call site, > But for those using net_random, you probably don't care too much about > the source of your random bits. If you did really want true random vs. > pseudo-random data, you need to explicitly use the right call. You're previous > patch series did good cleanup on differentiating the different random calls, but > this just seems like its removing what is otherwise useful indirection. I overlooked the importance of net_random() indirection. Thanks for the feedback. I'll leave all net_random() callers as-is in the next version. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 25, 2012 at 08:47:26PM +0900, Akinobu Mita wrote: > 2012/12/25 Neil Horman <nhorman@tuxdriver.com>: > > On Mon, Dec 24, 2012 at 11:14:15AM +0900, Akinobu Mita wrote: > >> Use more preferable function name which implies using a pseudo-random > >> number generator. > >> > >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > >> Cc: Jesse Gross <jesse@nicira.com> > >> Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com> > >> Cc: Vlad Yasevich <vyasevich@gmail.com> > >> Cc: Sridhar Samudrala <sri@us.ibm.com> > >> Cc: Neil Horman <nhorman@tuxdriver.com> > >> Cc: Steffen Klassert <steffen.klassert@secunet.com> > >> Cc: Herbert Xu <herbert@gondor.apana.org.au> > >> Cc: "David S. Miller" <davem@davemloft.net> > >> Cc: linux-sctp@vger.kernel.org > >> Cc: dev@openvswitch.org > >> Cc: netdev@vger.kernel.org > >> --- > >> include/net/red.h | 2 +- > >> net/802/garp.c | 2 +- > >> net/openvswitch/actions.c | 2 +- > >> net/rds/bind.c | 2 +- > >> net/sctp/socket.c | 2 +- > >> net/xfrm/xfrm_state.c | 2 +- > >> 6 files changed, 6 insertions(+), 6 deletions(-) > >> > > I'm largely indifferent to this patch, but I kind of feel like its just churn. > > Whats the real advantage in making this change? I grant that it clearly > > indicates the type of random number generator we're using at a given call site, > > But for those using net_random, you probably don't care too much about > > the source of your random bits. If you did really want true random vs. > > pseudo-random data, you need to explicitly use the right call. You're previous > > patch series did good cleanup on differentiating the different random calls, but > > this just seems like its removing what is otherwise useful indirection. > > I overlooked the importance of net_random() indirection. > Thanks for the feedback. I'll leave all net_random() callers as-is in > the next version. Well, I guess I should qualify my opinion. I find it useful personally (the generation of nonces in many cases can be left to most any pseudo random generator that the system deems is a 'good enough' balance between a fast generator that doesn't block on low entropy and a reasonably secure one that doesn't allow for easy prediction. As those needs and factors change, its nice to have a set point to change them at. If you (or anyone else has a differing opinion, I'm happy to listen to it. Regards Neil > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/red.h b/include/net/red.h index ef46058..168bb2f 100644 --- a/include/net/red.h +++ b/include/net/red.h @@ -303,7 +303,7 @@ static inline unsigned long red_calc_qavg(const struct red_parms *p, static inline u32 red_random(const struct red_parms *p) { - return reciprocal_divide(net_random(), p->max_P_reciprocal); + return reciprocal_divide(prandom_u32(), p->max_P_reciprocal); } static inline int red_mark_probability(const struct red_parms *p, diff --git a/net/802/garp.c b/net/802/garp.c index 8456f5d..cf7410d 100644 --- a/net/802/garp.c +++ b/net/802/garp.c @@ -397,7 +397,7 @@ static void garp_join_timer_arm(struct garp_applicant *app) { unsigned long delay; - delay = (u64)msecs_to_jiffies(garp_join_time) * net_random() >> 32; + delay = (u64)msecs_to_jiffies(garp_join_time) * prandom_u32() >> 32; mod_timer(&app->join_timer, jiffies + delay); } diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c index ac2defe..257bc36 100644 --- a/net/openvswitch/actions.c +++ b/net/openvswitch/actions.c @@ -404,7 +404,7 @@ static int sample(struct datapath *dp, struct sk_buff *skb, a = nla_next(a, &rem)) { switch (nla_type(a)) { case OVS_SAMPLE_ATTR_PROBABILITY: - if (net_random() >= nla_get_u32(a)) + if (prandom_u32() >= nla_get_u32(a)) return 0; break; diff --git a/net/rds/bind.c b/net/rds/bind.c index 637bde5..7f95f4b 100644 --- a/net/rds/bind.c +++ b/net/rds/bind.c @@ -118,7 +118,7 @@ static int rds_add_bound(struct rds_sock *rs, __be32 addr, __be16 *port) rover = be16_to_cpu(*port); last = rover; } else { - rover = max_t(u16, net_random(), 2); + rover = max_t(u16, prandom_u32(), 2); last = rover - 1; } diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 9e65758..95860aa 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -5899,7 +5899,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr) inet_get_local_port_range(&low, &high); remaining = (high - low) + 1; - rover = net_random() % remaining + low; + rover = prandom_u32() % remaining + low; do { rover++; diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 3459692..35ddaab 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -1546,7 +1546,7 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high) } else { u32 spi = 0; for (h=0; h<high-low+1; h++) { - spi = low + net_random()%(high-low+1); + spi = low + prandom_u32() % (high - low + 1); x0 = xfrm_state_lookup(net, mark, &x->id.daddr, htonl(spi), x->id.proto, x->props.family); if (x0 == NULL) { x->id.spi = htonl(spi);
Use more preferable function name which implies using a pseudo-random number generator. Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: Jesse Gross <jesse@nicira.com> Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com> Cc: Vlad Yasevich <vyasevich@gmail.com> Cc: Sridhar Samudrala <sri@us.ibm.com> Cc: Neil Horman <nhorman@tuxdriver.com> Cc: Steffen Klassert <steffen.klassert@secunet.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-sctp@vger.kernel.org Cc: dev@openvswitch.org Cc: netdev@vger.kernel.org --- include/net/red.h | 2 +- net/802/garp.c | 2 +- net/openvswitch/actions.c | 2 +- net/rds/bind.c | 2 +- net/sctp/socket.c | 2 +- net/xfrm/xfrm_state.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)