Thanks Darrell for your review.
I agree with your comments, if we have to consider R/W commands with
more than one parameter is much better to have specific functions
as you explained and showed.

I'll rework accordingly and post a v4.

Regards,
-Antonio

> -----Original Message-----
> From: Darrell Ball [mailto:db...@vmware.com]
> Sent: Monday, December 11, 2017 6:44 PM
> To: Fischetti, Antonio <antonio.fische...@intel.com>;
> d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v3 2/5] conntrack: add commands to r/w CT
> parameters.
> 
> One extra note inline
> 
> Thanks Darrell
> 
> On 12/11/17, 8:35 AM, "Darrell Ball" <db...@vmware.com> wrote:
> 
>     Thanks Antonio for doing all this and pushing it forward.
> 
>     Regarding patches 2-4:
> 
>     I understand you want to save some code for various possible set and
> get operations.
>     The prior art for these commands is however not generic set and get
> commands.
>     Sometimes, we have specific commands that can take different numbers
> of arguments but
>     those are specific and the context is clear.
>     If we take some examples, we might see there is an issue with the
> approach here.
>     Take per zone limits as one example, which is an internal
> requirement that we are working on
>     across datapaths.
>     This is a set operation with 2 arguments plus the datapath = 3
> arguments.
>     This won’t work with the generic set functions here.
>     The corresponding get will not work either.
>     Trying to make this work generically will get pretty messy and the
> parameter validation error prone.
> 
>     I would like to propose an alternative (a simple one; also with code
> consolidation and more error checking) below:
>     Please let me know what you think.
> 
>     Thanks Darrell
> 
>     ///////////////////////////////////////////////////////////////
> 
>     diff --git a/lib/conntrack.c b/lib/conntrack.c
>     index f5a3aa9..c8ad548 100644
>     --- a/lib/conntrack.c
>     +++ b/lib/conntrack.c
>     @@ -2485,6 +2485,27 @@ conntrack_flush(struct conntrack *ct, const
> uint16_t *zone)
>          return 0;
>      }
> 
>     +int
>     +conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns)
>     +{
>     +    atomic_init(&ct->n_conn_limit, maxconns);
>     +    return 0;
>     +}
>     +
>     +int
>     +conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns)
>     +{
>     +    atomic_read_relaxed(&ct->n_conn_limit, maxconns);
>     +    return 0;
>     +}
>     +
>     +int
>     +conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns)
>     +{
>     +    *nconns = atomic_count_get(&ct->n_conn);
>     +    return 0;
>     +}
>     +
>      /* This function must be called with the ct->resources read lock
> taken. */
>      static struct alg_exp_node *
>      expectation_lookup(struct hmap *alg_expectations,
>     diff --git a/lib/conntrack.h b/lib/conntrack.h
>     index fbeef1c..8652724 100644
>     --- a/lib/conntrack.h
>     +++ b/lib/conntrack.h
>     @@ -114,6 +114,9 @@ int conntrack_dump_next(struct conntrack_dump *,
> struct ct_dpif_entry *);
>      int conntrack_dump_done(struct conntrack_dump *);
> 
>      int conntrack_flush(struct conntrack *, const uint16_t *zone);
>     +int conntrack_set_maxconns(struct conntrack *ct, uint32_t
> maxconns);
>     +int conntrack_get_maxconns(struct conntrack *ct, uint32_t
> *maxconns);
>     +int conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns);
>      ^L
>      /* 'struct ct_lock' is a wrapper for an adaptive mutex.  It's
> useful to try
>       * different types of locks (e.g. spinlocks) */
>     diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
>     index 239c848..5fa3a97 100644
>     --- a/lib/ct-dpif.c
>     +++ b/lib/ct-dpif.c
>     @@ -140,6 +140,30 @@ ct_dpif_flush(struct dpif *dpif, const uint16_t
> *zone,
>                  : EOPNOTSUPP);
>      }
> 
>     +int
>     +ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns)
>     +{
>     +    return (dpif->dpif_class->ct_set_maxconns
>     +            ? dpif->dpif_class->ct_set_maxconns(dpif, maxconns)
>     +            : EOPNOTSUPP);
>     +}
>     +
>     +int
>     +ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns)
>     +{
>     +    return (dpif->dpif_class->ct_get_maxconns
>     +            ? dpif->dpif_class->ct_get_maxconns(dpif, maxconns)
>     +            : EOPNOTSUPP);
>     +}
>     +
>     +int
>     +ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns)
>     +{
>     +    return (dpif->dpif_class->ct_get_nconns
>     +            ? dpif->dpif_class->ct_get_nconns(dpif, nconns)
>     +            : EOPNOTSUPP);
>     +}
>     +
>      void
>      ct_dpif_entry_uninit(struct ct_dpif_entry *entry)
>      {
>     diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
>     index 5e2de53..09e7698 100644
>     --- a/lib/ct-dpif.h
>     +++ b/lib/ct-dpif.h
>     @@ -197,6 +197,9 @@ int ct_dpif_dump_next(struct ct_dpif_dump_state
> *, struct ct_dpif_entry *);
>      int ct_dpif_dump_done(struct ct_dpif_dump_state *);
>      int ct_dpif_flush(struct dpif *, const uint16_t *zone,
>                        const struct ct_dpif_tuple *);
>     +int ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns);
>     +int ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns);
>     +int ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns);
>      void ct_dpif_entry_uninit(struct ct_dpif_entry *);
>      void ct_dpif_format_entry(const struct ct_dpif_entry *, struct ds
> *,
>                                bool verbose, bool print_stats);
>     diff --git a/lib/dpctl.c b/lib/dpctl.c
>     index a28ded9..215d1c3 100644
>     --- a/lib/dpctl.c
>     +++ b/lib/dpctl.c
>     @@ -1654,6 +1654,96 @@ dpctl_ct_bkts(int argc, const char *argv[],
>          return error;
>      }
>      ^L
>     +static int
>     +dpctl_ct_open_dp(int argc, const char *argv[],
>     +                 struct dpctl_params *dpctl_p, struct dpif **dpif)
>     +{
>     +    int error = 0;
>     +    /* The datapath name is not a mandatory parameter for this
> command.
>     +     * If it is not specified - so argc < 3 - we retrieve it from
> the
>     +     * current setup, assuming only one exists. */
>     +    char *dpname = argc == 3 ? xstrdup(argv[1]) :
> get_one_dp(dpctl_p);
> 
> 
> [Darrell] The ‘3’ here would be replaced with a parameter representing
> the number
>               of parameters expected when the dp is included in the
> parameter list.
> 
> 
>     +    if (!dpname) {
>     +        error = EINVAL;
>     +        dpctl_error(dpctl_p, error, "datapath not found");
>     +    } else {
>     +        error = parsed_dpif_open(dpname, false, dpif);
>     +        free(dpname);
>     +        if (error) {
>     +            dpctl_error(dpctl_p, error, "opening datapath");
>     +        }
>     +    }
>     +    return error;
>     +}
>     +
>     +static int
>     +dpctl_ct_set_maxconns(int argc, const char *argv[],
>     +                      struct dpctl_params *dpctl_p)
>     +{
>     +    struct dpif *dpif;
>     +    int error = dpctl_ct_open_dp(argc, argv, dpctl_p, &dpif);
>     +    if (!error) {
>     +        uint32_t maxconns;
>     +        if (ovs_scan(argv[argc - 1], "%"SCNu32, &maxconns)) {
>     +            error = ct_dpif_set_maxconns(dpif, maxconns);
>     +            dpif_close(dpif);
>     +
>     +            if (!error) {
>     +                dpctl_print(dpctl_p, "Ok");
>     +            } else {
>     +                dpctl_error(dpctl_p, error, "ct set maxconns
> failed");
>     +            }
>     +        } else {
>     +            error = EINVAL;
>     +            dpctl_error(dpctl_p, error, "maxconns missing or
> malformed");
>     +        }
>     +    }
>     +
>     +    return error;
>     +}
>     +
>     +static int
>     +dpctl_ct_get_maxconns(int argc, const char *argv[],
>     +                    struct dpctl_params *dpctl_p)
>     +{
>     +    struct dpif *dpif;
>     +    int error = dpctl_ct_open_dp(argc, argv, dpctl_p, &dpif);
>     +    if (!error) {
>     +        uint32_t maxconns;
>     +        error = ct_dpif_get_maxconns(dpif, &maxconns);
>     +        dpif_close(dpif);
>     +
>     +        if (!error) {
>     +            dpctl_print(dpctl_p, "%u\n", maxconns);
>     +        } else {
>     +            dpctl_error(dpctl_p, error, "maxconns could not be
> retrieved");
>     +        }
>     +    }
>     +
>     +    return error;
>     +}
>     +
>     +static int
>     +dpctl_ct_get_nconns(int argc, const char *argv[],
>     +                    struct dpctl_params *dpctl_p)
>     +{
>     +    struct dpif *dpif;
>     +    int error = dpctl_ct_open_dp(argc, argv, dpctl_p, &dpif);
>     +    if (!error) {
>     +        uint32_t nconns;
>     +        error = ct_dpif_get_nconns(dpif, &nconns);
>     +        dpif_close(dpif);
>     +
>     +        if (!error) {
>     +            dpctl_print(dpctl_p, "%u\n", nconns);
>     +        } else {
>     +            dpctl_error(dpctl_p, error, "nconns could not be
> retrieved");
>     +        }
>     +    }
>     +
>     +    return error;
>     +}
>     +
>      /* Undocumented commands for unit testing. */
> 
>      static int
>     @@ -1950,6 +2040,9 @@ static const struct dpctl_command
> all_commands[] = {
>          { "ct-stats-show", "[dp] [zone=N] [verbose]",
>            0, 3, dpctl_ct_stats_show, DP_RO },
>          { "ct-bkts", "[dp] [gt=N]", 0, 2, dpctl_ct_bkts, DP_RO },
>     +    { "ct-set-maxconns", "[dp] maxconns", 1, 2,
> dpctl_ct_set_maxconns, DP_RW },
>     +    { "ct-get-maxconns", "[dp] maxconns", 1, 2,
> dpctl_ct_get_maxconns, DP_RO },
>     +    { "ct-get-nconns", "[dp] nconns", 1, 2, dpctl_ct_get_nconns,
> DP_RO },
>          { "help", "", 0, INT_MAX, dpctl_help, DP_RO },
>          { "list-commands", "", 0, INT_MAX, dpctl_list_commands, DP_RO
> },
> 
>     diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>     index b1ef9a6..9432859 100644
>     --- a/lib/dpif-netdev.c
>     +++ b/lib/dpif-netdev.c
>     @@ -5745,6 +5745,30 @@ dpif_netdev_ct_flush(struct dpif *dpif, const
> uint16_t *zone,
>          return conntrack_flush(&dp->conntrack, zone);
>      }
> 
>     +static int
>     +dpif_netdev_ct_set_maxconns(struct dpif *dpif, uint32_t maxconns)
>     +{
>     +    struct dp_netdev *dp = get_dp_netdev(dpif);
>     +
>     +    return conntrack_set_maxconns(&dp->conntrack, maxconns);
>     +}
>     +
>     +static int
>     +dpif_netdev_ct_get_maxconns(struct dpif *dpif, uint32_t *maxconns)
>     +{
>     +    struct dp_netdev *dp = get_dp_netdev(dpif);
>     +
>     +    return conntrack_get_nconns(&dp->conntrack, maxconns);
>     +}
>     +
>     +static int
>     +dpif_netdev_ct_get_nconns(struct dpif *dpif, uint32_t *nconns)
>     +{
>     +    struct dp_netdev *dp = get_dp_netdev(dpif);
>     +
>     +    return conntrack_get_nconns(&dp->conntrack, nconns);
>     +}
>     +
>      const struct dpif_class dpif_netdev_class = {
>          "netdev",
>          dpif_netdev_init,
>     @@ -5790,6 +5814,9 @@ const struct dpif_class dpif_netdev_class = {
>          dpif_netdev_ct_dump_next,
>          dpif_netdev_ct_dump_done,
>          dpif_netdev_ct_flush,
>     +    dpif_netdev_ct_set_maxconns,
>     +    dpif_netdev_ct_get_maxconns,
>     +    dpif_netdev_ct_get_nconns,
>          dpif_netdev_meter_get_features,
>          dpif_netdev_meter_set,
>          dpif_netdev_meter_get,
>     diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>     index 3f76128..f8d75eb 100644
>     --- a/lib/dpif-netlink.c
>     +++ b/lib/dpif-netlink.c
>     @@ -2989,6 +2989,9 @@ const struct dpif_class dpif_netlink_class = {
>          dpif_netlink_ct_dump_next,
>          dpif_netlink_ct_dump_done,
>          dpif_netlink_ct_flush,
>     +    NULL,                       /* ct_set_maxconns */
>     +    NULL,                       /* ct_get_maxconns */
>     +    NULL,                       /* ct_get_nconns */
>          dpif_netlink_meter_get_features,
>          dpif_netlink_meter_set,
>          dpif_netlink_meter_get,
>     diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
>     index 947bf5e..62b3598 100644
>     --- a/lib/dpif-provider.h
>     +++ b/lib/dpif-provider.h
>     @@ -437,6 +437,12 @@ struct dpif_class {
>           *     (zone 0). */
>          int (*ct_flush)(struct dpif *, const uint16_t *zone,
>                          const struct ct_dpif_tuple *tuple);
>     +    /* Set max connections allowed. */
>     +    int (*ct_set_maxconns)(struct dpif *, uint32_t maxconns);
>     +    /* Get max connections allowed. */
>     +    int (*ct_get_maxconns)(struct dpif *, uint32_t *maxconns);
>     +    /* Get number of connections tracked. */
>     +    int (*ct_get_nconns)(struct dpif *, uint32_t *nconns);
> 
>          /* Meters */
> 
> 
>     ///////////////////////////////////////////////////////////////
> 
> 
> 
>     On 10/13/17, 1:27 AM, "ovs-dev-boun...@openvswitch.org on behalf of
> antonio.fische...@intel.com" <ovs-dev-boun...@openvswitch.org on behalf
> of antonio.fische...@intel.com> wrote:
> 
>         Add infrastructure to implement:
>          - dpctl/ct-get-glbl-cfg to read a current value of available
>            conntrack parameters.
>          - dpctl/ct-set-glbl-cfg to set a value to the available
> conntrack
>            parameters.
> 
>         CC: Darrell Ball <dlu...@gmail.com>
>         CC: Kevin Traynor <ktray...@redhat.com>
>         Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
>         ---
>          lib/conntrack.c     | 60
> ++++++++++++++++++++++++++++++++++++++++++
>          lib/conntrack.h     |  3 +++
>          lib/ct-dpif.c       | 28 ++++++++++++++++++++
>          lib/ct-dpif.h       |  2 ++
>          lib/dpctl.c         | 76
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>          lib/dpif-netdev.c   | 19 ++++++++++++++
>          lib/dpif-netlink.c  |  2 ++
>          lib/dpif-provider.h |  4 +++
>          8 files changed, 194 insertions(+)
> 
>         diff --git a/lib/conntrack.c b/lib/conntrack.c
>         index e555b55..29889c9 100644
>         --- a/lib/conntrack.c
>         +++ b/lib/conntrack.c
>         @@ -67,6 +67,13 @@ enum ct_alg_mode {
>              CT_TFTP_MODE,
>          };
> 
>         +/* Variable to manage read/write on CT parameters. */
>         +struct ct_cfg_params {
>         +    char *cli;      /* Parameter name in human format. */
>         +    int (*wr)(struct conntrack *, uint32_t);
>         +    int (*rd)(struct conntrack *, uint32_t *);
>         +};
>         +
>          static bool conn_key_extract(struct conntrack *, struct
> dp_packet *,
>                                       ovs_be16 dl_type, struct
> conn_lookup_ctx *,
>                                       uint16_t zone);
>         @@ -2485,6 +2492,59 @@ conntrack_flush(struct conntrack *ct,
> const uint16_t *zone)
>              return 0;
>          }
> 
>         +/* List of parameters that can be read/written at run-time. */
>         +struct ct_cfg_params cfg_params[] = {};
>         +
>         +int
>         +conntrack_set_param(struct conntrack *ct,
>         +        const char *set_param)
>         +{
>         +    uint32_t max_conn;
>         +    char buf[16] = "";
>         +
>         +    /* Check if the specified param can be managed. */
>         +    for (int i = 0; i < sizeof(cfg_params) / sizeof(struct
> ct_cfg_params);
>         +             i++) {
>         +        if (!strncmp(set_param, cfg_params[i].cli,
>         +                strlen(cfg_params[i].cli))) {
>         +            ovs_strzcpy(buf, cfg_params[i].cli, sizeof(buf) -
> 1);
>         +            strncat(buf, "=%"SCNu32, sizeof(buf) - 1 -
> strlen(buf));
>         +            if (ovs_scan(set_param, buf, &max_conn)) {
>         +                return (cfg_params[i].wr
>         +                        ? cfg_params[i].wr(ct, max_conn)
>         +                        : EOPNOTSUPP);
>         +            } else {
>         +                return EINVAL;
>         +            }
>         +        }
>         +    }
>         +    /* The entered param didn't match any in the list. */
>         +    VLOG_WARN("%s parameter is not managed by this command.",
> set_param);
>         +
>         +    return EINVAL;
>         +}
>         +
>         +int
>         +conntrack_get_param(struct conntrack *ct,
>         +        const char *get_param, uint32_t *val)
>         +{
>         +    /* Check if the specified param can be managed. */
>         +    for (int i = 0; i < sizeof(cfg_params) / sizeof(struct
> ct_cfg_params);
>         +             i++) {
>         +        if (!strncmp(get_param, cfg_params[i].cli,
>         +                strlen(cfg_params[i].cli))) {
>         +
>         +            return (cfg_params[i].rd
>         +                    ? cfg_params[i].rd(ct, val)
>         +                    : EOPNOTSUPP);
>         +        }
>         +    }
>         +    /* The entered param didn't match any in the list. */
>         +    VLOG_WARN("%s parameter is not managed by this command.",
> get_param);
>         +
>         +    return EINVAL;
>         +}
>         +
>          /* This function must be called with the ct->resources read
> lock taken. */
>          static struct alg_exp_node *
>          expectation_lookup(struct hmap *alg_expectations,
>         diff --git a/lib/conntrack.h b/lib/conntrack.h
>         index fbeef1c..4eb9a9a 100644
>         --- a/lib/conntrack.h
>         +++ b/lib/conntrack.h
>         @@ -114,6 +114,9 @@ int conntrack_dump_next(struct
> conntrack_dump *, struct ct_dpif_entry *);
>          int conntrack_dump_done(struct conntrack_dump *);
> 
>          int conntrack_flush(struct conntrack *, const uint16_t *zone);
>         +int conntrack_set_param(struct conntrack *, const char
> *set_param);
>         +int conntrack_get_param(struct conntrack *, const char
> *get_param,
>         +                        uint32_t *val);
>          ?
>          /* 'struct ct_lock' is a wrapper for an adaptive mutex.  It's
> useful to try
>           * different types of locks (e.g. spinlocks) */
>         diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
>         index c79e69e..599bc57 100644
>         --- a/lib/ct-dpif.c
>         +++ b/lib/ct-dpif.c
>         @@ -127,6 +127,34 @@ ct_dpif_flush(struct dpif *dpif, const
> uint16_t *zone)
>                      : EOPNOTSUPP);
>          }
> 
>         +int
>         +ct_dpif_set_param(struct dpif *dpif, const char *set_param)
>         +{
>         +    if (!set_param) {
>         +        VLOG_DBG("%s: ct_set_param: no input param",
> dpif_name(dpif));
>         +        return EINVAL;
>         +    }
>         +    VLOG_DBG("%s: ct_set_param: %s", dpif_name(dpif),
> set_param);
>         +
>         +    return (dpif->dpif_class->ct_set_param
>         +            ? dpif->dpif_class->ct_set_param(dpif, set_param)
>         +            : EOPNOTSUPP);
>         +}
>         +
>         +int
>         +ct_dpif_get_param(struct dpif *dpif, const char *get_param,
> uint32_t *val)
>         +{
>         +    if (!get_param) {
>         +        VLOG_DBG("%s: ct_get_param: no input param",
> dpif_name(dpif));
>         +        return EINVAL;
>         +    }
>         +    VLOG_DBG("%s: ct_get_param: %s", dpif_name(dpif),
> get_param);
>         +
>         +    return (dpif->dpif_class->ct_get_param
>         +            ? dpif->dpif_class->ct_get_param(dpif, get_param,
> val)
>         +            : EOPNOTSUPP);
>         +}
>         +
>          void
>          ct_dpif_entry_uninit(struct ct_dpif_entry *entry)
>          {
>         diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
>         index d5f9661..92ce3e9 100644
>         --- a/lib/ct-dpif.h
>         +++ b/lib/ct-dpif.h
>         @@ -196,6 +196,8 @@ int ct_dpif_dump_start(struct dpif *, struct
> ct_dpif_dump_state **,
>          int ct_dpif_dump_next(struct ct_dpif_dump_state *, struct
> ct_dpif_entry *);
>          int ct_dpif_dump_done(struct ct_dpif_dump_state *);
>          int ct_dpif_flush(struct dpif *, const uint16_t *zone);
>         +int ct_dpif_set_param(struct dpif *dpif, const char
> *set_param);
>         +int ct_dpif_get_param(struct dpif *dpif, const char *get_param,
> uint32_t *val);
>          void ct_dpif_entry_uninit(struct ct_dpif_entry *);
>          void ct_dpif_format_entry(const struct ct_dpif_entry *, struct
> ds *,
>                                    bool verbose, bool print_stats);
>         diff --git a/lib/dpctl.c b/lib/dpctl.c
>         index b6eecf0..5b02951 100644
>         --- a/lib/dpctl.c
>         +++ b/lib/dpctl.c
>         @@ -1589,6 +1589,80 @@ dpctl_ct_bkts(int argc, const char
> *argv[],
>              free(conn_per_bkts);
>              return error;
>          }
>         +
>         +static int
>         +dpctl_ct_set_param(int argc, const char *argv[],
>         +        struct dpctl_params *dpctl_p)
>         +{
>         +    struct dpif *dpif;
>         +    char *name;
>         +    int error;
>         +
>         +    /* The datapath name is not a mandatory parameter for this
> command.
>         +     * If it is not specified - so argc < 3 - we retrieve it
> from the
>         +     * current setup, assuming only one exists. */
>         +    name = argc == 3 ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
>         +    if (!name) {
>         +        return EINVAL;
>         +    }
>         +    error = parsed_dpif_open(name, false, &dpif);
>         +    free(name);
>         +    if (error) {
>         +        dpctl_error(dpctl_p, error, "opening datapath");
>         +        return error;
>         +    }
>         +
>         +    error = ct_dpif_set_param(dpif, argv[argc - 1]);
>         +
>         +    if (!error) {
>         +        dpctl_print(dpctl_p, "Ok");
>         +    } else {
>         +        dpctl_print(dpctl_p, "CT set global cfg failed (%s)",
>         +                        ovs_strerror(error));
>         +    }
>         +
>         +    dpif_close(dpif);
>         +
>         +    return error;
>         +}
>         +
>         +static int
>         +dpctl_ct_get_param(int argc, const char *argv[],
>         +        struct dpctl_params *dpctl_p)
>         +{
>         +    struct dpif *dpif;
>         +    uint32_t param_val;
>         +    char *name;
>         +    int error;
>         +
>         +    /* The datapath name is not a mandatory parameter for this
> command.
>         +     * If it is not specified - so argc < 3 - we retrieve it
> from the
>         +     * current setup, assuming only one exists. */
>         +    name = argc == 3 ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
>         +    if (!name) {
>         +        return EINVAL;
>         +    }
>         +    error = parsed_dpif_open(name, false, &dpif);
>         +    free(name);
>         +    if (error) {
>         +        dpctl_error(dpctl_p, error, "opening datapath");
>         +        return error;
>         +    }
>         +
>         +    error = ct_dpif_get_param(dpif, argv[argc - 1],
> &param_val);
>         +
>         +    if (!error) {
>         +        dpctl_print(dpctl_p, "Current value: %d", param_val);
>         +    } else {
>         +        dpctl_print(dpctl_p, "CT get global cfg failed (%s)",
>         +                        ovs_strerror(error));
>         +    }
>         +
>         +    dpif_close(dpif);
>         +
>         +    return error;
>         +}
>         +
>          ?
>          /* Undocumented commands for unit testing. */
> 
>         @@ -1885,6 +1959,8 @@ static const struct dpctl_command
> all_commands[] = {
>              { "ct-stats-show", "[dp] [zone=N] [verbose]",
>                0, 3, dpctl_ct_stats_show, DP_RO },
>              { "ct-bkts", "[dp] [gt=N]", 0, 2, dpctl_ct_bkts, DP_RO },
>         +    { "ct-set-glbl-cfg", "[dp] param=..", 1, 2,
> dpctl_ct_set_param, DP_RW },
>         +    { "ct-get-glbl-cfg", "[dp] param", 1, 2,
> dpctl_ct_get_param, DP_RO },
>              { "help", "", 0, INT_MAX, dpctl_help, DP_RO },
>              { "list-commands", "", 0, INT_MAX, dpctl_list_commands,
> DP_RO },
> 
>         diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>         index d5eb830..4c0e7cf 100644
>         --- a/lib/dpif-netdev.c
>         +++ b/lib/dpif-netdev.c
>         @@ -5721,6 +5721,23 @@ dpif_netdev_ct_flush(struct dpif *dpif,
> const uint16_t *zone)
>              return conntrack_flush(&dp->conntrack, zone);
>          }
> 
>         +static int
>         +dpif_netdev_ct_set_param(struct dpif *dpif, const char
> *set_param)
>         +{
>         +    struct dp_netdev *dp = get_dp_netdev(dpif);
>         +
>         +    return conntrack_set_param(&dp->conntrack, set_param);
>         +}
>         +
>         +static int
>         +dpif_netdev_ct_get_param(struct dpif *dpif, const char
> *get_param,
>         +                         uint32_t *val)
>         +{
>         +    struct dp_netdev *dp = get_dp_netdev(dpif);
>         +
>         +    return conntrack_get_param(&dp->conntrack, get_param, val);
>         +}
>         +
>          const struct dpif_class dpif_netdev_class = {
>              "netdev",
>              dpif_netdev_init,
>         @@ -5766,6 +5783,8 @@ const struct dpif_class dpif_netdev_class
> = {
>              dpif_netdev_ct_dump_next,
>              dpif_netdev_ct_dump_done,
>              dpif_netdev_ct_flush,
>         +    dpif_netdev_ct_set_param,
>         +    dpif_netdev_ct_get_param,
>              dpif_netdev_meter_get_features,
>              dpif_netdev_meter_set,
>              dpif_netdev_meter_get,
>         diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>         index 29001fb..0945fad 100644
>         --- a/lib/dpif-netlink.c
>         +++ b/lib/dpif-netlink.c
>         @@ -2986,6 +2986,8 @@ const struct dpif_class dpif_netlink_class
> = {
>              dpif_netlink_ct_dump_next,
>              dpif_netlink_ct_dump_done,
>              dpif_netlink_ct_flush,
>         +    NULL,                       /* ct_set_param */
>         +    NULL,                       /* ct_get_param */
>              dpif_netlink_meter_get_features,
>              dpif_netlink_meter_set,
>              dpif_netlink_meter_get,
>         diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
>         index 1d82a09..262b2e0 100644
>         --- a/lib/dpif-provider.h
>         +++ b/lib/dpif-provider.h
>         @@ -427,6 +427,10 @@ struct dpif_class {
>              /* Flushes the connection tracking tables. If 'zone' is not
> NULL,
>               * only deletes connections in '*zone'. */
>              int (*ct_flush)(struct dpif *, const uint16_t *zone);
>         +    /* Set a value to a connection tracking working parameter.
> */
>         +    int (*ct_set_param)(struct dpif *, const char *set_param);
>         +    /* Read the current value of a connection tracking working
> parameter. */
>         +    int (*ct_get_param)(struct dpif *, const char *get_param,
> uint32_t *val);
> 
>              /* Meters */
> 
>         --
>         2.4.11
> 
>         _______________________________________________
>         dev mailing list
>         d...@openvswitch.org
>         https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__mail.openvswitch.org_mailman_listinfo_ovs-
> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=m6PHM-SNZk7M88zlL6EQWY-S5kl3XTbB-YNfA-yk7xE&s=u0Gt2hVhSfd4NYQ0-
> mRy8r1P_U_vFyUqgE_Xd6rC9Qk&e=
> 
> 
> 
> 
> 
> 
> 
> 
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to