diff mbox series

[ovs-dev,v7] ovn-controller: Reserve zones for upcoming ports.

Message ID 20240930064737.31313-1-mansi.sharma@nutanix.com
State Superseded
Headers show
Series [ovs-dev,v7] ovn-controller: 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 fail github build: failed

Commit Message

Mansi Sharma Sept. 30, 2024, 6:47 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>
---
v6->v7
1. Added logic in ct zones runtime handler.
---
 controller/ct-zone.c            |  53 ++++++++++++++-
 controller/ct-zone.h            |   4 +-
 controller/ovn-controller.8.xml |  15 +++-
 controller/ovn-controller.c     |   3 +-
 tests/ovn-controller.at         | 117 +++++++++++++++++++++++++++++---
 5 files changed, 180 insertions(+), 12 deletions(-)

Comments

0-day Robot Sept. 30, 2024, 7 a.m. UTC | #1
References:  <20240930064737.31313-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: Line has trailing whitespace
#111 FILE: controller/ct-zone.h:89:
                                const struct ovsrec_open_vswitch_table 

Lines checked: 315, Warnings: 1, Errors: 0


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

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/controller/ct-zone.c b/controller/ct-zone.c
index 77eb16ac9..a47a1e829 100644
--- a/controller/ct-zone.c
+++ b/controller/ct-zone.c
@@ -183,6 +183,31 @@  ct_zones_update(const struct sset *local_lports,
         sset_add(&all_users, local_lport);
     }
 
+    /* Add local_lport 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) {
+        const char *reserve_ct_zone_request_list = smap_get(
+                &cfg->external_ids, "reserve_ct_zones");
+
+        if (reserve_ct_zone_request_list) {
+            char *dup_reserve = xstrdup(reserve_ct_zone_request_list);
+            char *reserve_port;
+            char *save_ptr = NULL;
+
+            for (reserve_port = strtok_r(dup_reserve, ",", &save_ptr);
+                 reserve_port != NULL;
+                 reserve_port = strtok_r(NULL, ",", &save_ptr)) {
+                   sset_add(&all_users, reserve_port);
+            }
+
+            free(dup_reserve);
+        }
+    }
+
     /* Local patched datapath (gateway routers) need zones assigned. */
     const struct local_datapath *ld;
     HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
@@ -396,9 +421,33 @@  bool
 ct_zone_handle_port_update(struct ct_zone_ctx *ctx,
                            const struct sbrec_port_binding *pb,
                            bool updated, int *scan_start,
-                           int min_ct_zone, int max_ct_zone)
+                           int min_ct_zone, int max_ct_zone,
+                           const struct ovsrec_open_vswitch_table *ovs_table)
 {
     struct shash_node *node = shash_find(&ctx->current, pb->logical_port);
+    bool is_reserved = false;
+    const struct ovsrec_open_vswitch *cfg;
+    cfg = ovsrec_open_vswitch_table_first(ovs_table);
+
+    if (cfg) {
+        const char *reserve_ct_zone_request_list = smap_get(
+                &cfg->external_ids, "reserve_ct_zones");
+
+        if (reserve_ct_zone_request_list) {
+            char *dup_reserve = xstrdup(reserve_ct_zone_request_list);
+            char *reserve_port;
+            char *save_ptr = NULL;
+
+            for (reserve_port = strtok_r(dup_reserve, ",", &save_ptr);
+                 reserve_port != NULL;
+                 reserve_port = strtok_r(NULL, ",", &save_ptr)) {
+                   is_reserved = true;
+                   break;
+            }
+
+            free(dup_reserve);
+        }
+    }
 
     if (node) {
         struct ct_zone *ct_zone = node->data;
@@ -415,6 +464,8 @@  ct_zone_handle_port_update(struct ct_zone_ctx *ctx,
         }
         ct_zone_limit_update(ctx, pb->logical_port, ct_zone_get_pb_limit(pb));
         return true;
+    } else if (node && is_reserved) {
+        return true;
     } else if (node && ct_zone_remove(ctx, node->name)) {
         return true;
     }
diff --git a/controller/ct-zone.h b/controller/ct-zone.h
index 6df03975c..0ae456a61 100644
--- a/controller/ct-zone.h
+++ b/controller/ct-zone.h
@@ -85,7 +85,9 @@  bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
 bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx,
                                 const struct sbrec_port_binding *pb,
                                 bool updated, int *scan_start,
-                                int min_ct_zone, int max_ct_zone);
+                                int min_ct_zone, int max_ct_zone,
+                                const struct ovsrec_open_vswitch_table 
+                                *ovs_table);
 uint16_t ct_zone_find_zone(const struct shash *ct_zones, const char *name);
 void ct_zones_limits_sync(struct ct_zone_ctx *ctx,
                           const struct hmap *local_datapaths,
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index faefa77b9..b070b724c 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -626,7 +626,20 @@ 
           <code>external_ids:ovn-installed-ts</code>.
         </p>
       </dd>
-    </dl>
+
+      <dt>
+        <code>external-ids:reserve_ct_zones</code> in the <code>Bridge</code>
+        table
+      </dt>
+
+      <dd>
+        <p>
+          This key represents list of ports which are supposed to come up on
+          the chassis, and hence need advance reservation of ct-zones.
+          It is comma seprated list of port names.
+        </p>
+      </dd>
+   </dl>
 
     <h1>OVN Southbound Database Usage</h1>
 
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6a7cca673..694f2983d 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2321,7 +2321,8 @@  ct_zones_runtime_data_handler(struct engine_node *node, void *data)
             updated |= ct_zone_handle_port_update(&ct_zones_data->ctx,
                                                   t_lport->pb,
                                                   port_updated, &scan_start,
-                                                  min_ct_zone, max_ct_zone);
+                                                  min_ct_zone, max_ct_zone,
+                                                  ovs_table);
         }
     }
 
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 74bff9035..c2fd35a9c 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2533,8 +2533,14 @@  check_ovsdb_zone() {
     test $ct_zone -eq $db_zone
 }
 
+check_duplicates() {
+    output=$1
+    AT_CHECK([printf "$output" | tr ' ' '\n' | sort | uniq -d | grep .], [1])
+}
+
 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
 
@@ -2560,17 +2566,18 @@  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"
-
 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])
+
+check_duplicates "$ct_zones"
 
 # Now purposely request an SNAT zone for lr0 that conflicts with a zone
 # currently assigned to a logical port
@@ -2585,15 +2592,109 @@  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"
+
+check_duplicates "$ct_zones"
 
 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"
+check_duplicates "$ct_zones"
+
+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])
+
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv3
+check test $? -eq 0
+
+# 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"
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv3
+check test $? -eq 0
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv4
+check test $? -eq 1
+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)
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv5
+check test $? -eq 0
+
+check ovs-vsctl remove interface hv5-vif5 external_ids iface-id
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5)
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv5
+check test $? -eq 0
+check test "$req_port5_zone" -eq "$req_port5_zone_new"
+check_duplicates "$ct_zones"
+
+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])
+
+check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv6
+check ovn-nbctl lsp-add ls0 ls0-req-hv6
+check ovs-vsctl -- add-port br-int hv6-vif6 -- \
+    set interface hv6-vif6 external-ids:iface-id=ls0-req-hv6
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+req_port6_zone=$(get_zone_num "$ct_zones" ls0-req-hv6)
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv6
+check test $? -eq 0
+
+ovn-nbctl --wait=hv lsp-del ls0-req-hv6
+ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
+echo "$ct_zones"
+ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv6
+check test $? -eq 0
+req_port6_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv6)
+check_duplicates "$ct_zones"
+check test "$req_port6_zone" -eq "$req_port6_zone_new"
+check_duplicates "$ct_zones"
 
 # Now create a conflict in the OVSDB and restart ovn-controller.