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 <num...@ovn.org> Cc: Flavio Leitner <f...@sysclose.org> Suggested-by: Ben Pfaff <b...@ovn.org> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1782834 Signed-off-by: Aaron Conole <acon...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev