[ovs-dev] [PATCH ovn] controller: Allow pinctrl thread to handle packet-ins when version mismatch with northd.
numans at ovn.org
numans at ovn.org
Sun Nov 22 14:44:58 UTC 2020
From: Numan Siddique <numans at ovn.org>
The commit 1dd27ea7aea4 added the support to pin ovn-controller to a
specific version of ovn-northd. This patch did not handle the
scenario the packet-in scenario when ovn-controller is started and it
detects version mismatch with ovn-northd. In this case, pinctrl
thread is not configured with the proper integration bridge name,
because of which it cannot handle any packet-ins.
This patch addresses this scenario. This is required so that any
existing VMs do not loose DHCP address during renewal.
Fixes: 1dd27ea7aea4("Provide the option to pin ovn-controller and ovn-northd to a specific version")
Signed-off-by: Numan Siddique <numans at ovn.org>
---
controller/ovn-controller.c | 27 ++++++++++++-----
controller/pinctrl.c | 34 ++++++++++++++-------
controller/pinctrl.h | 2 +-
tests/ovn.at | 59 ++++++++++++++++++++++++++++++++++++-
4 files changed, 102 insertions(+), 20 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 41e2678cf4..61de0af8d9 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2648,25 +2648,29 @@ main(int argc, char *argv[])
engine_set_context(&eng_ctx);
- if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) &&
+ bool northd_version_match =
check_northd_version(ovs_idl_loop.idl, ovnsb_idl_loop.idl,
- ovn_version)) {
+ ovn_version);
+
+ const struct ovsrec_bridge_table *bridge_table =
+ ovsrec_bridge_table_get(ovs_idl_loop.idl);
+ const struct ovsrec_open_vswitch_table *ovs_table =
+ ovsrec_open_vswitch_table_get(ovs_idl_loop.idl);
+ const struct ovsrec_bridge *br_int =
+ process_br_int(ovs_idl_txn, bridge_table, ovs_table);
+
+ if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) &&
+ northd_version_match) {
/* Contains the transport zones that this Chassis belongs to */
struct sset transport_zones = SSET_INITIALIZER(&transport_zones);
sset_from_delimited_string(&transport_zones,
get_transport_zones(ovsrec_open_vswitch_table_get(
ovs_idl_loop.idl)), ",");
- const struct ovsrec_bridge_table *bridge_table =
- ovsrec_bridge_table_get(ovs_idl_loop.idl);
- const struct ovsrec_open_vswitch_table *ovs_table =
- ovsrec_open_vswitch_table_get(ovs_idl_loop.idl);
const struct sbrec_chassis_table *chassis_table =
sbrec_chassis_table_get(ovnsb_idl_loop.idl);
const struct sbrec_chassis_private_table *chassis_pvt_table =
sbrec_chassis_private_table_get(ovnsb_idl_loop.idl);
- const struct ovsrec_bridge *br_int =
- process_br_int(ovs_idl_txn, bridge_table, ovs_table);
const char *chassis_id = get_ovs_chassis_id(ovs_table);
const struct sbrec_chassis *chassis = NULL;
const struct sbrec_chassis_private *chassis_private = NULL;
@@ -2836,6 +2840,13 @@ main(int argc, char *argv[])
}
}
+ if (!northd_version_match && br_int) {
+ /* Set the integration bridge name to pinctrl so that the pinctrl
+ * thread can handle any packet-ins when we are not processing
+ * any DB updates due to version mismatch. */
+ pinctrl_set_br_int_name(br_int->name);
+ }
+
unixctl_server_run(unixctl);
unixctl_server_wait(unixctl);
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index a5236564b4..c9dd5ea301 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -3114,6 +3114,29 @@ pinctrl_handler(void *arg_)
return NULL;
}
+static void
+pinctrl_set_br_int_name_(char *br_int_name)
+{
+ if (br_int_name && (!pinctrl.br_int_name || strcmp(pinctrl.br_int_name,
+ br_int_name))) {
+ if (pinctrl.br_int_name) {
+ free(pinctrl.br_int_name);
+ }
+ pinctrl.br_int_name = xstrdup(br_int_name);
+ /* Notify pinctrl_handler that integration bridge is
+ * set/changed. */
+ notify_pinctrl_handler();
+ }
+}
+
+void
+pinctrl_set_br_int_name(char *br_int_name)
+{
+ ovs_mutex_lock(&pinctrl_mutex);
+ pinctrl_set_br_int_name_(br_int_name);
+ ovs_mutex_unlock(&pinctrl_mutex);
+}
+
/* Called by ovn-controller. */
void
pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
@@ -3133,16 +3156,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct sset *active_tunnels)
{
ovs_mutex_lock(&pinctrl_mutex);
- if (br_int && (!pinctrl.br_int_name || strcmp(pinctrl.br_int_name,
- br_int->name))) {
- if (pinctrl.br_int_name) {
- free(pinctrl.br_int_name);
- }
- pinctrl.br_int_name = xstrdup(br_int->name);
- /* Notify pinctrl_handler that integration bridge is
- * set/changed. */
- notify_pinctrl_handler();
- }
+ pinctrl_set_br_int_name_(br_int->name);
run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
sbrec_port_binding_by_key,
sbrec_mac_binding_by_lport_ip);
diff --git a/controller/pinctrl.h b/controller/pinctrl.h
index 8fa4baae9e..4b101ec925 100644
--- a/controller/pinctrl.h
+++ b/controller/pinctrl.h
@@ -49,5 +49,5 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct sset *active_tunnels);
void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn);
void pinctrl_destroy(void);
-
+void pinctrl_set_br_int_name(char *br_int_name);
#endif /* controller/pinctrl.h */
diff --git a/tests/ovn.at b/tests/ovn.at
index b322b681af..e682f9edf4 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6008,7 +6008,64 @@ expected_dhcp_opts=${boofile}330400000e100104ffffff0003040a00000136040a000001
test_dhcp 19 1 f00000000001 01 0 $ciaddr $offer_ip $request_ip 1 0 ff1000000001 $server_ip 02 $expected_dhcp_opts
compare_dhcp_packets 1
-OVN_CLEANUP([hv1])
+# Test that ovn-controller pinctrl thread handles dhcp requests when it
+# connects to a wrong version of ovn-northd at startup.
+
+# Stop ovn-northd so that we can modify the northd_version.
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as northd-backup
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+northd_version=$(ovn-sbctl get SB_Global . options:northd_internal_version | sed s/\"//g)
+echo "northd version = $northd_version"
+
+check ovn-sbctl set SB_Global . options:northd_internal_version=foo
+
+echo
+echo "__file__:__line__: Stop ovn-controller."
+as hv1
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+echo
+echo "__file__:__line__: Pin ovn-controller to ovn-northd version."
+
+as hv1
+check ovs-vsctl set open . external_ids:ovn-match-northd-version=true
+
+# Start ovn-controller
+as hv1
+start_daemon ovn-controller
+
+OVS_WAIT_UNTIL(
+ [test 1 = $(grep -c "controller version - $northd_version mismatch with northd version - foo" hv1/ovn-controller.log)
+])
+
+reset_pcap_file hv1-vif1 hv1/vif1
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 1.expected
+rm -f 2.expected
+
+# ----------------------------------------------------------------------
+
+offer_ip=`ip_to_hex 10 0 0 4`
+server_ip=`ip_to_hex 10 0 0 1`
+ciaddr=`ip_to_hex 0 0 0 0`
+request_ip=0
+boofile=4308626f6f7466696c65
+expected_dhcp_opts=${boofile}330400000e100104ffffff0003040a00000136040a000001
+test_dhcp 20 1 f00000000001 01 0 $ciaddr $offer_ip $request_ip 1 0 ff1000000001 $server_ip 02 $expected_dhcp_opts
+compare_dhcp_packets 1
+
+as hv1
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
AT_CLEANUP
--
2.28.0
More information about the dev
mailing list