diff mbox series

[ovs-dev] controller: Allow pinctrl thread to handle packet-ins when version mismatch with northd.

Message ID 20201122144458.3857059-1-numans@ovn.org
State Accepted
Headers show
Series [ovs-dev] controller: Allow pinctrl thread to handle packet-ins when version mismatch with northd. | expand

Commit Message

Numan Siddique Nov. 22, 2020, 2:44 p.m. UTC
From: Numan Siddique <numans@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@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(-)

Comments

Flavio Fernandes Nov. 22, 2020, 10:30 p.m. UTC | #1
Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>>
Tested-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>>


> On Nov 22, 2020, at 9:44 AM, numans@ovn.org wrote:
> 
> From: Numan Siddique <numans@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@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
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Flavio Fernandes Nov. 22, 2020, 10:37 p.m. UTC | #2
Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>>


> On Nov 22, 2020, at 9:44 AM, numans@ovn.org wrote:
> 
> From: Numan Siddique <numans@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@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
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Flavio Fernandes Nov. 22, 2020, 10:37 p.m. UTC | #3
Tested-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>>


> On Nov 22, 2020, at 9:44 AM, numans@ovn.org wrote:
> 
> From: Numan Siddique <numans@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@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
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique Nov. 23, 2020, 6:15 a.m. UTC | #4
On Mon, Nov 23, 2020 at 4:09 AM Flavio Fernandes <flavio@flaviof.com> wrote:
>
> Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>>
> Tested-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>>

Thanks a lot for the review and testing it.

I applied this patch to master.

Numan

>
>
> > On Nov 22, 2020, at 9:44 AM, numans@ovn.org wrote:
> >
> > From: Numan Siddique <numans@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@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
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

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