[ovs-dev] [PATCH] connmgr: support changing openflow versions without restarting
Numan Siddique
numans at ovn.org
Tue Aug 11 10:27:47 UTC 2020
On Sat, Aug 8, 2020 at 3:02 AM Aaron Conole <aconole at redhat.com> wrote:
>
> 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")
> 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>
Thanks Aaron. Tested this patch with OVN and ovn-controller will now
reconnect successfully to the openflow
connection if the supported version is added to the bridge protocol.
Acked-by: Numan Siddique <numans at ovn.org>
Tested-by: Numan Siddique <numans at ovn.org>
Thanks
Numan
> ---
> NOTE: The log on line 608 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 | 25 +++++++++++++++++++------
> tests/bridge.at | 17 +++++++++++++++++
> 2 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index 51d656cba9..b57a381097 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -190,8 +190,9 @@ 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 bool ofservice_reconfigure(struct ofservice *,
> + const struct ofproto_controller *,
> + bool)
> OVS_REQUIRES(ofproto_mutex);
> static void ofservice_create(struct connmgr *mgr, const char *target,
> const struct ofproto_controller *)
> @@ -602,7 +603,14 @@ connmgr_set_controllers(struct connmgr *mgr, struct shash *controllers)
> target);
> ofservice_destroy(ofservice);
> } else {
> - ofservice_reconfigure(ofservice, c);
> + if (ofservice_reconfigure(ofservice, c, true) == false) {
> + char *target_to_restore = xstrdup(target);
> + VLOG_INFO("%s: restarting controller \"%s\" due to version change",
> + mgr->name, target);
> + ofservice_destroy(ofservice);
> + ofservice_create(mgr, target_to_restore, c);
> + free(target_to_restore);
> + }
> }
> }
>
> @@ -1935,7 +1943,7 @@ ofservice_create(struct connmgr *mgr, const char *target,
> ofservice->rconn = rconn;
> ofservice->pvconn = pvconn;
> ofservice->s = *c;
> - ofservice_reconfigure(ofservice, c);
> + (void)ofservice_reconfigure(ofservice, c, false);
>
> VLOG_INFO("%s: added %s controller \"%s\"",
> mgr->name, ofconn_type_to_string(ofservice->type), target);
> @@ -2011,9 +2019,10 @@ ofservice_wait(struct ofservice *ofservice)
> }
> }
>
> -static void
> +static bool
> ofservice_reconfigure(struct ofservice *ofservice,
> - const struct ofproto_controller *settings)
> + const struct ofproto_controller *settings,
> + bool reject_version)
> OVS_REQUIRES(ofproto_mutex)
> {
> /* If the allowed OpenFlow versions change, close all of the existing
> @@ -2021,6 +2030,9 @@ ofservice_reconfigure(struct ofservice *ofservice,
> * version. */
> if (ofservice->s.allowed_versions != settings->allowed_versions) {
> ofservice_close_all(ofservice);
> + if (reject_version) {
> + return false;
> + }
> }
>
> ofservice->s = *settings;
> @@ -2029,6 +2041,7 @@ ofservice_reconfigure(struct ofservice *ofservice,
> LIST_FOR_EACH (ofconn, ofservice_node, &ofservice->conns) {
> ofconn_reconfigure(ofconn, settings);
> }
> + return true;
> }
>
> /* 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list