From 561f1c3ee471e3b3212111abeb8239122eb8efd4 Mon Sep 17 00:00:00 2001

From: Numan Siddique <numans@ovn.org>

Date: Sun, 22 Nov 2020 18:47:25 +0530

Subject: [PATCH 02/16] controller: Allow pinctrl thread to handle packet-ins

 when version mismatch with northd.

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")

Acked-by: Flavio Fernandes <flavio@flaviof.com>

Tested-by: Flavio Fernandes <flavio@flaviof.com>

Signed-off-by: Numan Siddique <numans@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 faae787f3..3b41cc390 100644

--- a/controller/ovn-controller.c

+++ b/controller/ovn-controller.c

@@ -2552,25 +2552,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;

@@ -2751,6 +2755,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 dd9fff959..728fb3063 100644

--- a/controller/pinctrl.c

+++ b/controller/pinctrl.c

@@ -3113,6 +3113,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,

@@ -3132,16 +3155,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 8fa4baae9..4b101ec92 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 f771b7563..f56f8a696 100644

--- a/tests/ovn.at

+++ b/tests/ovn.at

@@ -6093,7 +6093,64 @@ AT_CHECK([cat 1.packets | cut -c -48], [0], [expout])

 cat 1.expected | cut -c 53- > expout

 AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout])

-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