diff mbox series

[ovs-dev,03/12] ovs-vsctl: Add datapath and CT zone commands.

Message ID 1564097054-72663-4-git-send-email-yihung.wei@gmail.com
State Changes Requested
Headers show
Series Support zone-based conntrack timeout policy | expand

Commit Message

Yi-Hung Wei July 25, 2019, 11:24 p.m. UTC
From: William Tu <u9012063@gmail.com>

The patch adds the following commands
  $ ovs-vsctl {add,del,list}-dp
for creating/deleting/listing the datapath, and
  $ ovs-vsctl {add,del,list}-zone-tp
for conntrack zones and timeout policies.

Signed-off-by: William Tu <u9012063@gmail.com>
---
 tests/ovs-vsctl.at       |  20 +++-
 utilities/ovs-vsctl.8.in |  29 ++++++
 utilities/ovs-vsctl.c    | 245 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 292 insertions(+), 2 deletions(-)

Comments

William Tu July 26, 2019, 3:04 p.m. UTC | #1
On Thu, Jul 25, 2019 at 04:24:05PM -0700, Yi-Hung Wei wrote:
> From: William Tu <u9012063@gmail.com>
> 
> The patch adds the following commands
>   $ ovs-vsctl {add,del,list}-dp
> for creating/deleting/listing the datapath, and
>   $ ovs-vsctl {add,del,list}-zone-tp
> for conntrack zones and timeout policies.
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  tests/ovs-vsctl.at       |  20 +++-
>  utilities/ovs-vsctl.8.in |  29 ++++++
>  utilities/ovs-vsctl.c    | 245 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 292 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index 77604c58a2bc..8854138ecb1e 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -805,6 +805,22 @@ AT_CHECK(
>    [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567"'])])
>  AT_CHECK(
>    [RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
> +
> +AT_CHECK([RUN_OVS_VSCTL([add-dp netdev])])
> +AT_CHECK([RUN_OVS_VSCTL([add-dp system])])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1 icmp_reply=2])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout Policies: 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([list-zone-tp netdev])], [0], [Zone:1, Timeout Policies: icmp_first=1 icmp_reply=2
> +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
> +])
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> +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-dp netdev])])
> +AT_CHECK([RUN_OVS_VSCTL([list-dp | sed 's/ uuid.*$//'])], [0], [system
> +])
>  OVS_VSCTL_CLEANUP
>  AT_CLEANUP
>  

I realize I should add more test cases here.
The above has only positive checks, I will add negative checks.

<snip>

Thanks
William
Darrell Ball July 26, 2019, 6:13 p.m. UTC | #2
Thanks for the patch

Not a full review; just some initial testing


1/ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
icmp_reply_blah=3])])

The above syntax is NOT flagged as an error


2/ AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=2
icmp_first=2 icmp_reply=3])])

The above "--may-exist" option fails with
+ovs-vsctl: 'add-zone-tp' command has no '--may-exist' option

AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
is also failing
+ovs-vsctl: 'del-zone-tp' command has no '--if-exists' option

Please support both "--may-exist" and "--if-exists"


3/ The below should fail, but it is accepted.

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])])


4/ The below fails (which is good), but the error is in idl, rather than
the 'del-zone-tp' command

AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
fails with
+2019-07-26T17:56:10Z|00002|ovsdb_idl|WARN|Trying to delete a key that
doesn't exist in the map.



5/ Please support --may-exist for add-dp

6/ Please support --if-exists for del-dp


7/ Few comments below


Thanks Darrell


On Thu, Jul 25, 2019 at 4:26 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote:

> From: William Tu <u9012063@gmail.com>
>
> The patch adds the following commands
>   $ ovs-vsctl {add,del,list}-dp
> for creating/deleting/listing the datapath, and
>   $ ovs-vsctl {add,del,list}-zone-tp
> for conntrack zones and timeout policies.
>
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  tests/ovs-vsctl.at       |  20 +++-
>  utilities/ovs-vsctl.8.in |  29 ++++++
>  utilities/ovs-vsctl.c    | 245
> +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 292 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index 77604c58a2bc..8854138ecb1e 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -805,6 +805,22 @@ AT_CHECK(
>    [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567
> "'])])
>  AT_CHECK(
>    [RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
> +
> +AT_CHECK([RUN_OVS_VSCTL([add-dp netdev])])
> +AT_CHECK([RUN_OVS_VSCTL([add-dp system])])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> icmp_reply=2])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> Policies: icmp_first=1 icmp_reply=2
> +])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icmp_reply=3])])
>

Add all possible keys as part of positive tests so we know thye work




> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> Policies: icmp_first=1 icmp_reply=2
> +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
> +])
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> +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-dp netdev])])
> +AT_CHECK([RUN_OVS_VSCTL([list-dp | sed 's/ uuid.*$//'])], [0], [system
> +])
>  OVS_VSCTL_CLEANUP
>  AT_CLEANUP
>
> @@ -890,10 +906,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0
> flood_vlans=-1])],
>  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
>    [1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid
> range 0 to 4095 (inclusive)
>  ])
> -AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
>

unrelated change



>    [1], [], [[ovs-vsctl: constraint violation: xyz is not one of the
> allowed values ([in-band, out-of-band])
>  ]])
> -AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
>

unrelated change



>    [1], [], [ovs-vsctl: cannot specify key to set for non-map column
> connection_mode
>  ])
>  AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index 7c09df79bd29..f8ec995247e7 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -353,6 +353,35 @@ list.
>  Prints the name of the bridge that contains \fIiface\fR on standard
>  output.
>  .
> +.SS "Datapath Commands"
> +These commands examine and manipulate Open vSwitch datapath.
> +.
> +.IP "\fBadd\-dp \fIdatapath\fR"
> +Creates a new datapath named \fIdatapath\fR.  Use "netdev" for userspace
> +datapath and "system" for kernel datapath.  Initially the datapath will
> +have no CT zones or other data.
> +.IP "\fBdel\-dp \fIdatapath\fR"
> +Deletes \fIdatapath\fR.
> +.IP "\fBlist\-dp \fIdatapath\fR"
> +Prints the datapath name and its uuid.
> +.
> +.SS "Conntrack Zone Commands"
> +These commands query and modify datapath CT zones and Timeout Policies.
> +.
> +.IP "\fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id \fIpolicies\fR"
> +Creates a conntrack zone with \fIzone_id\fR under the datapath
> \fIdatapath\fR.
> +Associate the conntrack timeout policies to it by a list of
> +\fIkey\fB=\fIvalue\fR pairs, separated by space.  For example, specifying
> +30-second timeout policy for first icmp packet, and 60-second for icmp
> reply packet
> +by doing \fBicmp_first=30 icmp_reply=60\fR.  See CT_Timeout_Policy TABLE
> +at \fBovs-vswitchd.conf.db\fR(5) for all available configurations.
> +.
> +.IP "\fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR"
> +Delete a zone under \fIdatapath\fR by specifying its zone ID.
> +.
> +.IP "\fBlist\-zone\-tp \fIdatapath\fR"
> +Prints the timeout policies of all zones under the \fIdatapath\fR.
> +.
>  .SS "OpenFlow Controller Connectivity"
>  .
>  \fBovs\-vswitchd\fR can perform all configured bridging and switching
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index 4948137efe8c..3ec9b2b05f35 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -40,6 +40,7 @@
>  #include "ovsdb-idl.h"
>  #include "openvswitch/poll-loop.h"
>  #include "process.h"
> +#include "simap.h"
>  #include "stream.h"
>  #include "stream-ssl.h"
>  #include "smap.h"
> @@ -49,6 +50,7 @@
>  #include "table.h"
>  #include "timeval.h"
>  #include "util.h"
> +#include "openvswitch/ofp-parse.h"
>  #include "openvswitch/vconn.h"
>  #include "openvswitch/vlog.h"
>
> @@ -1154,6 +1156,239 @@ cmd_emer_reset(struct ctl_context *ctx)
>  }
>
>  static void
> +cmd_add_dp(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
> +    struct ovsrec_datapath *dp;
> +    const char *dp_name;
> +    int i;
> +
> +    dp_name = ctx->argv[1];
> +
> +    for (i = 0; i < ovs->n_datapaths; i++) {
> +        if (!strcmp(dp_name, ovs->key_datapaths[i])) {
> +            VLOG_ERR("Datapath %s alread exists", dp_name);
> +            return;
> +        }
> +    }
> +
> +    dp = ovsrec_datapath_insert(ctx->txn);
> +    ovsrec_open_vswitch_update_datapaths_setkey(ovs, dp_name, dp);
> +}
> +
> +static void
> +cmd_del_dp(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
> +
> +    ovsrec_open_vswitch_update_datapaths_delkey(ovs, ctx->argv[1]);
> +}
> +
> +static void
> +cmd_list_dp(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
> +    int i;
> +
> +    for (i = 0; i < ovs->n_datapaths; i++) {
> +        struct ovsrec_datapath *dp = ovs->value_datapaths[i];
> +        char *key;
> +
> +        key = ovs->key_datapaths[i];
> +        ds_put_format(&ctx->output, "%s uuid="UUID_FMT"\n",
> +                      key, UUID_ARGS(&dp->header_.uuid));
> +    }
> +}
> +
> +static void
> +pre_get_dp(struct ctl_context *ctx)
> +{
> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
> +}
> +
> +static struct ovsrec_datapath *
> +find_datapath(struct vsctl_context *vsctl_ctx, const char *dp_name)
> +{
> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
> +    int i;
> +
> +    for (i = 0; i < ovs->n_datapaths; i++) {
> +        if (!strcmp(ovs->key_datapaths[i], dp_name)) {
> +            return ovs->value_datapaths[i];
> +        }
> +    }
> +    return NULL;
> +}
> +
> +static struct ovsrec_ct_zone *
> +find_ct_zone(struct ovsrec_datapath *dp, const int64_t zone_id)
> +{
> +    int i;
> +
> +    for (i = 0; i < dp->n_ct_zones; i++) {
> +        if (dp->key_ct_zones[i] == zone_id) {
> +            return dp->value_ct_zones[i];
> +        }
> +    }
> +    return NULL;
> +}
> +
> +static struct ovsrec_ct_timeout_policy *
> +create_timeout_policy(struct ctl_context *ctx, char **argv, int n_tps)
> +{
> +    const struct ovsrec_ct_timeout_policy_table *tp_table;
> +    const struct ovsrec_ct_timeout_policy *row;
> +    struct ovsrec_ct_timeout_policy *tp = NULL;
> +    const char **key_timeouts;
> +    int64_t *value_timeouts;
> +    struct simap new_tp, s;
> +    uint32_t hash_new_tp;
> +    int i, j;
> +
> +    simap_init(&new_tp);
> +
> +    key_timeouts = xmalloc(sizeof *key_timeouts * n_tps);
> +    value_timeouts = xmalloc(sizeof *value_timeouts * n_tps);
> +
> +    /* Parse timeout arguments. */
> +    for (i = 0; i < n_tps; i++) {
> +        char *key, *value, *pos, *copy;
> +
> +        pos = copy = xstrdup(argv[i]);
> +        if (!ofputil_parse_key_value(&pos, &key, &value)) {
> +            goto done;
> +        }
> +        key_timeouts[i] = key;
> +        value_timeouts[i] = atoi(value);
> +        simap_put(&new_tp, key, (unsigned int)value_timeouts[i]);
> +    }
> +done:
> +    hash_new_tp = simap_hash(&new_tp);
> +
> +    tp_table = ovsrec_ct_timeout_policy_table_get(ctx->idl);
> +    OVSREC_CT_TIMEOUT_POLICY_TABLE_FOR_EACH (row, tp_table) {
> +        simap_init(&s);
> +        /* Covert to simap. */
> +        for (j = 0; j < row->n_timeouts; j++) {
> +            simap_put(&s, row->key_timeouts[j], row->value_timeouts[j]);
> +        }
> +        if (simap_hash(&s) == hash_new_tp) {
> +            tp = CONST_CAST(struct ovsrec_ct_timeout_policy *, row);
> +            simap_destroy(&s);
> +            break;
> +        }
> +        simap_destroy(&s);
> +    }
> +
> +    if (!tp) {
> +        tp = ovsrec_ct_timeout_policy_insert(ctx->txn);
> +        ovsrec_ct_timeout_policy_set_timeouts(tp, key_timeouts,
> +                                              (const int64_t
> *)value_timeouts,
> +                                              n_tps);
> +    }
> +
> +    free(key_timeouts);
> +    free(value_timeouts);
> +    return tp;
> +}
> +
> +static void
> +cmd_add_zone(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    struct ovsrec_ct_timeout_policy *tp;
> +    struct ovsrec_ct_zone *zone;
> +    struct ovsrec_datapath *dp;
> +    const char *dp_name;
> +    int64_t zone_id;
> +    int n_tps;
> +
> +    dp_name = ctx->argv[1];
> +    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
> +
> +    dp = find_datapath(vsctl_ctx, dp_name);
> +    if (!dp) {
> +        VLOG_ERR("datapath: %s record not found", dp_name);
> +        return;
> +    }
> +
> +    n_tps = ctx->argc - 3;
> +    tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps);
> +
> +    zone = find_ct_zone(dp, zone_id);
> +    if (zone) {
> +        ovsrec_ct_zone_set_timeout_policy(zone, tp);
> +    } else {
> +        zone = ovsrec_ct_zone_insert(ctx->txn);
> +        ovsrec_ct_zone_set_timeout_policy(zone, tp);
> +        ovsrec_datapath_update_ct_zones_setkey(dp, zone_id, zone);
> +    }
> +}
> +
> +static void
> +cmd_del_zone(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    struct ovsrec_datapath *dp;
> +    const char *dp_name;
> +    int64_t zone_id;
> +
> +    dp_name = ctx->argv[1];
> +    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
> +
> +    dp = find_datapath(vsctl_ctx, dp_name);
> +    if (!dp) {
> +        VLOG_ERR("datapath: %s record not found", dp_name);
> +        return;
> +    }
> +
> +    ovsrec_datapath_update_ct_zones_delkey(dp, zone_id);
> +}
> +
> +static void
> +cmd_list_zone(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    struct ovsrec_ct_timeout_policy *tp;
> +    struct ovsrec_ct_zone *zone;
> +    struct ovsrec_datapath *dp;
> +    int i, j;
> +
> +    dp = find_datapath(vsctl_ctx, ctx->argv[1]);
> +    if (!dp) {
> +        VLOG_ERR("datapath: %s record not found", ctx->argv[1]);
> +        return;
> +    }
> +
> +    for (i = 0; i < dp->n_ct_zones; i++) {
> +        zone = dp->value_ct_zones[i];
> +        ds_put_format(&ctx->output, "Zone:%"PRIu64", Timeout Policies: ",
> +                      dp->key_ct_zones[i]);
> +
> +        tp = zone->timeout_policy;
> +
> +        for (j = 0; j < tp->n_timeouts - 1; j++) {
> +            ds_put_format(&ctx->output, "%s=%"PRIu64" ",
> +                          tp->key_timeouts[j], tp->value_timeouts[j]);
> +        }
> +        ds_put_format(&ctx->output, "%s=%"PRIu64"\n",
> +                      tp->key_timeouts[j], tp->value_timeouts[j]);
> +    }
> +}
> +
> +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_timeout_policy_col_timeouts);
> +}
> +
> +static void
>  cmd_add_br(struct ctl_context *ctx)
>  {
>      struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> @@ -2896,6 +3131,16 @@ static const struct ctl_command_syntax
> vsctl_commands[] = {
>      /* Switch commands. */
>      {"emer-reset", 0, 0, "", pre_cmd_emer_reset, cmd_emer_reset, NULL,
> "", RW},
>
> +    /* Datapath commands. */
> +    {"add-dp", 1, 1, "", pre_get_dp, cmd_add_dp, NULL, "", RW},
> +    {"del-dp", 1, 1, "", pre_get_dp, cmd_del_dp, NULL, "", RW},
> +    {"list-dp", 0, 0, "", pre_get_dp, cmd_list_dp, NULL, "", RO},
> +
> +    /* Zone and CT Timeout Policy commands. */
> +    {"add-zone-tp", 2, 19, "", pre_get_zone, cmd_add_zone, NULL, "", RW},
> +    {"del-zone-tp", 2, 2, "", pre_get_zone, cmd_del_zone, NULL, "", RW},
> +    {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone, NULL, "", RO},
> +
>      {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
>  };
>
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Darrell Ball July 26, 2019, 11:10 p.m. UTC | #3
added one more comment for now


On Fri, Jul 26, 2019 at 11:13 AM Darrell Ball <dlu998@gmail.com> wrote:

> Thanks for the patch
>
> Not a full review; just some initial testing
>
>
> 1/ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icmp_reply_blah=3])])
>
> The above syntax is NOT flagged as an error
>
>
> 2/ AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=2
> icmp_first=2 icmp_reply=3])])
>
> The above "--may-exist" option fails with
> +ovs-vsctl: 'add-zone-tp' command has no '--may-exist' option
>
> AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
> is also failing
> +ovs-vsctl: 'del-zone-tp' command has no '--if-exists' option
>
> Please support both "--may-exist" and "--if-exists"
>
>
> 3/ The below should fail, but it is accepted.
>
> 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])])
>
>
> 4/ The below fails (which is good), but the error is in idl, rather than
> the 'del-zone-tp' command
>
> AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> fails with
> +2019-07-26T17:56:10Z|00002|ovsdb_idl|WARN|Trying to delete a key that
> doesn't exist in the map.
>
>
>
> 5/ Please support --may-exist for add-dp
>
> 6/ Please support --if-exists for del-dp
>
>
> 7/ Few comments below
>
>
> Thanks Darrell
>
>
> On Thu, Jul 25, 2019 at 4:26 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote:
>
>> From: William Tu <u9012063@gmail.com>
>>
>> The patch adds the following commands
>>   $ ovs-vsctl {add,del,list}-dp
>> for creating/deleting/listing the datapath, and
>>   $ ovs-vsctl {add,del,list}-zone-tp
>> for conntrack zones and timeout policies.
>>
>> Signed-off-by: William Tu <u9012063@gmail.com>
>> ---
>>  tests/ovs-vsctl.at       |  20 +++-
>>  utilities/ovs-vsctl.8.in |  29 ++++++
>>  utilities/ovs-vsctl.c    | 245
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 292 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
>> index 77604c58a2bc..8854138ecb1e 100644
>> --- a/tests/ovs-vsctl.at
>> +++ b/tests/ovs-vsctl.at
>> @@ -805,6 +805,22 @@ AT_CHECK(
>>    [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567
>> "'])])
>>  AT_CHECK(
>>    [RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
>> +
>> +AT_CHECK([RUN_OVS_VSCTL([add-dp netdev])])
>> +AT_CHECK([RUN_OVS_VSCTL([add-dp system])])
>> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
>> icmp_reply=2])])
>> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
>> Policies: icmp_first=1 icmp_reply=2
>> +])
>> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
>> icmp_reply=3])])
>>
>
> Add all possible keys as part of positive tests so we know thye work
>
>
>
>
>> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
>> Policies: icmp_first=1 icmp_reply=2
>> +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
>> +])
>> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
>> +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-dp netdev])])
>> +AT_CHECK([RUN_OVS_VSCTL([list-dp | sed 's/ uuid.*$//'])], [0], [system
>> +])
>>  OVS_VSCTL_CLEANUP
>>  AT_CLEANUP
>>
>> @@ -890,10 +906,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0
>> flood_vlans=-1])],
>>  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
>>    [1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid
>> range 0 to 4095 (inclusive)
>>  ])
>> -AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
>> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
>>
>
> unrelated change
>
>
>
>>    [1], [], [[ovs-vsctl: constraint violation: xyz is not one of the
>> allowed values ([in-band, out-of-band])
>>  ]])
>> -AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
>> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
>>
>
> unrelated change
>
>
>
>>    [1], [], [ovs-vsctl: cannot specify key to set for non-map column
>> connection_mode
>>  ])
>>  AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
>> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
>> index 7c09df79bd29..f8ec995247e7 100644
>> --- a/utilities/ovs-vsctl.8.in
>> +++ b/utilities/ovs-vsctl.8.in
>> @@ -353,6 +353,35 @@ list.
>>  Prints the name of the bridge that contains \fIiface\fR on standard
>>  output.
>>  .
>> +.SS "Datapath Commands"
>> +These commands examine and manipulate Open vSwitch datapath.
>> +.
>> +.IP "\fBadd\-dp \fIdatapath\fR"
>> +Creates a new datapath named \fIdatapath\fR.  Use "netdev" for userspace
>> +datapath and "system" for kernel datapath.  Initially the datapath will
>> +have no CT zones or other data.
>> +.IP "\fBdel\-dp \fIdatapath\fR"
>> +Deletes \fIdatapath\fR.
>> +.IP "\fBlist\-dp \fIdatapath\fR"
>> +Prints the datapath name and its uuid.
>> +.
>> +.SS "Conntrack Zone Commands"
>> +These commands query and modify datapath CT zones and Timeout Policies.
>> +.
>> +.IP "\fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id \fIpolicies\fR"
>> +Creates a conntrack zone with \fIzone_id\fR under the datapath
>> \fIdatapath\fR.
>> +Associate the conntrack timeout policies to it by a list of
>> +\fIkey\fB=\fIvalue\fR pairs, separated by space.  For example, specifying
>> +30-second timeout policy for first icmp packet, and 60-second for icmp
>> reply packet
>> +by doing \fBicmp_first=30 icmp_reply=60\fR.  See CT_Timeout_Policy TABLE
>> +at \fBovs-vswitchd.conf.db\fR(5) for all available configurations.
>> +.
>> +.IP "\fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR"
>> +Delete a zone under \fIdatapath\fR by specifying its zone ID.
>> +.
>> +.IP "\fBlist\-zone\-tp \fIdatapath\fR"
>> +Prints the timeout policies of all zones under the \fIdatapath\fR.
>> +.
>>  .SS "OpenFlow Controller Connectivity"
>>  .
>>  \fBovs\-vswitchd\fR can perform all configured bridging and switching
>> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
>> index 4948137efe8c..3ec9b2b05f35 100644
>> --- a/utilities/ovs-vsctl.c
>> +++ b/utilities/ovs-vsctl.c
>> @@ -40,6 +40,7 @@
>>  #include "ovsdb-idl.h"
>>  #include "openvswitch/poll-loop.h"
>>  #include "process.h"
>> +#include "simap.h"
>>  #include "stream.h"
>>  #include "stream-ssl.h"
>>  #include "smap.h"
>> @@ -49,6 +50,7 @@
>>  #include "table.h"
>>  #include "timeval.h"
>>  #include "util.h"
>> +#include "openvswitch/ofp-parse.h"
>>  #include "openvswitch/vconn.h"
>>  #include "openvswitch/vlog.h"
>>
>> @@ -1154,6 +1156,239 @@ cmd_emer_reset(struct ctl_context *ctx)
>>  }
>>
>>  static void
>> +cmd_add_dp(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>> +    struct ovsrec_datapath *dp;
>> +    const char *dp_name;
>> +    int i;
>> +
>> +    dp_name = ctx->argv[1];
>> +
>> +    for (i = 0; i < ovs->n_datapaths; i++) {
>> +        if (!strcmp(dp_name, ovs->key_datapaths[i])) {
>> +            VLOG_ERR("Datapath %s alread exists", dp_name);
>> +            return;
>> +        }
>> +    }
>> +
>> +    dp = ovsrec_datapath_insert(ctx->txn);
>> +    ovsrec_open_vswitch_update_datapaths_setkey(ovs, dp_name, dp);
>> +}
>> +
>> +static void
>> +cmd_del_dp(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>> +
>> +    ovsrec_open_vswitch_update_datapaths_delkey(ovs, ctx->argv[1]);
>> +}
>> +
>> +static void
>> +cmd_list_dp(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>> +    int i;
>> +
>> +    for (i = 0; i < ovs->n_datapaths; i++) {
>> +        struct ovsrec_datapath *dp = ovs->value_datapaths[i];
>> +        char *key;
>> +
>> +        key = ovs->key_datapaths[i];
>> +        ds_put_format(&ctx->output, "%s uuid="UUID_FMT"\n",
>> +                      key, UUID_ARGS(&dp->header_.uuid));
>> +    }
>> +}
>> +
>> +static void
>> +pre_get_dp(struct ctl_context *ctx)
>> +{
>> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
>> +}
>> +
>> +static struct ovsrec_datapath *
>> +find_datapath(struct vsctl_context *vsctl_ctx, const char *dp_name)
>> +{
>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>> +    int i;
>> +
>> +    for (i = 0; i < ovs->n_datapaths; i++) {
>> +        if (!strcmp(ovs->key_datapaths[i], dp_name)) {
>> +            return ovs->value_datapaths[i];
>> +        }
>> +    }
>> +    return NULL;
>> +}
>> +
>> +static struct ovsrec_ct_zone *
>> +find_ct_zone(struct ovsrec_datapath *dp, const int64_t zone_id)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < dp->n_ct_zones; i++) {
>> +        if (dp->key_ct_zones[i] == zone_id) {
>> +            return dp->value_ct_zones[i];
>> +        }
>> +    }
>> +    return NULL;
>> +}
>> +
>> +static struct ovsrec_ct_timeout_policy *
>> +create_timeout_policy(struct ctl_context *ctx, char **argv, int n_tps)
>> +{
>> +    const struct ovsrec_ct_timeout_policy_table *tp_table;
>> +    const struct ovsrec_ct_timeout_policy *row;
>> +    struct ovsrec_ct_timeout_policy *tp = NULL;
>> +    const char **key_timeouts;
>> +    int64_t *value_timeouts;
>> +    struct simap new_tp, s;
>> +    uint32_t hash_new_tp;
>> +    int i, j;
>> +
>> +    simap_init(&new_tp);
>> +
>> +    key_timeouts = xmalloc(sizeof *key_timeouts * n_tps);
>> +    value_timeouts = xmalloc(sizeof *value_timeouts * n_tps);
>> +
>> +    /* Parse timeout arguments. */
>> +    for (i = 0; i < n_tps; i++) {
>> +        char *key, *value, *pos, *copy;
>> +
>> +        pos = copy = xstrdup(argv[i]);
>> +        if (!ofputil_parse_key_value(&pos, &key, &value)) {
>> +            goto done;
>> +        }
>> +        key_timeouts[i] = key;
>> +        value_timeouts[i] = atoi(value);
>> +        simap_put(&new_tp, key, (unsigned int)value_timeouts[i]);
>> +    }
>> +done:
>> +    hash_new_tp = simap_hash(&new_tp);
>> +
>> +    tp_table = ovsrec_ct_timeout_policy_table_get(ctx->idl);
>> +    OVSREC_CT_TIMEOUT_POLICY_TABLE_FOR_EACH (row, tp_table) {
>> +        simap_init(&s);
>> +        /* Covert to simap. */
>> +        for (j = 0; j < row->n_timeouts; j++) {
>> +            simap_put(&s, row->key_timeouts[j], row->value_timeouts[j]);
>> +        }
>> +        if (simap_hash(&s) == hash_new_tp) {
>>
>
In this case an existing timeouts policy is found with the same hash value.
The following logic skips creating a new policy.
However, although the hash is the same, the timeout policy may be different
Maybe you need to compare all the key and values to confirm they are the
same.



> +            tp = CONST_CAST(struct ovsrec_ct_timeout_policy *, row);
>> +            simap_destroy(&s);
>> +            break;
>> +        }
>> +        simap_destroy(&s);
>> +    }
>> +
>> +    if (!tp) {
>> +        tp = ovsrec_ct_timeout_policy_insert(ctx->txn);
>> +        ovsrec_ct_timeout_policy_set_timeouts(tp, key_timeouts,
>> +                                              (const int64_t
>> *)value_timeouts,
>> +                                              n_tps);
>> +    }
>> +
>> +    free(key_timeouts);
>> +    free(value_timeouts);
>> +    return tp;
>> +}
>> +
>> +static void
>> +cmd_add_zone(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    struct ovsrec_ct_timeout_policy *tp;
>> +    struct ovsrec_ct_zone *zone;
>> +    struct ovsrec_datapath *dp;
>> +    const char *dp_name;
>> +    int64_t zone_id;
>> +    int n_tps;
>> +
>> +    dp_name = ctx->argv[1];
>> +    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
>> +
>> +    dp = find_datapath(vsctl_ctx, dp_name);
>> +    if (!dp) {
>> +        VLOG_ERR("datapath: %s record not found", dp_name);
>> +        return;
>> +    }
>> +
>> +    n_tps = ctx->argc - 3;
>> +    tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps);
>> +
>> +    zone = find_ct_zone(dp, zone_id);
>> +    if (zone) {
>> +        ovsrec_ct_zone_set_timeout_policy(zone, tp);
>> +    } else {
>> +        zone = ovsrec_ct_zone_insert(ctx->txn);
>> +        ovsrec_ct_zone_set_timeout_policy(zone, tp);
>> +        ovsrec_datapath_update_ct_zones_setkey(dp, zone_id, zone);
>> +    }
>> +}
>> +
>> +static void
>> +cmd_del_zone(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    struct ovsrec_datapath *dp;
>> +    const char *dp_name;
>> +    int64_t zone_id;
>> +
>> +    dp_name = ctx->argv[1];
>> +    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
>> +
>> +    dp = find_datapath(vsctl_ctx, dp_name);
>> +    if (!dp) {
>> +        VLOG_ERR("datapath: %s record not found", dp_name);
>> +        return;
>> +    }
>> +
>> +    ovsrec_datapath_update_ct_zones_delkey(dp, zone_id);
>> +}
>> +
>> +static void
>> +cmd_list_zone(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    struct ovsrec_ct_timeout_policy *tp;
>> +    struct ovsrec_ct_zone *zone;
>> +    struct ovsrec_datapath *dp;
>> +    int i, j;
>> +
>> +    dp = find_datapath(vsctl_ctx, ctx->argv[1]);
>> +    if (!dp) {
>> +        VLOG_ERR("datapath: %s record not found", ctx->argv[1]);
>> +        return;
>> +    }
>> +
>> +    for (i = 0; i < dp->n_ct_zones; i++) {
>> +        zone = dp->value_ct_zones[i];
>> +        ds_put_format(&ctx->output, "Zone:%"PRIu64", Timeout Policies: ",
>> +                      dp->key_ct_zones[i]);
>> +
>> +        tp = zone->timeout_policy;
>> +
>> +        for (j = 0; j < tp->n_timeouts - 1; j++) {
>> +            ds_put_format(&ctx->output, "%s=%"PRIu64" ",
>> +                          tp->key_timeouts[j], tp->value_timeouts[j]);
>> +        }
>> +        ds_put_format(&ctx->output, "%s=%"PRIu64"\n",
>> +                      tp->key_timeouts[j], tp->value_timeouts[j]);
>> +    }
>> +}
>> +
>> +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_timeout_policy_col_timeouts);
>> +}
>> +
>> +static void
>>  cmd_add_br(struct ctl_context *ctx)
>>  {
>>      struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> @@ -2896,6 +3131,16 @@ static const struct ctl_command_syntax
>> vsctl_commands[] = {
>>      /* Switch commands. */
>>      {"emer-reset", 0, 0, "", pre_cmd_emer_reset, cmd_emer_reset, NULL,
>> "", RW},
>>
>> +    /* Datapath commands. */
>> +    {"add-dp", 1, 1, "", pre_get_dp, cmd_add_dp, NULL, "", RW},
>> +    {"del-dp", 1, 1, "", pre_get_dp, cmd_del_dp, NULL, "", RW},
>> +    {"list-dp", 0, 0, "", pre_get_dp, cmd_list_dp, NULL, "", RO},
>> +
>> +    /* Zone and CT Timeout Policy commands. */
>> +    {"add-zone-tp", 2, 19, "", pre_get_zone, cmd_add_zone, NULL, "", RW},
>> +    {"del-zone-tp", 2, 2, "", pre_get_zone, cmd_del_zone, NULL, "", RW},
>> +    {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone, NULL, "",
>> RO},
>> +
>>      {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
>>  };
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
Darrell Ball July 29, 2019, 9:02 p.m. UTC | #4
added one more comment.

On Fri, Jul 26, 2019 at 4:10 PM Darrell Ball <dlu998@gmail.com> wrote:

> added one more comment for now
>
>
> On Fri, Jul 26, 2019 at 11:13 AM Darrell Ball <dlu998@gmail.com> wrote:
>
>> Thanks for the patch
>>
>> Not a full review; just some initial testing
>>
>>
>> 1/ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
>> icmp_reply_blah=3])])
>>
>> The above syntax is NOT flagged as an error
>>
>>
>> 2/ AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=2
>> icmp_first=2 icmp_reply=3])])
>>
>> The above "--may-exist" option fails with
>> +ovs-vsctl: 'add-zone-tp' command has no '--may-exist' option
>>
>> AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
>> is also failing
>> +ovs-vsctl: 'del-zone-tp' command has no '--if-exists' option
>>
>> Please support both "--may-exist" and "--if-exists"
>>
>>
>> 3/ The below should fail, but it is accepted.
>>
>> 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])])
>>
>>
>> 4/ The below fails (which is good), but the error is in idl, rather than
>> the 'del-zone-tp' command
>>
>> AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
>> AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
>> fails with
>> +2019-07-26T17:56:10Z|00002|ovsdb_idl|WARN|Trying to delete a key that
>> doesn't exist in the map.
>>
>>
>>
>> 5/ Please support --may-exist for add-dp
>>
>> 6/ Please support --if-exists for del-dp
>>
>>
>> 7/ Few comments below
>>
>>
>> Thanks Darrell
>>
>>
>> On Thu, Jul 25, 2019 at 4:26 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote:
>>
>>> From: William Tu <u9012063@gmail.com>
>>>
>>> The patch adds the following commands
>>>   $ ovs-vsctl {add,del,list}-dp
>>> for creating/deleting/listing the datapath, and
>>>   $ ovs-vsctl {add,del,list}-zone-tp
>>> for conntrack zones and timeout policies.
>>>
>>> Signed-off-by: William Tu <u9012063@gmail.com>
>>> ---
>>>  tests/ovs-vsctl.at       |  20 +++-
>>>  utilities/ovs-vsctl.8.in |  29 ++++++
>>>  utilities/ovs-vsctl.c    | 245
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 292 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
>>> index 77604c58a2bc..8854138ecb1e 100644
>>> --- a/tests/ovs-vsctl.at
>>> +++ b/tests/ovs-vsctl.at
>>> @@ -805,6 +805,22 @@ AT_CHECK(
>>>    [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567
>>> "'])])
>>>  AT_CHECK(
>>>    [RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
>>> +
>>> +AT_CHECK([RUN_OVS_VSCTL([add-dp netdev])])
>>> +AT_CHECK([RUN_OVS_VSCTL([add-dp system])])
>>> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
>>> icmp_reply=2])])
>>> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
>>> Policies: icmp_first=1 icmp_reply=2
>>> +])
>>> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
>>> icmp_reply=3])])
>>>
>>
>> Add all possible keys as part of positive tests so we know thye work
>>
>>
>>
>>
>>> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
>>> Policies: icmp_first=1 icmp_reply=2
>>> +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
>>> +])
>>> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
>>> +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-dp netdev])])
>>> +AT_CHECK([RUN_OVS_VSCTL([list-dp | sed 's/ uuid.*$//'])], [0], [system
>>> +])
>>>  OVS_VSCTL_CLEANUP
>>>  AT_CLEANUP
>>>
>>> @@ -890,10 +906,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0
>>> flood_vlans=-1])],
>>>  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
>>>    [1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid
>>> range 0 to 4095 (inclusive)
>>>  ])
>>> -AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
>>> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
>>>
>>
>> unrelated change
>>
>>
>>
>>>    [1], [], [[ovs-vsctl: constraint violation: xyz is not one of the
>>> allowed values ([in-band, out-of-band])
>>>  ]])
>>> -AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
>>> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
>>>
>>
>> unrelated change
>>
>>
>>
>>>    [1], [], [ovs-vsctl: cannot specify key to set for non-map column
>>> connection_mode
>>>  ])
>>>  AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
>>> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
>>> index 7c09df79bd29..f8ec995247e7 100644
>>> --- a/utilities/ovs-vsctl.8.in
>>> +++ b/utilities/ovs-vsctl.8.in
>>> @@ -353,6 +353,35 @@ list.
>>>  Prints the name of the bridge that contains \fIiface\fR on standard
>>>  output.
>>>  .
>>> +.SS "Datapath Commands"
>>> +These commands examine and manipulate Open vSwitch datapath.
>>> +.
>>> +.IP "\fBadd\-dp \fIdatapath\fR"
>>> +Creates a new datapath named \fIdatapath\fR.  Use "netdev" for userspace
>>> +datapath and "system" for kernel datapath.  Initially the datapath will
>>> +have no CT zones or other data.
>>> +.IP "\fBdel\-dp \fIdatapath\fR"
>>> +Deletes \fIdatapath\fR.
>>> +.IP "\fBlist\-dp \fIdatapath\fR"
>>> +Prints the datapath name and its uuid.
>>> +.
>>> +.SS "Conntrack Zone Commands"
>>> +These commands query and modify datapath CT zones and Timeout Policies.
>>> +.
>>> +.IP "\fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id \fIpolicies\fR"
>>> +Creates a conntrack zone with \fIzone_id\fR under the datapath
>>> \fIdatapath\fR.
>>> +Associate the conntrack timeout policies to it by a list of
>>> +\fIkey\fB=\fIvalue\fR pairs, separated by space.  For example,
>>> specifying
>>> +30-second timeout policy for first icmp packet, and 60-second for icmp
>>> reply packet
>>> +by doing \fBicmp_first=30 icmp_reply=60\fR.  See CT_Timeout_Policy TABLE
>>> +at \fBovs-vswitchd.conf.db\fR(5) for all available configurations.
>>> +.
>>> +.IP "\fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR"
>>> +Delete a zone under \fIdatapath\fR by specifying its zone ID.
>>> +.
>>> +.IP "\fBlist\-zone\-tp \fIdatapath\fR"
>>> +Prints the timeout policies of all zones under the \fIdatapath\fR.
>>> +.
>>>  .SS "OpenFlow Controller Connectivity"
>>>  .
>>>  \fBovs\-vswitchd\fR can perform all configured bridging and switching
>>> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
>>> index 4948137efe8c..3ec9b2b05f35 100644
>>> --- a/utilities/ovs-vsctl.c
>>> +++ b/utilities/ovs-vsctl.c
>>> @@ -40,6 +40,7 @@
>>>  #include "ovsdb-idl.h"
>>>  #include "openvswitch/poll-loop.h"
>>>  #include "process.h"
>>> +#include "simap.h"
>>>  #include "stream.h"
>>>  #include "stream-ssl.h"
>>>  #include "smap.h"
>>> @@ -49,6 +50,7 @@
>>>  #include "table.h"
>>>  #include "timeval.h"
>>>  #include "util.h"
>>> +#include "openvswitch/ofp-parse.h"
>>>  #include "openvswitch/vconn.h"
>>>  #include "openvswitch/vlog.h"
>>>
>>> @@ -1154,6 +1156,239 @@ cmd_emer_reset(struct ctl_context *ctx)
>>>  }
>>>
>>>  static void
>>> +cmd_add_dp(struct ctl_context *ctx)
>>> +{
>>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>>> +    struct ovsrec_datapath *dp;
>>> +    const char *dp_name;
>>> +    int i;
>>> +
>>> +    dp_name = ctx->argv[1];
>>> +
>>> +    for (i = 0; i < ovs->n_datapaths; i++) {
>>> +        if (!strcmp(dp_name, ovs->key_datapaths[i])) {
>>> +            VLOG_ERR("Datapath %s alread exists", dp_name);
>>> +            return;
>>> +        }
>>> +    }
>>> +
>>> +    dp = ovsrec_datapath_insert(ctx->txn);
>>> +    ovsrec_open_vswitch_update_datapaths_setkey(ovs, dp_name, dp);
>>> +}
>>> +
>>> +static void
>>> +cmd_del_dp(struct ctl_context *ctx)
>>> +{
>>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>>> +
>>> +    ovsrec_open_vswitch_update_datapaths_delkey(ovs, ctx->argv[1]);
>>> +}
>>> +
>>> +static void
>>> +cmd_list_dp(struct ctl_context *ctx)
>>> +{
>>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>>> +    int i;
>>> +
>>> +    for (i = 0; i < ovs->n_datapaths; i++) {
>>> +        struct ovsrec_datapath *dp = ovs->value_datapaths[i];
>>> +        char *key;
>>> +
>>> +        key = ovs->key_datapaths[i];
>>> +        ds_put_format(&ctx->output, "%s uuid="UUID_FMT"\n",
>>> +                      key, UUID_ARGS(&dp->header_.uuid));
>>> +    }
>>> +}
>>> +
>>> +static void
>>> +pre_get_dp(struct ctl_context *ctx)
>>> +{
>>> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
>>> +}
>>> +
>>> +static struct ovsrec_datapath *
>>> +find_datapath(struct vsctl_context *vsctl_ctx, const char *dp_name)
>>> +{
>>> +    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
>>> +    int i;
>>> +
>>> +    for (i = 0; i < ovs->n_datapaths; i++) {
>>> +        if (!strcmp(ovs->key_datapaths[i], dp_name)) {
>>> +            return ovs->value_datapaths[i];
>>> +        }
>>> +    }
>>> +    return NULL;
>>> +}
>>> +
>>> +static struct ovsrec_ct_zone *
>>> +find_ct_zone(struct ovsrec_datapath *dp, const int64_t zone_id)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < dp->n_ct_zones; i++) {
>>> +        if (dp->key_ct_zones[i] == zone_id) {
>>> +            return dp->value_ct_zones[i];
>>> +        }
>>> +    }
>>> +    return NULL;
>>> +}
>>> +
>>> +static struct ovsrec_ct_timeout_policy *
>>> +create_timeout_policy(struct ctl_context *ctx, char **argv, int n_tps)
>>> +{
>>> +    const struct ovsrec_ct_timeout_policy_table *tp_table;
>>> +    const struct ovsrec_ct_timeout_policy *row;
>>> +    struct ovsrec_ct_timeout_policy *tp = NULL;
>>> +    const char **key_timeouts;
>>> +    int64_t *value_timeouts;
>>> +    struct simap new_tp, s;
>>> +    uint32_t hash_new_tp;
>>> +    int i, j;
>>> +
>>> +    simap_init(&new_tp);
>>> +
>>> +    key_timeouts = xmalloc(sizeof *key_timeouts * n_tps);
>>> +    value_timeouts = xmalloc(sizeof *value_timeouts * n_tps);
>>> +
>>> +    /* Parse timeout arguments. */
>>> +    for (i = 0; i < n_tps; i++) {
>>> +        char *key, *value, *pos, *copy;
>>> +
>>> +        pos = copy = xstrdup(argv[i]);
>>> +        if (!ofputil_parse_key_value(&pos, &key, &value)) {
>>> +            goto done;
>>> +        }
>>> +        key_timeouts[i] = key;
>>> +        value_timeouts[i] = atoi(value);
>>> +        simap_put(&new_tp, key, (unsigned int)value_timeouts[i]);
>>> +    }
>>> +done:
>>> +    hash_new_tp = simap_hash(&new_tp);
>>> +
>>> +    tp_table = ovsrec_ct_timeout_policy_table_get(ctx->idl);
>>> +    OVSREC_CT_TIMEOUT_POLICY_TABLE_FOR_EACH (row, tp_table) {
>>> +        simap_init(&s);
>>> +        /* Covert to simap. */
>>> +        for (j = 0; j < row->n_timeouts; j++) {
>>> +            simap_put(&s, row->key_timeouts[j], row->value_timeouts[j]);
>>> +        }
>>> +        if (simap_hash(&s) == hash_new_tp) {
>>>
>>
> In this case an existing timeouts policy is found with the same hash value.
> The following logic skips creating a new policy.
> However, although the hash is the same, the timeout policy may be different
> Maybe you need to compare all the key and values to confirm they are the
> same.
>
>
>
>> +            tp = CONST_CAST(struct ovsrec_ct_timeout_policy *, row);
>>> +            simap_destroy(&s);
>>> +            break;
>>> +        }
>>> +        simap_destroy(&s);
>>> +    }
>>> +
>>> +    if (!tp) {
>>> +        tp = ovsrec_ct_timeout_policy_insert(ctx->txn);
>>> +        ovsrec_ct_timeout_policy_set_timeouts(tp, key_timeouts,
>>> +                                              (const int64_t
>>> *)value_timeouts,
>>> +                                              n_tps);
>>> +    }
>>> +
>>> +    free(key_timeouts);
>>> +    free(value_timeouts);
>>>
>>
Memory leak; need to add:

free(copy);



> +    return tp;
>>> +}
>>> +
>>> +static void
>>> +cmd_add_zone(struct ctl_context *ctx)
>>> +{
>>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> +    struct ovsrec_ct_timeout_policy *tp;
>>> +    struct ovsrec_ct_zone *zone;
>>> +    struct ovsrec_datapath *dp;
>>> +    const char *dp_name;
>>> +    int64_t zone_id;
>>> +    int n_tps;
>>> +
>>> +    dp_name = ctx->argv[1];
>>> +    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
>>> +
>>> +    dp = find_datapath(vsctl_ctx, dp_name);
>>> +    if (!dp) {
>>> +        VLOG_ERR("datapath: %s record not found", dp_name);
>>> +        return;
>>> +    }
>>> +
>>> +    n_tps = ctx->argc - 3;
>>> +    tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps);
>>> +
>>> +    zone = find_ct_zone(dp, zone_id);
>>> +    if (zone) {
>>> +        ovsrec_ct_zone_set_timeout_policy(zone, tp);
>>> +    } else {
>>> +        zone = ovsrec_ct_zone_insert(ctx->txn);
>>> +        ovsrec_ct_zone_set_timeout_policy(zone, tp);
>>> +        ovsrec_datapath_update_ct_zones_setkey(dp, zone_id, zone);
>>> +    }
>>> +}
>>> +
>>> +static void
>>> +cmd_del_zone(struct ctl_context *ctx)
>>> +{
>>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> +    struct ovsrec_datapath *dp;
>>> +    const char *dp_name;
>>> +    int64_t zone_id;
>>> +
>>> +    dp_name = ctx->argv[1];
>>> +    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
>>> +
>>> +    dp = find_datapath(vsctl_ctx, dp_name);
>>> +    if (!dp) {
>>> +        VLOG_ERR("datapath: %s record not found", dp_name);
>>> +        return;
>>> +    }
>>> +
>>> +    ovsrec_datapath_update_ct_zones_delkey(dp, zone_id);
>>> +}
>>> +
>>> +static void
>>> +cmd_list_zone(struct ctl_context *ctx)
>>> +{
>>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> +    struct ovsrec_ct_timeout_policy *tp;
>>> +    struct ovsrec_ct_zone *zone;
>>> +    struct ovsrec_datapath *dp;
>>> +    int i, j;
>>> +
>>> +    dp = find_datapath(vsctl_ctx, ctx->argv[1]);
>>> +    if (!dp) {
>>> +        VLOG_ERR("datapath: %s record not found", ctx->argv[1]);
>>> +        return;
>>> +    }
>>> +
>>> +    for (i = 0; i < dp->n_ct_zones; i++) {
>>> +        zone = dp->value_ct_zones[i];
>>> +        ds_put_format(&ctx->output, "Zone:%"PRIu64", Timeout Policies:
>>> ",
>>> +                      dp->key_ct_zones[i]);
>>> +
>>> +        tp = zone->timeout_policy;
>>> +
>>> +        for (j = 0; j < tp->n_timeouts - 1; j++) {
>>> +            ds_put_format(&ctx->output, "%s=%"PRIu64" ",
>>> +                          tp->key_timeouts[j], tp->value_timeouts[j]);
>>> +        }
>>> +        ds_put_format(&ctx->output, "%s=%"PRIu64"\n",
>>> +                      tp->key_timeouts[j], tp->value_timeouts[j]);
>>> +    }
>>> +}
>>> +
>>> +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_timeout_policy_col_timeouts);
>>> +}
>>> +
>>> +static void
>>>  cmd_add_br(struct ctl_context *ctx)
>>>  {
>>>      struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>>> @@ -2896,6 +3131,16 @@ static const struct ctl_command_syntax
>>> vsctl_commands[] = {
>>>      /* Switch commands. */
>>>      {"emer-reset", 0, 0, "", pre_cmd_emer_reset, cmd_emer_reset, NULL,
>>> "", RW},
>>>
>>> +    /* Datapath commands. */
>>> +    {"add-dp", 1, 1, "", pre_get_dp, cmd_add_dp, NULL, "", RW},
>>> +    {"del-dp", 1, 1, "", pre_get_dp, cmd_del_dp, NULL, "", RW},
>>> +    {"list-dp", 0, 0, "", pre_get_dp, cmd_list_dp, NULL, "", RO},
>>> +
>>> +    /* Zone and CT Timeout Policy commands. */
>>> +    {"add-zone-tp", 2, 19, "", pre_get_zone, cmd_add_zone, NULL, "",
>>> RW},
>>> +    {"del-zone-tp", 2, 2, "", pre_get_zone, cmd_del_zone, NULL, "", RW},
>>> +    {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone, NULL, "",
>>> RO},
>>> +
>>>      {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
>>>  };
>>>
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
diff mbox series

Patch

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 77604c58a2bc..8854138ecb1e 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -805,6 +805,22 @@  AT_CHECK(
   [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567"'])])
 AT_CHECK(
   [RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
+
+AT_CHECK([RUN_OVS_VSCTL([add-dp netdev])])
+AT_CHECK([RUN_OVS_VSCTL([add-dp system])])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1 icmp_reply=2])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout Policies: 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([list-zone-tp netdev])], [0], [Zone:1, Timeout Policies: icmp_first=1 icmp_reply=2
+Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
+])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
+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-dp netdev])])
+AT_CHECK([RUN_OVS_VSCTL([list-dp | sed 's/ uuid.*$//'])], [0], [system
+])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
@@ -890,10 +906,10 @@  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=-1])],
 AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
   [1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid range 0 to 4095 (inclusive)
 ])
-AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
+AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
   [1], [], [[ovs-vsctl: constraint violation: xyz is not one of the allowed values ([in-band, out-of-band])
 ]])
-AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
+AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
   [1], [], [ovs-vsctl: cannot specify key to set for non-map column connection_mode
 ])
 AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 7c09df79bd29..f8ec995247e7 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -353,6 +353,35 @@  list.
 Prints the name of the bridge that contains \fIiface\fR on standard
 output.
 .
+.SS "Datapath Commands"
+These commands examine and manipulate Open vSwitch datapath.
+.
+.IP "\fBadd\-dp \fIdatapath\fR"
+Creates a new datapath named \fIdatapath\fR.  Use "netdev" for userspace
+datapath and "system" for kernel datapath.  Initially the datapath will
+have no CT zones or other data.
+.IP "\fBdel\-dp \fIdatapath\fR"
+Deletes \fIdatapath\fR.
+.IP "\fBlist\-dp \fIdatapath\fR"
+Prints the datapath name and its uuid.
+.
+.SS "Conntrack Zone Commands"
+These commands query and modify datapath CT zones and Timeout Policies.
+.
+.IP "\fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id \fIpolicies\fR"
+Creates a conntrack zone with \fIzone_id\fR under the datapath \fIdatapath\fR.
+Associate the conntrack timeout policies to it by a list of
+\fIkey\fB=\fIvalue\fR pairs, separated by space.  For example, specifying
+30-second timeout policy for first icmp packet, and 60-second for icmp reply packet
+by doing \fBicmp_first=30 icmp_reply=60\fR.  See CT_Timeout_Policy TABLE
+at \fBovs-vswitchd.conf.db\fR(5) for all available configurations.
+.
+.IP "\fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR"
+Delete a zone under \fIdatapath\fR by specifying its zone ID.
+.
+.IP "\fBlist\-zone\-tp \fIdatapath\fR"
+Prints the timeout policies of all zones under the \fIdatapath\fR.
+.
 .SS "OpenFlow Controller Connectivity"
 .
 \fBovs\-vswitchd\fR can perform all configured bridging and switching
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 4948137efe8c..3ec9b2b05f35 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -40,6 +40,7 @@ 
 #include "ovsdb-idl.h"
 #include "openvswitch/poll-loop.h"
 #include "process.h"
+#include "simap.h"
 #include "stream.h"
 #include "stream-ssl.h"
 #include "smap.h"
@@ -49,6 +50,7 @@ 
 #include "table.h"
 #include "timeval.h"
 #include "util.h"
+#include "openvswitch/ofp-parse.h"
 #include "openvswitch/vconn.h"
 #include "openvswitch/vlog.h"
 
@@ -1154,6 +1156,239 @@  cmd_emer_reset(struct ctl_context *ctx)
 }
 
 static void
+cmd_add_dp(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
+    struct ovsrec_datapath *dp;
+    const char *dp_name;
+    int i;
+
+    dp_name = ctx->argv[1];
+
+    for (i = 0; i < ovs->n_datapaths; i++) {
+        if (!strcmp(dp_name, ovs->key_datapaths[i])) {
+            VLOG_ERR("Datapath %s alread exists", dp_name);
+            return;
+        }
+    }
+
+    dp = ovsrec_datapath_insert(ctx->txn);
+    ovsrec_open_vswitch_update_datapaths_setkey(ovs, dp_name, dp);
+}
+
+static void
+cmd_del_dp(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
+
+    ovsrec_open_vswitch_update_datapaths_delkey(ovs, ctx->argv[1]);
+}
+
+static void
+cmd_list_dp(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
+    int i;
+
+    for (i = 0; i < ovs->n_datapaths; i++) {
+        struct ovsrec_datapath *dp = ovs->value_datapaths[i];
+        char *key;
+
+        key = ovs->key_datapaths[i];
+        ds_put_format(&ctx->output, "%s uuid="UUID_FMT"\n",
+                      key, UUID_ARGS(&dp->header_.uuid));
+    }
+}
+
+static void
+pre_get_dp(struct ctl_context *ctx)
+{
+    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
+}
+
+static struct ovsrec_datapath *
+find_datapath(struct vsctl_context *vsctl_ctx, const char *dp_name)
+{
+    const struct ovsrec_open_vswitch *ovs = vsctl_ctx->ovs;
+    int i;
+
+    for (i = 0; i < ovs->n_datapaths; i++) {
+        if (!strcmp(ovs->key_datapaths[i], dp_name)) {
+            return ovs->value_datapaths[i];
+        }
+    }
+    return NULL;
+}
+
+static struct ovsrec_ct_zone *
+find_ct_zone(struct ovsrec_datapath *dp, const int64_t zone_id)
+{
+    int i;
+
+    for (i = 0; i < dp->n_ct_zones; i++) {
+        if (dp->key_ct_zones[i] == zone_id) {
+            return dp->value_ct_zones[i];
+        }
+    }
+    return NULL;
+}
+
+static struct ovsrec_ct_timeout_policy *
+create_timeout_policy(struct ctl_context *ctx, char **argv, int n_tps)
+{
+    const struct ovsrec_ct_timeout_policy_table *tp_table;
+    const struct ovsrec_ct_timeout_policy *row;
+    struct ovsrec_ct_timeout_policy *tp = NULL;
+    const char **key_timeouts;
+    int64_t *value_timeouts;
+    struct simap new_tp, s;
+    uint32_t hash_new_tp;
+    int i, j;
+
+    simap_init(&new_tp);
+
+    key_timeouts = xmalloc(sizeof *key_timeouts * n_tps);
+    value_timeouts = xmalloc(sizeof *value_timeouts * n_tps);
+
+    /* Parse timeout arguments. */
+    for (i = 0; i < n_tps; i++) {
+        char *key, *value, *pos, *copy;
+
+        pos = copy = xstrdup(argv[i]);
+        if (!ofputil_parse_key_value(&pos, &key, &value)) {
+            goto done;
+        }
+        key_timeouts[i] = key;
+        value_timeouts[i] = atoi(value);
+        simap_put(&new_tp, key, (unsigned int)value_timeouts[i]);
+    }
+done:
+    hash_new_tp = simap_hash(&new_tp);
+
+    tp_table = ovsrec_ct_timeout_policy_table_get(ctx->idl);
+    OVSREC_CT_TIMEOUT_POLICY_TABLE_FOR_EACH (row, tp_table) {
+        simap_init(&s);
+        /* Covert to simap. */
+        for (j = 0; j < row->n_timeouts; j++) {
+            simap_put(&s, row->key_timeouts[j], row->value_timeouts[j]);
+        }
+        if (simap_hash(&s) == hash_new_tp) {
+            tp = CONST_CAST(struct ovsrec_ct_timeout_policy *, row);
+            simap_destroy(&s);
+            break;
+        }
+        simap_destroy(&s);
+    }
+
+    if (!tp) {
+        tp = ovsrec_ct_timeout_policy_insert(ctx->txn);
+        ovsrec_ct_timeout_policy_set_timeouts(tp, key_timeouts,
+                                              (const int64_t *)value_timeouts,
+                                              n_tps);
+    }
+
+    free(key_timeouts);
+    free(value_timeouts);
+    return tp;
+}
+
+static void
+cmd_add_zone(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    struct ovsrec_ct_timeout_policy *tp;
+    struct ovsrec_ct_zone *zone;
+    struct ovsrec_datapath *dp;
+    const char *dp_name;
+    int64_t zone_id;
+    int n_tps;
+
+    dp_name = ctx->argv[1];
+    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
+
+    dp = find_datapath(vsctl_ctx, dp_name);
+    if (!dp) {
+        VLOG_ERR("datapath: %s record not found", dp_name);
+        return;
+    }
+
+    n_tps = ctx->argc - 3;
+    tp = create_timeout_policy(ctx, &ctx->argv[3], n_tps);
+
+    zone = find_ct_zone(dp, zone_id);
+    if (zone) {
+        ovsrec_ct_zone_set_timeout_policy(zone, tp);
+    } else {
+        zone = ovsrec_ct_zone_insert(ctx->txn);
+        ovsrec_ct_zone_set_timeout_policy(zone, tp);
+        ovsrec_datapath_update_ct_zones_setkey(dp, zone_id, zone);
+    }
+}
+
+static void
+cmd_del_zone(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    struct ovsrec_datapath *dp;
+    const char *dp_name;
+    int64_t zone_id;
+
+    dp_name = ctx->argv[1];
+    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
+
+    dp = find_datapath(vsctl_ctx, dp_name);
+    if (!dp) {
+        VLOG_ERR("datapath: %s record not found", dp_name);
+        return;
+    }
+
+    ovsrec_datapath_update_ct_zones_delkey(dp, zone_id);
+}
+
+static void
+cmd_list_zone(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    struct ovsrec_ct_timeout_policy *tp;
+    struct ovsrec_ct_zone *zone;
+    struct ovsrec_datapath *dp;
+    int i, j;
+
+    dp = find_datapath(vsctl_ctx, ctx->argv[1]);
+    if (!dp) {
+        VLOG_ERR("datapath: %s record not found", ctx->argv[1]);
+        return;
+    }
+
+    for (i = 0; i < dp->n_ct_zones; i++) {
+        zone = dp->value_ct_zones[i];
+        ds_put_format(&ctx->output, "Zone:%"PRIu64", Timeout Policies: ",
+                      dp->key_ct_zones[i]);
+
+        tp = zone->timeout_policy;
+
+        for (j = 0; j < tp->n_timeouts - 1; j++) {
+            ds_put_format(&ctx->output, "%s=%"PRIu64" ",
+                          tp->key_timeouts[j], tp->value_timeouts[j]);
+        }
+        ds_put_format(&ctx->output, "%s=%"PRIu64"\n",
+                      tp->key_timeouts[j], tp->value_timeouts[j]);
+    }
+}
+
+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_timeout_policy_col_timeouts);
+}
+
+static void
 cmd_add_br(struct ctl_context *ctx)
 {
     struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
@@ -2896,6 +3131,16 @@  static const struct ctl_command_syntax vsctl_commands[] = {
     /* Switch commands. */
     {"emer-reset", 0, 0, "", pre_cmd_emer_reset, cmd_emer_reset, NULL, "", RW},
 
+    /* Datapath commands. */
+    {"add-dp", 1, 1, "", pre_get_dp, cmd_add_dp, NULL, "", RW},
+    {"del-dp", 1, 1, "", pre_get_dp, cmd_del_dp, NULL, "", RW},
+    {"list-dp", 0, 0, "", pre_get_dp, cmd_list_dp, NULL, "", RO},
+
+    /* Zone and CT Timeout Policy commands. */
+    {"add-zone-tp", 2, 19, "", pre_get_zone, cmd_add_zone, NULL, "", RW},
+    {"del-zone-tp", 2, 2, "", pre_get_zone, cmd_del_zone, NULL, "", RW},
+    {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone, NULL, "", RO},
+
     {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
 };