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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to