[ovs-dev] [PATCH v2] connmgr: support changing openflow versions without restarting

Aaron Conole aconole at redhat.com
Wed Aug 12 20:07:55 UTC 2020


When commit a0baa7dfa4fe ("connmgr: Make treatment of active and passive
connections more uniform") was applied, it did not take into account
that a reconfiguration of the allowed_versions setting would require a
reload of the ofservice object (only accomplished via a restart of OvS).

For now, during the reconfigure cycle, we delete the ofservice object and
then recreate it immediately.  A new test is added to ensure we do not
break this behavior again.

Fixes: a0baa7dfa4fe ("connmgr: Make treatment of active and passive connections more uniform")
Cc: Numan Siddique <numans at ovn.org>
Cc: Flavio Leitner <fbl at sysclose.org>
Suggested-by: Ben Pfaff <blp at ovn.org>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1782834
Signed-off-by: Aaron Conole <aconole at redhat.com>
---
v2: Keep most of the original API.

NOTE: The log on line 607 will flag the 0-day robot, but I thought
      for string searching purposes it's better to keep it all one
      line.

 ofproto/connmgr.c | 24 ++++++++++++++++--------
 tests/bridge.at   | 17 +++++++++++++++++
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 51d656cba9..d37e9ff1ab 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -190,8 +190,8 @@ struct ofservice {
 
 static void ofservice_run(struct ofservice *);
 static void ofservice_wait(struct ofservice *);
-static void ofservice_reconfigure(struct ofservice *,
-                                  const struct ofproto_controller *)
+static int ofservice_reconfigure(struct ofservice *,
+                                 const struct ofproto_controller *)
     OVS_REQUIRES(ofproto_mutex);
 static void ofservice_create(struct connmgr *mgr, const char *target,
                              const struct ofproto_controller *)
@@ -602,7 +602,14 @@ connmgr_set_controllers(struct connmgr *mgr, struct shash *controllers)
                       target);
             ofservice_destroy(ofservice);
         } else {
-            ofservice_reconfigure(ofservice, c);
+            if (ofservice_reconfigure(ofservice, c)) {
+                char *target_to_restore = xstrdup(target);
+                VLOG_INFO("%s: Changes to controller \"%s\" expects re-initialization: Re-initializing now.",
+                          mgr->name, target);
+                ofservice_destroy(ofservice);
+                ofservice_create(mgr, target_to_restore, c);
+                free(target_to_restore);
+            }
         }
     }
 
@@ -2011,16 +2018,15 @@ ofservice_wait(struct ofservice *ofservice)
     }
 }
 
-static void
+static int
 ofservice_reconfigure(struct ofservice *ofservice,
                       const struct ofproto_controller *settings)
     OVS_REQUIRES(ofproto_mutex)
 {
-    /* If the allowed OpenFlow versions change, close all of the existing
-     * connections to allow them to reconnect and possibly negotiate a new
-     * version. */
+    /* If the allowed OpenFlow versions change, a full cleanup is needed
+     * for the ofservice and connections. */
     if (ofservice->s.allowed_versions != settings->allowed_versions) {
-        ofservice_close_all(ofservice);
+        return -EINVAL;
     }
 
     ofservice->s = *settings;
@@ -2029,6 +2035,8 @@ ofservice_reconfigure(struct ofservice *ofservice,
     LIST_FOR_EACH (ofconn, ofservice_node, &ofservice->conns) {
         ofconn_reconfigure(ofconn, settings);
     }
+
+    return 0;
 }
 
 /* Finds and returns the ofservice within 'mgr' that has the given
diff --git a/tests/bridge.at b/tests/bridge.at
index d48463e263..904f1381c7 100644
--- a/tests/bridge.at
+++ b/tests/bridge.at
@@ -103,3 +103,20 @@ AT_CHECK([ovs-appctl -t ovs-vswitchd version], [0], [ignore])
 OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 AT_CLEANUP
+
+AT_SETUP([bridge - change ofproto versions])
+dnl Start vswitch and add a version test bridge
+OVS_VSWITCHD_START(
+    [add-br vr_test0 -- \
+     set bridge vr_test0 datapath-type=dummy \
+                         protocols=OpenFlow10])
+
+dnl set the version to include, say, OpenFlow14
+AT_CHECK([ovs-vsctl set bridge vr_test0 protocols=OpenFlow10,OpenFlow14])
+
+dnl now try to use bundle action on a flow
+AT_CHECK([ovs-ofctl add-flow vr_test0 --bundle actions=normal])
+
+OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+AT_CLEANUP
-- 
2.25.4



More information about the dev mailing list