[ovs-dev] [PATCH v2] ovn: Add lflow-list to ovn-sbctl.

Alex Wang alexw at nicira.com
Wed Aug 12 22:51:20 UTC 2015


Thx a lot for the refinement,

Minor comments below,


On Wed, Aug 12, 2015 at 3:04 PM, Russell Bryant <rbryant at 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 <rbryant at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list