Message ID | 1434488879-10663-1-git-send-email-willemb@google.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, Jun 16, 2015 at 5:07 PM, Willem de Bruijn <willemb@google.com> wrote: > From: Willem de Bruijn <willemb@google.com> > > PACKET_FANOUT_LB computes f->rr_cur such that it is modulo > f->num_members. It returns the old value unconditionally, but > f->num_members may have changed since the last store. This can be > fixed with > > - return cur > + return cur < num ? : 0; Well, that test is bad. Should be return cur < num ? cur : 0. But the patch is more concise, anyway. > > When modifying the logic, simplify it further by replacing the loop > with an unconditional atomic increment. > > Fixes: dc99f600698d ("packet: Add fanout support.") > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> -- 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
Hello. On 6/17/2015 12:07 AM, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > PACKET_FANOUT_LB computes f->rr_cur such that it is modulo > f->num_members. It returns the old value unconditionally, but > f->num_members may have changed since the last store. This can be > fixed with > - return cur > + return cur < num ? : 0; > When modifying the logic, simplify it further by replacing the loop > with an unconditional atomic increment. > Fixes: dc99f600698d ("packet: Add fanout support.") > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> > --- > net/packet/af_packet.c | 19 ++----------------- > 1 file changed, 2 insertions(+), 17 deletions(-) > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index b5989c6..efd35e8 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c [...] > @@ -1293,13 +1283,8 @@ static unsigned int fanout_demux_lb(struct packet_fanout *f, > struct sk_buff *skb, > unsigned int num) > { > - int cur, old; > - > - cur = atomic_read(&f->rr_cur); > - while ((old = atomic_cmpxchg(&f->rr_cur, cur, > - fanout_rr_next(f, num))) != cur) > - cur = old; > - return cur; > + unsigned int val = atomic_inc_return(&f->rr_cur); Please insert an empty line after declaration, as it was before your patch. > + return val % num; > } [...] WBR, Sergei -- 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/net/packet/af_packet.c b/net/packet/af_packet.c index b5989c6..efd35e8 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1272,16 +1272,6 @@ static void packet_sock_destruct(struct sock *sk) sk_refcnt_debug_dec(sk); } -static int fanout_rr_next(struct packet_fanout *f, unsigned int num) -{ - int x = atomic_read(&f->rr_cur) + 1; - - if (x >= num) - x = 0; - - return x; -} - static unsigned int fanout_demux_hash(struct packet_fanout *f, struct sk_buff *skb, unsigned int num) @@ -1293,13 +1283,8 @@ static unsigned int fanout_demux_lb(struct packet_fanout *f, struct sk_buff *skb, unsigned int num) { - int cur, old; - - cur = atomic_read(&f->rr_cur); - while ((old = atomic_cmpxchg(&f->rr_cur, cur, - fanout_rr_next(f, num))) != cur) - cur = old; - return cur; + unsigned int val = atomic_inc_return(&f->rr_cur); + return val % num; } static unsigned int fanout_demux_cpu(struct packet_fanout *f,