On Mon, Oct 16, 2023 at 9:09 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 10/10/23 16:12, 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 <amu...@redhat.com>
> > ---
> > 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 | 8 ++
> > tests/ovs-vsctl.at | 92 ++++++++++++++++++++
> > utilities/ovs-vsctl.8.in | 45 ++++++++--
> > utilities/ovs-vsctl.c | 171 ++++++++++++++++++++++++++++++++++++-
> > vswitchd/vswitch.ovsschema | 14 ++-
> > vswitchd/vswitch.xml | 11 +++
> > 6 files changed, 331 insertions(+), 10 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index df98e75a0..c0d96b894 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -9,6 +9,14 @@ Post-v3.2.0
> > - ovs-dpctl:
> > * Support removal of default CT zone limit via ovs-dpctl, e.g.
> > "ovs-appctl dpctl/ct-del-limits default"
> > + - 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.
> > + * New command 'set-zone-default-limit' and
> 'del-zone-default-limit' to
> > + manage the maximum number of connections in conntrack zones that
> are
> > + not explicitly defined otherwise via new 'ct_zone_default_limit'
> column
> > + in the 'Datapath' table.
>
> Can we just add a way to specify 'default' instead of 'zone=N'
> in the add/del-zone-limit commands? Two separate commands seem
> redundant.
>
> >
> >
> > v3.2.0 - 17 Aug 2023
> > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > index a368bff6e..0d2fa68fb 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([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([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([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([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([set-zone-default-limit netdev 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([--may-exist set-zone-default-limit netdev
> 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-default-limit netdev])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Zone: 10, Limit: 5
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-default-limit netdev])])
> > +
> >
> > 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 +1184,37 @@ 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 have limit
> > +])
> > +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 has limit
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-default-limit netdevxx limit=1])],
> > + [1], [], [ovs-vsctl: datapath netdevxx does not exist
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-default-limit netdev limit=-1])],
> > + [1], [], [ovs-vsctl: limit (-1) out of range
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([del-zone-default-limit netdev])],
> > + [1], [], [ovs-vsctl: datapath netdev does not have limit
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-default-limit netdev limit=1])])
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-default-limit netdev limit=2])],
> > + [1], [], [ovs-vsctl: datapath netdev already has 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..f8a04d707 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,55 @@ 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 \fIpolicies\fR for
>
> 'a policies' doesn't sound right here. Should be 'a policy'.
>
> > +\fIzone_id\fR that already exists is an error.
>
> This is not true. At least, shouldn't be. An error will be if
> there is already timeout policy configured for a specified zone
> id, not if zone id exists. 'zone_id' exists is also a strange
> way to say that now that existance of the zone record is not
> tied to existance of timeout policy in it.
>
> > With \fB\-\-may\-exist\fR,
> > +this command updates the \fIpolicies\fR if \fIzone_id\fR already exists.
>
> Is it a behavior change? The 'add' command did nothing before,
> now it updates?
>
> The 'add' is not 'set', so it shoudln't update the exisitng value.
>
> > .
> > .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
> > +Without \fB\-\-if\-exists\fR, attempting to delete a policies for zone
> that
>
> 'a policy', zone can only have one.
>
> > does not exist is an error.
>
> If the zone doesn't have a timeout policy it will be an error as well.
>
> > With \fB\-\-if\-exists\fR, attempting to
> > delete a zone that does not exist has no effect.
>
> We're not deleting the zone anymore.
>
> All the same things applies to the new commands below.
>
> > .
> > .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 \fIlimit\fR for
> > +\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\-limit \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 limit for 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.
> > +.
> > +.IP "[\fB\-\-may\-exist\fR] \fBset\-zone\-default\-limit \fIdatapath
> \fBlimit=\fIdefault_limit"
> > +Sets a conntrack default zone limit in \fIdatapath\fR.
> > +The \fIlimit\fR with value \fB0\fR means unlimited.
> > +.IP
> > +Without \fB\-\-may\-exist\fR, attempting to add a default limit for
> > +datapath that already has default limit is an error.
> > +With \fB\-\-may\-exist\fR, this command updates the default limit if
> > +it is already set for specified datapath.
> > +.
> > +.IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-default\-limit \fIdatapath"
> > +Delete the default limit associated with \fIdatapath\fR.
> > +.IP
> > +Without \fB\-\-if\-exists\fR, attempting to delete a default limit for
> > +datapath that does not have default limit is an error.
> > +With \fB\-\-if\-exists\fR, attempting to delete a default limit that is
> not
> > +set has no effect.
> > +.
> > .SS "Datapath Capabilities Command"
> > The command query datapath capabilities.
> > .
> > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> > index 5e549df00..7e01deaec 100644
> > --- a/utilities/ovs-vsctl.c
> > +++ b/utilities/ovs-vsctl.c
> > @@ -1302,7 +1302,7 @@ cmd_add_zone_tp(struct ctl_context *ctx)
> > ctl_fatal("No timeout policy");
> > }
> >
> > - if (zone && !may_exist) {
> > + if (zone && zone->timeout_policy && !may_exist) {
> > ctl_fatal("zone id %"PRIu64" already exists", zone_id);
>
> This error message is not correct anymore. Smae for logs in other
> commands below.
>
> I didn't read into implementations below too much as they will need
> changes for eroror messages and 'add' vs 'set' and removal of the
> separate commands for default limits. But there are a few more
> comments for documentation below.
>
> > }
> >
> > @@ -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) {
> > + if (must_exist && !(zone && zone->timeout_policy)) {
> > 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,156 @@ 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;
> > +
> > + bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> > + const char *dp_name = ctx->argv[1];
> > +
> > + 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 && zone->limit && !may_exist) {
> > + ctl_fatal("zone_id %"PRIi64" already has limit", 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 && 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
> > +cmd_set_zone_default_limit(struct ctl_context *ctx)
> > +{
> > + struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> > + int64_t limit = -1;
> > +
> > + bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> > + const char *dp_name = ctx->argv[1];
> > +
> > + ovs_scan(ctx->argv[2], "limit=%"SCNi64, &limit);
> > +
> > + 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);
> > + }
> > +
> > + if (dp->ct_zone_default_limit && !may_exist) {
> > + ctl_fatal("datapath %s already has limit", dp_name);
> > + }
> > +
> > + ovsrec_datapath_set_ct_zone_default_limit(dp, &limit, 1);
> > +}
> > +
> > +static void
> > +cmd_del_zone_default_limit(struct ctl_context *ctx)
> > +{
> > + struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> > +
> > + bool must_exist = !shash_find(&ctx->options, "--if-exists");
> > + const char *dp_name = ctx->argv[1];
> > +
> > + struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
> > + if (!dp) {
> > + ctl_fatal("datapath %s does not exist", dp_name);
> > + }
> > +
> > + 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);
> > +}
> > +
> > 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 +3312,18 @@ 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},
> > + {"set-zone-default-limit", 2, 2, "", pre_get_zone,
> > + cmd_set_zone_default_limit, NULL, "--may-exist", RW},
> > + {"del-zone-default-limit", 1, 1, "", pre_get_zone,
> > + cmd_del_zone_default_limit, NULL, "--if-exists", RW},
> > +
> > {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 cfcde34ff..84b514e01 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -6417,6 +6417,12 @@ 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 limit explicitly.
>
> Please, use the <ref table="" column="" /> to refererence the exact place
> where the per-zone value is specified.
>
> > + If the limit is unspecified
> > + the datapath configuration is left intact.
>
> I'm not sure this is correct thing to say. We should mention that the
> datapath configuration is left intact only if both the default and the
> per-zone limit are not specified.
>
> > + 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.
> > @@ -6433,6 +6439,11 @@ 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 datapath configuration is left intact.
>
> The same here. We may just say that if not specified than the defualt
> value will be used referencing the table and the column where default
> value is stored.
>
> > + 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.
>
>
Thank you for the review, all should be addressed in v5.
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
