Thx a lot for the refinement,
Minor comments below,
On Wed, Aug 12, 2015 at 3:04 PM, Russell Bryant <rbry...@redhat.com> wrote:
> I frequently view the contents of the Logical_Flow table while working
> on OVN. Add a command that can output the contents of this table in a
> sorted way that makes it easier to read through. It's sorted by
> logical datapath, pipeline, table id, priority, and match.
>
> Signed-off-by: Russell Bryant <rbry...@redhat.com>
> ---
> ovn/utilities/ovn-sbctl.8.in | 6 +++
> ovn/utilities/ovn-sbctl.c | 116
> +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 122 insertions(+)
>
>
> v1->v2
> - Address Alex's review feedback
> - Add a friendly "dump-flows" alias
>
> Example output:
>
> $ ./ovn-2port-setup.sh
> + ovn-nbctl lswitch-add sw0
> + ovn-nbctl lport-add sw0 sw0-port1
> + ovn-nbctl lport-add sw0 sw0-port2
> + ovn-nbctl lport-set-macs sw0-port1 00:00:00:00:00:01
> + ovn-nbctl lport-set-macs sw0-port2 00:00:00:00:00:02
> + ovn-nbctl lport-set-port-security sw0-port1 00:00:00:00:00:01
> + ovn-nbctl lport-set-port-security sw0-port2 00:00:00:00:00:02
> + ovs-vsctl add-port br-int lport1 -- set Interface lport1
> external_ids:iface-id=sw0-port1
> + ovs-vsctl add-port br-int lport2 -- set Interface lport2
> external_ids:iface-id=sw0-port2
> $ ovn-sbctl lflow-list
> Datapath: 0f9a9e4e-0ef0-4afb-bed4-09887387fd10 Pipeline: ingress
> table_id=0, priority=100, match=(eth.src[40]), action=(drop;)
> table_id=0, priority=100, match=(vlan.present), action=(drop;)
> table_id=0, priority= 50, match=(inport == "sw0-port1" && eth.src ==
> {00:00:00:00:00:01}), action=(next;)
> table_id=0, priority= 50, match=(inport == "sw0-port2" && eth.src ==
> {00:00:00:00:00:02}), action=(next;)
> table_id=0, priority= 0, match=(1), action=(drop;)
> table_id=1, priority=100, match=(eth.dst[40]), action=(outport =
> "_MC_flood"; output;)
> table_id=1, priority= 50, match=(eth.dst == 00:00:00:00:00:01),
> action=(outport = "sw0-port1"; output;)
> table_id=1, priority= 50, match=(eth.dst == 00:00:00:00:00:02),
> action=(outport = "sw0-port2"; output;)
> Datapath: 0f9a9e4e-0ef0-4afb-bed4-09887387fd10 Pipeline: egress
> table_id=0, priority= 0, match=(1), action=(next;)
> table_id=1, priority=100, match=(eth.dst[40]), action=(output;)
> table_id=1, priority= 50, match=(outport == "sw0-port1" && eth.dst ==
> {00:00:00:00:00:01}), action=(output;)
> table_id=1, priority= 50, match=(outport == "sw0-port2" && eth.dst ==
> {00:00:00:00:00:02}), action=(output;)
>
>
>
> diff --git a/ovn/utilities/ovn-sbctl.8.in b/ovn/utilities/ovn-sbctl.8.in
> index b5c796e..9f9168e 100644
> --- a/ovn/utilities/ovn-sbctl.8.in
> +++ b/ovn/utilities/ovn-sbctl.8.in
> @@ -146,6 +146,12 @@ Without \fB\-\-if\-exists\fR, attempting to unbind a
> logical port
> that is not bound is an error. With \fB\-\-if\-exists\fR, attempting
> to unbind logical port that is not bound has no effect.
> .
> +.SS "Logical Flow Commands"
> +.
> +.IP "\fBlflow\-list\fR [\fIlogical\-datapath\fR]"
> +List logical flows. If \fIlogical\-datapath\fR is specified, only list
> flows for
> +that logical datapath.
> +.
> .so lib/db-ctl-base.man
> .SH "EXIT STATUS"
> .IP "0"
> diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
> index cbde60a..5d988f5 100644
> --- a/ovn/utilities/ovn-sbctl.c
> +++ b/ovn/utilities/ovn-sbctl.c
> @@ -300,6 +300,9 @@ Port binding commands:\n\
> lport-bind LPORT CHASSIS bind logical port LPORT to CHASSIS\n\
> lport-unbind LPORT reset the port binding of logical port
> LPORT\n\
> \n\
> +Logical flow commands:\n\
> + lflow-list [DATAPATH] List logical flows for all or a single
> datapath\n\
> +\n\
>
Could we also add usage for the alias 'dump-flows'
> %s\
> \n\
> Options:\n\
> @@ -470,6 +473,13 @@ pre_get_info(struct ctl_context *ctx)
>
> ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port);
> ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis);
> +
> + ovsdb_idl_add_column(ctx->idl,
> &sbrec_logical_flow_col_logical_datapath);
> + ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_pipeline);
> + ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_actions);
> + ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_priority);
> + ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_table_id);
> + ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_match);
> }
>
> static struct cmd_show_table cmd_show_tables[] = {
> @@ -584,6 +594,106 @@ cmd_lport_unbind(struct ctl_context *ctx)
> }
> }
>
> +enum {
> + PL_INGRESS,
> + PL_EGRESS,
> +};
> +
> +/* Help ensure we catch any future pipeline values */
> +static int
> +pipeline_encode(const char *pl)
> +{
> + if (!strcmp(pl, "ingress")) {
> + return PL_INGRESS;
> + } else if (!strcmp(pl, "egress")) {
> + return PL_EGRESS;
> + }
> +
> + OVS_NOT_REACHED();
> +}
> +
> +static int
> +lflow_cmp(const void *lf1_, const void *lf2_)
> +{
> + const struct sbrec_logical_flow *lf1, *lf2;
> +
> + lf1 = *((struct sbrec_logical_flow **) lf1_);
> + lf2 = *((struct sbrec_logical_flow **) lf2_);
> +
> + int pl1 = pipeline_encode(lf1->pipeline);
> + int pl2 = pipeline_encode(lf2->pipeline);
> +
> +#define CMP(expr) \
> + do { \
> + int res; \
> + res = (expr); \
> + if (res) { \
> + return res; \
> + } \
> + } while (0)
> +
> + CMP(uuid_compare_3way(&lf1->logical_datapath->header_.uuid,
> + &lf2->logical_datapath->header_.uuid));
> + CMP(strcmp(lf1->pipeline, lf2->pipeline) * -1);
>
Should remove this line?
> + CMP(pl1 > pl2 ? 1 : (pl1 < pl2 ? -1 : 0));
>
Can we just subtract?
Thanks,
Alex Wang,
> + CMP(lf1->table_id > lf2->table_id ? 1 :
> + (lf1->table_id < lf2->table_id ? -1 : 0));
> + CMP(lf1->priority > lf2->priority ? -1 :
> + (lf1->priority < lf2->priority ? 1 : 0));
> + CMP(strcmp(lf1->match, lf2->match));
> +
> +#undef CMP
> +
> + return 0;
> +}
> +
> +static void
> +cmd_lflow_list(struct ctl_context *ctx)
> +{
> + const char *datapath = ctx->argc == 2 ? ctx->argv[1] : NULL;
> + struct uuid datapath_uuid = { .parts = { 0, }};
> + const struct sbrec_logical_flow **lflows;
> + const struct sbrec_logical_flow *lflow;
> + size_t n_flows = 0, n_capacity = 64;
> +
> + if (datapath && !uuid_from_string(&datapath_uuid, datapath)) {
> + VLOG_ERR("Invalid format of datapath UUID");
> + return;
> + }
> +
> + lflows = xmalloc(sizeof *lflows * n_capacity);
> + SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->idl) {
> + if (n_flows == n_capacity) {
> + lflows = x2nrealloc(lflows, &n_capacity, sizeof *lflows);
> + }
> + lflows[n_flows] = lflow;
> + n_flows++;
> + }
> +
> + qsort(lflows, n_flows, sizeof *lflows, lflow_cmp);
> +
> + const char *cur_pipeline = "";
> + size_t i;
> + for (i = 0; i < n_flows; i++) {
> + lflow = lflows[i];
> + if (datapath && !uuid_equals(&datapath_uuid,
> +
> &lflow->logical_datapath->header_.uuid)) {
> + continue;
> + }
> + if (strcmp(cur_pipeline, lflow->pipeline)) {
> + printf("Datapath: " UUID_FMT " Pipeline: %s\n",
> + UUID_ARGS(&lflow->logical_datapath->header_.uuid),
> + lflow->pipeline);
> + cur_pipeline = lflow->pipeline;
> + }
> + printf(" table_id=%" PRId64 ", priority=%3" PRId64 ", "
> + "match=(%s), action=(%s)\n",
> + lflow->table_id, lflow->priority,
> + lflow->match, lflow->actions);
> + }
> +
> + free(lflows);
> +}
>
> static const struct ctl_table_class tables[] = {
> {&sbrec_table_chassis,
> @@ -858,6 +968,12 @@ static const struct ctl_command_syntax
> sbctl_commands[] = {
> {"lport-unbind", 1, 1, "LPORT", pre_get_info, cmd_lport_unbind, NULL,
> "--if-exists", RW},
>
> + /* Logical flow commands */
> + {"lflow-list", 0, 1, "[DATAPATH]", pre_get_info, cmd_lflow_list, NULL,
> + "", RO},
> + {"dump-flows", 0, 1, "[DATAPATH]", pre_get_info, cmd_lflow_list, NULL,
> + "", RO}, /* Friendly alias for lflow-list */
> +
> /* SSL commands (To Be Added). */
>
> {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
> --
> 2.4.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
