diff mbox series

[ovs-dev] ct-zones: reserve zones for upcoming ports

Message ID 20240723065716.165340-1-mansi.sharma@nutanix.com
State Changes Requested
Headers show
Series [ovs-dev] ct-zones: reserve zones for upcoming ports | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Mansi Sharma July 23, 2024, 6:57 a.m. UTC
This change will be useful for migration cases, where it can be
used to sync ct-entries before port is up on new chassis,
resulting in reduced network package drops.
It also fulfills the need of any other service which might need
advance ct-zone reservation in future.

Signed-off-by: Mansi Sharma <mansi.sharma@nutanix.com>
---
 controller/ct-zone.c        | 24 +++++++++-
 controller/ct-zone.h        |  3 +-
 controller/ovn-controller.c |  2 +-
 tests/ovn-controller.at     | 92 ++++++++++++++++++++++++++++++++++---
 4 files changed, 111 insertions(+), 10 deletions(-)

Comments

0-day Robot July 23, 2024, 7:20 a.m. UTC | #1
References:  <20240723065716.165340-1-mansi.sharma@nutanix.com>
 

Bleep bloop.  Greetings Mansi Sharma, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: The subject summary should start with a capital.
WARNING: The subject summary should end with a dot.
Subject: ct-zones: reserve zones for upcoming ports
WARNING: Line has trailing whitespace
#39 FILE: controller/ct-zone.c:109:
     * chassis and might need ct-zone reservation in advance. 

WARNING: Line is 81 characters long (recommended limit is 79)
#47 FILE: controller/ct-zone.c:117:
            char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list);

WARNING: Line has trailing whitespace
#50 FILE: controller/ct-zone.c:120:
            for (reserve_port = strtok_r(duplicate_reserve_list, ",", 

WARNING: Line has trailing whitespace
#51 FILE: controller/ct-zone.c:121:
                 &save_ptr); reserve_port != NULL; 

Lines checked: 222, Warnings: 6, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Mansi Sharma Aug. 6, 2024, 4:28 a.m. UTC | #2
Ping for review.

From: Mansi Sharma <mansi.sharma@nutanix.com>
Date: Tuesday, 23 July 2024 at 12:27 PM
To: dev@openvswitch.org <dev@openvswitch.org>
Cc: Mansi Sharma <mansi.sharma@nutanix.com>
Subject: [PATCH ovn] ct-zones: reserve zones for upcoming ports
This change will be useful for migration cases, where it can be
used to sync ct-entries before port is up on new chassis,
resulting in reduced network package drops.
It also fulfills the need of any other service which might need
advance ct-zone reservation in future.

Signed-off-by: Mansi Sharma <mansi.sharma@nutanix.com>
---
 controller/ct-zone.c        | 24 +++++++++-
 controller/ct-zone.h        |  3 +-
 controller/ovn-controller.c |  2 +-
 tests/ovn-controller.at     | 92 ++++++++++++++++++++++++++++++++++---
 4 files changed, 111 insertions(+), 10 deletions(-)

diff --git a/controller/ct-zone.c b/controller/ct-zone.c
index e4f66a52a..a3c5b90ff 100644
--- a/controller/ct-zone.c
+++ b/controller/ct-zone.c
@@ -89,7 +89,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx,

 void
 ct_zones_update(const struct sset *local_lports,
-                const struct hmap *local_datapaths, struct ct_zone_ctx *ctx)
+                const struct hmap *local_datapaths, struct ct_zone_ctx *ctx,
+                struct ovsrec_open_vswitch_table *ovs_table)
 {
     struct simap_node *ct_zone;
     int scan_start = 1;
@@ -104,6 +105,27 @@ ct_zones_update(const struct sset *local_lports,
         sset_add(&all_users, local_lport);
     }

+    /* Add local_loprt name which are supposed to come up on the
+     * chassis and might need ct-zone reservation in advance.
+     * The data is picked up from ovs_vswitch table. */
+    const struct ovsrec_open_vswitch *cfg;
+    cfg = ovsrec_open_vswitch_table_first(ovs_table);
+    if (cfg) {
+        char *reserve_ct_zone_request_list = smap_get(&cfg->external_ids,
+                                                      "reserve_ct_zones");
+        if (reserve_ct_zone_request_list) {
+            char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list);
+            char *reserve_port;
+            char *save_ptr = NULL;
+            for (reserve_port = strtok_r(duplicate_reserve_list, ",",
+                 &save_ptr); reserve_port != NULL;
+                 reserve_port = strtok_r(NULL, ",", &save_ptr)) {
+                   sset_add(&all_users, reserve_port);
+            }
+            free(duplicate_reserve_list);
+        }
+    }
+
     /* Local patched datapath (gateway routers) need zones assigned. */
     const struct local_datapath *ld;
     HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
diff --git a/controller/ct-zone.h b/controller/ct-zone.h
index 889bdf2fc..f5a6b793e 100644
--- a/controller/ct-zone.h
+++ b/controller/ct-zone.h
@@ -62,7 +62,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx,
                       const struct ovsrec_bridge *br_int);
 void ct_zones_update(const struct sset *local_lports,
                      const struct hmap *local_datapaths,
-                     struct ct_zone_ctx *ctx);
+                     struct ct_zone_ctx *ctx,
+                     struct ovsrec_open_vswitch_table *ovs_table);
 void ct_zones_commit(const struct ovsrec_bridge *br_int,
                      struct shash *pending_ct_zones);
 void ct_zones_pending_clear_commited(struct shash *pending);
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d6d001b1a..8764dfefa 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2231,7 +2231,7 @@ en_ct_zones_run(struct engine_node *node, void *data)

     ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int);
     ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths,
-                    &ct_zones_data->ctx);
+                    &ct_zones_data->ctx, ovs_table);

     ct_zones_data->recomputed = true;
     engine_set_node_state(node, EN_UPDATED);
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 2cb86dc98..6872dc587 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2529,6 +2529,7 @@ check_ovsdb_zone() {

 check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1
 check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2
+check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4

 check ovn-nbctl lr-add lr0

@@ -2554,17 +2555,25 @@ echo "$ct_zones"

 port1_zone=$(get_zone_num "$ct_zones" ls0-hv1)
 port2_zone=$(get_zone_num "$ct_zones" ls0-hv2)
-
+req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3)
+req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4)
 snat_zone=$(get_zone_num "$ct_zones" lr0_snat)
 echo "snat_zone is $snat_zone"

-check test "$port1_zone" -ne "$port2_zone"
-check test "$port2_zone" -ne "$snat_zone"
-check test "$port1_zone" -ne "$snat_zone"
+zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone))
+for i in $zone_list; do
+    for j in $zone_list; do
+        if ["$i" != "$j"]; then
+            check tests "$i" -ne "$j"
+        fi
+    done
+done

 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])

 # Now purposely request an SNAT zone for lr0 that conflicts with a zone
 # currently assigned to a logical port
@@ -2579,15 +2588,84 @@ echo "$ct_zones"
 port1_zone=$(get_zone_num "$ct_zones" ls0-hv1)
 port2_zone=$(get_zone_num "$ct_zones" ls0-hv2)
 snat_zone=$(get_zone_num "$ct_zones" lr0_snat)
+req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3)
+req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4)

 check test "$snat_zone" -eq "$snat_req_zone"
-check test "$port1_zone" -ne "$port2_zone"
-check test "$port2_zone" -ne "$snat_zone"
-check test "$port1_zone" -ne "$snat_zone"
+
+zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone))
+for i in $zone_list; do
+    for j in $zone_list; do
+        if ["$i" != "$j"]; then
+            check tests "$i" -ne "$j"
+        fi
+    done
+done
+
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
+
+# Add port named ls0-req-hv3 and check if same zone assigned
+# previously get assigned to it this time as well.
+
+check ovn-nbctl lsp-add ls0 ls0-req-hv3
+check ovs-vsctl -- add-port br-int hv3-vif3 -- \
+    set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+
+check ovn-nbctl --wait=hv sync
+
+req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3)
+
+check test "$req_port3_zone -eq $req_port3_zone_new"
+
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
+
+# Checks for two cases after removing entry from ovs_vswitch table -
+# 1. If port is already up, ct-zone should be reserved.
+# 2. If port is not up yet, ct-zone should not be reserved.
+
+check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones
+
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+
+req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3)
+req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4)
+
+check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete"
+check test "$req_port4_zone_after_delete" == ""
+
+# Checks for case when a ct-zone is reserved it comes up on that chassis, and
+# gets deleted, but its persisted in ovs_vswitch table, it should persist the
+# same zone throughout.
+
+check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5
+check ovn-nbctl lsp-add ls0 ls0-req-hv5
+check ovs-vsctl -- add-port br-int hv5-vif5 -- \
+    set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5)
+
+check ovs-vsctl remove interface hv5-vif5 external_ids iface-id
+echo "$ct_zones"
+req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5)
+
+check test "req_port5_zone" -eq "req_port5_zone_new"

 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])

 # Now create a conflict in the OVSDB and restart ovn-controller.

--
2.22.3
Numan Siddique Aug. 6, 2024, 8:53 p.m. UTC | #3
On Tue, Aug 6, 2024 at 12:29 AM Mansi Sharma <mansi.sharma@nutanix.com> wrote:
>
> Ping for review.

Thanks for the patch.  This patch needs a rebase.
And it is failing compilation

---
env REAL_CC="gcc" CHECK="sparse -Wsparse-error -I
/workspace/ovn-tmp/ovs/include/sparse -m64 -I /usr/local/include -I
/usr/include/x86_64-linux-gnu " cgcc -target=x86_64 -DHAVE_CONFIG_H
-I. -I../.. -I ../../include -I ../../include -I ../../ovn -I
./include -I ../../lib -I ./lib -I /workspace/ovn-tmp/ovs/include -I
/workspace/ovn-tmp/ovs/include -I /workspace/ovn-tmp/ovs/lib -I
/workspace/ovn-tmp/ovs/lib -I /workspace/ovn-tmp/ovs -I
/workspace/ovn-tmp/ovs -Wstrict-prototypes -Wall -Wextra
-Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes
-Wmissing-field-initializers -fno-strict-aliasing -Wswitch-bool
-Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare
-Wshift-negative-value -Wduplicated-cond -Wshadow
-Wmultistatement-macros -Wcast-align=strict -Werror -Werror -MT
controller/physical.o -MD -MP -MF $depbase.Tpo -c -o
controller/physical.o ../../controller/physical.c &&\
3918mv -f $depbase.Tpo $depbase.Po
3919../../controller/ovn-controller.c:2234:42: error: incorrect type
in argument 4 (different modifiers)
3920../../controller/ovn-controller.c:2234:42: expected struct
ovsrec_open_vswitch_table *ovs_table
3921../../controller/ovn-controller.c:2234:42: got struct
ovsrec_open_vswitch_table const *ovs_table
3922make[2]: *** [Makefile:2385: controller/ovn-controller.o] Error 1
3923make[2]: *** Waiting for unfinished jobs....
3924make[2]: Leaving directory '/workspace/ovn-tmp/ovn-24.03.90/_build/sub'
3925make[1]: *** [Makefile:1537: all] Error 2
3926make[1]: Leaving directory '/workspace/ovn-tmp/ovn-24.03.90/_build/sub'
3927make: *** [Makefile:2878: distcheck] Error 1
3928+ cat '*/_build/sub/tests/testsuite.log'
3929cat: '*/_build/sub/tests/testsuite.log': No such file or directory
----

Please see the CI run for this patch -
https://github.com/ovsrobot/ovn/actions/runs/10054521567/job/27789433135

Also you need to document  the option "reserve_ct_zones" in
controller/ovn-controller.8.xml

Thanks
Numan

>
> From: Mansi Sharma <mansi.sharma@nutanix.com>
> Date: Tuesday, 23 July 2024 at 12:27 PM
> To: dev@openvswitch.org <dev@openvswitch.org>
> Cc: Mansi Sharma <mansi.sharma@nutanix.com>
> Subject: [PATCH ovn] ct-zones: reserve zones for upcoming ports
> This change will be useful for migration cases, where it can be
> used to sync ct-entries before port is up on new chassis,
> resulting in reduced network package drops.
> It also fulfills the need of any other service which might need
> advance ct-zone reservation in future.
>
> Signed-off-by: Mansi Sharma <mansi.sharma@nutanix.com>
> ---
>  controller/ct-zone.c        | 24 +++++++++-
>  controller/ct-zone.h        |  3 +-
>  controller/ovn-controller.c |  2 +-
>  tests/ovn-controller.at     | 92 ++++++++++++++++++++++++++++++++++---
>  4 files changed, 111 insertions(+), 10 deletions(-)
>
> diff --git a/controller/ct-zone.c b/controller/ct-zone.c
> index e4f66a52a..a3c5b90ff 100644
> --- a/controller/ct-zone.c
> +++ b/controller/ct-zone.c
> @@ -89,7 +89,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx,
>
>  void
>  ct_zones_update(const struct sset *local_lports,
> -                const struct hmap *local_datapaths, struct ct_zone_ctx *ctx)
> +                const struct hmap *local_datapaths, struct ct_zone_ctx *ctx,
> +                struct ovsrec_open_vswitch_table *ovs_table)
>  {
>      struct simap_node *ct_zone;
>      int scan_start = 1;
> @@ -104,6 +105,27 @@ ct_zones_update(const struct sset *local_lports,
>          sset_add(&all_users, local_lport);
>      }
>
> +    /* Add local_loprt name which are supposed to come up on the
> +     * chassis and might need ct-zone reservation in advance.
> +     * The data is picked up from ovs_vswitch table. */
> +    const struct ovsrec_open_vswitch *cfg;
> +    cfg = ovsrec_open_vswitch_table_first(ovs_table);
> +    if (cfg) {
> +        char *reserve_ct_zone_request_list = smap_get(&cfg->external_ids,
> +                                                      "reserve_ct_zones");
> +        if (reserve_ct_zone_request_list) {
> +            char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list);
> +            char *reserve_port;
> +            char *save_ptr = NULL;
> +            for (reserve_port = strtok_r(duplicate_reserve_list, ",",
> +                 &save_ptr); reserve_port != NULL;
> +                 reserve_port = strtok_r(NULL, ",", &save_ptr)) {
> +                   sset_add(&all_users, reserve_port);
> +            }
> +            free(duplicate_reserve_list);
> +        }
> +    }
> +
>      /* Local patched datapath (gateway routers) need zones assigned. */
>      const struct local_datapath *ld;
>      HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
> diff --git a/controller/ct-zone.h b/controller/ct-zone.h
> index 889bdf2fc..f5a6b793e 100644
> --- a/controller/ct-zone.h
> +++ b/controller/ct-zone.h
> @@ -62,7 +62,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx,
>                        const struct ovsrec_bridge *br_int);
>  void ct_zones_update(const struct sset *local_lports,
>                       const struct hmap *local_datapaths,
> -                     struct ct_zone_ctx *ctx);
> +                     struct ct_zone_ctx *ctx,
> +                     struct ovsrec_open_vswitch_table *ovs_table);
>  void ct_zones_commit(const struct ovsrec_bridge *br_int,
>                       struct shash *pending_ct_zones);
>  void ct_zones_pending_clear_commited(struct shash *pending);
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index d6d001b1a..8764dfefa 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2231,7 +2231,7 @@ en_ct_zones_run(struct engine_node *node, void *data)
>
>      ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int);
>      ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths,
> -                    &ct_zones_data->ctx);
> +                    &ct_zones_data->ctx, ovs_table);
>
>      ct_zones_data->recomputed = true;
>      engine_set_node_state(node, EN_UPDATED);
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 2cb86dc98..6872dc587 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -2529,6 +2529,7 @@ check_ovsdb_zone() {
>
>  check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1
>  check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2
> +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4
>
>  check ovn-nbctl lr-add lr0
>
> @@ -2554,17 +2555,25 @@ echo "$ct_zones"
>
>  port1_zone=$(get_zone_num "$ct_zones" ls0-hv1)
>  port2_zone=$(get_zone_num "$ct_zones" ls0-hv2)
> -
> +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3)
> +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4)
>  snat_zone=$(get_zone_num "$ct_zones" lr0_snat)
>  echo "snat_zone is $snat_zone"
>
> -check test "$port1_zone" -ne "$port2_zone"
> -check test "$port2_zone" -ne "$snat_zone"
> -check test "$port1_zone" -ne "$snat_zone"
> +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone))
> +for i in $zone_list; do
> +    for j in $zone_list; do
> +        if ["$i" != "$j"]; then
> +            check tests "$i" -ne "$j"
> +        fi
> +    done
> +done
>
>  OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
>  OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
>  OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
>
>  # Now purposely request an SNAT zone for lr0 that conflicts with a zone
>  # currently assigned to a logical port
> @@ -2579,15 +2588,84 @@ echo "$ct_zones"
>  port1_zone=$(get_zone_num "$ct_zones" ls0-hv1)
>  port2_zone=$(get_zone_num "$ct_zones" ls0-hv2)
>  snat_zone=$(get_zone_num "$ct_zones" lr0_snat)
> +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3)
> +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4)
>
>  check test "$snat_zone" -eq "$snat_req_zone"
> -check test "$port1_zone" -ne "$port2_zone"
> -check test "$port2_zone" -ne "$snat_zone"
> -check test "$port1_zone" -ne "$snat_zone"
> +
> +zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone))
> +for i in $zone_list; do
> +    for j in $zone_list; do
> +        if ["$i" != "$j"]; then
> +            check tests "$i" -ne "$j"
> +        fi
> +    done
> +done
> +
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
> +
> +# Add port named ls0-req-hv3 and check if same zone assigned
> +# previously get assigned to it this time as well.
> +
> +check ovn-nbctl lsp-add ls0 ls0-req-hv3
> +check ovs-vsctl -- add-port br-int hv3-vif3 -- \
> +    set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3
> +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
> +echo "$ct_zones"
> +
> +check ovn-nbctl --wait=hv sync
> +
> +req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3)
> +
> +check test "$req_port3_zone -eq $req_port3_zone_new"
> +
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
> +
> +# Checks for two cases after removing entry from ovs_vswitch table -
> +# 1. If port is already up, ct-zone should be reserved.
> +# 2. If port is not up yet, ct-zone should not be reserved.
> +
> +check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones
> +
> +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
> +echo "$ct_zones"
> +
> +req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3)
> +req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4)
> +
> +check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete"
> +check test "$req_port4_zone_after_delete" == ""
> +
> +# Checks for case when a ct-zone is reserved it comes up on that chassis, and
> +# gets deleted, but its persisted in ovs_vswitch table, it should persist the
> +# same zone throughout.
> +
> +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5
> +check ovn-nbctl lsp-add ls0 ls0-req-hv5
> +check ovs-vsctl -- add-port br-int hv5-vif5 -- \
> +    set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5
> +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
> +echo "$ct_zones"
> +req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5)
> +
> +check ovs-vsctl remove interface hv5-vif5 external_ids iface-id
> +echo "$ct_zones"
> +req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5)
> +
> +check test "req_port5_zone" -eq "req_port5_zone_new"
>
>  OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
>  OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
>  OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
> +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
>
>  # Now create a conflict in the OVSDB and restart ovn-controller.
>
> --
> 2.22.3
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/ct-zone.c b/controller/ct-zone.c
index e4f66a52a..a3c5b90ff 100644
--- a/controller/ct-zone.c
+++ b/controller/ct-zone.c
@@ -89,7 +89,8 @@  ct_zones_restore(struct ct_zone_ctx *ctx,
 
 void
 ct_zones_update(const struct sset *local_lports,
-                const struct hmap *local_datapaths, struct ct_zone_ctx *ctx)
+                const struct hmap *local_datapaths, struct ct_zone_ctx *ctx,
+                struct ovsrec_open_vswitch_table *ovs_table)
 {
     struct simap_node *ct_zone;
     int scan_start = 1;
@@ -104,6 +105,27 @@  ct_zones_update(const struct sset *local_lports,
         sset_add(&all_users, local_lport);
     }
 
+    /* Add local_loprt name which are supposed to come up on the
+     * chassis and might need ct-zone reservation in advance. 
+     * The data is picked up from ovs_vswitch table. */
+    const struct ovsrec_open_vswitch *cfg;
+    cfg = ovsrec_open_vswitch_table_first(ovs_table);
+    if (cfg) {
+        char *reserve_ct_zone_request_list = smap_get(&cfg->external_ids,
+                                                      "reserve_ct_zones");
+        if (reserve_ct_zone_request_list) {
+            char *duplicate_reserve_list = xstrdup(reserve_ct_zone_request_list);
+            char *reserve_port;
+            char *save_ptr = NULL;
+            for (reserve_port = strtok_r(duplicate_reserve_list, ",", 
+                 &save_ptr); reserve_port != NULL; 
+                 reserve_port = strtok_r(NULL, ",", &save_ptr)) {
+                   sset_add(&all_users, reserve_port);
+            }
+            free(duplicate_reserve_list);
+        }
+    }
+
     /* Local patched datapath (gateway routers) need zones assigned. */
     const struct local_datapath *ld;
     HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
diff --git a/controller/ct-zone.h b/controller/ct-zone.h
index 889bdf2fc..f5a6b793e 100644
--- a/controller/ct-zone.h
+++ b/controller/ct-zone.h
@@ -62,7 +62,8 @@  void ct_zones_restore(struct ct_zone_ctx *ctx,
                       const struct ovsrec_bridge *br_int);
 void ct_zones_update(const struct sset *local_lports,
                      const struct hmap *local_datapaths,
-                     struct ct_zone_ctx *ctx);
+                     struct ct_zone_ctx *ctx,
+                     struct ovsrec_open_vswitch_table *ovs_table);
 void ct_zones_commit(const struct ovsrec_bridge *br_int,
                      struct shash *pending_ct_zones);
 void ct_zones_pending_clear_commited(struct shash *pending);
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d6d001b1a..8764dfefa 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2231,7 +2231,7 @@  en_ct_zones_run(struct engine_node *node, void *data)
 
     ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int);
     ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths,
-                    &ct_zones_data->ctx);
+                    &ct_zones_data->ctx, ovs_table);
 
     ct_zones_data->recomputed = true;
     engine_set_node_state(node, EN_UPDATED);
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 2cb86dc98..6872dc587 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2529,6 +2529,7 @@  check_ovsdb_zone() {
 
 check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1
 check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2
+check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4
 
 check ovn-nbctl lr-add lr0
 
@@ -2554,17 +2555,25 @@  echo "$ct_zones"
 
 port1_zone=$(get_zone_num "$ct_zones" ls0-hv1)
 port2_zone=$(get_zone_num "$ct_zones" ls0-hv2)
-
+req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3)
+req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4)
 snat_zone=$(get_zone_num "$ct_zones" lr0_snat)
 echo "snat_zone is $snat_zone"
 
-check test "$port1_zone" -ne "$port2_zone"
-check test "$port2_zone" -ne "$snat_zone"
-check test "$port1_zone" -ne "$snat_zone"
+zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone))
+for i in $zone_list; do
+    for j in $zone_list; do
+        if ["$i" != "$j"]; then
+            check tests "$i" -ne "$j"
+        fi
+    done
+done
 
 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
 
 # Now purposely request an SNAT zone for lr0 that conflicts with a zone
 # currently assigned to a logical port
@@ -2579,15 +2588,84 @@  echo "$ct_zones"
 port1_zone=$(get_zone_num "$ct_zones" ls0-hv1)
 port2_zone=$(get_zone_num "$ct_zones" ls0-hv2)
 snat_zone=$(get_zone_num "$ct_zones" lr0_snat)
+req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3)
+req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4)
 
 check test "$snat_zone" -eq "$snat_req_zone"
-check test "$port1_zone" -ne "$port2_zone"
-check test "$port2_zone" -ne "$snat_zone"
-check test "$port1_zone" -ne "$snat_zone"
+
+zone_list=$(seq(port1_zone, port2_zone, req_port2_zone, req_port4_zone, snat_zone))
+for i in $zone_list; do
+    for j in $zone_list; do
+        if ["$i" != "$j"]; then
+            check tests "$i" -ne "$j"
+        fi
+    done
+done
+
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
+
+# Add port named ls0-req-hv3 and check if same zone assigned
+# previously get assigned to it this time as well.
+
+check ovn-nbctl lsp-add ls0 ls0-req-hv3
+check ovs-vsctl -- add-port br-int hv3-vif3 -- \
+    set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+
+check ovn-nbctl --wait=hv sync
+
+req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3)
+
+check test "$req_port3_zone -eq $req_port3_zone_new"
+
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone])
+
+# Checks for two cases after removing entry from ovs_vswitch table -
+# 1. If port is already up, ct-zone should be reserved.
+# 2. If port is not up yet, ct-zone should not be reserved.
+
+check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones
+
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+
+req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3)
+req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4)
+
+check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete"
+check test "$req_port4_zone_after_delete" == ""
+
+# Checks for case when a ct-zone is reserved it comes up on that chassis, and
+# gets deleted, but its persisted in ovs_vswitch table, it should persist the
+# same zone throughout.
+
+check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5
+check ovn-nbctl lsp-add ls0 ls0-req-hv5
+check ovs-vsctl -- add-port br-int hv5-vif5 -- \
+    set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5)
+
+check ovs-vsctl remove interface hv5-vif5 external_ids iface-id
+echo "$ct_zones"
+req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5)
+
+check test "req_port5_zone" -eq "req_port5_zone_new"
 
 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone])
 OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone])
+OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone])
 
 # Now create a conflict in the OVSDB and restart ovn-controller.