| Message ID | 20210529160134.393412-1-aroytman@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v6] ovn-sbctl.c Add logical flows count numbers | expand |
Bleep bloop. Greetings Alexey Roytman, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line lacks whitespace around operator #146 FILE: utilities/ovn-sbctl.c:108: lflow-list [DATAPATH] [LFLOW...] list logical flows for DATAPATH\n\ WARNING: Line lacks whitespace around operator #147 FILE: utilities/ovn-sbctl.c:109: dump-flows [DATAPATH] [LFLOW...] alias for lflow-list\n\ WARNING: Line lacks whitespace around operator #148 FILE: utilities/ovn-sbctl.c:110: count-flows [DATAPATH] count logical flows for DATAPATH\n\ Lines checked: 350, Warnings: 3, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
Hi Aaron, These are not operators, but command names. Could you suggest please how to prevent these warnings. ? Thanks Alexey. On Sat, May 29, 2021 at 8:01 PM 0-day Robot <robot@bytheb.org> wrote: > Bleep bloop. Greetings Alexey Roytman, I am a robot and I have tried out > your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > checkpatch: > WARNING: Line lacks whitespace around operator > #146 FILE: utilities/ovn-sbctl.c:108: > lflow-list [DATAPATH] [LFLOW...] list logical flows for DATAPATH\n\ > > WARNING: Line lacks whitespace around operator > #147 FILE: utilities/ovn-sbctl.c:109: > dump-flows [DATAPATH] [LFLOW...] alias for lflow-list\n\ > > WARNING: Line lacks whitespace around operator > #148 FILE: utilities/ovn-sbctl.c:110: > count-flows [DATAPATH] count logical flows for DATAPATH\n\ > > Lines checked: 350, Warnings: 3, Errors: 0 > > > Please check this out. If you feel there has been an error, please email > aconole@redhat.com > > Thanks, > 0-day Robot >
Alexey Roytman <aroytman@gmail.com> writes: > Hi Aaron, > > These are not operators, but command names. > Could you suggest please how to prevent these warnings. ? There isn't a good way at the moment. We probably need some additional logic in checkpatch that can scan the context from the resulting file. For now, the maintainers know when these warnings are false positives. Sorry for the noise. > Thanks Alexey. > > On Sat, May 29, 2021 at 8:01 PM 0-day Robot <robot@bytheb.org> wrote: > > Bleep bloop. Greetings Alexey Roytman, I am a robot and I have tried out your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > checkpatch: > WARNING: Line lacks whitespace around operator > #146 FILE: utilities/ovn-sbctl.c:108: > lflow-list [DATAPATH] [LFLOW...] list logical flows for DATAPATH\n\ > > WARNING: Line lacks whitespace around operator > #147 FILE: utilities/ovn-sbctl.c:109: > dump-flows [DATAPATH] [LFLOW...] alias for lflow-list\n\ > > WARNING: Line lacks whitespace around operator > #148 FILE: utilities/ovn-sbctl.c:110: > count-flows [DATAPATH] count logical flows for DATAPATH\n\ > > Lines checked: 350, Warnings: 3, Errors: 0 > > Please check this out. If you feel there has been an error, please email aconole@redhat.com > > Thanks, > 0-day Robot
Hi Alexey, Because of commit b6f0e51d8b52cf2381503c3c1c5c2a0d6bd7afa6, this needs a rebase. It also needs to have its printf() calls removed in favor of writing to the ctx->output dynamic string. See below for some other comments, some admittedly kind of nitpicky. On 5/29/21 12:01 PM, Alexey Roytman wrote: > From: Alexey Roytman <roytman@il.ibm.com> > > For big scale deployments, when number of logical flows can be 2M+, > sometimes users just need to know the total number of logical flows > and numbers of logical flows per table/per datapath. > > New command output > ovn-sbctl count-flows > Datapath: "sw1" (4b1e53d8-9f0f-4768-b4a6-6cbc58a4bfda) pipeline: ingress > table=0 (ls_in_port_sec_l2 ) lflows=2 > table=1 (ls_in_port_sec_ip ) lflows=1 > table=2 (ls_in_port_sec_nd ) lflows=1 > table=3 (ls_in_lookup_fdb ) lflows=1 > table=4 (ls_in_put_fdb ) lflows=1 > table=5 (ls_in_pre_acl ) lflows=2 > table=6 (ls_in_pre_lb ) lflows=3 > table=7 (ls_in_pre_stateful ) lflows=2 > table=8 (ls_in_acl_hint ) lflows=1 > table=9 (ls_in_acl ) lflows=2 > table=10(ls_in_qos_mark ) lflows=1 > table=11(ls_in_qos_meter ) lflows=1 > table=12(ls_in_lb ) lflows=1 > table=13(ls_in_stateful ) lflows=8 > table=14(ls_in_pre_hairpin ) lflows=1 > table=15(ls_in_nat_hairpin ) lflows=1 > table=16(ls_in_hairpin ) lflows=1 > table=17(ls_in_arp_rsp ) lflows=1 > table=18(ls_in_dhcp_options ) lflows=1 > table=19(ls_in_dhcp_response) lflows=1 > table=20(ls_in_dns_lookup ) lflows=1 > table=21(ls_in_dns_response ) lflows=1 > table=22(ls_in_external_port) lflows=1 > table=23(ls_in_l2_lkup ) lflows=3 > table=24(ls_in_l2_unknown ) lflows=2 > Total number of logical flows in the datapath "sw1" (4b1e53d8-9f0f-4768-b4a6-6cbc58a4bfda) pipeline: ingress = 41 > > Datapath: "sw1" (4b1e53d8-9f0f-4768-b4a6-6cbc58a4bfda) pipeline: egress > table=0 (ls_out_pre_lb ) lflows=3 > table=1 (ls_out_pre_acl ) lflows=2 > table=2 (ls_out_pre_stateful) lflows=2 > table=3 (ls_out_lb ) lflows=1 > table=4 (ls_out_acl_hint ) lflows=1 > table=5 (ls_out_acl ) lflows=2 > table=6 (ls_out_qos_mark ) lflows=1 > table=7 (ls_out_qos_meter ) lflows=1 > table=8 (ls_out_stateful ) lflows=3 > table=9 (ls_out_port_sec_ip ) lflows=1 > table=10(ls_out_port_sec_l2 ) lflows=1 > Total number of logical flows in the datapath "sw1" (4b1e53d8-9f0f-4768-b4a6-6cbc58a4bfda) pipeline: egress = 18 > > Total number of logical flows = 59 > > Signed-off-by: Alexey Roytman <roytman@il.ibm.com> > --- > v5 -> v6 > * Addressed Ben's comments about replacemen the --count flag of lflow-list/dump-flows by a a "count-flows" command. > v3 -> v4 > * Addressed review comments from Mark > > NOTE: In the current ovn-sbctl (and probably ovn-nbctl) implementation, some of the methods do not work, when the > utility is running in the daemon mode (there is no output). All printf methods should be replaced by "ds_put_*" methods. > I can work on that in a separate path. Meantime, I added to temporary functions "print_datapath_name_new" and > "print_datapath_prompt_new" just for finishing this task. > > tests/ovn-sbctl.at | 45 ++++++++++- > utilities/ovn-sbctl.8.xml | 4 + > utilities/ovn-sbctl.c | 153 +++++++++++++++++++++++++++++++++++--- > 3 files changed, 190 insertions(+), 12 deletions(-) > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > index f49134381..ff727281e 100644 > --- a/tests/ovn-sbctl.at > +++ b/tests/ovn-sbctl.at > @@ -175,4 +175,47 @@ inactivity_probe : 30000 > > OVN_SBCTL_TEST([ovn_sbctl_invalid_0x_flow], [invalid 0x flow], [ > check ovn-sbctl lflow-list 0x12345678 > -]) > \ No newline at end of file If you see this, add a new line to the end of the file. > +]) > + > +dnl --------------------------------------------------------------------- > + > +OVN_SBCTL_TEST([ovn_sbctl_count_flows], [ovn-sbctl - count-flows], [ > + > +count_entries() { > + ovn-sbctl --column=_uuid list Logical_Flow | sed -r '/^\s*$/d' | wc -l > +} > + > +count_pipeline() { > + ovn-sbctl --column=pipeline list Logical_Flow | grep $1 | sed -r '/^\s*$/d' | wc -l > +} > + > +# we start with empty Logical_Flow table > +# validate that the table is indeed empty > +AT_CHECK([count_entries], [0], [dnl > +0 > +]) > + > +AT_CHECK([ovn-sbctl count-flows], [0], [stdout], [stderr]) > + > +AT_CHECK([ovn-sbctl count-flows], [0], [dnl > +Total number of logical flows = 0 > +]) > + > +# create some logical flows > +check ovn-nbctl ls-add count-test > + > +OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) > + > +egress_lflows=`count_pipeline egress` > +ingress_lflows=`count_pipeline ingress` > + > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep "flows =" | awk 'NF>1{print $NF}'], [0], [dnl > +$total_lflows > +]) > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep Total | grep egress | awk 'NF>1{print $NF}'], [0], [dnl > +$egress_lflows > +]) > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep Total | grep ingress | awk 'NF>1{print $NF}'], [0], [dnl > +$ingress_lflows > +]) > +]) > diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml > index 4e6b21c47..ad16f5fa3 100644 > --- a/utilities/ovn-sbctl.8.xml > +++ b/utilities/ovn-sbctl.8.xml > @@ -416,10 +416,14 @@ > <var>chassis</var>. The <code>--ovs</code> and <code>--stats</code> > can also be used in conjunction with <code>--vflows</code>. > </p> > + > </dd> > > <dt>[<code>--uuid</code>] <code>dump-flows</code> [<var>logical-datapath</var>]</dt> > <dd>Alias for <code>lflow-list</code>.</dd> > + > + <dt><code>count-flows</code> [<var>logical-datapath</var>]</dt> > + <dd>prints numbers of logical flows per table and per datapath.</dd> > </dl> > > <h2>Remote Connectivity Commands</h2> > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c > index 53f10cdd8..ba352c05b 100644 > --- a/utilities/ovn-sbctl.c > +++ b/utilities/ovn-sbctl.c > @@ -105,8 +105,9 @@ Port binding commands:\n\ > lsp-unbind PORT reset the port binding of logical port PORT\n\ > \n\ > Logical flow commands:\n\ > - lflow-list [DATAPATH] [LFLOW...] List logical flows for DATAPATH\n\ > - dump-flows [DATAPATH] [LFLOW...] Alias for lflow-list\n\ > + lflow-list [DATAPATH] [LFLOW...] list logical flows for DATAPATH\n\ > + dump-flows [DATAPATH] [LFLOW...] alias for lflow-list\n\ > + count-flows [DATAPATH] count logical flows for DATAPATH\n\ > \n\ > Connection commands:\n\ > get-connection print the connections\n\ > @@ -682,6 +683,24 @@ print_datapath_name(const struct sbrec_datapath_binding *dp) > } > } > > +/* > + A temp function, will replace the original 'print_datapath_name' > + when all prints will be replaced by ds_put methods. I suspect that because of the commit referenced at the beginning of my message, you won't need a temporary function. > +*/ > +static void > +print_datapath_name_new(struct ds *ds, const struct sbrec_datapath_binding *dp) > +{ > + const struct smap *ids = &dp->external_ids; > + const char *name = smap_get(ids, "name"); > + const char *name2 = smap_get(ids, "name2"); > + if (name && name2) { > + ds_put_format(ds, "\"%s\" aka \"%s\"", name, name2); > + } else if (name || name2) { > + ds_put_format(ds, "\"%s\"", name ? name : name2); > + } > +} > + > + > static void > print_vflow_datapath_name(const struct sbrec_datapath_binding *dp, > bool do_print) > @@ -898,14 +917,120 @@ sbctl_lflow_add(struct sbctl_lflow **lflows, > if (*n_flows == *n_capacity) { > *lflows = x2nrealloc(*lflows, n_capacity, sizeof **lflows); > } > - (*lflows)[*n_flows].lflow = lflow; > - (*lflows)[*n_flows].dp = dp; > + (*lflows)[ *n_flows ].lflow = lflow; > + (*lflows)[ *n_flows ].dp = dp; The changes on the two lines above are not necessary. > (*n_flows)++; > } > > +static void > +print_datapath_prompt(const struct sbrec_datapath_binding *dp, > + const struct uuid *uuid, > + char *pipeline) { > + printf("Datapath: "); > + print_datapath_name(dp); > + printf(" ("UUID_FMT") pipeline: %s\n", UUID_ARGS(uuid), pipeline); > +} > + > +/* > + A temp function, will replace the original 'print_datapath_prompt' > + when all prints will be replaced by ds_put methods. > +*/ Similarly to print_datapath_name_new(), I think you can get rid of this temporary function. > +static void > +print_datapath_prompt_new(struct ds *ds, > + const struct sbrec_datapath_binding *dp, > + const struct uuid *uuid, > + char *pipeline) { > + ds_put_cstr(ds, "Datapath: "); > + print_datapath_name_new(ds, dp); > + ds_put_format(ds, > + " ("UUID_FMT") pipeline: %s\n", > + UUID_ARGS(uuid), pipeline); > +} > + > + > +static void > +print_datapath_sum(struct ds *ds, > + const struct sbrec_datapath_binding *dp, > + const struct uuid *uuid, > + char *pipeline, > + long lflows) { > + ds_put_cstr(ds, "Total number of logical flows in the datapath "); > + print_datapath_name_new(ds, dp); > + ds_put_format(ds, " ("UUID_FMT") pipeline: %s = %ld\n\n", > + UUID_ARGS(uuid), pipeline, lflows); > +} > + > + > +static void > +print_lflows_count(struct ds *ds, > + int64_t table_id, > + const char *name, > + long count) { > + ds_put_format(ds, " table=%-2"PRId64"(%-19s) lflows=%ld\n", \ > + table_id, name, count); > +} > + > +static void > +print_lflow_counters(struct ds *ds, size_t n_flows, struct sbctl_lflow *lflows) > +{ > + const struct sbctl_lflow *curr, *prev = NULL; > + long table_lflows = 0; > + long dp_lflows = 0; > + > + for (size_t i = 0; i < n_flows; i++) { > + bool new_datapath = false; > + curr = &lflows[i]; > + if (!prev > + || prev->dp != curr->dp > + || strcmp(prev->lflow->pipeline, curr->lflow->pipeline)) { > + new_datapath = true; > + } > + > + if (prev && > + (prev->lflow->table_id != curr->lflow->table_id || new_datapath)) { > + print_lflows_count(ds, prev->lflow->table_id, > + smap_get_def(&prev->lflow->external_ids, "stage-name", ""), > + table_lflows); > + table_lflows = 1; > + if (new_datapath) { > + print_datapath_sum(ds, prev->dp, &prev->dp->header_.uuid, > + prev->lflow->pipeline, dp_lflows); > + dp_lflows = 1; > + } else { > + dp_lflows++; > + } > + } else { > + dp_lflows++; > + table_lflows++; > + } > + > + if (new_datapath) { > + print_datapath_prompt_new(ds, > + curr->dp, > + &curr->dp->header_.uuid, > + curr->lflow->pipeline); > + } > + prev = curr; > + } > + if (n_flows > 0) { > + print_lflows_count(ds, > + prev->lflow->table_id, > + smap_get_def(&prev->lflow->external_ids, > + "stage-name", > + ""), > + table_lflows); > + print_datapath_sum(ds, > + prev->dp, &prev->dp->header_.uuid, > + prev->lflow->pipeline, dp_lflows); > + > + } > + ds_put_format(ds, "Total number of logical flows = %ld\n", n_flows); > +} > + > static void > cmd_lflow_list(struct ctl_context *ctx) > { > + const char *cmd = ctx->argv[0]; > const struct sbrec_datapath_binding *datapath = NULL; > if (ctx->argc > 1) { > const struct ovsdb_idl_row *row; > @@ -966,6 +1091,11 @@ cmd_lflow_list(struct ctl_context *ctx) > qsort(lflows, n_flows, sizeof *lflows, sbctl_lflow_cmp); > } > > + if (strcmp(cmd, "count-flows")==0) { I don't think there's actually a coding guideline that states this, but it's unusual to compare the result of strcmp like this. I think most of us are trained to see "strcmp()" as meaning "the strings are different" and "!strcmp()" as meaning "the strings are the same". So it tripped me up for a second to see it written this way. Also, please put a space before and after the "==" operator when you're using it. I'm actually surprised checkpatch didn't flag that. > + print_lflow_counters(&ctx->output, n_flows, lflows); > + goto cleanup; > + } > + > bool print_uuid = shash_find(&ctx->options, "--uuid") != NULL; > > const struct sbctl_lflow *curr, *prev = NULL; > @@ -997,11 +1127,9 @@ cmd_lflow_list(struct ctl_context *ctx) > if (!prev > || prev->dp != curr->dp > || strcmp(prev->lflow->pipeline, curr->lflow->pipeline)) { > - printf("Datapath: "); > - print_datapath_name(curr->dp); > - printf(" ("UUID_FMT") Pipeline: %s\n", > - UUID_ARGS(&curr->dp->header_.uuid), > - curr->lflow->pipeline); > + print_datapath_prompt(curr->dp, > + &curr->dp->header_.uuid, > + curr->lflow->pipeline); > } > > /* Print the flow. */ > @@ -1028,6 +1156,7 @@ cmd_lflow_list(struct ctl_context *ctx) > cmd_lflow_list_load_balancers(ctx, vconn, datapath, stats, print_uuid); > } > > +cleanup: > vconn_close(vconn); > free(lflows); > } > @@ -1378,8 +1507,10 @@ static const struct ctl_command_syntax sbctl_commands[] = { > "--uuid,--ovs?,--stats,--vflows?", RO}, > {"dump-flows", 0, INT_MAX, "[DATAPATH] [LFLOW...]", > pre_get_info, cmd_lflow_list, NULL, > - "--uuid,--ovs?,--stats,--vflows?", > - RO}, /* Friendly alias for lflow-list */ > + "--uuid,--ovs?,--stats,--vflows?", RO}, > + /* Friendly alias for lflow-list */ > + {"count-flows", 0, 1, "[DATAPATH]", > + pre_get_info, cmd_lflow_list, NULL, "", RO}, > > /* IP multicast commands. */ > {"ip-multicast-flush", 0, 1, "SWITCH", >
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at index f49134381..ff727281e 100644 --- a/tests/ovn-sbctl.at +++ b/tests/ovn-sbctl.at @@ -175,4 +175,47 @@ inactivity_probe : 30000 OVN_SBCTL_TEST([ovn_sbctl_invalid_0x_flow], [invalid 0x flow], [ check ovn-sbctl lflow-list 0x12345678 -]) \ No newline at end of file +]) + +dnl --------------------------------------------------------------------- + +OVN_SBCTL_TEST([ovn_sbctl_count_flows], [ovn-sbctl - count-flows], [ + +count_entries() { + ovn-sbctl --column=_uuid list Logical_Flow | sed -r '/^\s*$/d' | wc -l +} + +count_pipeline() { + ovn-sbctl --column=pipeline list Logical_Flow | grep $1 | sed -r '/^\s*$/d' | wc -l +} + +# we start with empty Logical_Flow table +# validate that the table is indeed empty +AT_CHECK([count_entries], [0], [dnl +0 +]) + +AT_CHECK([ovn-sbctl count-flows], [0], [stdout], [stderr]) + +AT_CHECK([ovn-sbctl count-flows], [0], [dnl +Total number of logical flows = 0 +]) + +# create some logical flows +check ovn-nbctl ls-add count-test + +OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) + +egress_lflows=`count_pipeline egress` +ingress_lflows=`count_pipeline ingress` + +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep "flows =" | awk 'NF>1{print $NF}'], [0], [dnl +$total_lflows +]) +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep Total | grep egress | awk 'NF>1{print $NF}'], [0], [dnl +$egress_lflows +]) +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep Total | grep ingress | awk 'NF>1{print $NF}'], [0], [dnl +$ingress_lflows +]) +]) diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml index 4e6b21c47..ad16f5fa3 100644 --- a/utilities/ovn-sbctl.8.xml +++ b/utilities/ovn-sbctl.8.xml @@ -416,10 +416,14 @@ <var>chassis</var>. The <code>--ovs</code> and <code>--stats</code> can also be used in conjunction with <code>--vflows</code>. </p> + </dd> <dt>[<code>--uuid</code>] <code>dump-flows</code> [<var>logical-datapath</var>]</dt> <dd>Alias for <code>lflow-list</code>.</dd> + + <dt><code>count-flows</code> [<var>logical-datapath</var>]</dt> + <dd>prints numbers of logical flows per table and per datapath.</dd> </dl> <h2>Remote Connectivity Commands</h2> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c index 53f10cdd8..ba352c05b 100644 --- a/utilities/ovn-sbctl.c +++ b/utilities/ovn-sbctl.c @@ -105,8 +105,9 @@ Port binding commands:\n\ lsp-unbind PORT reset the port binding of logical port PORT\n\ \n\ Logical flow commands:\n\ - lflow-list [DATAPATH] [LFLOW...] List logical flows for DATAPATH\n\ - dump-flows [DATAPATH] [LFLOW...] Alias for lflow-list\n\ + lflow-list [DATAPATH] [LFLOW...] list logical flows for DATAPATH\n\ + dump-flows [DATAPATH] [LFLOW...] alias for lflow-list\n\ + count-flows [DATAPATH] count logical flows for DATAPATH\n\ \n\ Connection commands:\n\ get-connection print the connections\n\ @@ -682,6 +683,24 @@ print_datapath_name(const struct sbrec_datapath_binding *dp) } } +/* + A temp function, will replace the original 'print_datapath_name' + when all prints will be replaced by ds_put methods. +*/ +static void +print_datapath_name_new(struct ds *ds, const struct sbrec_datapath_binding *dp) +{ + const struct smap *ids = &dp->external_ids; + const char *name = smap_get(ids, "name"); + const char *name2 = smap_get(ids, "name2"); + if (name && name2) { + ds_put_format(ds, "\"%s\" aka \"%s\"", name, name2); + } else if (name || name2) { + ds_put_format(ds, "\"%s\"", name ? name : name2); + } +} + + static void print_vflow_datapath_name(const struct sbrec_datapath_binding *dp, bool do_print) @@ -898,14 +917,120 @@ sbctl_lflow_add(struct sbctl_lflow **lflows, if (*n_flows == *n_capacity) { *lflows = x2nrealloc(*lflows, n_capacity, sizeof **lflows); } - (*lflows)[*n_flows].lflow = lflow; - (*lflows)[*n_flows].dp = dp; + (*lflows)[ *n_flows ].lflow = lflow; + (*lflows)[ *n_flows ].dp = dp; (*n_flows)++; } +static void +print_datapath_prompt(const struct sbrec_datapath_binding *dp, + const struct uuid *uuid, + char *pipeline) { + printf("Datapath: "); + print_datapath_name(dp); + printf(" ("UUID_FMT") pipeline: %s\n", UUID_ARGS(uuid), pipeline); +} + +/* + A temp function, will replace the original 'print_datapath_prompt' + when all prints will be replaced by ds_put methods. +*/ +static void +print_datapath_prompt_new(struct ds *ds, + const struct sbrec_datapath_binding *dp, + const struct uuid *uuid, + char *pipeline) { + ds_put_cstr(ds, "Datapath: "); + print_datapath_name_new(ds, dp); + ds_put_format(ds, + " ("UUID_FMT") pipeline: %s\n", + UUID_ARGS(uuid), pipeline); +} + + +static void +print_datapath_sum(struct ds *ds, + const struct sbrec_datapath_binding *dp, + const struct uuid *uuid, + char *pipeline, + long lflows) { + ds_put_cstr(ds, "Total number of logical flows in the datapath "); + print_datapath_name_new(ds, dp); + ds_put_format(ds, " ("UUID_FMT") pipeline: %s = %ld\n\n", + UUID_ARGS(uuid), pipeline, lflows); +} + + +static void +print_lflows_count(struct ds *ds, + int64_t table_id, + const char *name, + long count) { + ds_put_format(ds, " table=%-2"PRId64"(%-19s) lflows=%ld\n", \ + table_id, name, count); +} + +static void +print_lflow_counters(struct ds *ds, size_t n_flows, struct sbctl_lflow *lflows) +{ + const struct sbctl_lflow *curr, *prev = NULL; + long table_lflows = 0; + long dp_lflows = 0; + + for (size_t i = 0; i < n_flows; i++) { + bool new_datapath = false; + curr = &lflows[i]; + if (!prev + || prev->dp != curr->dp + || strcmp(prev->lflow->pipeline, curr->lflow->pipeline)) { + new_datapath = true; + } + + if (prev && + (prev->lflow->table_id != curr->lflow->table_id || new_datapath)) { + print_lflows_count(ds, prev->lflow->table_id, + smap_get_def(&prev->lflow->external_ids, "stage-name", ""), + table_lflows); + table_lflows = 1; + if (new_datapath) { + print_datapath_sum(ds, prev->dp, &prev->dp->header_.uuid, + prev->lflow->pipeline, dp_lflows); + dp_lflows = 1; + } else { + dp_lflows++; + } + } else { + dp_lflows++; + table_lflows++; + } + + if (new_datapath) { + print_datapath_prompt_new(ds, + curr->dp, + &curr->dp->header_.uuid, + curr->lflow->pipeline); + } + prev = curr; + } + if (n_flows > 0) { + print_lflows_count(ds, + prev->lflow->table_id, + smap_get_def(&prev->lflow->external_ids, + "stage-name", + ""), + table_lflows); + print_datapath_sum(ds, + prev->dp, &prev->dp->header_.uuid, + prev->lflow->pipeline, dp_lflows); + + } + ds_put_format(ds, "Total number of logical flows = %ld\n", n_flows); +} + static void cmd_lflow_list(struct ctl_context *ctx) { + const char *cmd = ctx->argv[0]; const struct sbrec_datapath_binding *datapath = NULL; if (ctx->argc > 1) { const struct ovsdb_idl_row *row; @@ -966,6 +1091,11 @@ cmd_lflow_list(struct ctl_context *ctx) qsort(lflows, n_flows, sizeof *lflows, sbctl_lflow_cmp); } + if (strcmp(cmd, "count-flows")==0) { + print_lflow_counters(&ctx->output, n_flows, lflows); + goto cleanup; + } + bool print_uuid = shash_find(&ctx->options, "--uuid") != NULL; const struct sbctl_lflow *curr, *prev = NULL; @@ -997,11 +1127,9 @@ cmd_lflow_list(struct ctl_context *ctx) if (!prev || prev->dp != curr->dp || strcmp(prev->lflow->pipeline, curr->lflow->pipeline)) { - printf("Datapath: "); - print_datapath_name(curr->dp); - printf(" ("UUID_FMT") Pipeline: %s\n", - UUID_ARGS(&curr->dp->header_.uuid), - curr->lflow->pipeline); + print_datapath_prompt(curr->dp, + &curr->dp->header_.uuid, + curr->lflow->pipeline); } /* Print the flow. */ @@ -1028,6 +1156,7 @@ cmd_lflow_list(struct ctl_context *ctx) cmd_lflow_list_load_balancers(ctx, vconn, datapath, stats, print_uuid); } +cleanup: vconn_close(vconn); free(lflows); } @@ -1378,8 +1507,10 @@ static const struct ctl_command_syntax sbctl_commands[] = { "--uuid,--ovs?,--stats,--vflows?", RO}, {"dump-flows", 0, INT_MAX, "[DATAPATH] [LFLOW...]", pre_get_info, cmd_lflow_list, NULL, - "--uuid,--ovs?,--stats,--vflows?", - RO}, /* Friendly alias for lflow-list */ + "--uuid,--ovs?,--stats,--vflows?", RO}, + /* Friendly alias for lflow-list */ + {"count-flows", 0, 1, "[DATAPATH]", + pre_get_info, cmd_lflow_list, NULL, "", RO}, /* IP multicast commands. */ {"ip-multicast-flush", 0, 1, "SWITCH",