[ovs-dev] [TCP_CRR 1/6] dpif-linux: Remove poll_fd_woke() optimization from dpif_linux_recv().

[Ben Pfaff]

Maybe the commit message is badly phrased: the change that this
*reverts* yields a 37% benefit, so this patch actually yields a 37%
*slowdown*.  But it's a necessary prerequisite for the later
improvements.

I'll reword the commit message.

On Thu, Nov 24, 2011 at 09:05:08AM -0800, Justin Pettit wrote:
> Wow.  Great catch.
> 
> The description says it stopped calling poll_fd_woke() (which it's
> ultimately doing), but the call in this function is nl_sock_woke().
> 
> It might be nice to explain why this improvement works over a
> theoretical optimization.
> 
> --Justin
> 
> 
> On Nov 22, 2011, at 3:12 PM, Ben Pfaff wrote:
> 
> > This optimization on its own provides about 37% benefit against a
> > load of a single netperf CRR test, but at the same time it penalizes
> > ovs-benchmark by about 11%.  We can get back the CRR performance
> > loss, and more, other ways, so the first step is to revert this
> > patch.
> > ---
> > lib/dpif-linux.c |   47 +++++++++++++++++++++++------------------------
> > 1 files changed, 23 insertions(+), 24 deletions(-)
> > 
> > diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> > index 88193c3..4b4ac55 100644
> > --- a/lib/dpif-linux.c
> > +++ b/lib/dpif-linux.c
> > @@ -1097,39 +1097,38 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall)
> > 
> >     for (i = 0; i < N_UPCALL_SOCKS; i++) {
> >         struct nl_sock *upcall_sock;
> > +        int dp_ifindex;
> > +
> >         dpif->last_read_upcall = (dpif->last_read_upcall + 1) &
> >                                  (N_UPCALL_SOCKS - 1);
> >         upcall_sock = dpif->upcall_socks[dpif->last_read_upcall];
> > 
> > -        if (nl_sock_woke(upcall_sock)) {
> > -            int dp_ifindex;
> > 
> > -            for (;;) {
> > -                struct ofpbuf *buf;
> > -                int error;
> > +        for (;;) {
> > +            struct ofpbuf *buf;
> > +            int error;
> > 
> > -                if (++read_tries > 50) {
> > -                    return EAGAIN;
> > -                }
> > +            if (++read_tries > 50) {
> > +                return EAGAIN;
> > +            }
> > 
> > -                error = nl_sock_recv(upcall_sock, &buf, false);
> > -                if (error == EAGAIN) {
> > -                    break;
> > -                } else if (error) {
> > -                    return error;
> > -                }
> > +            error = nl_sock_recv(upcall_sock, &buf, false);
> > +            if (error == EAGAIN) {
> > +                break;
> > +            } else if (error) {
> > +                return error;
> > +            }
> > 
> > -                error = parse_odp_packet(buf, upcall, &dp_ifindex);
> > -                if (!error
> > -                    && dp_ifindex == dpif->dp_ifindex
> > -                    && dpif->listen_mask & (1u << upcall->type)) {
> > -                    return 0;
> > -                }
> > +            error = parse_odp_packet(buf, upcall, &dp_ifindex);
> > +            if (!error
> > +                && dp_ifindex == dpif->dp_ifindex
> > +                && dpif->listen_mask & (1u << upcall->type)) {
> > +                return 0;
> > +            }
> > 
> > -                ofpbuf_delete(buf);
> > -                if (error) {
> > -                    return error;
> > -                }
> > +            ofpbuf_delete(buf);
> > +            if (error) {
> > +                return error;
> >             }
> >         }
> >     }
> > -- 
> > 1.7.4.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>