I did some testing; display looks nice to me Other comments inline
> 2017-07-21T06:38:33.215Z|00053|unixctl|DBG|received request > dpctl/ct-bkts["netdev@ovs-netdev","gt=0"], id=0 > 2017-07-21T06:38:33.215Z|00054|dpctl|INFO|set_names=0 verbosity=0 names=0 > 2017-07-21T06:38:33.215Z|00055|dpctl|WARN|DARRELL gt 0 > 2017-07-21T06:38:33.215Z|00056|dpctl|WARN|DARRELL name netdev@ovs-netdev > 2017-07-21T06:38:33.215Z|00057|unixctl|DBG|replying with success, id=0: > "Total Buckets: 256 > Current Connections: 1 > > +-----------+-----------------------------------------+ > | Buckets | Connections per Buckets | > +-----------+-----------------------------------------+ > 0.. 7 | . . . . . . . . > 8.. 15 | . . . . . . . . > 16.. 23 | . . . . . . . . > 24.. 31 | . . . . . . . . > 32.. 39 | . . . . . . . . > 40.. 47 | . . . . . . . . > 48.. 55 | . . . . . . . . > 56.. 63 | . . . . . . . . > 64.. 71 | . . . . . . . . > 72.. 79 | . . . . . . . . > 80.. 87 | . . . . . . . . > 88.. 95 | . . . . . . . . > 96..103 | . . . . . . . . > 104..111 | . . . . . . . . > 112..119 | . . . . . . . . > 120..127 | . . . . . . . . > 128..135 | . . . . . . . . > 136..143 | . . . . . . . . > 144..151 | . . . . . . . . > 152..159 | . . . . . . . . > 160..167 | . . . . . . . . > 168..175 | 1 . . . . . . . > 176..183 | . . . . . . . . > 184..191 | . . . . . . . . > 192..199 | . . . . . . . . > 200..207 | . . . . . . . . > 208..215 | . . . . . . . . > 216..223 | . . . . . . . . > 224..231 | . . . . . . . . > 232..239 | . . . . . . . . > 240..247 | . . . . . . . . > 248..255 | . . . . . . . . -----Original Message----- From: <[email protected]> on behalf of "[email protected]" <[email protected]> Date: Tuesday, July 18, 2017 at 6:03 AM To: "[email protected]" <[email protected]> Subject: [ovs-dev] [PATCH v3] dpctl: Add new 'ct-bkts' command. From: Antonio Fischetti <[email protected]> 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 <[email protected]> Signed-off-by: Bhanuprakash Bodireddy <[email protected]> Co-authored-by: Bhanuprakash Bodireddy <[email protected]> --- lib/conntrack.c | 10 ++-- lib/conntrack.h | 2 +- lib/ct-dpif.c | 4 +- lib/ct-dpif.h | 3 +- lib/dpctl.c | 103 ++++++++++++++++++++++++++++++++++++++++- lib/dpctl.man | 8 ++++ lib/dpif-netdev.c | 4 +- lib/dpif-netlink.c | 4 +- lib/dpif-provider.h | 2 +- lib/netlink-conntrack.c | 3 +- lib/netlink-conntrack.h | 3 +- tests/test-netlink-conntrack.c | 2 +- utilities/ovs-dpctl.c | 1 + 13 files changed, 132 insertions(+), 17 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index de46a6b..77ce4cc 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, uint32_t *ptot_bkts) { memset(dump, 0, sizeof(*dump)); if (pzone) { @@ -1966,6 +1967,9 @@ conntrack_dump_start(struct conntrack *ct, struct conntrack_dump *dump, dump->filter_zone = true; } dump->ct = ct; + if (ptot_bkts) { + *ptot_bkts = CONNTRACK_BUCKETS; + } return 0; } @@ -1991,7 +1995,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..bec37e6 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, uint32_t *); 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..e3a7121 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, uint32_t *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..dd2b174 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, uint32_t *); 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..fe8737a 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -1277,7 +1277,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, NULL); if (error) { dpctl_error(dpctl_p, error, "starting conntrack dump"); dpif_close(dpif); @@ -1373,7 +1373,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, NULL); if (error) { dpctl_error(dpctl_p, error, "starting conntrack dump"); dpif_close(dpif); @@ -1464,6 +1464,104 @@ 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; + uint32_t tot_bkts = 0; + int lastargc = 0; + + int error; + + if (!dpctl_p->is_appctl) { + /* Not called by ovs-appctl command then exit. */ + dpctl_print(dpctl_p, + "Command is available for UserSpace ConnTracker only.\n"); + return 0; + } [Darrell] This will not fully work; ovs-appctl is valid for the kernel. For the kernel, I get ../../tests/system-traffic.at:2683: ovs-appctl dpctl/ct-bkts system@ovs-system --- /dev/null 2017-03-07 19:53:40.602811030 -0800 +++ /home/dball/ovs/_gcc/tests/system-kmod-testsuite.dir/at-groups/59/stdout 2017-07-20 22:28:00.242705054 -0700 @@ -0,0 +1,23 @@ +Total Buckets: 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Bucket nr out of range: 0 >= 0 +Current Connections: 16 + ++-----------+-----------------------------------------+ +| Buckets | Connections per Buckets | ++-----------+-----------------------------------------+ + + 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, >)) { + argc--; [Darrell] I think you want a ‘break’ here; the below processes both ‘gt=’ with the first (last one processed from the end) winning out 10.1.1.241 - - [20/Jul/2017 23:48:35] "GET / HTTP/1.1" 200 - ../../tests/system-traffic.at:2682: ovs-appctl dpctl/ct-bkts gt=1 gt=0 --- /dev/null 2017-03-07 19:53:40.602811030 -0800 +++ /home/dball/ovs/_gcc/tests/system-userspace-testsuite.dir/at-groups/59/stdout 2017-07-20 23:48:35.072443722 -0700 @@ -0,0 +1,39 @@ +Total Buckets: 256 +Current Connections: 1 + ++-----------+-----------------------------------------+ +| Buckets | Connections per Buckets | ++-----------+-----------------------------------------+ + 0.. 7 | . . . . . . . . + 8.. 15 | . . . . . . . . + 16.. 23 | . . . . . . . . + 24.. 31 | . . . . . . . . + 32.. 39 | . . . . . . . . + 40.. 47 | . . . . . . . . + 48.. 55 | . . . . . . . . + 56.. 63 | . . . . . . . . + 64.. 71 | . . . . . . . . + 72.. 79 | . . . . . . . . + 80.. 87 | . . . . . . . . + 88.. 95 | . . . . . . . . + 96..103 | . . . . . . . . + 104..111 | . . . . . . . . + 112..119 | . . . . . . . . + 120..127 | . . . . . . . . + 128..135 | . . . . . . . . + 136..143 | . . . . . . . . + 144..151 | . . . . . . . . + 152..159 | . . . . . . . . + 160..167 | . . . . . . . . + 168..175 | . . . . . . . . + 176..183 | . . . . . . . . + 184..191 | . . . . . . . . + 192..199 | . . . . . . . . + 200..207 | . . . . . . . . + 208..215 | . . . . . . . . + 216..223 | . . . . . . . . + 224..231 | . . . . . . . . + 232..239 | . . . . . . . . + 240..247 | . . . . . . . . + 248..255 | . . . . . . . . + } + } + } + + 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; + } + + 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 (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_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 +1858,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..1c77085 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 +greater than \fIThreshold\fR. [Darrell] Instead of +greater than \fIThreshold\fR. maybe something like +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 1dd0d63..e6ad3c3 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -5346,7 +5346,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, uint32_t *ptot_bkts) { struct dp_netdev *dp = get_dp_netdev(dpif); struct dp_netdev_ct_dump *dump; @@ -5355,7 +5355,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 562f613..842e17b 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, uint32_t *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..2ac2955 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, uint32_t *); 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..bbd70e6 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, + uint32_t *ptot_bkts OVS_UNUSED) { [Darrell] You might try using a signed parameter and setting -1 for the kernel Then ptot_bkts is used here as well. Then you could detect in ct_dpif_dump_start() in dpctl.c and bail out with cleanup. struct nl_ct_dump_state *state; diff --git a/lib/netlink-conntrack.h b/lib/netlink-conntrack.h index 1263b21..8e21919 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, + uint32_t *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..c454123 100644 --- a/tests/test-netlink-conntrack.c +++ b/tests/test-netlink-conntrack.c @@ -113,7 +113,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, NULL); [Darrell] May need to adjust this based on other comments 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 [email protected] https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=rdBCsIsBSudTQtKs5ujJ7Fn98RnzMGfPsma1i8KT58E&s=7AkB4CRrOwYCOOvpQ3RKmYs7HygKbEjrGJbfdU3hw44&e= _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
