diff mbox series

[ovs-dev,v2,1/3] ovs-vsctl: Add limit to CT zone

Message ID 20230926100352.136273-2-amusil@redhat.com
State Changes Requested
Headers show
Series Expose CT limit via DB | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ales Musil Sept. 26, 2023, 10:03 a.m. UTC
Add limit to the CT zone DB table with ovs-vsctl
helper methods. The limit has two special values
besides any number, 0 is unlimited and empty limit
is to leave the value untouched in the datapath.

This is preparation step and the value is not yet
propagated to the datapath.

Signed-off-by: Ales Musil <amusil@redhat.com>
---
 NEWS                       |   2 +
 tests/ovs-vsctl.at         |  58 ++++++++++++++++++++
 utilities/ovs-vsctl.8.in   |  20 ++++++-
 utilities/ovs-vsctl.c      | 108 ++++++++++++++++++++++++++++++++++++-
 vswitchd/vswitch.ovsschema |   9 +++-
 vswitchd/vswitch.xml       |   5 ++
 6 files changed, 198 insertions(+), 4 deletions(-)

Comments

Simon Horman Sept. 28, 2023, 7:34 a.m. UTC | #1
On Tue, Sep 26, 2023 at 12:03:50PM +0200, Ales Musil wrote:
> Add limit to the CT zone DB table with ovs-vsctl
> helper methods. The limit has two special values
> besides any number, 0 is unlimited and empty limit
> is to leave the value untouched in the datapath.
> 
> This is preparation step and the value is not yet
> propagated to the datapath.
> 
> Signed-off-by: Ales Musil <amusil@redhat.com>

Hi Ales,

Please address the 0-day Robot request for the patch subject to end in a '.'

The above comment notwithstanding,

Acked-by: Simon Horman <horms@ovn.org>
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 6b45492f1..e86e7f364 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,8 @@  Post-v3.2.0
        from older version is supported but it may trigger more leader elections
        during the process, and error logs complaining unrecognized fields may
        be observed on old nodes.
+  - CT:
+    * Add support for setting CT zone limit via DB.
 
 
 v3.2.0 - 17 Aug 2023
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index a368bff6e..b033eaf1f 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -975,6 +975,47 @@  AT_CHECK(
   [0], [stdout])
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:10, Timeout Policies: system default
 ])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=10])])
+
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
+
+AT_CHECK([RUN_OVS_VSCTL([add-zone-limit netdev zone=1 limit=1])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 1, Limit: 1
+])
+
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-limit netdev zone=1 limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 1, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=1])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
+
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-limit netdev zone=10 limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 10, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=10 icmp_first=1 icmp_reply=2])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
+Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
+])
+
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=10])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
+Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
+])
+
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-limit netdev zone=10 limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=10])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 10, Limit: 5
+])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
+Zone:10, Timeout Policies: system default
+])
 
 AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
 AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
@@ -1123,6 +1164,23 @@  AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=11])],
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
 ])
 
+AT_CHECK([RUN_OVS_VSCTL([add-zone-limit netdevxx zone=5 limit=1])],
+  [1], [], [ovs-vsctl: datapath netdevxx does not exist
+])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-limit netdev zone=88888 limit=1])],
+  [1], [], [ovs-vsctl: zone_id (88888) out of range
+])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-limit netdev zone=5 limit=-1])],
+  [1], [], [ovs-vsctl: limit (-1) out of range
+])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=10])],
+  [1], [], [ovs-vsctl: zone_id 10 does not exist
+])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-limit netdev zone=5 limit=1])])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-limit netdev zone=5 limit=2])],
+  [1], [], [ovs-vsctl: zone_id 5 already exists
+])
+
 AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
 AT_CHECK([RUN_OVS_VSCTL([list-dp-cap nosystem])],
   [1], [], [ovs-vsctl: datapath "nosystem" record not found
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 9e319aa1c..e6c0d6b2c 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -354,7 +354,7 @@  Prints the name of the bridge that contains \fIiface\fR on standard
 output.
 .
 .SS "Conntrack Zone Commands"
-These commands query and modify datapath CT zones and Timeout Policies.
+These commands query and modify datapath CT zones, Timeout Policies and Limits.
 .
 .IP "[\fB\-\-may\-exist\fR] \fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id \fIpolicies\fR"
 Creates a conntrack zone timeout policy with \fIzone_id\fR in
@@ -379,6 +379,24 @@  delete a zone that does not exist has no effect.
 .IP "\fBlist\-zone\-tp \fIdatapath\fR"
 Prints the timeout policies of all zones in \fIdatapath\fR.
 .
+.IP "[\fB\-\-may\-exist\fR] \fBadd\-zone\-limit \fIdatapath \fBzone=\fIzone_id \fBlimit=\fIzone_limit"
+Sets a conntrack zone limit with \fIzone_id\fR in
+\fIdatapath\fR.  The \fIlimit\fR with value \fB0\fR means unlimited.
+.IP
+Without \fB\-\-may\-exist\fR, attempting to add a \fIzone_id\fR that
+already exists is an error.  With \fB\-\-may\-exist\fR,
+this command updates the \fIlimit\fR if \fIzone_id\fR already exists.
+.
+.IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-imit \fIdatapath \fBzone=\fIzone_id\fR"
+Delete the limit associated with \fIzone_id\fR from \fIdatapath\fR.
+.IP
+Without \fB\-\-if\-exists\fR, attempting to delete a zone that
+does not exist is an error.  With \fB\-\-if\-exists\fR, attempting to
+delete a zone that does not exist has no effect.
+.
+.IP "\fBlist\-zone\-limit \fIdatapath\fR"
+Prints the limits of all zones in \fIdatapath\fR.
+.
 .SS "Datapath Capabilities Command"
 The command query datapath capabilities.
 .
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 5e549df00..e6b51459f 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1336,7 +1336,16 @@  cmd_del_zone_tp(struct ctl_context *ctx)
         ctl_fatal("zone id %"PRIu64" does not exist", zone_id);
     }
 
-    if (zone) {
+    if (!zone) {
+        return;
+    }
+
+    if (zone->limit) {
+        if (zone->timeout_policy) {
+            ovsrec_ct_timeout_policy_delete(zone->timeout_policy);
+        }
+        ovsrec_ct_zone_set_timeout_policy(zone, NULL);
+    } else {
         ovsrec_datapath_update_ct_zones_delkey(dp, zone_id);
     }
 }
@@ -1371,12 +1380,101 @@  cmd_list_zone_tp(struct ctl_context *ctx)
     }
 }
 
+static void
+cmd_add_zone_limit(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    int64_t zone_id = -1;
+    int64_t limit = -1;
+
+    const char *dp_name = ctx->argv[1];
+    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
+    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
+    ovs_scan(ctx->argv[3], "limit=%"SCNi64, &limit);
+
+    if (zone_id < 0 || zone_id > UINT16_MAX) {
+        ctl_fatal("zone_id (%"PRIi64") out of range", zone_id);
+    }
+
+    if (limit < 0 || limit > UINT32_MAX) {
+        ctl_fatal("limit (%"PRIi64") out of range", limit);
+    }
+
+    struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
+    if (!dp) {
+        ctl_fatal("datapath %s does not exist", dp_name);
+    }
+
+    struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id);
+    if (zone && !may_exist) {
+        ctl_fatal("zone_id %"PRIi64" already exists", zone_id);
+    }
+
+    if (!zone) {
+        zone = ovsrec_ct_zone_insert(ctx->txn);
+        ovsrec_datapath_update_ct_zones_setkey(dp, zone_id, zone);
+    }
+
+    ovsrec_ct_zone_set_limit(zone, &limit, 1);
+}
+
+static void
+cmd_del_zone_limit(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    int64_t zone_id;
+
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
+    const char *dp_name = ctx->argv[1];
+    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
+
+    struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
+    if (!dp) {
+        ctl_fatal("datapath %s does not exist", dp_name);
+    }
+
+    struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id);
+    if (must_exist && !zone) {
+        ctl_fatal("zone_id %"PRIi64" does not exist", zone_id);
+    }
+
+    if (!zone) {
+        return;
+    }
+
+    if (zone->timeout_policy) {
+        ovsrec_ct_zone_set_limit(zone, NULL, 0);
+    } else {
+        ovsrec_datapath_update_ct_zones_delkey(dp, zone_id);
+    }
+}
+
+static void
+cmd_list_zone_limit(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+
+    struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, ctx->argv[1]);
+    if (!dp) {
+        ctl_fatal("datapath: %s record not found", ctx->argv[1]);
+    }
+
+    for (int i = 0; i < dp->n_ct_zones; i++) {
+        struct ovsrec_ct_zone *zone = dp->value_ct_zones[i];
+        if (zone->limit) {
+            ds_put_format(&ctx->output, "Zone: %"PRIu64", Limit: %"PRIu64"\n",
+                          dp->key_ct_zones[i], *zone->limit);
+        }
+    }
+}
+
 static void
 pre_get_zone(struct ctl_context *ctx)
 {
     ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
     ovsdb_idl_add_column(ctx->idl, &ovsrec_datapath_col_ct_zones);
     ovsdb_idl_add_column(ctx->idl, &ovsrec_ct_zone_col_timeout_policy);
+    ovsdb_idl_add_column(ctx->idl, &ovsrec_ct_zone_col_limit);
     ovsdb_idl_add_column(ctx->idl, &ovsrec_ct_timeout_policy_col_timeouts);
 }
 
@@ -3159,6 +3257,14 @@  static const struct ctl_command_syntax vsctl_commands[] = {
     /* Datapath capabilities. */
     {"list-dp-cap", 1, 1, "", pre_get_dp_cap, cmd_list_dp_cap, NULL, "", RO},
 
+    /* CT zone limit. */
+    {"add-zone-limit", 3, 3, "", pre_get_zone, cmd_add_zone_limit, NULL,
+     "--may-exist", RW},
+    {"del-zone-limit", 2, 2, "", pre_get_zone, cmd_del_zone_limit, NULL,
+     "--if-exists", RW},
+    {"list-zone-limit", 1, 1, "", pre_get_zone, cmd_list_zone_limit, NULL,
+     "", RO},
+
     {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
 };
 
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 2d395ff95..ed90c57d0 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@ 
 {"name": "Open_vSwitch",
- "version": "8.4.0",
- "cksum": "2738838700 27127",
+ "version": "8.5.0",
+ "cksum": "2723382443 27334",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -679,6 +679,11 @@ 
          "type": {"key": {"type": "uuid",
                           "refTable": "CT_Timeout_Policy"},
                   "min": 0, "max": 1}},
+       "limit": {
+          "type": { "key": {"type": "integer",
+                            "minInteger": 0,
+                            "maxInteger": 4294967295},
+                    "min": 0, "max": 1}},
        "external_ids": {
          "type": {"key": "string", "value": "string",
                   "min": 0, "max": "unlimited"}}}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index cfcde34ff..d1674170a 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -6428,6 +6428,11 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
   <table name="CT_Zone">
     Connection tracking zone configuration
 
+    <column name="limit">
+      Connection tracking limit for this zone. If the limit is unspecified
+      the datapath configuration is left intact. The value 0 means unlimited.
+    </column>
+
     <column name="timeout_policy">
       Connection tracking timeout policy for this zone. If a timeout policy
       is not specified, it defaults to the timeout policy in the system.