| Message ID | 20231002103347.101274-2-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| 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 |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 10/2/23 12:33, 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> > Acked-by: Simon Horman <horms@ovn.org> > --- > v3: Rebase on top of current master. > Add ack from Simon and fix the missing '.'. Hi, Ales. Thanks for the patch! See some comemnts inline. > --- > 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(-) > > 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. The indentation here is off. However, more importantly, this line is not user-friendly and it doesn't tell much about what changed and how to use the functionality. First, please use 'connection tracking' or 'conntrack' whenever possible instead of obscure 'CT'. Secondly, we should talk about new commands being added to ovs-vsctl and a new column name in the database, because these are interfaces users will care about. Maybe smething like this: - ovs-vsctl: * New commands 'add-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. > > > 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. These restrictions look strange to me. I would expect following two commands to both work and regardless of their order: ovs-vsctl add-zone-tp netdev zone=10 icmp_first=1 icmp_reply=2 ovs-vsctl add-zone-limit netdev zone=10 limit=1 From the user-facing API point of view these commands are not related to each other, but the second one will always fail without --may-exist in the current implementation. add-zone-limit should fail only if the limit exists. add-zone-tp should fail only if timeout policy already exists. But they should not fail if only the other type of resource exists. Both values in the database are nullable, so it should be possible to track that without much trouble. BTW, the following should work as well: ovs-vsctl add-zone-tp netdev zone=10 icmp_first=1 icmp_reply=2 \ -- add-zone-limit netdev zone=10 limit=1 > +. > +.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; An empty line would be nice. Also, reverse x-mass tree. > + 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); This is strange. > + } > + > + 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]; Empty line. > + 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 A period is missing? > + 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. Also, might make sense to list columns in the same order they are in the schema. Best regards, Ilya Maximets.
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.