[ovs-discuss] High OpenFlow upcall RTTs introduced between 2.0 and 2.1?

[Ben Pfaff]

On Wed, Apr 30, 2014 at 04:45:00PM +0200, Christian Stigen Larsen wrote:
> 
> Ben Pfaff <blp at nicira.com>:
> >> RTTs were up from an average of 87 ms to 541 ms.
> > 
> > Hmm.  I thought about this for a minute longer.
> > 
> > Can you try applying this patch and see if it makes any difference?
> 
> Ah, I hadn't tested 2.2! (Isn't that in-development, as in "don't rely on it"?)
> 
> 2.2 is definitely fine -- I had only tested 2.0 and 2.1, and the patch doesn't
> apply on 2.1.  But you do point-releases on 2.1?  I see ofproto-dpif.c has
> significant changes from 2.1 to 2.2, so I wasn't able to test any more.

Ah, you're right.

It looks like branch-2.1 needs a commit that only made it onto master.
The log message didn't make it clear that it was a bug fix.

Please try applying commit cfc50ae514f805dcd9c14589f21158185424daf6 to
your local copy of OVS 2.1.  For convenience, I'm enclosing a copy
below.  If it fixes the problem, I'll apply it to branch-2.1 and it
will be in the next point release.

--8<--------------------------cut here-------------------------->8--

>From cfc50ae514f805dcd9c14589f21158185424daf6 Mon Sep 17 00:00:00 2001
From: Alex Wang <alexw at nicira.com>
Date: Thu, 17 Apr 2014 12:24:45 -0700
Subject: [PATCH] ofproto-dpif: Use sequence number to wake up main thread for
 packet-in I/O.

This commit adds per 'struct ofproto_dpif' sequence number for
packet-in I/O.  Whenever ofproto_dpif_send_packet_in() is called,
the calling thread will change the sequence number to wake up the
main thread.

Signed-off-by: Alex Wang <alexw at nicira.com>
Acked-by: Joe Stringer <joestringer at nicira.com>
---
 ofproto/ofproto-dpif.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 30cbb24..0407302 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -317,6 +317,8 @@ struct ofproto_dpif {
 
     /* Work queues. */
     struct guarded_list pins;      /* Contains "struct ofputil_packet_in"s. */
+    struct seq *pins_seq;          /* For notifying 'pins' reception. */
+    uint64_t pins_seqno;
 };
 
 /* All existing ofproto_dpif instances, indexed by ->up.name. */
@@ -388,6 +390,9 @@ ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto,
         free(CONST_CAST(void *, pin->up.packet));
         free(pin);
     }
+
+    /* Wakes up main thread for packet-in I/O. */
+    seq_change(ofproto->pins_seq);
 }
 
 /* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
@@ -1160,6 +1165,9 @@ construct(struct ofproto *ofproto_)
     sset_init(&ofproto->port_poll_set);
     ofproto->port_poll_errno = 0;
     ofproto->change_seq = 0;
+    ofproto->pins_seq = seq_create();
+    ofproto->pins_seqno = seq_read(ofproto->pins_seq);
+
 
     SHASH_FOR_EACH_SAFE (node, next, &init_ofp_ports) {
         struct iface_hint *iface_hint = node->data;
@@ -1333,6 +1341,8 @@ destruct(struct ofproto *ofproto_)
     ovs_mutex_destroy(&ofproto->stats_mutex);
     ovs_mutex_destroy(&ofproto->vsp_mutex);
 
+    seq_destroy(ofproto->pins_seq);
+
     close_dpif_backer(ofproto->backer);
 }
 
@@ -1365,6 +1375,12 @@ run(struct ofproto *ofproto_)
         }
     }
 
+    /* Always updates the ofproto->pins_seqno to avoid frequent wakeup during
+     * flow restore.  Even though nothing is processed during flow restore,
+     * all queued 'pins' will be handled immediately when flow restore
+     * completes. */
+    ofproto->pins_seqno = seq_read(ofproto->pins_seq);
+
     if (ofproto->netflow) {
         netflow_run(ofproto->netflow);
     }
@@ -1474,6 +1490,7 @@ wait(struct ofproto *ofproto_)
     }
 
     seq_wait(udpif_dump_seq(ofproto->backer->udpif), ofproto->dump_seq);
+    seq_wait(ofproto->pins_seq, ofproto->pins_seqno);
 }
 
 static void
-- 
1.7.10.4