Thanks Dumitru. I agree, that chaning one line in a file doesn’t help much :) This work should be done at the whole file someday.
Regards, Vladislav Odintsov On 1 Feb 2022, at 11:33, Dumitru Ceara <[email protected]<mailto:[email protected]>> wrote: On 1/31/22 18:34, Vladislav Odintsov wrote: After commit [0] the mandatory vtep chassis's option 'is-vtep' has appeared. The upgrade scenario for ovn-controller-vtep was not supported: 'is-vtep' option was set only on vtep chassis creation. If chassis was created prior to a new codebase, it was left intact and HW VTEP connectivity was broken. This commit fixes such scenario and now 'is-vtep' is set for vtep chassis other_config if was not set yet. This patch also adds a related test. 0: https://github.com/ovn-org/ovn/commit/1c360bbd911cab9fadd6df8cd528d992ffa7a998 Fixes: 1c360bbd911c ("Fix basic multicast flows for vxlan (non-vtep) tunnels") Signed-off-by: Vladislav Odintsov <[email protected]<mailto:[email protected]>> Acked-by: Dumitru Ceara <[email protected]<mailto:[email protected]>> --- Thanks for the follow up, still looks good to me! controller-vtep/gateway.c | 5 +++++ tests/ovn-controller-vtep.at<http://ovn-controller-vtep.at> | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/controller-vtep/gateway.c b/controller-vtep/gateway.c index 288772dc4..9fbfc0337 100644 --- a/controller-vtep/gateway.c +++ b/controller-vtep/gateway.c @@ -22,6 +22,7 @@ #include "lib/util.h" #include "openvswitch/vlog.h" #include "lib/ovn-sb-idl.h" +#include "smap.h" #include "vtep/vtep-idl.h" #include "ovn-controller-vtep.h" @@ -117,6 +118,10 @@ revalidate_gateway(struct controller_vtep_ctx *ctx) "false"); sbrec_encap_set_options(chassis_rec->encaps[0], &options); } + if (!smap_get_bool(&chassis_rec->other_config, "is-vtep", false)) { + const struct smap oc = SMAP_CONST1(&oc, "is-vtep", "true"); + sbrec_chassis_set_other_config(chassis_rec, &oc); + } } else { if (gw_node) { VLOG_WARN("Chassis for VTEP physical switch (%s) disappears, " diff --git a/tests/ovn-controller-vtep.at<http://ovn-controller-vtep.at> b/tests/ovn-controller-vtep.at<http://ovn-controller-vtep.at> index b76fa5cef..08e1d13e7 100644 --- a/tests/ovn-controller-vtep.at<http://ovn-controller-vtep.at> +++ b/tests/ovn-controller-vtep.at<http://ovn-controller-vtep.at> @@ -102,6 +102,14 @@ Chassis br-vtep options: {csum="false"} ]) +# checks is-vtep option is in place, remove, check again +AT_CHECK([ovn-sbctl get chassis br-vtep other_config:is-vtep], [0], [dnl +"true" +]) + +check ovn-sbctl remove chassis br-vtep other_config is-vtep +OVS_WAIT_UNTIL([ovn-sbctl get chassis br-vtep other_config:is-vtep]) + I was tempted to comment to use fetch_column/wait_column here but we don't do that in most of the ovn-controller-vtep.at<http://ovn-controller-vtep.at/> file. So, I think it's fine, we might as well be consistent and tackle this change at a later time. # deletes the chassis via ovn-sbctl and check that it is readded back # with the log. AT_CHECK([ovn-sbctl chassis-del br-vtep]) Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
