[ovs-dev,v4] dpctl: Add new 'ct-bkts' command.
diff mbox

Message ID 151E7AB3-EE38-4537-B1A1-74D585CF54F4@vmware.com
State Changes Requested
Delegated to: Darrell Ball
Headers show

Commit Message

Darrell Ball July 23, 2017, 5:28 p.m. UTC
Minor comment:

When applying the patch, there is a complaint about new whitespace below

+the number of connections in a bucket is greater than \fIThreshold\fR.
+

Two comments inline


-----Original Message-----
From: <ovs-dev-bounces@openvswitch.org> on behalf of "antonio.fischetti@intel.com" <antonio.fischetti@intel.com>

Date: Saturday, July 22, 2017 at 7:38 AM
To: "dev@openvswitch.org" <dev@openvswitch.org>
Subject: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

    With the command:
     ovs-appctl dpctl/ct-bkts
    shows the number of connections per bucket.
    
    By using a threshold:
     ovs-appctl dpctl/ct-bkts gt=N
    for each bucket shows the number of connections when they
    are greater than N.
    
    Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>

    Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>

    Co-authored-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
    ---
     lib/conntrack.c                |   9 ++--
     lib/conntrack.h                |   2 +-
     lib/ct-dpif.c                  |   4 +-
     lib/ct-dpif.h                  |   3 +-
     lib/dpctl.c                    | 108 ++++++++++++++++++++++++++++++++++++++++-
     lib/dpctl.man                  |   8 +++
     lib/dpif-netdev.c              |   4 +-
     lib/dpif-netlink.c             |   4 +-
     lib/dpif-provider.h            |   2 +-
     lib/netlink-conntrack.c        |   6 ++-
     lib/netlink-conntrack.h        |   3 +-
     tests/test-netlink-conntrack.c |   3 +-
     utilities/ovs-dpctl.c          |   1 +
     13 files changed, 140 insertions(+), 17 deletions(-)
    
    diff --git a/lib/conntrack.c b/lib/conntrack.c
    index de46a6b..e290b20 100644
    --- a/lib/conntrack.c
    +++ b/lib/conntrack.c
    @@ -1931,7 +1931,7 @@ conn_key_to_tuple(const struct conn_key *key, struct ct_dpif_tuple *tuple)
     
     static void
     conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
    -                      long long now)
    +                      long long now, int bkt)
     {
         struct ct_l4_proto *class;
         long long expiration;
    @@ -1954,11 +1954,12 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
         if (class->conn_get_protoinfo) {
             class->conn_get_protoinfo(conn, &entry->protoinfo);
         }
    +    entry->bkt = bkt;
     }
     
     int
     conntrack_dump_start(struct conntrack *ct, struct conntrack_dump *dump,
    -                     const uint16_t *pzone)
    +                     const uint16_t *pzone, int *ptot_bkts)
     {
         memset(dump, 0, sizeof(*dump));
         if (pzone) {
    @@ -1967,6 +1968,8 @@ conntrack_dump_start(struct conntrack *ct, struct conntrack_dump *dump,
         }
         dump->ct = ct;
     
    +    *ptot_bkts = CONNTRACK_BUCKETS;
    +
         return 0;
     }
     
    @@ -1991,7 +1994,7 @@ conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry)
                 INIT_CONTAINER(conn, node, node);
                 if ((!dump->filter_zone || conn->key.zone == dump->zone) &&
                      (conn->conn_type != CT_CONN_TYPE_UN_NAT)) {
    -                conn_to_ct_dpif_entry(conn, entry, now);
    +                conn_to_ct_dpif_entry(conn, entry, now, dump->bucket);
                     break;
                 }
                 /* Else continue, until we find an entry in the appropriate zone
    diff --git a/lib/conntrack.h b/lib/conntrack.h
    index defde4c..3f48444 100644
    --- a/lib/conntrack.h
    +++ b/lib/conntrack.h
    @@ -108,7 +108,7 @@ struct conntrack_dump {
     struct ct_dpif_entry;
     
     int conntrack_dump_start(struct conntrack *, struct conntrack_dump *,
    -                         const uint16_t *pzone);
    +                         const uint16_t *pzone, int *);
     int conntrack_dump_next(struct conntrack_dump *, struct ct_dpif_entry *);
     int conntrack_dump_done(struct conntrack_dump *);
     
    diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
    index f8d2cf1..c79e69e 100644
    --- a/lib/ct-dpif.c
    +++ b/lib/ct-dpif.c
    @@ -65,12 +65,12 @@ static const struct flags ct_dpif_status_flags[] = {
      * that represents the error.  Otherwise it returns zero. */
     int
     ct_dpif_dump_start(struct dpif *dpif, struct ct_dpif_dump_state **dump,
    -                   const uint16_t *zone)
    +                   const uint16_t *zone, int *ptot_bkts)
     {
         int err;
     
         err = (dpif->dpif_class->ct_dump_start
    -           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone)
    +           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone, ptot_bkts)
                : EOPNOTSUPP);
     
         if (!err) {
    diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
    index cd35f3e..d5f9661 100644
    --- a/lib/ct-dpif.h
    +++ b/lib/ct-dpif.h
    @@ -169,6 +169,7 @@ struct ct_dpif_entry {
         /* Timeout for this entry in seconds */
         uint32_t timeout;
         uint32_t mark;
    +    uint32_t bkt;       /* CT bucket number. */
     };
     
     enum {
    @@ -191,7 +192,7 @@ struct ct_dpif_dump_state {
     };
     
     int ct_dpif_dump_start(struct dpif *, struct ct_dpif_dump_state **,
    -                       const uint16_t *zone);
    +                       const uint16_t *zone, int *);
     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);
    diff --git a/lib/dpctl.c b/lib/dpctl.c
    index 6aa6c8e..3ccc7a3 100644
    --- a/lib/dpctl.c
    +++ b/lib/dpctl.c
    @@ -1258,6 +1258,7 @@ dpctl_dump_conntrack(int argc, const char *argv[],
         struct ct_dpif_dump_state *dump;
         struct ct_dpif_entry cte;
         uint16_t zone, *pzone = NULL;
    +    int tot_bkts;
         struct dpif *dpif;
         char *name;
         int error;
    @@ -1277,7 +1278,7 @@ dpctl_dump_conntrack(int argc, const char *argv[],
             return error;
         }
     
    -    error = ct_dpif_dump_start(dpif, &dump, pzone);
    +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);
         if (error) {
             dpctl_error(dpctl_p, error, "starting conntrack dump");
             dpif_close(dpif);
    @@ -1339,6 +1340,7 @@ dpctl_ct_stats_show(int argc, const char *argv[],
         struct ct_dpif_dump_state *dump;
         struct ct_dpif_entry cte;
         uint16_t zone, *pzone = NULL;
    +    int tot_bkts;
         bool verbose = false;
         int lastargc = 0;
     
    @@ -1373,7 +1375,7 @@ dpctl_ct_stats_show(int argc, const char *argv[],
     
         memset(proto_stats, 0, sizeof(proto_stats));
         memset(tcp_conn_per_states, 0, sizeof(tcp_conn_per_states));
    -    error = ct_dpif_dump_start(dpif, &dump, pzone);
    +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);
         if (error) {
             dpctl_error(dpctl_p, error, "starting conntrack dump");
             dpif_close(dpif);
    @@ -1464,6 +1466,107 @@ dpctl_ct_stats_show(int argc, const char *argv[],
         dpif_close(dpif);
         return error;
     }
    +
    +#define CT_BKTS_GT "gt="
    +static int
    +dpctl_ct_bkts(int argc, const char *argv[],
    +                     struct dpctl_params *dpctl_p)
    +{
    +    struct dpif *dpif;
    +    char *name;
    +
    +    struct ct_dpif_dump_state *dump;
    +    struct ct_dpif_entry cte;
    +    uint16_t gt = 0; /* Threshold: display value when greater than gt. */
    +    uint16_t *pzone = NULL;
    +    int tot_bkts = 0;
    +    int lastargc = 0;
    +
    +    int error;
    +
    +    while (argc > 1 && lastargc != argc) {
    +        lastargc = argc;
    +        if (!strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {
    +            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {
    +                argc--;
    +                break;
    +            }
    +        }
    +    }

[Darrell]
When, I do this, I get a failure, which is expected.

> 2017-07-23T02:28:02.155Z|00052|connmgr|INFO|br0<->unix: 11 flow_mods in the last 0 s (11 adds)

> 2017-07-23T02:28:02.346Z|00053|unixctl|DBG|received request dpctl/ct-bkts["gt=0","netdev@ovs-netdev"], id=0

> 2017-07-23T02:28:02.346Z|00054|dpctl|INFO|set_names=0 verbosity=0 names=0

> 2017-07-23T02:28:02.346Z|00057|dpif|WARN|could not create datapath gt=0 of unknown type system

> 2017-07-23T02:28:02.347Z|00058|unixctl|DBG|replying with error, id=0: "ovs-vswitchd: opening datapath (Address family not supported by protocol)


But, your command requires a maximum of 2 parameters and if there are 2, the datapath name must be first and ‘gt=x’ second 
Hence, I think you can just remove the while loop completely since ‘gt=x’ must always be last, if present.

//////////////////////////////

    +
    +    name = (argc > 1) ? 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_dump_start(dpif, &dump, pzone, &tot_bkts);
    +    if (error) {
    +        dpctl_error(dpctl_p, error, "starting conntrack dump");
    +        dpif_close(dpif);
    +        return error;
    +    }
    +    if (tot_bkts == -1) {
    +         /* Command not available when called by kernel OvS. */
    +         dpctl_print(dpctl_p,
    +             "Command is available for UserSpace ConnTracker only.\n");
    +         ct_dpif_dump_done(dump);

[Darrell]
          dpctl_print(dpctl_p,
              "Command is available for UserSpace ConnTracker only.\n");
          ct_dpif_dump_done(dump);
+         dpif_close(dpif);
          return 0;
     }

//////////////////////////////


    +         return 0;
    +    }
    +
    +    dpctl_print(dpctl_p, "Total Buckets: %d\n", tot_bkts);
    +
    +    int tot_conn = 0;
    +    uint32_t conn_per_bkts[tot_bkts];
    +    memset(conn_per_bkts, 0, sizeof(uint32_t) * tot_bkts);
    +
    +    while (!ct_dpif_dump_next(dump, &cte)) {
    +        ct_dpif_entry_uninit(&cte);
    +        tot_conn++;
    +        if (tot_bkts > 0) {
    +            if (cte.bkt < tot_bkts) {
    +                conn_per_bkts[cte.bkt]++;
    +            } else {
    +                dpctl_print(dpctl_p, "Bucket nr out of range: %d >= %d\n",
    +                        cte.bkt, tot_bkts);
    +            }
    +        }
    +    }
    +
    +    dpctl_print(dpctl_p, "Current Connections: %d\n", tot_conn);
    +    dpctl_print(dpctl_p, "\n");
    +    if (tot_bkts && tot_conn) {
    +        dpctl_print(dpctl_p, "+-----------+"
    +                "-----------------------------------------+\n");
    +        dpctl_print(dpctl_p, "|  Buckets  |"
    +                "         Connections per Buckets         |\n");
    +        dpctl_print(dpctl_p, "+-----------+"
    +                "-----------------------------------------+");
    +#define NUM_BKTS_DIPLAYED_PER_ROW 8
    +        for (int i = 0; i < tot_bkts; i++) {
    +            if (i % NUM_BKTS_DIPLAYED_PER_ROW == 0) {
    +                 dpctl_print(dpctl_p, "\n %3d..%3d   | ",
    +                         i, i + NUM_BKTS_DIPLAYED_PER_ROW - 1);
    +            }
    +            if (conn_per_bkts[i] > gt) {
    +                dpctl_print(dpctl_p, "%5d", conn_per_bkts[i]);
    +            } else {
    +                dpctl_print(dpctl_p, "%5s", ".");
    +            }
    +        }
    +        dpctl_print(dpctl_p, "\n\n");
    +    }
    +
    +    ct_dpif_dump_done(dump);
    +    dpif_close(dpif);
    +    return error;
    +}
     ?
     /* Undocumented commands for unit testing. */
     
    @@ -1760,6 +1863,7 @@ static const struct dpctl_command all_commands[] = {
         { "flush-conntrack", "[dp] [zone=N]", 0, 2, dpctl_flush_conntrack, DP_RW },
         { "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 },
         { "help", "", 0, INT_MAX, dpctl_help, DP_RO },
         { "list-commands", "", 0, INT_MAX, dpctl_list_commands, DP_RO },
     
    diff --git a/lib/dpctl.man b/lib/dpctl.man
    index f95d54a..077fd99 100644
    --- a/lib/dpctl.man
    +++ b/lib/dpctl.man
    @@ -228,3 +228,11 @@ Displays the number of connections grouped by protocol used by \fIdp\fR.
     If \fBzone=\fIzone\fR is specified, numbers refer to the connections in
     \fBzone\fR. The \fBverbose\fR option allows to group by connection state
     for each protocol.
    +.
    +.TP
    +\*(DX\fBct\-bkts\fR [\fIdp\fR] [\fBgt=\fIThreshold\fR]
    +For each ConnTracker bucket, displays the number of connections used
    +by \fIdp\fR.
    +If \fBgt=\fIThreshold\fR is specified, bucket numbers are displayed when
    +the number of connections in a bucket is greater than \fIThreshold\fR.
    +
    diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
    index 47a9fa0..10435e7 100644
    --- a/lib/dpif-netdev.c
    +++ b/lib/dpif-netdev.c
    @@ -5333,7 +5333,7 @@ struct dp_netdev_ct_dump {
     
     static int
     dpif_netdev_ct_dump_start(struct dpif *dpif, struct ct_dpif_dump_state **dump_,
    -                          const uint16_t *pzone)
    +                          const uint16_t *pzone, int *ptot_bkts)
     {
         struct dp_netdev *dp = get_dp_netdev(dpif);
         struct dp_netdev_ct_dump *dump;
    @@ -5342,7 +5342,7 @@ dpif_netdev_ct_dump_start(struct dpif *dpif, struct ct_dpif_dump_state **dump_,
         dump->dp = dp;
         dump->ct = &dp->conntrack;
     
    -    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone);
    +    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone, ptot_bkts);
     
         *dump_ = &dump->up;
     
    diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
    index 55effd1..b87bb16 100644
    --- a/lib/dpif-netlink.c
    +++ b/lib/dpif-netlink.c
    @@ -2847,13 +2847,13 @@ struct dpif_netlink_ct_dump_state {
     static int
     dpif_netlink_ct_dump_start(struct dpif *dpif OVS_UNUSED,
                                struct ct_dpif_dump_state **dump_,
    -                           const uint16_t *zone)
    +                           const uint16_t *zone, int *ptot_bkts)
     {
         struct dpif_netlink_ct_dump_state *dump;
         int err;
     
         dump = xzalloc(sizeof *dump);
    -    err = nl_ct_dump_start(&dump->nl_ct_dump, zone);
    +    err = nl_ct_dump_start(&dump->nl_ct_dump, zone, ptot_bkts);
         if (err) {
             free(dump);
             return err;
    diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
    index 64ac2e2..1d82a09 100644
    --- a/lib/dpif-provider.h
    +++ b/lib/dpif-provider.h
    @@ -419,7 +419,7 @@ struct dpif_class {
          * ct_dump_done() should perform any cleanup necessary (including
          * deallocating the 'state' structure, if applicable). */
         int (*ct_dump_start)(struct dpif *, struct ct_dpif_dump_state **state,
    -                         const uint16_t *zone);
    +                         const uint16_t *zone, int *);
         int (*ct_dump_next)(struct dpif *, struct ct_dpif_dump_state *state,
                             struct ct_dpif_entry *entry);
         int (*ct_dump_done)(struct dpif *, struct ct_dpif_dump_state *state);
    diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
    index f0e2aea..ac48b15 100644
    --- a/lib/netlink-conntrack.c?
    +++ b/lib/netlink-conntrack.c
    @@ -123,7 +123,8 @@ struct nl_ct_dump_state {
     
     /* Initialize a conntrack netlink dump. */
     int
    -nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t *zone)
    +nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t *zone,
    +        int *ptot_bkts)
     {
         struct nl_ct_dump_state *state;
     
    @@ -140,6 +141,9 @@ nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t *zone)
         nl_dump_start(&state->dump, NETLINK_NETFILTER, &state->buf);
         ofpbuf_clear(&state->buf);
     
    +    /* Buckets to store connections are not used. */
    +    *ptot_bkts = -1;
    +

[Darrell]
I tested this and looks good.

         return 0;
     }
     
    diff --git a/lib/netlink-conntrack.h b/lib/netlink-conntrack.h
    index 1263b21..a95aa69 100644
    --- a/lib/netlink-conntrack.h
    +++ b/lib/netlink-conntrack.h
    @@ -35,7 +35,8 @@ enum nl_ct_event_type {
     
     struct nl_ct_dump_state;
     
    -int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t *zone);
    +int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t *zone,
    +        int *ptot_bkts);
     int nl_ct_dump_next(struct nl_ct_dump_state *, struct ct_dpif_entry *);
     int nl_ct_dump_done(struct nl_ct_dump_state *);
     
    diff --git a/tests/test-netlink-conntrack.c b/tests/test-netlink-conntrack.c
    index 000062d..0d9dace 100644
    --- a/tests/test-netlink-conntrack.c
    +++ b/tests/test-netlink-conntrack.c
    @@ -106,6 +106,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)
         uint16_t zone, *pzone = NULL;
         struct ct_dpif_entry entry;
         int err;
    +    int tot_bkts;
     
         if (ctx->argc >= 2) {
             if (!ovs_scan(ctx->argv[1], "zone=%"SCNu16, &zone)) {
    @@ -113,7 +114,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)
             }
             pzone = &zone;
         }
    -    err = nl_ct_dump_start(&dump, pzone);
    +    err = nl_ct_dump_start(&dump, pzone, &tot_bkts);
         if (err) {
             ovs_fatal(err, "Error creating conntrack netlink dump");
         }
    diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
    index 7292fca..7b005ac 100644
    --- a/utilities/ovs-dpctl.c
    +++ b/utilities/ovs-dpctl.c
    @@ -202,6 +202,7 @@ usage(void *userdata OVS_UNUSED)
                    "delete all conntrack entries in ZONE\n"
                "  ct-stats-show [DP] [zone=ZONE] [verbose] " \
                    "CT connections grouped by protocol\n"
    +           "  ct-bkts [DP] [gt=N] display connections per CT bucket\n"
                "Each IFACE on add-dp, add-if, and set-if may be followed by\n"
                "comma-separated options.  See ovs-dpctl(8) for syntax, or the\n"
                "Interface table in ovs-vswitchd.conf.db(5) for an options list.\n"
    -- 
    2.4.11
    
    _______________________________________________
    dev mailing list
    dev@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=DzNptgj_q8j7ukiP3_wi0zYxZ4WDkZkhE1w1OSsoeD0&s=wOJItkDVJYq7hVfEWV_b7k3n8wFSLxdVYOIT2toHc40&e=

Comments

Fischetti, Antonio July 24, 2017, 9:44 a.m. UTC | #1
Thanks Darrell, I will apply your suggetions.

-Antonio

> -----Original Message-----

> From: Darrell Ball [mailto:dball@vmware.com]

> Sent: Sunday, July 23, 2017 6:28 PM

> To: Fischetti, Antonio <antonio.fischetti@intel.com>; dev@openvswitch.org

> Subject: Re: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

> 

> Minor comment:

> 

> When applying the patch, there is a complaint about new whitespace below


[Antonio] will remove it.


> 

> +the number of connections in a bucket is greater than \fIThreshold\fR.

> +

> 

> Two comments inline

> 

> 

> -----Original Message-----

> From: <ovs-dev-bounces@openvswitch.org> on behalf of

> "antonio.fischetti@intel.com" <antonio.fischetti@intel.com>

> Date: Saturday, July 22, 2017 at 7:38 AM

> To: "dev@openvswitch.org" <dev@openvswitch.org>

> Subject: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

> 

>     With the command:

>      ovs-appctl dpctl/ct-bkts

>     shows the number of connections per bucket.

> 

>     By using a threshold:

>      ovs-appctl dpctl/ct-bkts gt=N

>     for each bucket shows the number of connections when they

>     are greater than N.

> 

>     Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>

>     Signed-off-by: Bhanuprakash Bodireddy

> <bhanuprakash.bodireddy@intel.com>

>     Co-authored-by: Bhanuprakash Bodireddy

> <bhanuprakash.bodireddy@intel.com>

>     ---

>      lib/conntrack.c                |   9 ++--

>      lib/conntrack.h                |   2 +-

>      lib/ct-dpif.c                  |   4 +-

>      lib/ct-dpif.h                  |   3 +-

>      lib/dpctl.c                    | 108

> ++++++++++++++++++++++++++++++++++++++++-

>      lib/dpctl.man                  |   8 +++

>      lib/dpif-netdev.c              |   4 +-

>      lib/dpif-netlink.c             |   4 +-

>      lib/dpif-provider.h            |   2 +-

>      lib/netlink-conntrack.c        |   6 ++-

>      lib/netlink-conntrack.h        |   3 +-

>      tests/test-netlink-conntrack.c |   3 +-

>      utilities/ovs-dpctl.c          |   1 +

>      13 files changed, 140 insertions(+), 17 deletions(-)

> 

>     diff --git a/lib/conntrack.c b/lib/conntrack.c

>     index de46a6b..e290b20 100644

>     --- a/lib/conntrack.c

>     +++ b/lib/conntrack.c

>     @@ -1931,7 +1931,7 @@ conn_key_to_tuple(const struct conn_key *key,

> struct ct_dpif_tuple *tuple)

> 

>      static void

>      conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry

> *entry,

>     -                      long long now)

>     +                      long long now, int bkt)

>      {

>          struct ct_l4_proto *class;

>          long long expiration;

>     @@ -1954,11 +1954,12 @@ conn_to_ct_dpif_entry(const struct conn *conn,

> struct ct_dpif_entry *entry,

>          if (class->conn_get_protoinfo) {

>              class->conn_get_protoinfo(conn, &entry->protoinfo);

>          }

>     +    entry->bkt = bkt;

>      }

> 

>      int

>      conntrack_dump_start(struct conntrack *ct, struct conntrack_dump

> *dump,

>     -                     const uint16_t *pzone)

>     +                     const uint16_t *pzone, int *ptot_bkts)

>      {

>          memset(dump, 0, sizeof(*dump));

>          if (pzone) {

>     @@ -1967,6 +1968,8 @@ conntrack_dump_start(struct conntrack *ct,

> struct conntrack_dump *dump,

>          }

>          dump->ct = ct;

> 

>     +    *ptot_bkts = CONNTRACK_BUCKETS;

>     +

>          return 0;

>      }

> 

>     @@ -1991,7 +1994,7 @@ conntrack_dump_next(struct conntrack_dump *dump,

> struct ct_dpif_entry *entry)

>                  INIT_CONTAINER(conn, node, node);

>                  if ((!dump->filter_zone || conn->key.zone == dump->zone)

> &&

>                       (conn->conn_type != CT_CONN_TYPE_UN_NAT)) {

>     -                conn_to_ct_dpif_entry(conn, entry, now);

>     +                conn_to_ct_dpif_entry(conn, entry, now, dump-

> >bucket);

>                      break;

>                  }

>                  /* Else continue, until we find an entry in the

> appropriate zone

>     diff --git a/lib/conntrack.h b/lib/conntrack.h

>     index defde4c..3f48444 100644

>     --- a/lib/conntrack.h

>     +++ b/lib/conntrack.h

>     @@ -108,7 +108,7 @@ struct conntrack_dump {

>      struct ct_dpif_entry;

> 

>      int conntrack_dump_start(struct conntrack *, struct conntrack_dump *,

>     -                         const uint16_t *pzone);

>     +                         const uint16_t *pzone, int *);

>      int conntrack_dump_next(struct conntrack_dump *, struct ct_dpif_entry

> *);

>      int conntrack_dump_done(struct conntrack_dump *);

> 

>     diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c

>     index f8d2cf1..c79e69e 100644

>     --- a/lib/ct-dpif.c

>     +++ b/lib/ct-dpif.c

>     @@ -65,12 +65,12 @@ static const struct flags ct_dpif_status_flags[] =

> {

>       * that represents the error.  Otherwise it returns zero. */

>      int

>      ct_dpif_dump_start(struct dpif *dpif, struct ct_dpif_dump_state

> **dump,

>     -                   const uint16_t *zone)

>     +                   const uint16_t *zone, int *ptot_bkts)

>      {

>          int err;

> 

>          err = (dpif->dpif_class->ct_dump_start

>     -           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone)

>     +           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone,

> ptot_bkts)

>                 : EOPNOTSUPP);

> 

>          if (!err) {

>     diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h

>     index cd35f3e..d5f9661 100644

>     --- a/lib/ct-dpif.h

>     +++ b/lib/ct-dpif.h

>     @@ -169,6 +169,7 @@ struct ct_dpif_entry {

>          /* Timeout for this entry in seconds */

>          uint32_t timeout;

>          uint32_t mark;

>     +    uint32_t bkt;       /* CT bucket number. */

>      };

> 

>      enum {

>     @@ -191,7 +192,7 @@ struct ct_dpif_dump_state {

>      };

> 

>      int ct_dpif_dump_start(struct dpif *, struct ct_dpif_dump_state **,

>     -                       const uint16_t *zone);

>     +                       const uint16_t *zone, int *);

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

>     diff --git a/lib/dpctl.c b/lib/dpctl.c

>     index 6aa6c8e..3ccc7a3 100644

>     --- a/lib/dpctl.c

>     +++ b/lib/dpctl.c

>     @@ -1258,6 +1258,7 @@ dpctl_dump_conntrack(int argc, const char

> *argv[],

>          struct ct_dpif_dump_state *dump;

>          struct ct_dpif_entry cte;

>          uint16_t zone, *pzone = NULL;

>     +    int tot_bkts;

>          struct dpif *dpif;

>          char *name;

>          int error;

>     @@ -1277,7 +1278,7 @@ dpctl_dump_conntrack(int argc, const char

> *argv[],

>              return error;

>          }

> 

>     -    error = ct_dpif_dump_start(dpif, &dump, pzone);

>     +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);

>          if (error) {

>              dpctl_error(dpctl_p, error, "starting conntrack dump");

>              dpif_close(dpif);

>     @@ -1339,6 +1340,7 @@ dpctl_ct_stats_show(int argc, const char

> *argv[],

>          struct ct_dpif_dump_state *dump;

>          struct ct_dpif_entry cte;

>          uint16_t zone, *pzone = NULL;

>     +    int tot_bkts;

>          bool verbose = false;

>          int lastargc = 0;

> 

>     @@ -1373,7 +1375,7 @@ dpctl_ct_stats_show(int argc, const char

> *argv[],

> 

>          memset(proto_stats, 0, sizeof(proto_stats));

>          memset(tcp_conn_per_states, 0, sizeof(tcp_conn_per_states));

>     -    error = ct_dpif_dump_start(dpif, &dump, pzone);

>     +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);

>          if (error) {

>              dpctl_error(dpctl_p, error, "starting conntrack dump");

>              dpif_close(dpif);

>     @@ -1464,6 +1466,107 @@ dpctl_ct_stats_show(int argc, const char

> *argv[],

>          dpif_close(dpif);

>          return error;

>      }

>     +

>     +#define CT_BKTS_GT "gt="

>     +static int

>     +dpctl_ct_bkts(int argc, const char *argv[],

>     +                     struct dpctl_params *dpctl_p)

>     +{

>     +    struct dpif *dpif;

>     +    char *name;

>     +

>     +    struct ct_dpif_dump_state *dump;

>     +    struct ct_dpif_entry cte;

>     +    uint16_t gt = 0; /* Threshold: display value when greater than

> gt. */

>     +    uint16_t *pzone = NULL;

>     +    int tot_bkts = 0;

>     +    int lastargc = 0;

>     +

>     +    int error;

>     +

>     +    while (argc > 1 && lastargc != argc) {

>     +        lastargc = argc;

>     +        if (!strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT)))

> {

>     +            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

>     +                argc--;

>     +                break;

>     +            }

>     +        }

>     +    }

> 

> [Darrell]

> When, I do this, I get a failure, which is expected.

> 

> > 2017-07-23T02:28:02.155Z|00052|connmgr|INFO|br0<->unix: 11 flow_mods in

> the last 0 s (11 adds)

> > 2017-07-23T02:28:02.346Z|00053|unixctl|DBG|received request dpctl/ct-

> bkts["gt=0","netdev@ovs-netdev"], id=0

> > 2017-07-23T02:28:02.346Z|00054|dpctl|INFO|set_names=0 verbosity=0

> names=0

> > 2017-07-23T02:28:02.346Z|00057|dpif|WARN|could not create datapath gt=0

> of unknown type system

> > 2017-07-23T02:28:02.347Z|00058|unixctl|DBG|replying with error, id=0:

> "ovs-vswitchd: opening datapath (Address family not supported by protocol)

> 

> But, your command requires a maximum of 2 parameters and if there are 2,

> the datapath name must be first and ‘gt=x’ second

> Hence, I think you can just remove the while loop completely since ‘gt=x’

> must always be last, if present.

> 

> diff --git a/lib/dpctl.c b/lib/dpctl.c

> index cb791ca..5f3801c 100644

> --- a/lib/dpctl.c

> +++ b/lib/dpctl.c

> @@ -1480,17 +1480,11 @@ dpctl_ct_bkts(int argc, const char *argv[],

>      uint16_t gt = 0; /* Threshold: display value when greater than gt. */

>      uint16_t *pzone = NULL;

>      int tot_bkts = 0;

> -    int lastargc = 0;

> -

>      int error;

> 

> -    while (argc > 1 && lastargc != argc) {

> -        lastargc = argc;

> -        if (!strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {

> -            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

> -                argc--;

> -                break;

> -            }

> +    if (argc > 1 && !strncmp(argv[argc - 1], CT_BKTS_GT,

> strlen(CT_BKTS_GT))) {

> +        if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

> +            argc--;

>          }

>      }


[Antonio] ok will change it


> 

> //////////////////////////////

> 

>     +

>     +    name = (argc > 1) ? 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_dump_start(dpif, &dump, pzone, &tot_bkts);

>     +    if (error) {

>     +        dpctl_error(dpctl_p, error, "starting conntrack dump");

>     +        dpif_close(dpif);

>     +        return error;

>     +    }

>     +    if (tot_bkts == -1) {

>     +         /* Command not available when called by kernel OvS. */

>     +         dpctl_print(dpctl_p,

>     +             "Command is available for UserSpace ConnTracker

> only.\n");

>     +         ct_dpif_dump_done(dump);

> 

> [Darrell]

>           dpctl_print(dpctl_p,

>               "Command is available for UserSpace ConnTracker only.\n");

>           ct_dpif_dump_done(dump);

> +         dpif_close(dpif);

>           return 0;

>      }

> 

> //////////////////////////////

> 

> 

>     +         return 0;

>     +    }

>     +

>     +    dpctl_print(dpctl_p, "Total Buckets: %d\n", tot_bkts);

>     +

>     +    int tot_conn = 0;

>     +    uint32_t conn_per_bkts[tot_bkts];

>     +    memset(conn_per_bkts, 0, sizeof(uint32_t) * tot_bkts);

>     +

>     +    while (!ct_dpif_dump_next(dump, &cte)) {

>     +        ct_dpif_entry_uninit(&cte);

>     +        tot_conn++;

>     +        if (tot_bkts > 0) {

>     +            if (cte.bkt < tot_bkts) {

>     +                conn_per_bkts[cte.bkt]++;

>     +            } else {

>     +                dpctl_print(dpctl_p, "Bucket nr out of range: %d >=

> %d\n",

>     +                        cte.bkt, tot_bkts);

>     +            }

>     +        }

>     +    }

>     +

>     +    dpctl_print(dpctl_p, "Current Connections: %d\n", tot_conn);

>     +    dpctl_print(dpctl_p, "\n");

>     +    if (tot_bkts && tot_conn) {

>     +        dpctl_print(dpctl_p, "+-----------+"

>     +                "-----------------------------------------+\n");

>     +        dpctl_print(dpctl_p, "|  Buckets  |"

>     +                "         Connections per Buckets         |\n");

>     +        dpctl_print(dpctl_p, "+-----------+"

>     +                "-----------------------------------------+");

>     +#define NUM_BKTS_DIPLAYED_PER_ROW 8

>     +        for (int i = 0; i < tot_bkts; i++) {

>     +            if (i % NUM_BKTS_DIPLAYED_PER_ROW == 0) {

>     +                 dpctl_print(dpctl_p, "\n %3d..%3d   | ",

>     +                         i, i + NUM_BKTS_DIPLAYED_PER_ROW - 1);

>     +            }

>     +            if (conn_per_bkts[i] > gt) {

>     +                dpctl_print(dpctl_p, "%5d", conn_per_bkts[i]);

>     +            } else {

>     +                dpctl_print(dpctl_p, "%5s", ".");

>     +            }

>     +        }

>     +        dpctl_print(dpctl_p, "\n\n");

>     +    }

>     +

>     +    ct_dpif_dump_done(dump);

>     +    dpif_close(dpif);

>     +    return error;

>     +}

>      ?

>      /* Undocumented commands for unit testing. */

> 

>     @@ -1760,6 +1863,7 @@ static const struct dpctl_command all_commands[]

> = {

>          { "flush-conntrack", "[dp] [zone=N]", 0, 2,

> dpctl_flush_conntrack, DP_RW },

>          { "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 },

>          { "help", "", 0, INT_MAX, dpctl_help, DP_RO },

>          { "list-commands", "", 0, INT_MAX, dpctl_list_commands, DP_RO },

> 

>     diff --git a/lib/dpctl.man b/lib/dpctl.man

>     index f95d54a..077fd99 100644

>     --- a/lib/dpctl.man

>     +++ b/lib/dpctl.man

>     @@ -228,3 +228,11 @@ Displays the number of connections grouped by

> protocol used by \fIdp\fR.

>      If \fBzone=\fIzone\fR is specified, numbers refer to the connections

> in

>      \fBzone\fR. The \fBverbose\fR option allows to group by connection

> state

>      for each protocol.

>     +.

>     +.TP

>     +\*(DX\fBct\-bkts\fR [\fIdp\fR] [\fBgt=\fIThreshold\fR]

>     +For each ConnTracker bucket, displays the number of connections used

>     +by \fIdp\fR.

>     +If \fBgt=\fIThreshold\fR is specified, bucket numbers are displayed

> when

>     +the number of connections in a bucket is greater than

> \fIThreshold\fR.

>     +

>     diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c

>     index 47a9fa0..10435e7 100644

>     --- a/lib/dpif-netdev.c

>     +++ b/lib/dpif-netdev.c

>     @@ -5333,7 +5333,7 @@ struct dp_netdev_ct_dump {

> 

>      static int

>      dpif_netdev_ct_dump_start(struct dpif *dpif, struct

> ct_dpif_dump_state **dump_,

>     -                          const uint16_t *pzone)

>     +                          const uint16_t *pzone, int *ptot_bkts)

>      {

>          struct dp_netdev *dp = get_dp_netdev(dpif);

>          struct dp_netdev_ct_dump *dump;

>     @@ -5342,7 +5342,7 @@ dpif_netdev_ct_dump_start(struct dpif *dpif,

> struct ct_dpif_dump_state **dump_,

>          dump->dp = dp;

>          dump->ct = &dp->conntrack;

> 

>     -    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone);

>     +    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone,

> ptot_bkts);

> 

>          *dump_ = &dump->up;

> 

>     diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c

>     index 55effd1..b87bb16 100644

>     --- a/lib/dpif-netlink.c

>     +++ b/lib/dpif-netlink.c

>     @@ -2847,13 +2847,13 @@ struct dpif_netlink_ct_dump_state {

>      static int

>      dpif_netlink_ct_dump_start(struct dpif *dpif OVS_UNUSED,

>                                 struct ct_dpif_dump_state **dump_,

>     -                           const uint16_t *zone)

>     +                           const uint16_t *zone, int *ptot_bkts)

>      {

>          struct dpif_netlink_ct_dump_state *dump;

>          int err;

> 

>          dump = xzalloc(sizeof *dump);

>     -    err = nl_ct_dump_start(&dump->nl_ct_dump, zone);

>     +    err = nl_ct_dump_start(&dump->nl_ct_dump, zone, ptot_bkts);

>          if (err) {

>              free(dump);

>              return err;

>     diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h

>     index 64ac2e2..1d82a09 100644

>     --- a/lib/dpif-provider.h

>     +++ b/lib/dpif-provider.h

>     @@ -419,7 +419,7 @@ struct dpif_class {

>           * ct_dump_done() should perform any cleanup necessary (including

>           * deallocating the 'state' structure, if applicable). */

>          int (*ct_dump_start)(struct dpif *, struct ct_dpif_dump_state

> **state,

>     -                         const uint16_t *zone);

>     +                         const uint16_t *zone, int *);

>          int (*ct_dump_next)(struct dpif *, struct ct_dpif_dump_state

> *state,

>                              struct ct_dpif_entry *entry);

>          int (*ct_dump_done)(struct dpif *, struct ct_dpif_dump_state

> *state);

>     diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c

>     index f0e2aea..ac48b15 100644

>     --- a/lib/netlink-conntrack.c?

>     +++ b/lib/netlink-conntrack.c

>     @@ -123,7 +123,8 @@ struct nl_ct_dump_state {

> 

>      /* Initialize a conntrack netlink dump. */

>      int

>     -nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t

> *zone)

>     +nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t

> *zone,

>     +        int *ptot_bkts)

>      {

>          struct nl_ct_dump_state *state;

> 

>     @@ -140,6 +141,9 @@ nl_ct_dump_start(struct nl_ct_dump_state **statep,

> const uint16_t *zone)

>          nl_dump_start(&state->dump, NETLINK_NETFILTER, &state->buf);

>          ofpbuf_clear(&state->buf);

> 

>     +    /* Buckets to store connections are not used. */

>     +    *ptot_bkts = -1;

>     +

> 

> [Darrell]

> I tested this and looks good.

> 

>          return 0;

>      }

> 

>     diff --git a/lib/netlink-conntrack.h b/lib/netlink-conntrack.h

>     index 1263b21..a95aa69 100644

>     --- a/lib/netlink-conntrack.h

>     +++ b/lib/netlink-conntrack.h

>     @@ -35,7 +35,8 @@ enum nl_ct_event_type {

> 

>      struct nl_ct_dump_state;

> 

>     -int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t

> *zone);

>     +int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t

> *zone,

>     +        int *ptot_bkts);

>      int nl_ct_dump_next(struct nl_ct_dump_state *, struct ct_dpif_entry

> *);

>      int nl_ct_dump_done(struct nl_ct_dump_state *);

> 

>     diff --git a/tests/test-netlink-conntrack.c b/tests/test-netlink-

> conntrack.c

>     index 000062d..0d9dace 100644

>     --- a/tests/test-netlink-conntrack.c

>     +++ b/tests/test-netlink-conntrack.c

>     @@ -106,6 +106,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)

>          uint16_t zone, *pzone = NULL;

>          struct ct_dpif_entry entry;

>          int err;

>     +    int tot_bkts;

> 

>          if (ctx->argc >= 2) {

>              if (!ovs_scan(ctx->argv[1], "zone=%"SCNu16, &zone)) {

>     @@ -113,7 +114,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)

>              }

>              pzone = &zone;

>          }

>     -    err = nl_ct_dump_start(&dump, pzone);

>     +    err = nl_ct_dump_start(&dump, pzone, &tot_bkts);

>          if (err) {

>              ovs_fatal(err, "Error creating conntrack netlink dump");

>          }

>     diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c

>     index 7292fca..7b005ac 100644

>     --- a/utilities/ovs-dpctl.c

>     +++ b/utilities/ovs-dpctl.c

>     @@ -202,6 +202,7 @@ usage(void *userdata OVS_UNUSED)

>                     "delete all conntrack entries in ZONE\n"

>                 "  ct-stats-show [DP] [zone=ZONE] [verbose] " \

>                     "CT connections grouped by protocol\n"

>     +           "  ct-bkts [DP] [gt=N] display connections per CT

> bucket\n"

>                 "Each IFACE on add-dp, add-if, and set-if may be followed

> by\n"

>                 "comma-separated options.  See ovs-dpctl(8) for syntax, or

> the\n"

>                 "Interface table in ovs-vswitchd.conf.db(5) for an options

> list.\n"

>     --

>     2.4.11

> 

>     _______________________________________________

>     dev mailing list

>     dev@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=DzNptgj_q8j7ukiP3_wi0zYxZ4WDkZkhE1w1OSsoeD0&s=wOJItkDVJYq7hVfEWV_b

> 7k3n8wFSLxdVYOIT2toHc40&e=

> 

> 

>
Fischetti, Antonio July 25, 2017, 1:57 p.m. UTC | #2
Hi Darrell, I posted a v5 at http://patchwork.ozlabs.org/patch/792723/
where I added all your suggestions.

Thanks,
-Antonio

> -----Original Message-----

> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

> bounces@openvswitch.org] On Behalf Of Fischetti, Antonio

> Sent: Monday, July 24, 2017 10:45 AM

> To: Darrell Ball <dball@vmware.com>; dev@openvswitch.org

> Subject: Re: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

> 

> Thanks Darrell, I will apply your suggetions.

> 

> -Antonio

> 

> > -----Original Message-----

> > From: Darrell Ball [mailto:dball@vmware.com]

> > Sent: Sunday, July 23, 2017 6:28 PM

> > To: Fischetti, Antonio <antonio.fischetti@intel.com>;

> dev@openvswitch.org

> > Subject: Re: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

> >

> > Minor comment:

> >

> > When applying the patch, there is a complaint about new whitespace below

> 

> [Antonio] will remove it.

> 

> 

> >

> > +the number of connections in a bucket is greater than \fIThreshold\fR.

> > +

> >

> > Two comments inline

> >

> >

> > -----Original Message-----

> > From: <ovs-dev-bounces@openvswitch.org> on behalf of

> > "antonio.fischetti@intel.com" <antonio.fischetti@intel.com>

> > Date: Saturday, July 22, 2017 at 7:38 AM

> > To: "dev@openvswitch.org" <dev@openvswitch.org>

> > Subject: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

> >

> >     With the command:

> >      ovs-appctl dpctl/ct-bkts

> >     shows the number of connections per bucket.

> >

> >     By using a threshold:

> >      ovs-appctl dpctl/ct-bkts gt=N

> >     for each bucket shows the number of connections when they

> >     are greater than N.

> >

> >     Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>

> >     Signed-off-by: Bhanuprakash Bodireddy

> > <bhanuprakash.bodireddy@intel.com>

> >     Co-authored-by: Bhanuprakash Bodireddy

> > <bhanuprakash.bodireddy@intel.com>

> >     ---

> >      lib/conntrack.c                |   9 ++--

> >      lib/conntrack.h                |   2 +-

> >      lib/ct-dpif.c                  |   4 +-

> >      lib/ct-dpif.h                  |   3 +-

> >      lib/dpctl.c                    | 108

> > ++++++++++++++++++++++++++++++++++++++++-

> >      lib/dpctl.man                  |   8 +++

> >      lib/dpif-netdev.c              |   4 +-

> >      lib/dpif-netlink.c             |   4 +-

> >      lib/dpif-provider.h            |   2 +-

> >      lib/netlink-conntrack.c        |   6 ++-

> >      lib/netlink-conntrack.h        |   3 +-

> >      tests/test-netlink-conntrack.c |   3 +-

> >      utilities/ovs-dpctl.c          |   1 +

> >      13 files changed, 140 insertions(+), 17 deletions(-)

> >

> >     diff --git a/lib/conntrack.c b/lib/conntrack.c

> >     index de46a6b..e290b20 100644

> >     --- a/lib/conntrack.c

> >     +++ b/lib/conntrack.c

> >     @@ -1931,7 +1931,7 @@ conn_key_to_tuple(const struct conn_key *key,

> > struct ct_dpif_tuple *tuple)

> >

> >      static void

> >      conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry

> > *entry,

> >     -                      long long now)

> >     +                      long long now, int bkt)

> >      {

> >          struct ct_l4_proto *class;

> >          long long expiration;

> >     @@ -1954,11 +1954,12 @@ conn_to_ct_dpif_entry(const struct conn

> *conn,

> > struct ct_dpif_entry *entry,

> >          if (class->conn_get_protoinfo) {

> >              class->conn_get_protoinfo(conn, &entry->protoinfo);

> >          }

> >     +    entry->bkt = bkt;

> >      }

> >

> >      int

> >      conntrack_dump_start(struct conntrack *ct, struct conntrack_dump

> > *dump,

> >     -                     const uint16_t *pzone)

> >     +                     const uint16_t *pzone, int *ptot_bkts)

> >      {

> >          memset(dump, 0, sizeof(*dump));

> >          if (pzone) {

> >     @@ -1967,6 +1968,8 @@ conntrack_dump_start(struct conntrack *ct,

> > struct conntrack_dump *dump,

> >          }

> >          dump->ct = ct;

> >

> >     +    *ptot_bkts = CONNTRACK_BUCKETS;

> >     +

> >          return 0;

> >      }

> >

> >     @@ -1991,7 +1994,7 @@ conntrack_dump_next(struct conntrack_dump

> *dump,

> > struct ct_dpif_entry *entry)

> >                  INIT_CONTAINER(conn, node, node);

> >                  if ((!dump->filter_zone || conn->key.zone == dump-

> >zone)

> > &&

> >                       (conn->conn_type != CT_CONN_TYPE_UN_NAT)) {

> >     -                conn_to_ct_dpif_entry(conn, entry, now);

> >     +                conn_to_ct_dpif_entry(conn, entry, now, dump-

> > >bucket);

> >                      break;

> >                  }

> >                  /* Else continue, until we find an entry in the

> > appropriate zone

> >     diff --git a/lib/conntrack.h b/lib/conntrack.h

> >     index defde4c..3f48444 100644

> >     --- a/lib/conntrack.h

> >     +++ b/lib/conntrack.h

> >     @@ -108,7 +108,7 @@ struct conntrack_dump {

> >      struct ct_dpif_entry;

> >

> >      int conntrack_dump_start(struct conntrack *, struct conntrack_dump

> *,

> >     -                         const uint16_t *pzone);

> >     +                         const uint16_t *pzone, int *);

> >      int conntrack_dump_next(struct conntrack_dump *, struct

> ct_dpif_entry

> > *);

> >      int conntrack_dump_done(struct conntrack_dump *);

> >

> >     diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c

> >     index f8d2cf1..c79e69e 100644

> >     --- a/lib/ct-dpif.c

> >     +++ b/lib/ct-dpif.c

> >     @@ -65,12 +65,12 @@ static const struct flags ct_dpif_status_flags[]

> =

> > {

> >       * that represents the error.  Otherwise it returns zero. */

> >      int

> >      ct_dpif_dump_start(struct dpif *dpif, struct ct_dpif_dump_state

> > **dump,

> >     -                   const uint16_t *zone)

> >     +                   const uint16_t *zone, int *ptot_bkts)

> >      {

> >          int err;

> >

> >          err = (dpif->dpif_class->ct_dump_start

> >     -           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone)

> >     +           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone,

> > ptot_bkts)

> >                 : EOPNOTSUPP);

> >

> >          if (!err) {

> >     diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h

> >     index cd35f3e..d5f9661 100644

> >     --- a/lib/ct-dpif.h

> >     +++ b/lib/ct-dpif.h

> >     @@ -169,6 +169,7 @@ struct ct_dpif_entry {

> >          /* Timeout for this entry in seconds */

> >          uint32_t timeout;

> >          uint32_t mark;

> >     +    uint32_t bkt;       /* CT bucket number. */

> >      };

> >

> >      enum {

> >     @@ -191,7 +192,7 @@ struct ct_dpif_dump_state {

> >      };

> >

> >      int ct_dpif_dump_start(struct dpif *, struct ct_dpif_dump_state **,

> >     -                       const uint16_t *zone);

> >     +                       const uint16_t *zone, int *);

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

> >     diff --git a/lib/dpctl.c b/lib/dpctl.c

> >     index 6aa6c8e..3ccc7a3 100644

> >     --- a/lib/dpctl.c

> >     +++ b/lib/dpctl.c

> >     @@ -1258,6 +1258,7 @@ dpctl_dump_conntrack(int argc, const char

> > *argv[],

> >          struct ct_dpif_dump_state *dump;

> >          struct ct_dpif_entry cte;

> >          uint16_t zone, *pzone = NULL;

> >     +    int tot_bkts;

> >          struct dpif *dpif;

> >          char *name;

> >          int error;

> >     @@ -1277,7 +1278,7 @@ dpctl_dump_conntrack(int argc, const char

> > *argv[],

> >              return error;

> >          }

> >

> >     -    error = ct_dpif_dump_start(dpif, &dump, pzone);

> >     +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);

> >          if (error) {

> >              dpctl_error(dpctl_p, error, "starting conntrack dump");

> >              dpif_close(dpif);

> >     @@ -1339,6 +1340,7 @@ dpctl_ct_stats_show(int argc, const char

> > *argv[],

> >          struct ct_dpif_dump_state *dump;

> >          struct ct_dpif_entry cte;

> >          uint16_t zone, *pzone = NULL;

> >     +    int tot_bkts;

> >          bool verbose = false;

> >          int lastargc = 0;

> >

> >     @@ -1373,7 +1375,7 @@ dpctl_ct_stats_show(int argc, const char

> > *argv[],

> >

> >          memset(proto_stats, 0, sizeof(proto_stats));

> >          memset(tcp_conn_per_states, 0, sizeof(tcp_conn_per_states));

> >     -    error = ct_dpif_dump_start(dpif, &dump, pzone);

> >     +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);

> >          if (error) {

> >              dpctl_error(dpctl_p, error, "starting conntrack dump");

> >              dpif_close(dpif);

> >     @@ -1464,6 +1466,107 @@ dpctl_ct_stats_show(int argc, const char

> > *argv[],

> >          dpif_close(dpif);

> >          return error;

> >      }

> >     +

> >     +#define CT_BKTS_GT "gt="

> >     +static int

> >     +dpctl_ct_bkts(int argc, const char *argv[],

> >     +                     struct dpctl_params *dpctl_p)

> >     +{

> >     +    struct dpif *dpif;

> >     +    char *name;

> >     +

> >     +    struct ct_dpif_dump_state *dump;

> >     +    struct ct_dpif_entry cte;

> >     +    uint16_t gt = 0; /* Threshold: display value when greater than

> > gt. */

> >     +    uint16_t *pzone = NULL;

> >     +    int tot_bkts = 0;

> >     +    int lastargc = 0;

> >     +

> >     +    int error;

> >     +

> >     +    while (argc > 1 && lastargc != argc) {

> >     +        lastargc = argc;

> >     +        if (!strncmp(argv[argc - 1], CT_BKTS_GT,

> strlen(CT_BKTS_GT)))

> > {

> >     +            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt))

> {

> >     +                argc--;

> >     +                break;

> >     +            }

> >     +        }

> >     +    }

> >

> > [Darrell]

> > When, I do this, I get a failure, which is expected.

> >

> > > 2017-07-23T02:28:02.155Z|00052|connmgr|INFO|br0<->unix: 11 flow_mods

> in

> > the last 0 s (11 adds)

> > > 2017-07-23T02:28:02.346Z|00053|unixctl|DBG|received request dpctl/ct-

> > bkts["gt=0","netdev@ovs-netdev"], id=0

> > > 2017-07-23T02:28:02.346Z|00054|dpctl|INFO|set_names=0 verbosity=0

> > names=0

> > > 2017-07-23T02:28:02.346Z|00057|dpif|WARN|could not create datapath

> gt=0

> > of unknown type system

> > > 2017-07-23T02:28:02.347Z|00058|unixctl|DBG|replying with error, id=0:

> > "ovs-vswitchd: opening datapath (Address family not supported by

> protocol)

> >

> > But, your command requires a maximum of 2 parameters and if there are 2,

> > the datapath name must be first and ‘gt=x’ second

> > Hence, I think you can just remove the while loop completely since

> ‘gt=x’

> > must always be last, if present.

> >

> > diff --git a/lib/dpctl.c b/lib/dpctl.c

> > index cb791ca..5f3801c 100644

> > --- a/lib/dpctl.c

> > +++ b/lib/dpctl.c

> > @@ -1480,17 +1480,11 @@ dpctl_ct_bkts(int argc, const char *argv[],

> >      uint16_t gt = 0; /* Threshold: display value when greater than gt.

> */

> >      uint16_t *pzone = NULL;

> >      int tot_bkts = 0;

> > -    int lastargc = 0;

> > -

> >      int error;

> >

> > -    while (argc > 1 && lastargc != argc) {

> > -        lastargc = argc;

> > -        if (!strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {

> > -            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

> > -                argc--;

> > -                break;

> > -            }

> > +    if (argc > 1 && !strncmp(argv[argc - 1], CT_BKTS_GT,

> > strlen(CT_BKTS_GT))) {

> > +        if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

> > +            argc--;

> >          }

> >      }

> 

> [Antonio] ok will change it

> 

> 

> >

> > //////////////////////////////

> >

> >     +

> >     +    name = (argc > 1) ? 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_dump_start(dpif, &dump, pzone, &tot_bkts);

> >     +    if (error) {

> >     +        dpctl_error(dpctl_p, error, "starting conntrack dump");

> >     +        dpif_close(dpif);

> >     +        return error;

> >     +    }

> >     +    if (tot_bkts == -1) {

> >     +         /* Command not available when called by kernel OvS. */

> >     +         dpctl_print(dpctl_p,

> >     +             "Command is available for UserSpace ConnTracker

> > only.\n");

> >     +         ct_dpif_dump_done(dump);

> >

> > [Darrell]

> >           dpctl_print(dpctl_p,

> >               "Command is available for UserSpace ConnTracker only.\n");

> >           ct_dpif_dump_done(dump);

> > +         dpif_close(dpif);

> >           return 0;

> >      }

> >

> > //////////////////////////////

> >

> >

> >     +         return 0;

> >     +    }

> >     +

> >     +    dpctl_print(dpctl_p, "Total Buckets: %d\n", tot_bkts);

> >     +

> >     +    int tot_conn = 0;

> >     +    uint32_t conn_per_bkts[tot_bkts];

> >     +    memset(conn_per_bkts, 0, sizeof(uint32_t) * tot_bkts);

> >     +

> >     +    while (!ct_dpif_dump_next(dump, &cte)) {

> >     +        ct_dpif_entry_uninit(&cte);

> >     +        tot_conn++;

> >     +        if (tot_bkts > 0) {

> >     +            if (cte.bkt < tot_bkts) {

> >     +                conn_per_bkts[cte.bkt]++;

> >     +            } else {

> >     +                dpctl_print(dpctl_p, "Bucket nr out of range: %d >=

> > %d\n",

> >     +                        cte.bkt, tot_bkts);

> >     +            }

> >     +        }

> >     +    }

> >     +

> >     +    dpctl_print(dpctl_p, "Current Connections: %d\n", tot_conn);

> >     +    dpctl_print(dpctl_p, "\n");

> >     +    if (tot_bkts && tot_conn) {

> >     +        dpctl_print(dpctl_p, "+-----------+"

> >     +                "-----------------------------------------+\n");

> >     +        dpctl_print(dpctl_p, "|  Buckets  |"

> >     +                "         Connections per Buckets         |\n");

> >     +        dpctl_print(dpctl_p, "+-----------+"

> >     +                "-----------------------------------------+");

> >     +#define NUM_BKTS_DIPLAYED_PER_ROW 8

> >     +        for (int i = 0; i < tot_bkts; i++) {

> >     +            if (i % NUM_BKTS_DIPLAYED_PER_ROW == 0) {

> >     +                 dpctl_print(dpctl_p, "\n %3d..%3d   | ",

> >     +                         i, i + NUM_BKTS_DIPLAYED_PER_ROW - 1);

> >     +            }

> >     +            if (conn_per_bkts[i] > gt) {

> >     +                dpctl_print(dpctl_p, "%5d", conn_per_bkts[i]);

> >     +            } else {

> >     +                dpctl_print(dpctl_p, "%5s", ".");

> >     +            }

> >     +        }

> >     +        dpctl_print(dpctl_p, "\n\n");

> >     +    }

> >     +

> >     +    ct_dpif_dump_done(dump);

> >     +    dpif_close(dpif);

> >     +    return error;

> >     +}

> >      ?

> >      /* Undocumented commands for unit testing. */

> >

> >     @@ -1760,6 +1863,7 @@ static const struct dpctl_command

> all_commands[]

> > = {

> >          { "flush-conntrack", "[dp] [zone=N]", 0, 2,

> > dpctl_flush_conntrack, DP_RW },

> >          { "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 },

> >          { "help", "", 0, INT_MAX, dpctl_help, DP_RO },

> >          { "list-commands", "", 0, INT_MAX, dpctl_list_commands, DP_RO

> },

> >

> >     diff --git a/lib/dpctl.man b/lib/dpctl.man

> >     index f95d54a..077fd99 100644

> >     --- a/lib/dpctl.man

> >     +++ b/lib/dpctl.man

> >     @@ -228,3 +228,11 @@ Displays the number of connections grouped by

> > protocol used by \fIdp\fR.

> >      If \fBzone=\fIzone\fR is specified, numbers refer to the

> connections

> > in

> >      \fBzone\fR. The \fBverbose\fR option allows to group by connection

> > state

> >      for each protocol.

> >     +.

> >     +.TP

> >     +\*(DX\fBct\-bkts\fR [\fIdp\fR] [\fBgt=\fIThreshold\fR]

> >     +For each ConnTracker bucket, displays the number of connections

> used

> >     +by \fIdp\fR.

> >     +If \fBgt=\fIThreshold\fR is specified, bucket numbers are displayed

> > when

> >     +the number of connections in a bucket is greater than

> > \fIThreshold\fR.

> >     +

> >     diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c

> >     index 47a9fa0..10435e7 100644

> >     --- a/lib/dpif-netdev.c

> >     +++ b/lib/dpif-netdev.c

> >     @@ -5333,7 +5333,7 @@ struct dp_netdev_ct_dump {

> >

> >      static int

> >      dpif_netdev_ct_dump_start(struct dpif *dpif, struct

> > ct_dpif_dump_state **dump_,

> >     -                          const uint16_t *pzone)

> >     +                          const uint16_t *pzone, int *ptot_bkts)

> >      {

> >          struct dp_netdev *dp = get_dp_netdev(dpif);

> >          struct dp_netdev_ct_dump *dump;

> >     @@ -5342,7 +5342,7 @@ dpif_netdev_ct_dump_start(struct dpif *dpif,

> > struct ct_dpif_dump_state **dump_,

> >          dump->dp = dp;

> >          dump->ct = &dp->conntrack;

> >

> >     -    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone);

> >     +    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone,

> > ptot_bkts);

> >

> >          *dump_ = &dump->up;

> >

> >     diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c

> >     index 55effd1..b87bb16 100644

> >     --- a/lib/dpif-netlink.c

> >     +++ b/lib/dpif-netlink.c

> >     @@ -2847,13 +2847,13 @@ struct dpif_netlink_ct_dump_state {

> >      static int

> >      dpif_netlink_ct_dump_start(struct dpif *dpif OVS_UNUSED,

> >                                 struct ct_dpif_dump_state **dump_,

> >     -                           const uint16_t *zone)

> >     +                           const uint16_t *zone, int *ptot_bkts)

> >      {

> >          struct dpif_netlink_ct_dump_state *dump;

> >          int err;

> >

> >          dump = xzalloc(sizeof *dump);

> >     -    err = nl_ct_dump_start(&dump->nl_ct_dump, zone);

> >     +    err = nl_ct_dump_start(&dump->nl_ct_dump, zone, ptot_bkts);

> >          if (err) {

> >              free(dump);

> >              return err;

> >     diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h

> >     index 64ac2e2..1d82a09 100644

> >     --- a/lib/dpif-provider.h

> >     +++ b/lib/dpif-provider.h

> >     @@ -419,7 +419,7 @@ struct dpif_class {

> >           * ct_dump_done() should perform any cleanup necessary

> (including

> >           * deallocating the 'state' structure, if applicable). */

> >          int (*ct_dump_start)(struct dpif *, struct ct_dpif_dump_state

> > **state,

> >     -                         const uint16_t *zone);

> >     +                         const uint16_t *zone, int *);

> >          int (*ct_dump_next)(struct dpif *, struct ct_dpif_dump_state

> > *state,

> >                              struct ct_dpif_entry *entry);

> >          int (*ct_dump_done)(struct dpif *, struct ct_dpif_dump_state

> > *state);

> >     diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c

> >     index f0e2aea..ac48b15 100644

> >     --- a/lib/netlink-conntrack.c?

> >     +++ b/lib/netlink-conntrack.c

> >     @@ -123,7 +123,8 @@ struct nl_ct_dump_state {

> >

> >      /* Initialize a conntrack netlink dump. */

> >      int

> >     -nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t

> > *zone)

> >     +nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t

> > *zone,

> >     +        int *ptot_bkts)

> >      {

> >          struct nl_ct_dump_state *state;

> >

> >     @@ -140,6 +141,9 @@ nl_ct_dump_start(struct nl_ct_dump_state

> **statep,

> > const uint16_t *zone)

> >          nl_dump_start(&state->dump, NETLINK_NETFILTER, &state->buf);

> >          ofpbuf_clear(&state->buf);

> >

> >     +    /* Buckets to store connections are not used. */

> >     +    *ptot_bkts = -1;

> >     +

> >

> > [Darrell]

> > I tested this and looks good.

> >

> >          return 0;

> >      }

> >

> >     diff --git a/lib/netlink-conntrack.h b/lib/netlink-conntrack.h

> >     index 1263b21..a95aa69 100644

> >     --- a/lib/netlink-conntrack.h

> >     +++ b/lib/netlink-conntrack.h

> >     @@ -35,7 +35,8 @@ enum nl_ct_event_type {

> >

> >      struct nl_ct_dump_state;

> >

> >     -int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t

> > *zone);

> >     +int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t

> > *zone,

> >     +        int *ptot_bkts);

> >      int nl_ct_dump_next(struct nl_ct_dump_state *, struct ct_dpif_entry

> > *);

> >      int nl_ct_dump_done(struct nl_ct_dump_state *);

> >

> >     diff --git a/tests/test-netlink-conntrack.c b/tests/test-netlink-

> > conntrack.c

> >     index 000062d..0d9dace 100644

> >     --- a/tests/test-netlink-conntrack.c

> >     +++ b/tests/test-netlink-conntrack.c

> >     @@ -106,6 +106,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)

> >          uint16_t zone, *pzone = NULL;

> >          struct ct_dpif_entry entry;

> >          int err;

> >     +    int tot_bkts;

> >

> >          if (ctx->argc >= 2) {

> >              if (!ovs_scan(ctx->argv[1], "zone=%"SCNu16, &zone)) {

> >     @@ -113,7 +114,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)

> >              }

> >              pzone = &zone;

> >          }

> >     -    err = nl_ct_dump_start(&dump, pzone);

> >     +    err = nl_ct_dump_start(&dump, pzone, &tot_bkts);

> >          if (err) {

> >              ovs_fatal(err, "Error creating conntrack netlink dump");

> >          }

> >     diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c

> >     index 7292fca..7b005ac 100644

> >     --- a/utilities/ovs-dpctl.c

> >     +++ b/utilities/ovs-dpctl.c

> >     @@ -202,6 +202,7 @@ usage(void *userdata OVS_UNUSED)

> >                     "delete all conntrack entries in ZONE\n"

> >                 "  ct-stats-show [DP] [zone=ZONE] [verbose] " \

> >                     "CT connections grouped by protocol\n"

> >     +           "  ct-bkts [DP] [gt=N] display connections per CT

> > bucket\n"

> >                 "Each IFACE on add-dp, add-if, and set-if may be

> followed

> > by\n"

> >                 "comma-separated options.  See ovs-dpctl(8) for syntax,

> or

> > the\n"

> >                 "Interface table in ovs-vswitchd.conf.db(5) for an

> options

> > list.\n"

> >     --

> >     2.4.11

> >

> >     _______________________________________________

> >     dev mailing list

> >     dev@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=DzNptgj_q8j7ukiP3_wi0zYxZ4WDkZkhE1w1OSsoeD0&s=wOJItkDVJYq7hVfEWV_b

> > 7k3n8wFSLxdVYOIT2toHc40&e=

> >

> >

> >

> 

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Darrell Ball July 25, 2017, 3:18 p.m. UTC | #3
Hi Antonio

I had done a quick check of the two point changes, but, I did not retest, since I tested twice before.
Thanks for the enhancement.
This will go into 2.8.

Thanks Darrell

-----Original Message-----
From: "Fischetti, Antonio" <antonio.fischetti@intel.com>

Date: Tuesday, July 25, 2017 at 6:57 AM
To: "Fischetti, Antonio" <antonio.fischetti@intel.com>, Darrell Ball <dball@vmware.com>, "dev@openvswitch.org" <dev@openvswitch.org>
Subject: RE: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

    Hi Darrell, I posted a v5 at https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_792723_&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=Mxh35xY_PS5nbtJNTKQnPqKl_7BotQ7kLayJzzAmuck&s=rc3sgx0RTpbMglULZqL1zVX831XgJKNmQktLaFN6kIQ&e= 
    
    where I added all your suggestions.
    
    
    
    Thanks,
    
    -Antonio
    
    
    
    > -----Original Message-----

    
    > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

    
    > bounces@openvswitch.org] On Behalf Of Fischetti, Antonio

    
    > Sent: Monday, July 24, 2017 10:45 AM

    
    > To: Darrell Ball <dball@vmware.com>; dev@openvswitch.org

    
    > Subject: Re: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

    
    > 

    
    > Thanks Darrell, I will apply your suggetions.

    
    > 

    
    > -Antonio

    
    > 

    
    > > -----Original Message-----

    
    > > From: Darrell Ball [mailto:dball@vmware.com]

    
    > > Sent: Sunday, July 23, 2017 6:28 PM

    
    > > To: Fischetti, Antonio <antonio.fischetti@intel.com>;

    
    > dev@openvswitch.org

    
    > > Subject: Re: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

    
    > >

    
    > > Minor comment:

    
    > >

    
    > > When applying the patch, there is a complaint about new whitespace below

    
    > 

    
    > [Antonio] will remove it.

    
    > 

    
    > 

    
    > >

    
    > > +the number of connections in a bucket is greater than \fIThreshold\fR.

    
    > > +

    
    > >

    
    > > Two comments inline

    
    > >

    
    > >

    
    > > -----Original Message-----

    
    > > From: <ovs-dev-bounces@openvswitch.org> on behalf of

    
    > > "antonio.fischetti@intel.com" <antonio.fischetti@intel.com>

    
    > > Date: Saturday, July 22, 2017 at 7:38 AM

    
    > > To: "dev@openvswitch.org" <dev@openvswitch.org>

    
    > > Subject: [ovs-dev] [PATCH v4] dpctl: Add new 'ct-bkts' command.

    
    > >

    
    > >     With the command:

    
    > >      ovs-appctl dpctl/ct-bkts

    
    > >     shows the number of connections per bucket.

    
    > >

    
    > >     By using a threshold:

    
    > >      ovs-appctl dpctl/ct-bkts gt=N

    
    > >     for each bucket shows the number of connections when they

    
    > >     are greater than N.

    
    > >

    
    > >     Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>

    
    > >     Signed-off-by: Bhanuprakash Bodireddy

    
    > > <bhanuprakash.bodireddy@intel.com>

    
    > >     Co-authored-by: Bhanuprakash Bodireddy

    
    > > <bhanuprakash.bodireddy@intel.com>

    
    > >     ---

    
    > >      lib/conntrack.c                |   9 ++--

    
    > >      lib/conntrack.h                |   2 +-

    
    > >      lib/ct-dpif.c                  |   4 +-

    
    > >      lib/ct-dpif.h                  |   3 +-

    
    > >      lib/dpctl.c                    | 108

    
    > > ++++++++++++++++++++++++++++++++++++++++-

    
    > >      lib/dpctl.man                  |   8 +++

    
    > >      lib/dpif-netdev.c              |   4 +-

    
    > >      lib/dpif-netlink.c             |   4 +-

    
    > >      lib/dpif-provider.h            |   2 +-

    
    > >      lib/netlink-conntrack.c        |   6 ++-

    
    > >      lib/netlink-conntrack.h        |   3 +-

    
    > >      tests/test-netlink-conntrack.c |   3 +-

    
    > >      utilities/ovs-dpctl.c          |   1 +

    
    > >      13 files changed, 140 insertions(+), 17 deletions(-)

    
    > >

    
    > >     diff --git a/lib/conntrack.c b/lib/conntrack.c

    
    > >     index de46a6b..e290b20 100644

    
    > >     --- a/lib/conntrack.c

    
    > >     +++ b/lib/conntrack.c

    
    > >     @@ -1931,7 +1931,7 @@ conn_key_to_tuple(const struct conn_key *key,

    
    > > struct ct_dpif_tuple *tuple)

    
    > >

    
    > >      static void

    
    > >      conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry

    
    > > *entry,

    
    > >     -                      long long now)

    
    > >     +                      long long now, int bkt)

    
    > >      {

    
    > >          struct ct_l4_proto *class;

    
    > >          long long expiration;

    
    > >     @@ -1954,11 +1954,12 @@ conn_to_ct_dpif_entry(const struct conn

    
    > *conn,

    
    > > struct ct_dpif_entry *entry,

    
    > >          if (class->conn_get_protoinfo) {

    
    > >              class->conn_get_protoinfo(conn, &entry->protoinfo);

    
    > >          }

    
    > >     +    entry->bkt = bkt;

    
    > >      }

    
    > >

    
    > >      int

    
    > >      conntrack_dump_start(struct conntrack *ct, struct conntrack_dump

    
    > > *dump,

    
    > >     -                     const uint16_t *pzone)

    
    > >     +                     const uint16_t *pzone, int *ptot_bkts)

    
    > >      {

    
    > >          memset(dump, 0, sizeof(*dump));

    
    > >          if (pzone) {

    
    > >     @@ -1967,6 +1968,8 @@ conntrack_dump_start(struct conntrack *ct,

    
    > > struct conntrack_dump *dump,

    
    > >          }

    
    > >          dump->ct = ct;

    
    > >

    
    > >     +    *ptot_bkts = CONNTRACK_BUCKETS;

    
    > >     +

    
    > >          return 0;

    
    > >      }

    
    > >

    
    > >     @@ -1991,7 +1994,7 @@ conntrack_dump_next(struct conntrack_dump

    
    > *dump,

    
    > > struct ct_dpif_entry *entry)

    
    > >                  INIT_CONTAINER(conn, node, node);

    
    > >                  if ((!dump->filter_zone || conn->key.zone == dump-

    
    > >zone)

    
    > > &&

    
    > >                       (conn->conn_type != CT_CONN_TYPE_UN_NAT)) {

    
    > >     -                conn_to_ct_dpif_entry(conn, entry, now);

    
    > >     +                conn_to_ct_dpif_entry(conn, entry, now, dump-

    
    > > >bucket);

    
    > >                      break;

    
    > >                  }

    
    > >                  /* Else continue, until we find an entry in the

    
    > > appropriate zone

    
    > >     diff --git a/lib/conntrack.h b/lib/conntrack.h

    
    > >     index defde4c..3f48444 100644

    
    > >     --- a/lib/conntrack.h

    
    > >     +++ b/lib/conntrack.h

    
    > >     @@ -108,7 +108,7 @@ struct conntrack_dump {

    
    > >      struct ct_dpif_entry;

    
    > >

    
    > >      int conntrack_dump_start(struct conntrack *, struct conntrack_dump

    
    > *,

    
    > >     -                         const uint16_t *pzone);

    
    > >     +                         const uint16_t *pzone, int *);

    
    > >      int conntrack_dump_next(struct conntrack_dump *, struct

    
    > ct_dpif_entry

    
    > > *);

    
    > >      int conntrack_dump_done(struct conntrack_dump *);

    
    > >

    
    > >     diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c

    
    > >     index f8d2cf1..c79e69e 100644

    
    > >     --- a/lib/ct-dpif.c

    
    > >     +++ b/lib/ct-dpif.c

    
    > >     @@ -65,12 +65,12 @@ static const struct flags ct_dpif_status_flags[]

    
    > =

    
    > > {

    
    > >       * that represents the error.  Otherwise it returns zero. */

    
    > >      int

    
    > >      ct_dpif_dump_start(struct dpif *dpif, struct ct_dpif_dump_state

    
    > > **dump,

    
    > >     -                   const uint16_t *zone)

    
    > >     +                   const uint16_t *zone, int *ptot_bkts)

    
    > >      {

    
    > >          int err;

    
    > >

    
    > >          err = (dpif->dpif_class->ct_dump_start

    
    > >     -           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone)

    
    > >     +           ? dpif->dpif_class->ct_dump_start(dpif, dump, zone,

    
    > > ptot_bkts)

    
    > >                 : EOPNOTSUPP);

    
    > >

    
    > >          if (!err) {

    
    > >     diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h

    
    > >     index cd35f3e..d5f9661 100644

    
    > >     --- a/lib/ct-dpif.h

    
    > >     +++ b/lib/ct-dpif.h

    
    > >     @@ -169,6 +169,7 @@ struct ct_dpif_entry {

    
    > >          /* Timeout for this entry in seconds */

    
    > >          uint32_t timeout;

    
    > >          uint32_t mark;

    
    > >     +    uint32_t bkt;       /* CT bucket number. */

    
    > >      };

    
    > >

    
    > >      enum {

    
    > >     @@ -191,7 +192,7 @@ struct ct_dpif_dump_state {

    
    > >      };

    
    > >

    
    > >      int ct_dpif_dump_start(struct dpif *, struct ct_dpif_dump_state **,

    
    > >     -                       const uint16_t *zone);

    
    > >     +                       const uint16_t *zone, int *);

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

    
    > >     diff --git a/lib/dpctl.c b/lib/dpctl.c

    
    > >     index 6aa6c8e..3ccc7a3 100644

    
    > >     --- a/lib/dpctl.c

    
    > >     +++ b/lib/dpctl.c

    
    > >     @@ -1258,6 +1258,7 @@ dpctl_dump_conntrack(int argc, const char

    
    > > *argv[],

    
    > >          struct ct_dpif_dump_state *dump;

    
    > >          struct ct_dpif_entry cte;

    
    > >          uint16_t zone, *pzone = NULL;

    
    > >     +    int tot_bkts;

    
    > >          struct dpif *dpif;

    
    > >          char *name;

    
    > >          int error;

    
    > >     @@ -1277,7 +1278,7 @@ dpctl_dump_conntrack(int argc, const char

    
    > > *argv[],

    
    > >              return error;

    
    > >          }

    
    > >

    
    > >     -    error = ct_dpif_dump_start(dpif, &dump, pzone);

    
    > >     +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);

    
    > >          if (error) {

    
    > >              dpctl_error(dpctl_p, error, "starting conntrack dump");

    
    > >              dpif_close(dpif);

    
    > >     @@ -1339,6 +1340,7 @@ dpctl_ct_stats_show(int argc, const char

    
    > > *argv[],

    
    > >          struct ct_dpif_dump_state *dump;

    
    > >          struct ct_dpif_entry cte;

    
    > >          uint16_t zone, *pzone = NULL;

    
    > >     +    int tot_bkts;

    
    > >          bool verbose = false;

    
    > >          int lastargc = 0;

    
    > >

    
    > >     @@ -1373,7 +1375,7 @@ dpctl_ct_stats_show(int argc, const char

    
    > > *argv[],

    
    > >

    
    > >          memset(proto_stats, 0, sizeof(proto_stats));

    
    > >          memset(tcp_conn_per_states, 0, sizeof(tcp_conn_per_states));

    
    > >     -    error = ct_dpif_dump_start(dpif, &dump, pzone);

    
    > >     +    error = ct_dpif_dump_start(dpif, &dump, pzone, &tot_bkts);

    
    > >          if (error) {

    
    > >              dpctl_error(dpctl_p, error, "starting conntrack dump");

    
    > >              dpif_close(dpif);

    
    > >     @@ -1464,6 +1466,107 @@ dpctl_ct_stats_show(int argc, const char

    
    > > *argv[],

    
    > >          dpif_close(dpif);

    
    > >          return error;

    
    > >      }

    
    > >     +

    
    > >     +#define CT_BKTS_GT "gt="

    
    > >     +static int

    
    > >     +dpctl_ct_bkts(int argc, const char *argv[],

    
    > >     +                     struct dpctl_params *dpctl_p)

    
    > >     +{

    
    > >     +    struct dpif *dpif;

    
    > >     +    char *name;

    
    > >     +

    
    > >     +    struct ct_dpif_dump_state *dump;

    
    > >     +    struct ct_dpif_entry cte;

    
    > >     +    uint16_t gt = 0; /* Threshold: display value when greater than

    
    > > gt. */

    
    > >     +    uint16_t *pzone = NULL;

    
    > >     +    int tot_bkts = 0;

    
    > >     +    int lastargc = 0;

    
    > >     +

    
    > >     +    int error;

    
    > >     +

    
    > >     +    while (argc > 1 && lastargc != argc) {

    
    > >     +        lastargc = argc;

    
    > >     +        if (!strncmp(argv[argc - 1], CT_BKTS_GT,

    
    > strlen(CT_BKTS_GT)))

    
    > > {

    
    > >     +            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt))

    
    > {

    
    > >     +                argc--;

    
    > >     +                break;

    
    > >     +            }

    
    > >     +        }

    
    > >     +    }

    
    > >

    
    > > [Darrell]

    
    > > When, I do this, I get a failure, which is expected.

    
    > >

    
    > > > 2017-07-23T02:28:02.155Z|00052|connmgr|INFO|br0<->unix: 11 flow_mods

    
    > in

    
    > > the last 0 s (11 adds)

    
    > > > 2017-07-23T02:28:02.346Z|00053|unixctl|DBG|received request dpctl/ct-

    
    > > bkts["gt=0","netdev@ovs-netdev"], id=0

    
    > > > 2017-07-23T02:28:02.346Z|00054|dpctl|INFO|set_names=0 verbosity=0

    
    > > names=0

    
    > > > 2017-07-23T02:28:02.346Z|00057|dpif|WARN|could not create datapath

    
    > gt=0

    
    > > of unknown type system

    
    > > > 2017-07-23T02:28:02.347Z|00058|unixctl|DBG|replying with error, id=0:

    
    > > "ovs-vswitchd: opening datapath (Address family not supported by

    
    > protocol)

    
    > >

    
    > > But, your command requires a maximum of 2 parameters and if there are 2,

    
    > > the datapath name must be first and ‘gt=x’ second

    
    > > Hence, I think you can just remove the while loop completely since

    
    > ‘gt=x’

    
    > > must always be last, if present.

    
    > >

    
    > > diff --git a/lib/dpctl.c b/lib/dpctl.c

    
    > > index cb791ca..5f3801c 100644

    
    > > --- a/lib/dpctl.c

    
    > > +++ b/lib/dpctl.c

    
    > > @@ -1480,17 +1480,11 @@ dpctl_ct_bkts(int argc, const char *argv[],

    
    > >      uint16_t gt = 0; /* Threshold: display value when greater than gt.

    
    > */

    
    > >      uint16_t *pzone = NULL;

    
    > >      int tot_bkts = 0;

    
    > > -    int lastargc = 0;

    
    > > -

    
    > >      int error;

    
    > >

    
    > > -    while (argc > 1 && lastargc != argc) {

    
    > > -        lastargc = argc;

    
    > > -        if (!strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {

    
    > > -            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

    
    > > -                argc--;

    
    > > -                break;

    
    > > -            }

    
    > > +    if (argc > 1 && !strncmp(argv[argc - 1], CT_BKTS_GT,

    
    > > strlen(CT_BKTS_GT))) {

    
    > > +        if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {

    
    > > +            argc--;

    
    > >          }

    
    > >      }

    
    > 

    
    > [Antonio] ok will change it

    
    > 

    
    > 

    
    > >

    
    > > //////////////////////////////

    
    > >

    
    > >     +

    
    > >     +    name = (argc > 1) ? 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_dump_start(dpif, &dump, pzone, &tot_bkts);

    
    > >     +    if (error) {

    
    > >     +        dpctl_error(dpctl_p, error, "starting conntrack dump");

    
    > >     +        dpif_close(dpif);

    
    > >     +        return error;

    
    > >     +    }

    
    > >     +    if (tot_bkts == -1) {

    
    > >     +         /* Command not available when called by kernel OvS. */

    
    > >     +         dpctl_print(dpctl_p,

    
    > >     +             "Command is available for UserSpace ConnTracker

    
    > > only.\n");

    
    > >     +         ct_dpif_dump_done(dump);

    
    > >

    
    > > [Darrell]

    
    > >           dpctl_print(dpctl_p,

    
    > >               "Command is available for UserSpace ConnTracker only.\n");

    
    > >           ct_dpif_dump_done(dump);

    
    > > +         dpif_close(dpif);

    
    > >           return 0;

    
    > >      }

    
    > >

    
    > > //////////////////////////////

    
    > >

    
    > >

    
    > >     +         return 0;

    
    > >     +    }

    
    > >     +

    
    > >     +    dpctl_print(dpctl_p, "Total Buckets: %d\n", tot_bkts);

    
    > >     +

    
    > >     +    int tot_conn = 0;

    
    > >     +    uint32_t conn_per_bkts[tot_bkts];

    
    > >     +    memset(conn_per_bkts, 0, sizeof(uint32_t) * tot_bkts);

    
    > >     +

    
    > >     +    while (!ct_dpif_dump_next(dump, &cte)) {

    
    > >     +        ct_dpif_entry_uninit(&cte);

    
    > >     +        tot_conn++;

    
    > >     +        if (tot_bkts > 0) {

    
    > >     +            if (cte.bkt < tot_bkts) {

    
    > >     +                conn_per_bkts[cte.bkt]++;

    
    > >     +            } else {

    
    > >     +                dpctl_print(dpctl_p, "Bucket nr out of range: %d >=

    
    > > %d\n",

    
    > >     +                        cte.bkt, tot_bkts);

    
    > >     +            }

    
    > >     +        }

    
    > >     +    }

    
    > >     +

    
    > >     +    dpctl_print(dpctl_p, "Current Connections: %d\n", tot_conn);

    
    > >     +    dpctl_print(dpctl_p, "\n");

    
    > >     +    if (tot_bkts && tot_conn) {

    
    > >     +        dpctl_print(dpctl_p, "+-----------+"

    
    > >     +                "-----------------------------------------+\n");

    
    > >     +        dpctl_print(dpctl_p, "|  Buckets  |"

    
    > >     +                "         Connections per Buckets         |\n");

    
    > >     +        dpctl_print(dpctl_p, "+-----------+"

    
    > >     +                "-----------------------------------------+");

    
    > >     +#define NUM_BKTS_DIPLAYED_PER_ROW 8

    
    > >     +        for (int i = 0; i < tot_bkts; i++) {

    
    > >     +            if (i % NUM_BKTS_DIPLAYED_PER_ROW == 0) {

    
    > >     +                 dpctl_print(dpctl_p, "\n %3d..%3d   | ",

    
    > >     +                         i, i + NUM_BKTS_DIPLAYED_PER_ROW - 1);

    
    > >     +            }

    
    > >     +            if (conn_per_bkts[i] > gt) {

    
    > >     +                dpctl_print(dpctl_p, "%5d", conn_per_bkts[i]);

    
    > >     +            } else {

    
    > >     +                dpctl_print(dpctl_p, "%5s", ".");

    
    > >     +            }

    
    > >     +        }

    
    > >     +        dpctl_print(dpctl_p, "\n\n");

    
    > >     +    }

    
    > >     +

    
    > >     +    ct_dpif_dump_done(dump);

    
    > >     +    dpif_close(dpif);

    
    > >     +    return error;

    
    > >     +}

    
    > >      ?

    
    > >      /* Undocumented commands for unit testing. */

    
    > >

    
    > >     @@ -1760,6 +1863,7 @@ static const struct dpctl_command

    
    > all_commands[]

    
    > > = {

    
    > >          { "flush-conntrack", "[dp] [zone=N]", 0, 2,

    
    > > dpctl_flush_conntrack, DP_RW },

    
    > >          { "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 },

    
    > >          { "help", "", 0, INT_MAX, dpctl_help, DP_RO },

    
    > >          { "list-commands", "", 0, INT_MAX, dpctl_list_commands, DP_RO

    
    > },

    
    > >

    
    > >     diff --git a/lib/dpctl.man b/lib/dpctl.man

    
    > >     index f95d54a..077fd99 100644

    
    > >     --- a/lib/dpctl.man

    
    > >     +++ b/lib/dpctl.man

    
    > >     @@ -228,3 +228,11 @@ Displays the number of connections grouped by

    
    > > protocol used by \fIdp\fR.

    
    > >      If \fBzone=\fIzone\fR is specified, numbers refer to the

    
    > connections

    
    > > in

    
    > >      \fBzone\fR. The \fBverbose\fR option allows to group by connection

    
    > > state

    
    > >      for each protocol.

    
    > >     +.

    
    > >     +.TP

    
    > >     +\*(DX\fBct\-bkts\fR [\fIdp\fR] [\fBgt=\fIThreshold\fR]

    
    > >     +For each ConnTracker bucket, displays the number of connections

    
    > used

    
    > >     +by \fIdp\fR.

    
    > >     +If \fBgt=\fIThreshold\fR is specified, bucket numbers are displayed

    
    > > when

    
    > >     +the number of connections in a bucket is greater than

    
    > > \fIThreshold\fR.

    
    > >     +

    
    > >     diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c

    
    > >     index 47a9fa0..10435e7 100644

    
    > >     --- a/lib/dpif-netdev.c

    
    > >     +++ b/lib/dpif-netdev.c

    
    > >     @@ -5333,7 +5333,7 @@ struct dp_netdev_ct_dump {

    
    > >

    
    > >      static int

    
    > >      dpif_netdev_ct_dump_start(struct dpif *dpif, struct

    
    > > ct_dpif_dump_state **dump_,

    
    > >     -                          const uint16_t *pzone)

    
    > >     +                          const uint16_t *pzone, int *ptot_bkts)

    
    > >      {

    
    > >          struct dp_netdev *dp = get_dp_netdev(dpif);

    
    > >          struct dp_netdev_ct_dump *dump;

    
    > >     @@ -5342,7 +5342,7 @@ dpif_netdev_ct_dump_start(struct dpif *dpif,

    
    > > struct ct_dpif_dump_state **dump_,

    
    > >          dump->dp = dp;

    
    > >          dump->ct = &dp->conntrack;

    
    > >

    
    > >     -    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone);

    
    > >     +    conntrack_dump_start(&dp->conntrack, &dump->dump, pzone,

    
    > > ptot_bkts);

    
    > >

    
    > >          *dump_ = &dump->up;

    
    > >

    
    > >     diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c

    
    > >     index 55effd1..b87bb16 100644

    
    > >     --- a/lib/dpif-netlink.c

    
    > >     +++ b/lib/dpif-netlink.c

    
    > >     @@ -2847,13 +2847,13 @@ struct dpif_netlink_ct_dump_state {

    
    > >      static int

    
    > >      dpif_netlink_ct_dump_start(struct dpif *dpif OVS_UNUSED,

    
    > >                                 struct ct_dpif_dump_state **dump_,

    
    > >     -                           const uint16_t *zone)

    
    > >     +                           const uint16_t *zone, int *ptot_bkts)

    
    > >      {

    
    > >          struct dpif_netlink_ct_dump_state *dump;

    
    > >          int err;

    
    > >

    
    > >          dump = xzalloc(sizeof *dump);

    
    > >     -    err = nl_ct_dump_start(&dump->nl_ct_dump, zone);

    
    > >     +    err = nl_ct_dump_start(&dump->nl_ct_dump, zone, ptot_bkts);

    
    > >          if (err) {

    
    > >              free(dump);

    
    > >              return err;

    
    > >     diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h

    
    > >     index 64ac2e2..1d82a09 100644

    
    > >     --- a/lib/dpif-provider.h

    
    > >     +++ b/lib/dpif-provider.h

    
    > >     @@ -419,7 +419,7 @@ struct dpif_class {

    
    > >           * ct_dump_done() should perform any cleanup necessary

    
    > (including

    
    > >           * deallocating the 'state' structure, if applicable). */

    
    > >          int (*ct_dump_start)(struct dpif *, struct ct_dpif_dump_state

    
    > > **state,

    
    > >     -                         const uint16_t *zone);

    
    > >     +                         const uint16_t *zone, int *);

    
    > >          int (*ct_dump_next)(struct dpif *, struct ct_dpif_dump_state

    
    > > *state,

    
    > >                              struct ct_dpif_entry *entry);

    
    > >          int (*ct_dump_done)(struct dpif *, struct ct_dpif_dump_state

    
    > > *state);

    
    > >     diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c

    
    > >     index f0e2aea..ac48b15 100644

    
    > >     --- a/lib/netlink-conntrack.c?

    
    > >     +++ b/lib/netlink-conntrack.c

    
    > >     @@ -123,7 +123,8 @@ struct nl_ct_dump_state {

    
    > >

    
    > >      /* Initialize a conntrack netlink dump. */

    
    > >      int

    
    > >     -nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t

    
    > > *zone)

    
    > >     +nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t

    
    > > *zone,

    
    > >     +        int *ptot_bkts)

    
    > >      {

    
    > >          struct nl_ct_dump_state *state;

    
    > >

    
    > >     @@ -140,6 +141,9 @@ nl_ct_dump_start(struct nl_ct_dump_state

    
    > **statep,

    
    > > const uint16_t *zone)

    
    > >          nl_dump_start(&state->dump, NETLINK_NETFILTER, &state->buf);

    
    > >          ofpbuf_clear(&state->buf);

    
    > >

    
    > >     +    /* Buckets to store connections are not used. */

    
    > >     +    *ptot_bkts = -1;

    
    > >     +

    
    > >

    
    > > [Darrell]

    
    > > I tested this and looks good.

    
    > >

    
    > >          return 0;

    
    > >      }

    
    > >

    
    > >     diff --git a/lib/netlink-conntrack.h b/lib/netlink-conntrack.h

    
    > >     index 1263b21..a95aa69 100644

    
    > >     --- a/lib/netlink-conntrack.h

    
    > >     +++ b/lib/netlink-conntrack.h

    
    > >     @@ -35,7 +35,8 @@ enum nl_ct_event_type {

    
    > >

    
    > >      struct nl_ct_dump_state;

    
    > >

    
    > >     -int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t

    
    > > *zone);

    
    > >     +int nl_ct_dump_start(struct nl_ct_dump_state **, const uint16_t

    
    > > *zone,

    
    > >     +        int *ptot_bkts);

    
    > >      int nl_ct_dump_next(struct nl_ct_dump_state *, struct ct_dpif_entry

    
    > > *);

    
    > >      int nl_ct_dump_done(struct nl_ct_dump_state *);

    
    > >

    
    > >     diff --git a/tests/test-netlink-conntrack.c b/tests/test-netlink-

    
    > > conntrack.c

    
    > >     index 000062d..0d9dace 100644

    
    > >     --- a/tests/test-netlink-conntrack.c

    
    > >     +++ b/tests/test-netlink-conntrack.c

    
    > >     @@ -106,6 +106,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)

    
    > >          uint16_t zone, *pzone = NULL;

    
    > >          struct ct_dpif_entry entry;

    
    > >          int err;

    
    > >     +    int tot_bkts;

    
    > >

    
    > >          if (ctx->argc >= 2) {

    
    > >              if (!ovs_scan(ctx->argv[1], "zone=%"SCNu16, &zone)) {

    
    > >     @@ -113,7 +114,7 @@ test_nl_ct_dump(struct ovs_cmdl_context *ctx)

    
    > >              }

    
    > >              pzone = &zone;

    
    > >          }

    
    > >     -    err = nl_ct_dump_start(&dump, pzone);

    
    > >     +    err = nl_ct_dump_start(&dump, pzone, &tot_bkts);

    
    > >          if (err) {

    
    > >              ovs_fatal(err, "Error creating conntrack netlink dump");

    
    > >          }

    
    > >     diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c

    
    > >     index 7292fca..7b005ac 100644

    
    > >     --- a/utilities/ovs-dpctl.c

    
    > >     +++ b/utilities/ovs-dpctl.c

    
    > >     @@ -202,6 +202,7 @@ usage(void *userdata OVS_UNUSED)

    
    > >                     "delete all conntrack entries in ZONE\n"

    
    > >                 "  ct-stats-show [DP] [zone=ZONE] [verbose] " \

    
    > >                     "CT connections grouped by protocol\n"

    
    > >     +           "  ct-bkts [DP] [gt=N] display connections per CT

    
    > > bucket\n"

    
    > >                 "Each IFACE on add-dp, add-if, and set-if may be

    
    > followed

    
    > > by\n"

    
    > >                 "comma-separated options.  See ovs-dpctl(8) for syntax,

    
    > or

    
    > > the\n"

    
    > >                 "Interface table in ovs-vswitchd.conf.db(5) for an

    
    > options

    
    > > list.\n"

    
    > >     --

    
    > >     2.4.11

    
    > >

    
    > >     _______________________________________________

    
    > >     dev mailing list

    
    > >     dev@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=DzNptgj_q8j7ukiP3_wi0zYxZ4WDkZkhE1w1OSsoeD0&s=wOJItkDVJYq7hVfEWV_b

    
    > > 7k3n8wFSLxdVYOIT2toHc40&e=

    
    > >

    
    > >

    
    > >

    
    > 

    
    > _______________________________________________

    
    > dev mailing list

    
    > dev@openvswitch.org

    
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=Mxh35xY_PS5nbtJNTKQnPqKl_7BotQ7kLayJzzAmuck&s=zKWNbUybOinK7QqD4r15-GATfu7BSpN4kxkG5UYU5Nw&e=

Patch
diff mbox

diff --git a/lib/dpctl.c b/lib/dpctl.c
index cb791ca..5f3801c 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1480,17 +1480,11 @@  dpctl_ct_bkts(int argc, const char *argv[],
     uint16_t gt = 0; /* Threshold: display value when greater than gt. */
     uint16_t *pzone = NULL;
     int tot_bkts = 0;
-    int lastargc = 0;
-
     int error;
 
-    while (argc > 1 && lastargc != argc) {
-        lastargc = argc;
-        if (!strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {
-            if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {
-                argc--;
-                break;
-            }
+    if (argc > 1 && !strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {
+        if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {
+            argc--;
         }
     }