controller: Add the capability to specify a min/max value for ct_zone.

Introduce the capability to specify boundaries (max and min values) for the ct_zones dynamically selected by ovn-controller. Acked-by: Ales Musil <[email protected]> Reported-at: https://issues.redhat.com/browse/FDP-383 Signed-off-by: Lorenzo Bianconi <[email protected]> Signed-off-by: Mark Michelson <[email protected]>
ovsrobot · Jul 26, 2024 · f2363f4 · f2363f4
1 parent ee45e12
commit f2363f4
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 14 deletions.
diff --git a/NEWS b/NEWS
@@ -42,6 +42,8 @@ Post v24.03.0
     true, the traffic destined to a logical port of a provider logical switch
     (having a localnet port) will be tunnelled instead of sending it via the
     localnet port.
+  - Added support to define boundaries (min and max values) for selected ct
+    zones.
 
 OVN v24.03.0 - 01 Mar 2024
 --------------------------

diff --git a/controller/ct-zone.c b/controller/ct-zone.c
@@ -14,7 +14,9 @@
  */
 
 #include <config.h>
+#include <errno.h>
 
+#include "chassis.h"
 #include "ct-zone.h"
 #include "local_data.h"
 #include "openvswitch/vlog.h"
@@ -29,7 +31,8 @@ static void ct_zone_add_pending(struct shash *pending_ct_zones,
                                 int zone, bool add, const char *name);
 static int ct_zone_get_snat(const struct sbrec_datapath_binding *dp);
 static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx,
-                                  const char *zone_name, int *scan_start);
+                                  const char *zone_name,
+                                  int *scan_start, int scan_stop);
 static bool ct_zone_remove(struct ct_zone_ctx *ctx,
                            struct simap_node *ct_zone);
 
@@ -87,12 +90,61 @@ ct_zones_restore(struct ct_zone_ctx *ctx,
     }
 }
 
+void
+ct_zones_parse_range(const struct ovsrec_open_vswitch_table *ovs_table,
+                     int *min_ct_zone, int *max_ct_zone)
+{
+    /* Set default values. */
+    *min_ct_zone = 1;
+    *max_ct_zone = MAX_CT_ZONES;
+
+    const struct ovsrec_open_vswitch *cfg =
+        ovsrec_open_vswitch_table_first(ovs_table);
+    if (!cfg) {
+        return;
+    }
+
+    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+    const char *range = get_chassis_external_id_value(&cfg->external_ids,
+                                                      chassis_id,
+                                                      "ct-zone-range", NULL);
+    if (!range) {
+        return;
+    }
+
+    char *ptr = NULL, *tokstr = xstrdup(range);
+    char *range_min = strtok_r(tokstr, "-", &ptr);
+    if (!range_min) {
+        goto out;
+    }
+
+    int min = strtol(range_min, NULL, 10);
+    if (errno == EINVAL || min < 1) {
+        goto out;
+    }
+    *min_ct_zone = min;
+
+    char *range_max = strtok_r(NULL, "-", &ptr);
+    if (!range_max) {
+        goto out;
+    }
+
+    int max = strtol(range_max, NULL, 10);
+    if (errno == EINVAL || max > MAX_CT_ZONES) {
+        goto out;
+    }
+    *max_ct_zone = max;
+out:
+    free(tokstr);
+}
+
 void
 ct_zones_update(const struct sset *local_lports,
+                const struct ovsrec_open_vswitch_table *ovs_table,
                 const struct hmap *local_datapaths, struct ct_zone_ctx *ctx)
 {
+    int min_ct_zone, max_ct_zone;
     struct simap_node *ct_zone;
-    int scan_start = 1;
     const char *user;
     struct sset all_users = SSET_INITIALIZER(&all_users);
     struct simap req_snat_zones = SIMAP_INITIALIZER(&req_snat_zones);
@@ -131,9 +183,12 @@ ct_zones_update(const struct sset *local_lports,
         free(snat);
     }
 
+    ct_zones_parse_range(ovs_table, &min_ct_zone, &max_ct_zone);
+
     /* Delete zones that do not exist in above sset. */
     SIMAP_FOR_EACH_SAFE (ct_zone, &ctx->current) {
-        if (!sset_contains(&all_users, ct_zone->name)) {
+        if (!sset_contains(&all_users, ct_zone->name) ||
+            ct_zone->data < min_ct_zone || ct_zone->data > max_ct_zone) {
             ct_zone_remove(ctx, ct_zone);
         } else if (!simap_find(&req_snat_zones, ct_zone->name)) {
             bitmap_set1(unreq_snat_zones_map, ct_zone->data);
@@ -195,7 +250,7 @@ ct_zones_update(const struct sset *local_lports,
             continue;
         }
 
-        ct_zone_assign_unused(ctx, user, &scan_start);
+        ct_zone_assign_unused(ctx, user, &min_ct_zone, max_ct_zone);
     }
 
     simap_destroy(&req_snat_zones);
@@ -296,11 +351,19 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
 /* Returns "true" if there was an update to the context. */
 bool
 ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name,
-                           bool updated, int *scan_start)
+                           bool updated, int *scan_start,
+                           int min_ct_zone, int max_ct_zone)
 {
     struct simap_node *ct_zone = simap_find(&ctx->current, name);
+
+    if (ct_zone &&
+        (ct_zone->data < min_ct_zone || ct_zone->data > max_ct_zone)) {
+        ct_zone_remove(ctx, ct_zone);
+        ct_zone = NULL;
+    }
+
     if (updated && !ct_zone) {
-        ct_zone_assign_unused(ctx, name, scan_start);
+        ct_zone_assign_unused(ctx, name, scan_start, max_ct_zone);
         return true;
     } else if (!updated && ct_zone_remove(ctx, ct_zone)) {
         return true;
@@ -312,11 +375,11 @@ ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name,
 
 static bool
 ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name,
-                      int *scan_start)
+                      int *scan_start, int scan_stop)
 {
     /* We assume that there are 64K zones and that we own them all. */
-    int zone = bitmap_scan(ctx->bitmap, 0, *scan_start, MAX_CT_ZONES + 1);
-    if (zone == MAX_CT_ZONES + 1) {
+    int zone = bitmap_scan(ctx->bitmap, 0, *scan_start, scan_stop + 1);
+    if (zone == scan_stop + 1) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
         VLOG_WARN_RL(&rl, "exhausted all ct zones");
         return false;

diff --git a/controller/ct-zone.h b/controller/ct-zone.h
@@ -56,11 +56,14 @@ struct ct_zone_pending_entry {
     enum ct_zone_pending_state state;
 };
 
+void ct_zones_parse_range(const struct ovsrec_open_vswitch_table *ovs_table,
+                          int *min_ct_zone, int *max_ct_zone);
 void ct_zones_restore(struct ct_zone_ctx *ctx,
                       const struct ovsrec_open_vswitch_table *ovs_table,
                       const struct sbrec_datapath_binding_table *dp_table,
                       const struct ovsrec_bridge *br_int);
 void ct_zones_update(const struct sset *local_lports,
+                     const struct ovsrec_open_vswitch_table *ovs_table,
                      const struct hmap *local_datapaths,
                      struct ct_zone_ctx *ctx);
 void ct_zones_commit(const struct ovsrec_bridge *br_int,
@@ -69,6 +72,7 @@ void ct_zones_pending_clear_commited(struct shash *pending);
 bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
                               const struct sbrec_datapath_binding *dp);
 bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name,
-                                bool updated, int *scan_start);
+                                bool updated, int *scan_start,
+                                int min_ct_zone, int max_ct_zone);
 
 #endif /* controller/ct-zone.h */
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
@@ -474,6 +474,17 @@
         chassis.
       </dd>
 
+      <dt>
+        <code>external_ids:ct-zone-range</code> in the
+        <code>Open_vSwitch</code> table
+      </dt>
+      <dd>
+        The presence of this key identifies a minimum and maximum values for
+        ct-zone ids dynamically selected by ovn-controller (boundaries are
+        included in the range). Minimum value is 1 while maximum value is
+        65535.
+      </dd>
+
       <dt>
         <code>external_ids:ct-zone-*</code> in the <code>Bridge</code> table
       </dt>

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
@@ -2227,8 +2227,8 @@ en_ct_zones_run(struct engine_node *node, void *data)
     const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
 
     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_update(&rt_data->local_lports, ovs_table,
+                    &rt_data->local_datapaths, &ct_zones_data->ctx);
 
     ct_zones_data->recomputed = true;
     engine_set_node_state(node, EN_UPDATED);
@@ -2272,6 +2272,8 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data)
 {
     struct ed_type_runtime_data *rt_data =
         engine_get_input_data("runtime_data", node);
+    const struct ovsrec_open_vswitch_table *ovs_table =
+        EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
 
     /* There is no tracked data. Fall back to full recompute of ct_zones. */
     if (!rt_data->tracked) {
@@ -2281,11 +2283,14 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data)
     struct ed_type_ct_zones *ct_zones_data = data;
 
     struct hmap *tracked_dp_bindings = &rt_data->tracked_dp_bindings;
+    int scan_start, min_ct_zone, max_ct_zone;
     struct tracked_datapath *tdp;
-    int scan_start = 1;
 
     bool updated = false;
 
+    ct_zones_parse_range(ovs_table, &min_ct_zone, &max_ct_zone);
+    scan_start = min_ct_zone;
+
     HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) {
         if (tdp->tracked_type == TRACKED_RESOURCE_NEW) {
             /* A new datapath has been added. Fall back to full recompute. */
@@ -2309,7 +2314,8 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data)
                     t_lport->tracked_type == TRACKED_RESOURCE_UPDATED;
             updated |= ct_zone_handle_port_update(&ct_zones_data->ctx,
                                                   t_lport->pb->logical_port,
-                                                  port_updated, &scan_start);
+                                                  port_updated, &scan_start,
+                                                  min_ct_zone, max_ct_zone);
         }
     }
 

diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
@@ -3127,3 +3127,70 @@ OVS_WAIT_UNTIL([grep -q 'tcp:127.0.0.1:1235: connected' hv1/ovn-controller.log])
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - CT zone min/max boundaries])
+ovn_start
+
+check_ct_zone_min() {
+    min_val=$1
+    OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{print $2}' | sort | head -n1) -ge ${min_val}])
+}
+
+check_ct_zone_max() {
+    max_val=$1
+    AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{print $2}' | sort | tail -n1) -le ${max_val}])
+}
+
+net_add n1
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone
+
+check ovs-vsctl add-port br-int lsp0 \
+    -- set Interface lsp0 external-ids:iface-id=lsp0
+
+check ovn-nbctl lr-add lr
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls ls-lr
+check ovn-nbctl lsp-set-type ls-lr router
+check ovn-nbctl lsp-set-addresses ls-lr router
+check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1
+
+check ovn-nbctl lsp-add ls lsp0
+check ovn-nbctl lsp-set-addresses lsp0 "00:00:00:00:00:02 10.0.0.2"
+
+check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1
+check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1
+
+# check regular boundaries
+check_ct_zone_min 1
+check_ct_zone_max 10
+
+# increase boundaries
+ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-20\"
+check_ct_zone_min 10
+check_ct_zone_max 20
+
+# reset min boundary
+ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-20\"
+
+# add a new port to the switch
+check ovs-vsctl add-port br-int lsp1 \
+    -- set Interface lsp1 external-ids:iface-id=lsp1
+check ovn-nbctl lsp-add ls lsp1
+check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:03 10.0.0.3"
+check_ct_zone_min 5
+check_ct_zone_max 20
+
+check ovn-nbctl set logical_router lr options:snat-ct-zone=2
+check_ct_zone_min 2
+check_ct_zone_max 20
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])