On Wed, Jul 14, 2021 at 8:23 AM Mark Michelson <[email protected]> wrote: > > Looks good to me Alexey, thanks! > > Acked-by: Mark Michelson <[email protected]>
Thanks Alexey and Mark for the reviews. I applied this patch to the main branch. Numan > > On 6/19/21 3:38 PM, Alexey Roytman wrote: > > From: Alexey Roytman <[email protected]> > > > > 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 > > 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 <[email protected]> > > > > --- > > V6 -> V7 > > * Addressed commit b6f0e51d8b52cf2381503c3c1c5c2a0d6bd7afa6 and Matk's > > comments > > 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 > > > > --- > > tests/ovn-sbctl.at | 69 ++++++++++++++++++++++++- > > utilities/ovn-sbctl.8.xml | 3 ++ > > utilities/ovn-sbctl.c | 106 +++++++++++++++++++++++++++++++++++--- > > 3 files changed, 169 insertions(+), 9 deletions(-) > > > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > > index f49134381..16f5dabcc 100644 > > --- a/tests/ovn-sbctl.at > > +++ b/tests/ovn-sbctl.at > > @@ -175,4 +175,71 @@ 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], [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]) > > + > > +total_lflows=`count_entries` > > +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 > > +]) > > + > > +# add another datapath > > +check ovn-nbctl ls-add count-test2 > > + > > +# check total logical flows in 2 datapathes > > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep "flows =" | awk > > 'NF>1{print $NF}'], [0], [dnl > > +$(($total_lflows * 2)) > > +]) > > +# check total logical flows in a specific datapath > > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows count-test | grep "flows =" | awk > > 'NF>1{print $NF}'], [0], [dnl > > +$total_lflows > > +]) > > + > > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows count-test | grep Total | grep > > egress | awk 'NF>1{print $NF}'], [0], [dnl > > +$egress_lflows > > +]) > > +AT_CHECK_UNQUOTED([ovn-sbctl count-flows count-test | grep Total | grep > > ingress | awk 'NF>1{print $NF}'], [0], [dnl > > +$ingress_lflows > > +]) > > + > > +# check nonexistent datapath > > +AT_CHECK([ovn-sbctl count-flows wrongDatapath], [0], [dnl > > +Total number of logical flows = 0 > > +]) > > +]) > > + > > diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml > > index 4e6b21c47..6d08ff70a 100644 > > --- a/utilities/ovn-sbctl.8.xml > > +++ b/utilities/ovn-sbctl.8.xml > > @@ -420,6 +420,9 @@ > > > > <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 4146384e7..1ab148a67 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\ > > @@ -907,9 +908,88 @@ sbctl_lflow_add(struct sbctl_lflow **lflows, > > (*n_flows)++; > > } > > > > +static void > > +print_datapath_prompt(const struct sbrec_datapath_binding *dp, > > + const struct uuid *uuid, char *pipeline, > > + struct ds *s) { > > + ds_put_cstr(s, "Datapath: "); > > + print_datapath_name(dp, s); > > + ds_put_format(s, " ("UUID_FMT") Pipeline: %s\n", > > + UUID_ARGS(uuid), pipeline); > > +} > > + > > +static void > > +print_datapath_sum(const struct sbrec_datapath_binding *dp, > > + const struct uuid *uuid, char *pipeline, > > + long lflows, struct ds *s) { > > + ds_put_cstr(s, "Total number of logical flows in the datapath "); > > + print_datapath_name(dp, s); > > + ds_put_format(s, " ("UUID_FMT") Pipeline: %s = %ld\n\n", > > + UUID_ARGS(uuid), pipeline, lflows); > > +} > > + > > +static void > > +print_lflows_count(int64_t table_id, const char *name, > > + long count, struct ds *s) { > > + ds_put_format(s, " table=%-2"PRId64"(%-19s) lflows=%ld\n", \ > > + table_id, name, count); > > +} > > + > > +static void > > +print_lflow_counters(size_t n_flows, struct sbctl_lflow *lflows, struct ds > > *s) > > +{ > > + 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(prev->lflow->table_id, > > + smap_get_def(&prev->lflow->external_ids, "stage-name", ""), > > + table_lflows, s); > > + table_lflows = 1; > > + if (new_datapath) { > > + print_datapath_sum(prev->dp, &prev->dp->header_.uuid, > > + prev->lflow->pipeline, dp_lflows, s); > > + dp_lflows = 1; > > + } else { > > + dp_lflows++; > > + } > > + } else { > > + dp_lflows++; > > + table_lflows++; > > + } > > + > > + if (new_datapath) { > > + print_datapath_prompt(curr->dp, &curr->dp->header_.uuid, > > + curr->lflow->pipeline, s); > > + } > > + prev = curr; > > + } > > + if (n_flows > 0) { > > + print_lflows_count(prev->lflow->table_id, > > + smap_get_def(&prev->lflow->external_ids, > > + "stage-name", ""), > > + table_lflows, s); > > + print_datapath_sum(prev->dp, &prev->dp->header_.uuid, > > + prev->lflow->pipeline, dp_lflows, s); > > + > > + } > > + ds_put_format(s, "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; > > @@ -925,7 +1005,12 @@ cmd_lflow_list(struct ctl_context *ctx) > > if (datapath) { > > ctx->argc--; > > ctx->argv++; > > - } > > + } else if (!strcmp(cmd, "count-flows")) { > > + /* datapath is defined, but isn't found */ > > + print_lflow_counters(0, NULL, &ctx->output); > > + return; > > + } > > + > > } > > > > for (size_t i = 1; i < ctx->argc; i++) { > > @@ -970,6 +1055,11 @@ cmd_lflow_list(struct ctl_context *ctx) > > qsort(lflows, n_flows, sizeof *lflows, sbctl_lflow_cmp); > > } > > > > + if (!strcmp(cmd, "count-flows")) { > > + print_lflow_counters(n_flows, lflows, &ctx->output); > > + goto cleanup; > > + } > > + > > bool print_uuid = shash_find(&ctx->options, "--uuid") != NULL; > > > > const struct sbctl_lflow *curr, *prev = NULL; > > @@ -1001,11 +1091,8 @@ cmd_lflow_list(struct ctl_context *ctx) > > if (!prev > > || prev->dp != curr->dp > > || strcmp(prev->lflow->pipeline, curr->lflow->pipeline)) { > > - ds_put_cstr(&ctx->output, "Datapath: "); > > - print_datapath_name(curr->dp, &ctx->output); > > - ds_put_format(&ctx->output, " ("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, &ctx->output); > > } > > > > /* Print the flow. */ > > @@ -1035,6 +1122,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); > > } > > @@ -1387,6 +1475,8 @@ static const struct ctl_command_syntax > > sbctl_commands[] = { > > pre_get_info, cmd_lflow_list, NULL, > > "--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", > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
