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