| Message ID | 20231102120021.89725-4-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | Expose CT limit via DB | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 11/2/23 13:00, 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> > --- > v6: Rebase on top of current master. > Address comments from Ilya: > - Update the semantics and documentation of the set command. > v5: Rebase on top of current master. > Address comments from Ilya: > - Use only single command for setting zone and default limit. > - Correct the errors in the man page. > - Use references for the column description. > v4: Rebase on top of current master. > Address comments from Ilya: > - Make sure that the NEWS is clear on what has been added. > - Make the usage of --may-exist and --if-exists more intuitive for the new commands. > - Some cosmetics. > Add command and column for default limit. > --- > NEWS | 5 ++ > tests/ovs-vsctl.at | 88 +++++++++++++++++++++++- > utilities/ovs-vsctl.8.in | 31 +++++++-- > utilities/ovs-vsctl.c | 133 +++++++++++++++++++++++++++++++++++-- > vswitchd/vswitch.ovsschema | 14 +++- > vswitchd/vswitch.xml | 13 ++++ > 6 files changed, 268 insertions(+), 16 deletions(-) > > diff --git a/NEWS b/NEWS > index 7bc27b687..61b48ff12 100644 > --- a/NEWS > +++ b/NEWS > @@ -9,6 +9,11 @@ Post-v3.2.0 > - ovs-appctl: > * Added support removal of default CT zone limit, e.g. > "ovs-appctl dpctl/ct-del-limits default". > + - ovs-vsctl: > + * New commands 'set-zone-limit', 'del-zone-limit' and 'list-zone-limit' > + to manage the maximum number of connections in conntrack zones via > + a new 'limit' column in the 'CT_Zone' database table and > + 'ct_zone_default_limit' column in the 'Datapath' table. > > > v3.2.0 - 17 Aug 2023 > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > index a368bff6e..f88e986db 100644 > --- a/tests/ovs-vsctl.at > +++ b/tests/ovs-vsctl.at > @@ -975,6 +975,67 @@ 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([--if-exists del-zone-tp netdev zone=10])]) > + > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])]) > + > +AT_CHECK([RUN_OVS_VSCTL([set-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([set-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([set-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([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([set-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([set-zone-limit netdev default limit=5])]) > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl > +Default limit: 5 > +Zone: 10, Limit: 5 > +]) > + > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=10])]) > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl > +Default limit: 10 > +Zone: 10, Limit: 5 > +]) > + > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])]) > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl > +Zone: 10, Limit: 5 > +]) > + > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-limit netdev 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 > @@ -1113,16 +1174,39 @@ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1 icmp_first=1 icmp_reply=2]) > ]) > AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 icmp_reply=3])]) > AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 icmp_reply=3])], > - [1], [], [ovs-vsctl: zone id 2 already exists > + [1], [], [ovs-vsctl: zone id 2 already has a policy > ]) > 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([del-zone-tp netdev zone=11])], > - [1], [], [ovs-vsctl: zone id 11 does not exist > + [1], [], [ovs-vsctl: zone id 11 does not have policy > ]) > 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([set-zone-limit netdevxx zone=5 limit=1])], > + [1], [], [ovs-vsctl: datapath netdevxx does not exist > +]) > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=88888 limit=1])], > + [1], [], [ovs-vsctl: zone_id (88888) out of range > +]) > +AT_CHECK([RUN_OVS_VSCTL([set-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 have limit > +]) > + > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdevxx default limit=1])], > + [1], [], [ovs-vsctl: datapath netdevxx does not exist > +]) > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=-1])], > + [1], [], [ovs-vsctl: limit (-1) out of range > +]) > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])], > + [1], [], [ovs-vsctl: datapath netdev does not have limit > +]) > + > 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..9bc95b82c 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 > @@ -365,20 +365,37 @@ packet and a 60-second policy for ICMP reply packets. See the > \fBCT_Timeout_Policy\fR table in \fBovs-vswitchd.conf.db\fR(5) for the > supported keys. > .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 does nothing if \fIzone_id\fR already exists. > +Without \fB\-\-may\-exist\fR, attempting to add a \fIpolicy\fR for > +\fIzone_id\fR that already has a policy is an error. > + With \fB\-\-may\-exist\fR, this command does nothing if policy for > + \fIzone_id\fR already exists. > . > .IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR" > Delete the timeout policy 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. > +Without \fB\-\-if\-exists\fR, attempting to delete a policy for zone that > +does not exist or doesn't have a policy is an error. With > +\fB\-\-if\-exists\fR, attempting to delete a a policy that does not > +exist has no effect. > . > .IP "\fBlist\-zone\-tp \fIdatapath\fR" > Prints the timeout policies of all zones in \fIdatapath\fR. > . > +.IP "\fBset\-zone\-limit \fIdatapath \fBzone=\fIzone_id\fR|\fBdefault \fBlimit=\fIzone_limit\fR" > +Sets a conntrack zone limit with \fIzone_id\fR|\fIdefault\fR in > +\fIdatapath\fR. The \fIlimit\fR with value \fB0\fR means unlimited. > +.IP > +. > +.IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-limit \fIdatapath \fBzone=\fIzone_id\fR|\fBdefault\fR" > +Delete the limit associated with \fIzone_id\fR from \fIdatapath\fR. > +.IP > +Without \fB\-\-if\-exists\fR, attempting to delete a limit for zone that > +does not exist or doesn't have a limit is an error. With \fB\-\-if\-exists\fR, > +attempting to delete a limit 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..2cf569663 100644 > --- a/utilities/ovs-vsctl.c > +++ b/utilities/ovs-vsctl.c > @@ -1302,8 +1302,8 @@ cmd_add_zone_tp(struct ctl_context *ctx) > ctl_fatal("No timeout policy"); > } > > - if (zone && !may_exist) { > - ctl_fatal("zone id %"PRIu64" already exists", zone_id); > + if (zone && zone->timeout_policy && !may_exist) { > + ctl_fatal("zone id %"PRIu64" already has a policy", zone_id); > } > > tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps); > @@ -1332,11 +1332,20 @@ cmd_del_zone_tp(struct ctl_context *ctx) > } > > struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id); > - if (must_exist && !zone) { > - ctl_fatal("zone id %"PRIu64" does not exist", zone_id); > + if (must_exist && !(zone && zone->timeout_policy)) { > + ctl_fatal("zone id %"PRIu64" does not have policy", zone_id); 'a policy' > } > > - 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,118 @@ cmd_list_zone_tp(struct ctl_context *ctx) > } > } > > +static void > +cmd_set_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]; > + > + ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id); > + ovs_scan(ctx->argv[3], "limit=%"SCNi64, &limit); > + > + struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name); > + if (!dp) { > + ctl_fatal("datapath %s does not exist", dp_name); > + } > + > + if (limit < 0 || limit > UINT32_MAX) { > + ctl_fatal("limit (%"PRIi64") out of range", limit); > + } > + > + if (!strcmp(ctx->argv[2], "default")) { > + ovsrec_datapath_set_ct_zone_default_limit(dp, &limit, 1); > + return; > + } > + > + if (zone_id < 0 || zone_id > UINT16_MAX) { > + ctl_fatal("zone_id (%"PRIi64") out of range", zone_id); > + } > + > + struct ovsrec_ct_zone *zone = find_ct_zone(dp, 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); > + } > + > + if (!strcmp(ctx->argv[2], "default")) { > + if (must_exist && !dp->ct_zone_default_limit) { > + ctl_fatal("datapath %s does not have limit", dp_name); 'a limit' > + } > + > + ovsrec_datapath_set_ct_zone_default_limit(dp, NULL, 0); > + return; > + } > + > + struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id); > + if (must_exist && !(zone && zone->limit)) { > + ctl_fatal("zone_id %"PRIi64" does not have limit", zone_id); 'a limit' > + } > + > + 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]); > + } > + > + if (dp->ct_zone_default_limit) { > + ds_put_format(&ctx->output, "Default limit: %"PRIu64"\n", Nit: since he output is different from the dpctl anyway, maybe it's better to print something like 'Default, Limit: N' instead to be more inline with non-default limits? WDYT? > + *dp->ct_zone_default_limit); > + } > + > + 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_datapath_col_ct_zone_default_limit); > 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 +3274,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. */ > + {"set-zone-limit", 3, 3, "", pre_get_zone, cmd_set_zone_limit, NULL, > + "", 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}, I just noticed that these commands do not have a usage string for some reson. And we'll also need a usege() function update. I see that TP commands are also not ducumented there, but that's a separate issue. > + > {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO}, > }; > > diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema > index 2d395ff95..e2d5e2e85 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": "4040946650 27557", > "tables": { > "Open_vSwitch": { > "columns": { > @@ -670,6 +670,11 @@ > "capabilities": { > "type": {"key": "string", "value": "string", > "min": 0, "max": "unlimited"}}, > + "ct_zone_default_limit": { > + "type": { "key": {"type": "integer", > + "minInteger": 0, > + "maxInteger": 4294967295}, > + "min": 0, "max": 1}}, > "external_ids": { > "type": {"key": "string", "value": "string", > "min": 0, "max": "unlimited"}}}}, > @@ -679,6 +684,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 e400043ce..05af24acf 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -6465,6 +6465,13 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > </column> > </group> > > + <column name="ct_zone_default_limit"> > + Default connection tracking zone limit that is applied to all zones > + that didn't specify the <ref table="CT_Zone" column="limit"/> > + explicitly. If the limit is unspecified the datapath for default > + limit configuration is left intact. The value 0 means unlimited. 'the datapath for default limit' ? Maybe 'default limit for the datapath'? And maybe a comma. > + </column> > + > <group title="Common Columns"> > The overall purpose of these columns is described under <code>Common > Columns</code> at the beginning of this document. > @@ -6481,6 +6488,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > is not specified, it defaults to the timeout policy in the system. > </column> > > + <column name="limit"> > + Connection tracking limit for this zone. If the limit is unspecified > + the <ref table="Datapath" column="ct_zone_default_limit"/> will be used. > + The value 0 means unlimited. > + </column> > + > <group title="Common Columns"> > The overall purpose of these columns is described under <code>Common > Columns</code> at the beginning of this document.
On Tue, Nov 28, 2023 at 2:54 PM Ilya Maximets <i.maximets@ovn.org> wrote: > On 11/2/23 13:00, 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> > > --- > > v6: Rebase on top of current master. > > Address comments from Ilya: > > - Update the semantics and documentation of the set command. > > v5: Rebase on top of current master. > > Address comments from Ilya: > > - Use only single command for setting zone and default limit. > > - Correct the errors in the man page. > > - Use references for the column description. > > v4: Rebase on top of current master. > > Address comments from Ilya: > > - Make sure that the NEWS is clear on what has been added. > > - Make the usage of --may-exist and --if-exists more intuitive for > the new commands. > > - Some cosmetics. > > Add command and column for default limit. > > --- > > NEWS | 5 ++ > > tests/ovs-vsctl.at | 88 +++++++++++++++++++++++- > > utilities/ovs-vsctl.8.in | 31 +++++++-- > > utilities/ovs-vsctl.c | 133 +++++++++++++++++++++++++++++++++++-- > > vswitchd/vswitch.ovsschema | 14 +++- > > vswitchd/vswitch.xml | 13 ++++ > > 6 files changed, 268 insertions(+), 16 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 7bc27b687..61b48ff12 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -9,6 +9,11 @@ Post-v3.2.0 > > - ovs-appctl: > > * Added support removal of default CT zone limit, e.g. > > "ovs-appctl dpctl/ct-del-limits default". > > + - ovs-vsctl: > > + * New commands 'set-zone-limit', 'del-zone-limit' and > 'list-zone-limit' > > + to manage the maximum number of connections in conntrack zones > via > > + a new 'limit' column in the 'CT_Zone' database table and > > + 'ct_zone_default_limit' column in the 'Datapath' table. > > > > > > v3.2.0 - 17 Aug 2023 > > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > > index a368bff6e..f88e986db 100644 > > --- a/tests/ovs-vsctl.at > > +++ b/tests/ovs-vsctl.at > > @@ -975,6 +975,67 @@ 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([--if-exists del-zone-tp netdev zone=10])]) > > + > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])]) > > + > > +AT_CHECK([RUN_OVS_VSCTL([set-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([set-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([set-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([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([set-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([set-zone-limit netdev default limit=5])]) > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl > > +Default limit: 5 > > +Zone: 10, Limit: 5 > > +]) > > + > > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=10])]) > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl > > +Default limit: 10 > > +Zone: 10, Limit: 5 > > +]) > > + > > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])]) > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl > > +Zone: 10, Limit: 5 > > +]) > > + > > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-limit netdev 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 > > @@ -1113,16 +1174,39 @@ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx > zone=1 icmp_first=1 icmp_reply=2]) > > ]) > > AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 > icmp_reply=3])]) > > AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 > icmp_reply=3])], > > - [1], [], [ovs-vsctl: zone id 2 already exists > > + [1], [], [ovs-vsctl: zone id 2 already has a policy > > ]) > > 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([del-zone-tp netdev zone=11])], > > - [1], [], [ovs-vsctl: zone id 11 does not exist > > + [1], [], [ovs-vsctl: zone id 11 does not have policy > > ]) > > 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([set-zone-limit netdevxx zone=5 limit=1])], > > + [1], [], [ovs-vsctl: datapath netdevxx does not exist > > +]) > > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=88888 limit=1])], > > + [1], [], [ovs-vsctl: zone_id (88888) out of range > > +]) > > +AT_CHECK([RUN_OVS_VSCTL([set-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 have limit > > +]) > > + > > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdevxx default limit=1])], > > + [1], [], [ovs-vsctl: datapath netdevxx does not exist > > +]) > > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=-1])], > > + [1], [], [ovs-vsctl: limit (-1) out of range > > +]) > > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])], > > + [1], [], [ovs-vsctl: datapath netdev does not have limit > > +]) > > + > > 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..9bc95b82c 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 > > @@ -365,20 +365,37 @@ packet and a 60-second policy for ICMP reply > packets. See the > > \fBCT_Timeout_Policy\fR table in \fBovs-vswitchd.conf.db\fR(5) for the > > supported keys. > > .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 does nothing if \fIzone_id\fR already exists. > > +Without \fB\-\-may\-exist\fR, attempting to add a \fIpolicy\fR for > > +\fIzone_id\fR that already has a policy is an error. > > + With \fB\-\-may\-exist\fR, this command does nothing if policy for > > + \fIzone_id\fR already exists. > > . > > .IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-tp \fIdatapath > \fBzone=\fIzone_id\fR" > > Delete the timeout policy 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. > > +Without \fB\-\-if\-exists\fR, attempting to delete a policy for zone > that > > +does not exist or doesn't have a policy is an error. With > > +\fB\-\-if\-exists\fR, attempting to delete a a policy that does not > > +exist has no effect. > > . > > .IP "\fBlist\-zone\-tp \fIdatapath\fR" > > Prints the timeout policies of all zones in \fIdatapath\fR. > > . > > +.IP "\fBset\-zone\-limit \fIdatapath \fBzone=\fIzone_id\fR|\fBdefault > \fBlimit=\fIzone_limit\fR" > > +Sets a conntrack zone limit with \fIzone_id\fR|\fIdefault\fR in > > +\fIdatapath\fR. The \fIlimit\fR with value \fB0\fR means unlimited. > > +.IP > > +. > > +.IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-limit \fIdatapath > \fBzone=\fIzone_id\fR|\fBdefault\fR" > > +Delete the limit associated with \fIzone_id\fR from \fIdatapath\fR. > > +.IP > > +Without \fB\-\-if\-exists\fR, attempting to delete a limit for zone that > > +does not exist or doesn't have a limit is an error. With > \fB\-\-if\-exists\fR, > > +attempting to delete a limit 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..2cf569663 100644 > > --- a/utilities/ovs-vsctl.c > > +++ b/utilities/ovs-vsctl.c > > @@ -1302,8 +1302,8 @@ cmd_add_zone_tp(struct ctl_context *ctx) > > ctl_fatal("No timeout policy"); > > } > > > > - if (zone && !may_exist) { > > - ctl_fatal("zone id %"PRIu64" already exists", zone_id); > > + if (zone && zone->timeout_policy && !may_exist) { > > + ctl_fatal("zone id %"PRIu64" already has a policy", zone_id); > > } > > > > tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps); > > @@ -1332,11 +1332,20 @@ cmd_del_zone_tp(struct ctl_context *ctx) > > } > > > > struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id); > > - if (must_exist && !zone) { > > - ctl_fatal("zone id %"PRIu64" does not exist", zone_id); > > + if (must_exist && !(zone && zone->timeout_policy)) { > > + ctl_fatal("zone id %"PRIu64" does not have policy", zone_id); > > 'a policy' > > > } > > > > - 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,118 @@ cmd_list_zone_tp(struct ctl_context *ctx) > > } > > } > > > > +static void > > +cmd_set_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]; > > + > > + ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id); > > + ovs_scan(ctx->argv[3], "limit=%"SCNi64, &limit); > > + > > + struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name); > > + if (!dp) { > > + ctl_fatal("datapath %s does not exist", dp_name); > > + } > > + > > + if (limit < 0 || limit > UINT32_MAX) { > > + ctl_fatal("limit (%"PRIi64") out of range", limit); > > + } > > + > > + if (!strcmp(ctx->argv[2], "default")) { > > + ovsrec_datapath_set_ct_zone_default_limit(dp, &limit, 1); > > + return; > > + } > > + > > + if (zone_id < 0 || zone_id > UINT16_MAX) { > > + ctl_fatal("zone_id (%"PRIi64") out of range", zone_id); > > + } > > + > > + struct ovsrec_ct_zone *zone = find_ct_zone(dp, 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); > > + } > > + > > + if (!strcmp(ctx->argv[2], "default")) { > > + if (must_exist && !dp->ct_zone_default_limit) { > > + ctl_fatal("datapath %s does not have limit", dp_name); > > 'a limit' > > > + } > > + > > + ovsrec_datapath_set_ct_zone_default_limit(dp, NULL, 0); > > + return; > > + } > > + > > + struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id); > > + if (must_exist && !(zone && zone->limit)) { > > + ctl_fatal("zone_id %"PRIi64" does not have limit", zone_id); > > 'a limit' > > > + } > > + > > + 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]); > > + } > > + > > + if (dp->ct_zone_default_limit) { > > + ds_put_format(&ctx->output, "Default limit: %"PRIu64"\n", > > Nit: since he output is different from the dpctl anyway, > maybe it's better to print something like 'Default, Limit: N' > instead to be more inline with non-default limits? WDYT? > > Yeah that is reasonable. > > > + *dp->ct_zone_default_limit); > > + } > > + > > + 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_datapath_col_ct_zone_default_limit); > > 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 +3274,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. */ > > + {"set-zone-limit", 3, 3, "", pre_get_zone, cmd_set_zone_limit, NULL, > > + "", 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}, > > I just noticed that these commands do not have a usage string for some > reson. > And we'll also need a usege() function update. > > I see that TP commands are also not ducumented there, but that's a separate > issue. > > > + > > {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO}, > > }; > > > > diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema > > index 2d395ff95..e2d5e2e85 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": "4040946650 27557", > > "tables": { > > "Open_vSwitch": { > > "columns": { > > @@ -670,6 +670,11 @@ > > "capabilities": { > > "type": {"key": "string", "value": "string", > > "min": 0, "max": "unlimited"}}, > > + "ct_zone_default_limit": { > > + "type": { "key": {"type": "integer", > > + "minInteger": 0, > > + "maxInteger": 4294967295}, > > + "min": 0, "max": 1}}, > > "external_ids": { > > "type": {"key": "string", "value": "string", > > "min": 0, "max": "unlimited"}}}}, > > @@ -679,6 +684,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 e400043ce..05af24acf 100644 > > --- a/vswitchd/vswitch.xml > > +++ b/vswitchd/vswitch.xml > > @@ -6465,6 +6465,13 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 > type=patch options:peer=p1 \ > > </column> > > </group> > > > > + <column name="ct_zone_default_limit"> > > + Default connection tracking zone limit that is applied to all > zones > > + that didn't specify the <ref table="CT_Zone" column="limit"/> > > + explicitly. If the limit is unspecified the datapath for default > > + limit configuration is left intact. The value 0 means unlimited. > > 'the datapath for default limit' ? Maybe 'default limit for the datapath'? > And maybe a comma. > > > + </column> > > + > > <group title="Common Columns"> > > The overall purpose of these columns is described under > <code>Common > > Columns</code> at the beginning of this document. > > @@ -6481,6 +6488,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 > type=patch options:peer=p1 \ > > is not specified, it defaults to the timeout policy in the system. > > </column> > > > > + <column name="limit"> > > + Connection tracking limit for this zone. If the limit is > unspecified > > + the <ref table="Datapath" column="ct_zone_default_limit"/> will > be used. > > + The value 0 means unlimited. > > + </column> > > + > > <group title="Common Columns"> > > The overall purpose of these columns is described under > <code>Common > > Columns</code> at the beginning of this document. > > Everything should be addressed in v7. Thanks, Ales
diff --git a/NEWS b/NEWS index 7bc27b687..61b48ff12 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,11 @@ Post-v3.2.0 - ovs-appctl: * Added support removal of default CT zone limit, e.g. "ovs-appctl dpctl/ct-del-limits default". + - ovs-vsctl: + * New commands 'set-zone-limit', 'del-zone-limit' and 'list-zone-limit' + to manage the maximum number of connections in conntrack zones via + a new 'limit' column in the 'CT_Zone' database table and + 'ct_zone_default_limit' column in the 'Datapath' table. v3.2.0 - 17 Aug 2023 diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index a368bff6e..f88e986db 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -975,6 +975,67 @@ 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([--if-exists del-zone-tp netdev zone=10])]) + +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])]) + +AT_CHECK([RUN_OVS_VSCTL([set-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([set-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([set-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([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([set-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([set-zone-limit netdev default limit=5])]) +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl +Default limit: 5 +Zone: 10, Limit: 5 +]) + +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=10])]) +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl +Default limit: 10 +Zone: 10, Limit: 5 +]) + +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])]) +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl +Zone: 10, Limit: 5 +]) + +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-limit netdev 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 @@ -1113,16 +1174,39 @@ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1 icmp_first=1 icmp_reply=2]) ]) AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 icmp_reply=3])]) AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 icmp_reply=3])], - [1], [], [ovs-vsctl: zone id 2 already exists + [1], [], [ovs-vsctl: zone id 2 already has a policy ]) 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([del-zone-tp netdev zone=11])], - [1], [], [ovs-vsctl: zone id 11 does not exist + [1], [], [ovs-vsctl: zone id 11 does not have policy ]) 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([set-zone-limit netdevxx zone=5 limit=1])], + [1], [], [ovs-vsctl: datapath netdevxx does not exist +]) +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=88888 limit=1])], + [1], [], [ovs-vsctl: zone_id (88888) out of range +]) +AT_CHECK([RUN_OVS_VSCTL([set-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 have limit +]) + +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdevxx default limit=1])], + [1], [], [ovs-vsctl: datapath netdevxx does not exist +]) +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=-1])], + [1], [], [ovs-vsctl: limit (-1) out of range +]) +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])], + [1], [], [ovs-vsctl: datapath netdev does not have limit +]) + 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..9bc95b82c 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 @@ -365,20 +365,37 @@ packet and a 60-second policy for ICMP reply packets. See the \fBCT_Timeout_Policy\fR table in \fBovs-vswitchd.conf.db\fR(5) for the supported keys. .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 does nothing if \fIzone_id\fR already exists. +Without \fB\-\-may\-exist\fR, attempting to add a \fIpolicy\fR for +\fIzone_id\fR that already has a policy is an error. + With \fB\-\-may\-exist\fR, this command does nothing if policy for + \fIzone_id\fR already exists. . .IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR" Delete the timeout policy 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. +Without \fB\-\-if\-exists\fR, attempting to delete a policy for zone that +does not exist or doesn't have a policy is an error. With +\fB\-\-if\-exists\fR, attempting to delete a a policy that does not +exist has no effect. . .IP "\fBlist\-zone\-tp \fIdatapath\fR" Prints the timeout policies of all zones in \fIdatapath\fR. . +.IP "\fBset\-zone\-limit \fIdatapath \fBzone=\fIzone_id\fR|\fBdefault \fBlimit=\fIzone_limit\fR" +Sets a conntrack zone limit with \fIzone_id\fR|\fIdefault\fR in +\fIdatapath\fR. The \fIlimit\fR with value \fB0\fR means unlimited. +.IP +. +.IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-limit \fIdatapath \fBzone=\fIzone_id\fR|\fBdefault\fR" +Delete the limit associated with \fIzone_id\fR from \fIdatapath\fR. +.IP +Without \fB\-\-if\-exists\fR, attempting to delete a limit for zone that +does not exist or doesn't have a limit is an error. With \fB\-\-if\-exists\fR, +attempting to delete a limit 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..2cf569663 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -1302,8 +1302,8 @@ cmd_add_zone_tp(struct ctl_context *ctx) ctl_fatal("No timeout policy"); } - if (zone && !may_exist) { - ctl_fatal("zone id %"PRIu64" already exists", zone_id); + if (zone && zone->timeout_policy && !may_exist) { + ctl_fatal("zone id %"PRIu64" already has a policy", zone_id); } tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps); @@ -1332,11 +1332,20 @@ cmd_del_zone_tp(struct ctl_context *ctx) } struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id); - if (must_exist && !zone) { - ctl_fatal("zone id %"PRIu64" does not exist", zone_id); + if (must_exist && !(zone && zone->timeout_policy)) { + ctl_fatal("zone id %"PRIu64" does not have policy", 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,118 @@ cmd_list_zone_tp(struct ctl_context *ctx) } } +static void +cmd_set_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]; + + ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id); + ovs_scan(ctx->argv[3], "limit=%"SCNi64, &limit); + + struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name); + if (!dp) { + ctl_fatal("datapath %s does not exist", dp_name); + } + + if (limit < 0 || limit > UINT32_MAX) { + ctl_fatal("limit (%"PRIi64") out of range", limit); + } + + if (!strcmp(ctx->argv[2], "default")) { + ovsrec_datapath_set_ct_zone_default_limit(dp, &limit, 1); + return; + } + + if (zone_id < 0 || zone_id > UINT16_MAX) { + ctl_fatal("zone_id (%"PRIi64") out of range", zone_id); + } + + struct ovsrec_ct_zone *zone = find_ct_zone(dp, 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); + } + + if (!strcmp(ctx->argv[2], "default")) { + if (must_exist && !dp->ct_zone_default_limit) { + ctl_fatal("datapath %s does not have limit", dp_name); + } + + ovsrec_datapath_set_ct_zone_default_limit(dp, NULL, 0); + return; + } + + struct ovsrec_ct_zone *zone = find_ct_zone(dp, zone_id); + if (must_exist && !(zone && zone->limit)) { + ctl_fatal("zone_id %"PRIi64" does not have limit", 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]); + } + + if (dp->ct_zone_default_limit) { + ds_put_format(&ctx->output, "Default limit: %"PRIu64"\n", + *dp->ct_zone_default_limit); + } + + 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_datapath_col_ct_zone_default_limit); 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 +3274,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. */ + {"set-zone-limit", 3, 3, "", pre_get_zone, cmd_set_zone_limit, NULL, + "", 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..e2d5e2e85 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": "4040946650 27557", "tables": { "Open_vSwitch": { "columns": { @@ -670,6 +670,11 @@ "capabilities": { "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}}, + "ct_zone_default_limit": { + "type": { "key": {"type": "integer", + "minInteger": 0, + "maxInteger": 4294967295}, + "min": 0, "max": 1}}, "external_ids": { "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}}}}, @@ -679,6 +684,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 e400043ce..05af24acf 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -6465,6 +6465,13 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ </column> </group> + <column name="ct_zone_default_limit"> + Default connection tracking zone limit that is applied to all zones + that didn't specify the <ref table="CT_Zone" column="limit"/> + explicitly. If the limit is unspecified the datapath for default + limit configuration is left intact. The value 0 means unlimited. + </column> + <group title="Common Columns"> The overall purpose of these columns is described under <code>Common Columns</code> at the beginning of this document. @@ -6481,6 +6488,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ is not specified, it defaults to the timeout policy in the system. </column> + <column name="limit"> + Connection tracking limit for this zone. If the limit is unspecified + the <ref table="Datapath" column="ct_zone_default_limit"/> will be used. + The value 0 means unlimited. + </column> + <group title="Common Columns"> The overall purpose of these columns is described under <code>Common Columns</code> at the beginning of this document.
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> --- v6: Rebase on top of current master. Address comments from Ilya: - Update the semantics and documentation of the set command. v5: Rebase on top of current master. Address comments from Ilya: - Use only single command for setting zone and default limit. - Correct the errors in the man page. - Use references for the column description. v4: Rebase on top of current master. Address comments from Ilya: - Make sure that the NEWS is clear on what has been added. - Make the usage of --may-exist and --if-exists more intuitive for the new commands. - Some cosmetics. Add command and column for default limit. --- NEWS | 5 ++ tests/ovs-vsctl.at | 88 +++++++++++++++++++++++- utilities/ovs-vsctl.8.in | 31 +++++++-- utilities/ovs-vsctl.c | 133 +++++++++++++++++++++++++++++++++++-- vswitchd/vswitch.ovsschema | 14 +++- vswitchd/vswitch.xml | 13 ++++ 6 files changed, 268 insertions(+), 16 deletions(-)