[ovs-dev] [PATCH] ofproto: Prefer "master" and "other" connections for snooping over "slave".

[Ben Pfaff]

This makes "ovs-ofctl snoop" and anything else that connects to the
switch's "snoop" socket typically more useful in the presence of multiple
controllers, since the "master" connection is the one with the most
interesting traffic.

Suggested-by: Justin Pettit <jpettit at nicira.com>
---
 ofproto/ofproto.c        |   40 +++++++++++++++++++++++++++++++---------
 utilities/ovs-ofctl.8.in |   12 ++++++++++--
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0ee69ef..dcf8683 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -963,25 +963,47 @@ process_port_change(struct ofproto *ofproto, int error, char *devname)
     }
 }
 
+/* Returns a "preference level" for snooping 'ofconn'.  A higher return value
+ * means that 'ofconn' is more interesting for monitoring than a lower return
+ * value. */
+static int
+snoop_preference(const struct ofconn *ofconn)
+{
+    switch (ofconn->role) {
+    case NX_ROLE_MASTER:
+        return 3;
+    case NX_ROLE_OTHER:
+        return 2;
+    case NX_ROLE_SLAVE:
+        return 1;
+    default:
+        /* Shouldn't happen. */
+        return 0;
+    }
+}
+
 /* One of ofproto's "snoop" pvconns has accepted a new connection on 'vconn'.
  * Connects this vconn to a controller. */
 static void
 add_snooper(struct ofproto *ofproto, struct vconn *vconn)
 {
-    struct ofconn *ofconn;
+    struct ofconn *ofconn, *best;
 
-    /* Arbitrarily pick the first controller in the list for monitoring.  We
-     * could do something smarter or more flexible later, if it ever proves
-     * useful. */
+    /* Pick a controller for monitoring. */
+    best = NULL;
     LIST_FOR_EACH (ofconn, struct ofconn, node, &ofproto->all_conns) {
-        if (ofconn->type == OFCONN_CONTROLLER) {
-            rconn_add_monitor(ofconn->rconn, vconn);
-            return;
+        if (ofconn->type == OFCONN_CONTROLLER
+            && (!best || snoop_preference(ofconn) > snoop_preference(best))) {
+            best = ofconn;
         }
+    }
 
+    if (best) {
+        rconn_add_monitor(best->rconn, vconn);
+    } else {
+        VLOG_INFO_RL(&rl, "no controller connection to snoop");
+        vconn_close(vconn);
     }
-    VLOG_INFO_RL(&rl, "no controller connection to monitor");
-    vconn_close(vconn);
 }
 
 int
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index d84b481..5140f07 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -155,8 +155,16 @@ Thus, this command can be used to view OpenFlow protocol activity
 between a switch and its controller.
 .IP
 When a switch has more than one controller configured, only the
-protocol to and from a single controller, chosen arbitrarily by Open
-vSwitch, is given.  If a switch has no controller configured, or if
+protocol to and from a single controller is given.  If none of the
+controllers is configured as a master or a slave (using a Nicira
+extension to OpenFlow), then a controller is chosen arbitrarily among
+them.  If there is a master controller, it is chosen; otherwise, if
+there are any controllers that are not masters or slaves, one is
+chosen arbitrarily; otherwise, a slave controller is chosen
+arbitrarily.  This choice is made once at connection time and does not
+change as controllers reconfigure their roles.
+.IP
+If a switch has no controller configured, or if
 the configured controller is disconnected, no traffic is sent, so
 monitoring will not show any traffic.
 .
-- 
1.7.1