diff mbox series

[ovs-dev,ovn] Improve debuggability of OVN to OpenFlow translations.

Message ID 1572609915-24716-1-git-send-email-dceara@redhat.com
State Accepted
Headers show
Series [ovs-dev,ovn] Improve debuggability of OVN to OpenFlow translations. | expand

Commit Message

Dumitru Ceara Nov. 1, 2019, 12:05 p.m. UTC
Until now, when translating SB database contents to OpenFlow flows,
ovn-controller stored (part of) the SB record UUID in the cookie field
of the OpenFlow entry only when translating logical flows.

Extend this behavior to the following SB Database table records too:
- Port_Binding
- Mac_Binding
- Multicast_Group
- Chassis

This makes debugging easier by allowing the user to trace back the
original SB entry that generated the OpenFlow entry. Also, the
"ovn-sbctl lflow-list" command now supports an additional "--vflows"
argument (virtual flows). When present this will instruct ovn-sbctl
to also pretty print the contents of the above mentioned tables for
the given datapath (or all datapaths if none is specified).

Combined with the "--ovs" and "--stats" arguments it allows the
user to have a more complete view of how forwarding in the
logical network is translated to forwarding in OVS.

A usage example:
$ ovn-sbctl --ovs --vflows lflow-list
[...]
Port Bindings:
  datapath="ls", logical_port=vm2, tunnel_key=4
    cookie=0xeaee10a0, duration=2122.949s, table=65, n_packets=0,
      n_bytes=0, priority=100,reg15=0x4,metadata=0x2 actions=output:2
  datapath="rtr", logical_port=rtr-ls, tunnel_key=1
    cookie=0x640b6337, duration=2122.945s, table=65, n_packets=0,
      n_bytes=0, priority=100,reg15=0x1,metadata=0x1
      actions=clone(ct_clear,set_field:0->reg11,set_field:0->reg12,
        set_field:0->reg13,set_field:0x4->reg11,set_field:0x3->reg12,
        set_field:0x2->metadata,set_field:0x3->reg14,set_field:0->reg10,
        set_field:0->reg15,set_field:0->reg0,set_field:0->reg1,
        set_field:0->reg2,set_field:0->reg3,set_field:0->reg4,
        set_field:0->reg5,set_field:0->reg6,set_field:0->reg7,
        set_field:0->reg8,set_field:0->reg9,set_field:0->in_port,
        resubmit(,8))

MAC Bindings:
  datapath="rtr", logical_port=rtr-ls, ip=10::254, mac=00:00:00:00:00:01
    cookie=0x1c2b771d, duration=2122.946s, table=66, n_packets=0,
      n_bytes=0,priority=100,reg0=0x100000,reg1=0,reg2=0,reg3=0x254,
      reg15=0x1,metadata=0x1
      actions=set_field:00:00:00:00:00:01->eth_dst
    cookie=0x1c2b771d, duration=2122.945s, table=67, n_packets=0,
      n_bytes=0,priority=100,icmp6,reg0=0x100000,reg1=0,reg2=0,
      reg3=0x254,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:00:01,icmp_code=0
      actions=load:0x1->NXM_NX_REG10[6]

MC Groups:
  datapath="ls", name=_MC_flood, tunnel_key=32768, ports=(vm1, ls-rtr, vm2)
    cookie=0x4a196492, duration=2122.949s, table=33, n_packets=0,
      n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
      actions=set_field:0x1->reg13,set_field:0x2->reg15,resubmit(,34),
      set_field:0xc->reg13,set_field:0x4->reg15,resubmit(,34),
      set_field:0x8000->reg15
    cookie=0x4a196492, duration=2122.947s, table=32, n_packets=0,
      n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
      actions=set_field:0x3->reg15,resubmit(,34),set_field:0x8000->reg15,
      resubmit(,33)
[..]

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 controller/lflow.c       |   6 +-
 controller/physical.c    |  58 +++++++++-----
 utilities/ovn-sbctl.8.in |   8 +-
 utilities/ovn-sbctl.c    | 196 +++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 230 insertions(+), 38 deletions(-)

Comments

Ben Pfaff Nov. 1, 2019, 5:43 p.m. UTC | #1
On Fri, Nov 01, 2019 at 01:05:15PM +0100, Dumitru Ceara wrote:
> Until now, when translating SB database contents to OpenFlow flows,
> ovn-controller stored (part of) the SB record UUID in the cookie field
> of the OpenFlow entry only when translating logical flows.
> 
> Extend this behavior to the following SB Database table records too:
> - Port_Binding
> - Mac_Binding
> - Multicast_Group
> - Chassis
> 
> This makes debugging easier by allowing the user to trace back the
> original SB entry that generated the OpenFlow entry. Also, the
> "ovn-sbctl lflow-list" command now supports an additional "--vflows"
> argument (virtual flows). When present this will instruct ovn-sbctl
> to also pretty print the contents of the above mentioned tables for
> the given datapath (or all datapaths if none is specified).

This is smart and I fully endorse the idea.

I haven't properly reviewed the patch, so the following is for the idea:
Acked-by: Ben Pfaff <blp@ovn.org>
Mark Michelson Nov. 1, 2019, 9:09 p.m. UTC | #2
Great addition, Dumitru!

Acked-by: Mark Michelson <mmichels@redhat.com>

On 11/1/19 8:05 AM, Dumitru Ceara wrote:
> Until now, when translating SB database contents to OpenFlow flows,
> ovn-controller stored (part of) the SB record UUID in the cookie field
> of the OpenFlow entry only when translating logical flows.
> 
> Extend this behavior to the following SB Database table records too:
> - Port_Binding
> - Mac_Binding
> - Multicast_Group
> - Chassis
> 
> This makes debugging easier by allowing the user to trace back the
> original SB entry that generated the OpenFlow entry. Also, the
> "ovn-sbctl lflow-list" command now supports an additional "--vflows"
> argument (virtual flows). When present this will instruct ovn-sbctl
> to also pretty print the contents of the above mentioned tables for
> the given datapath (or all datapaths if none is specified).
> 
> Combined with the "--ovs" and "--stats" arguments it allows the
> user to have a more complete view of how forwarding in the
> logical network is translated to forwarding in OVS.
> 
> A usage example:
> $ ovn-sbctl --ovs --vflows lflow-list
> [...]
> Port Bindings:
>    datapath="ls", logical_port=vm2, tunnel_key=4
>      cookie=0xeaee10a0, duration=2122.949s, table=65, n_packets=0,
>        n_bytes=0, priority=100,reg15=0x4,metadata=0x2 actions=output:2
>    datapath="rtr", logical_port=rtr-ls, tunnel_key=1
>      cookie=0x640b6337, duration=2122.945s, table=65, n_packets=0,
>        n_bytes=0, priority=100,reg15=0x1,metadata=0x1
>        actions=clone(ct_clear,set_field:0->reg11,set_field:0->reg12,
>          set_field:0->reg13,set_field:0x4->reg11,set_field:0x3->reg12,
>          set_field:0x2->metadata,set_field:0x3->reg14,set_field:0->reg10,
>          set_field:0->reg15,set_field:0->reg0,set_field:0->reg1,
>          set_field:0->reg2,set_field:0->reg3,set_field:0->reg4,
>          set_field:0->reg5,set_field:0->reg6,set_field:0->reg7,
>          set_field:0->reg8,set_field:0->reg9,set_field:0->in_port,
>          resubmit(,8))
> 
> MAC Bindings:
>    datapath="rtr", logical_port=rtr-ls, ip=10::254, mac=00:00:00:00:00:01
>      cookie=0x1c2b771d, duration=2122.946s, table=66, n_packets=0,
>        n_bytes=0,priority=100,reg0=0x100000,reg1=0,reg2=0,reg3=0x254,
>        reg15=0x1,metadata=0x1
>        actions=set_field:00:00:00:00:00:01->eth_dst
>      cookie=0x1c2b771d, duration=2122.945s, table=67, n_packets=0,
>        n_bytes=0,priority=100,icmp6,reg0=0x100000,reg1=0,reg2=0,
>        reg3=0x254,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:00:01,icmp_code=0
>        actions=load:0x1->NXM_NX_REG10[6]
> 
> MC Groups:
>    datapath="ls", name=_MC_flood, tunnel_key=32768, ports=(vm1, ls-rtr, vm2)
>      cookie=0x4a196492, duration=2122.949s, table=33, n_packets=0,
>        n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
>        actions=set_field:0x1->reg13,set_field:0x2->reg15,resubmit(,34),
>        set_field:0xc->reg13,set_field:0x4->reg15,resubmit(,34),
>        set_field:0x8000->reg15
>      cookie=0x4a196492, duration=2122.947s, table=32, n_packets=0,
>        n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
>        actions=set_field:0x3->reg15,resubmit(,34),set_field:0x8000->reg15,
>        resubmit(,33)
> [..]
> 
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> ---
>   controller/lflow.c       |   6 +-
>   controller/physical.c    |  58 +++++++++-----
>   utilities/ovn-sbctl.8.in |   8 +-
>   utilities/ovn-sbctl.c    | 196 +++++++++++++++++++++++++++++++++++++++++++----
>   4 files changed, 230 insertions(+), 38 deletions(-)
> 
> diff --git a/controller/lflow.c b/controller/lflow.c
> index f34abce..36150bd 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -817,7 +817,8 @@ consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>       uint64_t stub[1024 / 8];
>       struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
>       put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
> -    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, 0, &get_arp_match,
> +    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
> +                    b->header_.uuid.parts[0], &get_arp_match,
>                       &ofpacts, &b->header_.uuid);
>   
>       ofpbuf_clear(&ofpacts);
> @@ -825,7 +826,8 @@ consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>       put_load(&value, sizeof value, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1,
>                &ofpacts);
>       match_set_dl_src(&lookup_arp_match, mac);
> -    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100, 0, &lookup_arp_match,
> +    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100,
> +                    b->header_.uuid.parts[0], &lookup_arp_match,
>                       &ofpacts, &b->header_.uuid);
>   
>       ofpbuf_uninit(&ofpacts);
> diff --git a/controller/physical.c b/controller/physical.c
> index 6e606d3..500d419 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -298,7 +298,8 @@ put_remote_port_redirect_bridged(const struct
>                                &value, NULL);
>   
>           put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p);
> -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> +                        binding->header_.uuid.parts[0],
>                           match, ofpacts_p, &binding->header_.uuid);
>   
>   }
> @@ -393,7 +394,8 @@ put_remote_port_redirect_overlay(const struct
>           bundle->fields = NX_HASH_FIELDS_ETH_SRC;
>           ofpact_finish_BUNDLE(ofpacts_p, &bundle);
>       }
> -    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
> +    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> +                    binding->header_.uuid.parts[0],
>                       match, ofpacts_p, &binding->header_.uuid);
>   }
>   
> @@ -406,6 +408,7 @@ struct remote_chassis_mac {
>       struct hmap_node hmap_node;
>       char *chassis_mac;
>       char *chassis_id;
> +    uint32_t chassis_sb_cookie;
>   };
>   
>   static void
> @@ -446,6 +449,8 @@ populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
>                           hash_string(chassis_mac_bridge, 0));
>               remote_chassis_mac->chassis_mac = xstrdup(chassis_mac_str);
>               remote_chassis_mac->chassis_id = xstrdup(chassis->name);
> +            remote_chassis_mac->chassis_sb_cookie =
> +                chassis->header_.uuid.parts[0];
>           }
>           free(tokstr);
>       }
> @@ -496,7 +501,8 @@ put_chassis_mac_conj_id_flow(const struct sbrec_chassis_table *chassis_table,
>           conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
>           conj->n_clauses = 2;
>           conj->clause = 0;
> -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> +                        mac->chassis_sb_cookie,
>                           &match, ofpacts_p, hc_uuid);
>       }
>   
> @@ -568,8 +574,9 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
>   
>           /* Resubmit to first logical ingress pipeline table. */
>           put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> -                        180, 0, &match, ofpacts_p, hc_uuid);
> +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> +                        rport_binding->header_.uuid.parts[0],
> +                        &match, ofpacts_p, hc_uuid);
>   
>           /* Provide second search criteria, i.e localnet port's
>            * vlan ID for conjunction flow */
> @@ -587,8 +594,9 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
>           conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
>           conj->n_clauses = 2;
>           conj->clause = 1;
> -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0, &match,
> -                        ofpacts_p, hc_uuid);
> +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> +                        rport_binding->header_.uuid.parts[0],
> +                        &match, ofpacts_p, hc_uuid);
>       }
>   }
>   
> @@ -687,7 +695,8 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index
>   
>           ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
>   
> -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150,
> +                        localnet_port->header_.uuid.parts[0],
>                           &match, ofpacts_p, &localnet_port->header_.uuid);
>       }
>   }
> @@ -902,7 +911,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>           ofpacts_p->header = clone;
>           ofpact_finish_CLONE(ofpacts_p, &clone);
>   
> -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> +                        binding->header_.uuid.parts[0],
>                           &match, ofpacts_p, &binding->header_.uuid);
>           return;
>       }
> @@ -971,7 +981,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>               put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
>           }
>   
> -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> +                        binding->header_.uuid.parts[0],
>                           &match, ofpacts_p, &binding->header_.uuid);
>   
>           goto out;
> @@ -1124,8 +1135,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>           /* Resubmit to first logical ingress pipeline table. */
>           put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
>           ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> -                        tag ? 150 : 100, 0, &match, ofpacts_p,
> -                        &binding->header_.uuid);
> +                        tag ? 150 : 100, binding->header_.uuid.parts[0],
> +                        &match, ofpacts_p, &binding->header_.uuid);
>   
>           if (!tag && (!strcmp(binding->type, "localnet")
>                        || !strcmp(binding->type, "l2gateway"))) {
> @@ -1135,7 +1146,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>                * action. */
>               ofpbuf_pull(ofpacts_p, ofpacts_orig_size);
>               match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
> -            ofctrl_add_flow(flow_table, 0, 100, 0, &match, ofpacts_p,
> +            ofctrl_add_flow(flow_table, 0, 100,
> +                            binding->header_.uuid.parts[0], &match, ofpacts_p,
>                               &binding->header_.uuid);
>           }
>   
> @@ -1168,7 +1180,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>                * switch will also contain the tag. */
>               ofpact_put_STRIP_VLAN(ofpacts_p);
>           }
> -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> +                        binding->header_.uuid.parts[0],
>                           &match, ofpacts_p, &binding->header_.uuid);
>   
>           if (!strcmp(binding->type, "localnet")) {
> @@ -1199,7 +1212,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>   
>           /* Resubmit to table 33. */
>           put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
> -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> +                        binding->header_.uuid.parts[0],
>                           &match, ofpacts_p, &binding->header_.uuid);
>       } else {
>   
> @@ -1322,7 +1336,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
>            * group as the logical output port. */
>           put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
>   
> -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> +                        mc->header_.uuid.parts[0],
>                           &match, &ofpacts, &mc->header_.uuid);
>       }
>   
> @@ -1360,7 +1375,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
>               if (local_ports) {
>                   put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts);
>               }
> -            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
> +            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> +                            mc->header_.uuid.parts[0],
>                               &match, &remote_ofpacts, &mc->header_.uuid);
>           }
>       }
> @@ -1672,8 +1688,9 @@ physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>               put_load(1, MFF_LOG_FLAGS, MLF_RCV_FROM_VXLAN_BIT, 1, &ofpacts);
>               put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
>   
> -            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, 0, &match,
> -                            &ofpacts, hc_uuid);
> +            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100,
> +                            binding->header_.uuid.parts[0],
> +                            &match, &ofpacts, hc_uuid);
>           }
>       }
>   
> @@ -1730,7 +1747,8 @@ physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>           if (pb && !strcmp(pb->type, "localport")) {
>               match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, pb->tunnel_key);
>               match_set_metadata(&match, htonll(pb->datapath->tunnel_key));
> -            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, 0,
> +            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150,
> +                            pb->header_.uuid.parts[0],
>                               &match, &ofpacts, hc_uuid);
>           }
>       }
> diff --git a/utilities/ovn-sbctl.8.in b/utilities/ovn-sbctl.8.in
> index 644f944..d9fc3a5 100644
> --- a/utilities/ovn-sbctl.8.in
> +++ b/utilities/ovn-sbctl.8.in
> @@ -167,7 +167,7 @@ to unbind logical port that is not bound has no effect.
>   .
>   .SS "Logical Flow Commands"
>   .
> -.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
> +.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] [\fB\-\-vflows\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
>   List logical flows.  If \fIlogical-datapath\fR is specified, only list
>   flows for that logical datapath.  The \fIlogical-datapath\fR may be
>   given as a UUID or as a datapath name (reporting an error if multiple
> @@ -197,6 +197,12 @@ for more information about the OpenFlow flow output.
>   By default, OpenFlow flow output includes only match and actions.  Add
>   \fB\-\-stats\fR to include all OpenFlow information, such as packet
>   and byte counters, duration, and timeouts.
> +.IP
> +If \fB\-\-vflows\fR is included, other southbound database records directly
> +used for generating OpenFlow flows are also listed. This includes:
> +\fIport-bindings\fR, \fImac-bindings\fR, \fImulticast-groups\fR,
> +\fIchassis\fR.  The \fB\-\-ovs\fR and \fB\-\-stats\fR can also be used in
> +conjunction with \fB\-\-vflows\fR.
>   .
>   .IP "[\fB\-\-uuid\fR] \fBdump\-flows\fR [\fIlogical-datapath\fR]"
>   Alias for \fBlflow\-list\fB.
> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> index ffcaee2..4facdb1 100644
> --- a/utilities/ovn-sbctl.c
> +++ b/utilities/ovn-sbctl.c
> @@ -516,7 +516,9 @@ pre_get_info(struct ctl_context *ctx)
>       ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip);
>   
>       ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key);
>       ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath);
>   
>       ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_logical_datapath);
>       ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_pipeline);
> @@ -530,6 +532,16 @@ pre_get_info(struct ctl_context *ctx)
>   
>       ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_datapath);
>       ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_seq_no);
> +
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_name);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_datapath);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_tunnel_key);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_ports);
> +
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_datapath);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_logical_port);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_ip);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
>   }
>   
>   static struct cmd_show_table cmd_show_tables[] = {
> @@ -844,6 +856,161 @@ sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats)
>   }
>   
>   static void
> +print_datapath_name(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) {
> +        printf("\"%s\" aka \"%s\"", name, name2);
> +    } else if (name || name2) {
> +        printf("\"%s\"", name ? name : name2);
> +    }
> +}
> +
> +static void
> +print_vflow_datapath_name(const struct sbrec_datapath_binding *dp,
> +                          bool do_print)
> +{
> +    if (!do_print) {
> +        return;
> +    }
> +    printf("datapath=");
> +    print_datapath_name(dp);
> +    printf(", ");
> +}
> +
> +static void
> +print_uuid_part(const struct uuid *uuid, bool do_print)
> +{
> +    if (!do_print) {
> +        return;
> +    }
> +    printf("uuid=0x%08"PRIx32", ", uuid->parts[0]);
> +}
> +
> +static void
> +cmd_lflow_list_port_bindings(struct ctl_context *ctx, struct vconn *vconn,
> +                             const struct sbrec_datapath_binding *datapath,
> +                             bool stats, bool print_uuid)
> +{
> +    const struct sbrec_port_binding *pb;
> +    const struct sbrec_port_binding *pb_prev = NULL;
> +    SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) {
> +
> +        if (datapath && pb->datapath != datapath) {
> +            continue;
> +        }
> +
> +        if (!pb_prev) {
> +            printf("\nPort Bindings:\n");
> +        }
> +
> +        printf("  ");
> +        print_uuid_part(&pb->header_.uuid, print_uuid);
> +        print_vflow_datapath_name(pb->datapath, !datapath);
> +        printf("logical_port=%s, tunnel_key=%-5"PRId64"\n",
> +               pb->logical_port, pb->tunnel_key);
> +        if (vconn) {
> +            sbctl_dump_openflow(vconn, &pb->header_.uuid, stats);
> +        }
> +
> +        pb_prev = pb;
> +    }
> +}
> +
> +static void
> +cmd_lflow_list_mac_bindings(struct ctl_context *ctx, struct vconn *vconn,
> +                            const struct sbrec_datapath_binding *datapath,
> +                            bool stats, bool print_uuid)
> +{
> +    const struct sbrec_mac_binding *mb;
> +    const struct sbrec_mac_binding *mb_prev = NULL;
> +    SBREC_MAC_BINDING_FOR_EACH (mb, ctx->idl) {
> +        if (datapath && mb->datapath != datapath) {
> +            continue;
> +        }
> +
> +        if (!mb_prev) {
> +            printf("\nMAC Bindings:\n");
> +        }
> +
> +        printf("  ");
> +        print_uuid_part(&mb->header_.uuid, print_uuid);
> +        print_vflow_datapath_name(mb->datapath, !datapath);
> +
> +        printf("logical_port=%s, ip=%s, mac=%s\n",
> +               mb->logical_port, mb->ip, mb->mac);
> +        if (vconn) {
> +            sbctl_dump_openflow(vconn, &mb->header_.uuid, stats);
> +        }
> +
> +        mb_prev = mb;
> +    }
> +}
> +
> +static void
> +cmd_lflow_list_mc_groups(struct ctl_context *ctx, struct vconn *vconn,
> +                         const struct sbrec_datapath_binding *datapath,
> +                         bool stats, bool print_uuid)
> +{
> +    const struct sbrec_multicast_group *mc;
> +    const struct sbrec_multicast_group *mc_prev = NULL;
> +    SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->idl) {
> +        if (datapath && mc->datapath != datapath) {
> +            continue;
> +        }
> +
> +        if (!mc_prev) {
> +            printf("\nMC Groups:\n");
> +        }
> +
> +        printf("  ");
> +        print_uuid_part(&mc->header_.uuid, print_uuid);
> +        print_vflow_datapath_name(mc->datapath, !datapath);
> +
> +        printf("name=%s, tunnel_key=%-5"PRId64", ports=(",
> +               mc->name, mc->tunnel_key);
> +        for (size_t i = 0; i < mc->n_ports; i++) {
> +            printf("%s", mc->ports[i]->logical_port);
> +            if (i != mc->n_ports - 1) {
> +                printf(", ");
> +            }
> +        }
> +        printf(")\n");
> +
> +        if (vconn) {
> +            sbctl_dump_openflow(vconn, &mc->header_.uuid, stats);
> +        }
> +
> +        mc_prev = mc;
> +    }
> +}
> +
> +static void
> +cmd_lflow_list_chassis(struct ctl_context *ctx, struct vconn *vconn,
> +                       bool stats, bool print_uuid)
> +{
> +    const struct sbrec_chassis *chassis;
> +    const struct sbrec_chassis *chassis_prev = NULL;
> +    SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) {
> +        if (!chassis_prev) {
> +            printf("\nChassis:\n");
> +        }
> +
> +        printf("  ");
> +        print_uuid_part(&chassis->header_.uuid, print_uuid);
> +
> +        printf("name=%s\n", chassis->name);
> +        if (vconn) {
> +            sbctl_dump_openflow(vconn, &chassis->header_.uuid, stats);
> +        }
> +
> +        chassis_prev = chassis;
> +    }
> +}
> +
> +static void
>   cmd_lflow_list(struct ctl_context *ctx)
>   {
>       const struct sbrec_datapath_binding *datapath = NULL;
> @@ -925,16 +1092,8 @@ cmd_lflow_list(struct ctl_context *ctx)
>           if (!prev
>               || prev->logical_datapath != lflow->logical_datapath
>               || strcmp(prev->pipeline, lflow->pipeline)) {
> -            printf("Datapath:");
> -
> -            const struct smap *ids = &lflow->logical_datapath->external_ids;
> -            const char *name = smap_get(ids, "name");
> -            const char *name2 = smap_get(ids, "name2");
> -            if (name && name2) {
> -                printf(" \"%s\" aka \"%s\"", name, name2);
> -            } else if (name || name2) {
> -                printf(" \"%s\"", name ? name : name2);
> -            }
> +            printf("Datapath: ");
> +            print_datapath_name(lflow->logical_datapath);
>               printf(" ("UUID_FMT")  Pipeline: %s\n",
>                      UUID_ARGS(&lflow->logical_datapath->header_.uuid),
>                      lflow->pipeline);
> @@ -942,9 +1101,7 @@ cmd_lflow_list(struct ctl_context *ctx)
>   
>           /* Print the flow. */
>           printf("  ");
> -        if (print_uuid) {
> -            printf("uuid=0x%08"PRIx32", ", lflow->header_.uuid.parts[0]);
> -        }
> +        print_uuid_part(&lflow->header_.uuid, print_uuid);
>           printf("table=%-2"PRId64"(%-19s), priority=%-5"PRId64
>                  ", match=(%s), action=(%s)\n",
>                  lflow->table_id,
> @@ -956,6 +1113,14 @@ cmd_lflow_list(struct ctl_context *ctx)
>           prev = lflow;
>       }
>   
> +    bool vflows = shash_find(&ctx->options, "--vflows") != NULL;
> +    if (vflows) {
> +        cmd_lflow_list_port_bindings(ctx, vconn, datapath, stats, print_uuid);
> +        cmd_lflow_list_mac_bindings(ctx, vconn, datapath, stats, print_uuid);
> +        cmd_lflow_list_mc_groups(ctx, vconn, datapath, stats, print_uuid);
> +        cmd_lflow_list_chassis(ctx, vconn, stats, print_uuid);
> +    }
> +
>       vconn_close(vconn);
>       free(lflows);
>   }
> @@ -1509,10 +1674,11 @@ static const struct ctl_command_syntax sbctl_commands[] = {
>       /* Logical flow commands */
>       {"lflow-list", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
>        pre_get_info, cmd_lflow_list, NULL,
> -     "--uuid,--ovs?,--stats", RO},
> +     "--uuid,--ovs?,--stats,--vflows?", RO},
>       {"dump-flows", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
>        pre_get_info, cmd_lflow_list, NULL,
> -     "--uuid,--ovs?,--stats", RO}, /* Friendly alias for lflow-list */
> +     "--uuid,--ovs?,--stats,--vflows?",
> +     RO}, /* Friendly alias for lflow-list */
>   
>       /* IP multicast commands. */
>       {"ip-multicast-flush", 0, 1, "SWITCH",
>
Numan Siddique Nov. 4, 2019, 11:49 a.m. UTC | #3
Thanks for the patch. It would be really helpful in debugging.

I applied this to master.

Thanks
Numan

On Sat, Nov 2, 2019 at 2:39 AM Mark Michelson <mmichels@redhat.com> wrote:
>
> Great addition, Dumitru!
>
> Acked-by: Mark Michelson <mmichels@redhat.com>
>
> On 11/1/19 8:05 AM, Dumitru Ceara wrote:
> > Until now, when translating SB database contents to OpenFlow flows,
> > ovn-controller stored (part of) the SB record UUID in the cookie field
> > of the OpenFlow entry only when translating logical flows.
> >
> > Extend this behavior to the following SB Database table records too:
> > - Port_Binding
> > - Mac_Binding
> > - Multicast_Group
> > - Chassis
> >
> > This makes debugging easier by allowing the user to trace back the
> > original SB entry that generated the OpenFlow entry. Also, the
> > "ovn-sbctl lflow-list" command now supports an additional "--vflows"
> > argument (virtual flows). When present this will instruct ovn-sbctl
> > to also pretty print the contents of the above mentioned tables for
> > the given datapath (or all datapaths if none is specified).
> >
> > Combined with the "--ovs" and "--stats" arguments it allows the
> > user to have a more complete view of how forwarding in the
> > logical network is translated to forwarding in OVS.
> >
> > A usage example:
> > $ ovn-sbctl --ovs --vflows lflow-list
> > [...]
> > Port Bindings:
> >    datapath="ls", logical_port=vm2, tunnel_key=4
> >      cookie=0xeaee10a0, duration=2122.949s, table=65, n_packets=0,
> >        n_bytes=0, priority=100,reg15=0x4,metadata=0x2 actions=output:2
> >    datapath="rtr", logical_port=rtr-ls, tunnel_key=1
> >      cookie=0x640b6337, duration=2122.945s, table=65, n_packets=0,
> >        n_bytes=0, priority=100,reg15=0x1,metadata=0x1
> >        actions=clone(ct_clear,set_field:0->reg11,set_field:0->reg12,
> >          set_field:0->reg13,set_field:0x4->reg11,set_field:0x3->reg12,
> >          set_field:0x2->metadata,set_field:0x3->reg14,set_field:0->reg10,
> >          set_field:0->reg15,set_field:0->reg0,set_field:0->reg1,
> >          set_field:0->reg2,set_field:0->reg3,set_field:0->reg4,
> >          set_field:0->reg5,set_field:0->reg6,set_field:0->reg7,
> >          set_field:0->reg8,set_field:0->reg9,set_field:0->in_port,
> >          resubmit(,8))
> >
> > MAC Bindings:
> >    datapath="rtr", logical_port=rtr-ls, ip=10::254, mac=00:00:00:00:00:01
> >      cookie=0x1c2b771d, duration=2122.946s, table=66, n_packets=0,
> >        n_bytes=0,priority=100,reg0=0x100000,reg1=0,reg2=0,reg3=0x254,
> >        reg15=0x1,metadata=0x1
> >        actions=set_field:00:00:00:00:00:01->eth_dst
> >      cookie=0x1c2b771d, duration=2122.945s, table=67, n_packets=0,
> >        n_bytes=0,priority=100,icmp6,reg0=0x100000,reg1=0,reg2=0,
> >        reg3=0x254,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:00:01,icmp_code=0
> >        actions=load:0x1->NXM_NX_REG10[6]
> >
> > MC Groups:
> >    datapath="ls", name=_MC_flood, tunnel_key=32768, ports=(vm1, ls-rtr, vm2)
> >      cookie=0x4a196492, duration=2122.949s, table=33, n_packets=0,
> >        n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
> >        actions=set_field:0x1->reg13,set_field:0x2->reg15,resubmit(,34),
> >        set_field:0xc->reg13,set_field:0x4->reg15,resubmit(,34),
> >        set_field:0x8000->reg15
> >      cookie=0x4a196492, duration=2122.947s, table=32, n_packets=0,
> >        n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
> >        actions=set_field:0x3->reg15,resubmit(,34),set_field:0x8000->reg15,
> >        resubmit(,33)
> > [..]
> >
> > Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> > ---
> >   controller/lflow.c       |   6 +-
> >   controller/physical.c    |  58 +++++++++-----
> >   utilities/ovn-sbctl.8.in |   8 +-
> >   utilities/ovn-sbctl.c    | 196 +++++++++++++++++++++++++++++++++++++++++++----
> >   4 files changed, 230 insertions(+), 38 deletions(-)
> >
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index f34abce..36150bd 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -817,7 +817,8 @@ consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >       uint64_t stub[1024 / 8];
> >       struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
> >       put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
> > -    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, 0, &get_arp_match,
> > +    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
> > +                    b->header_.uuid.parts[0], &get_arp_match,
> >                       &ofpacts, &b->header_.uuid);
> >
> >       ofpbuf_clear(&ofpacts);
> > @@ -825,7 +826,8 @@ consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >       put_load(&value, sizeof value, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1,
> >                &ofpacts);
> >       match_set_dl_src(&lookup_arp_match, mac);
> > -    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100, 0, &lookup_arp_match,
> > +    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100,
> > +                    b->header_.uuid.parts[0], &lookup_arp_match,
> >                       &ofpacts, &b->header_.uuid);
> >
> >       ofpbuf_uninit(&ofpacts);
> > diff --git a/controller/physical.c b/controller/physical.c
> > index 6e606d3..500d419 100644
> > --- a/controller/physical.c
> > +++ b/controller/physical.c
> > @@ -298,7 +298,8 @@ put_remote_port_redirect_bridged(const struct
> >                                &value, NULL);
> >
> >           put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p);
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > +                        binding->header_.uuid.parts[0],
> >                           match, ofpacts_p, &binding->header_.uuid);
> >
> >   }
> > @@ -393,7 +394,8 @@ put_remote_port_redirect_overlay(const struct
> >           bundle->fields = NX_HASH_FIELDS_ETH_SRC;
> >           ofpact_finish_BUNDLE(ofpacts_p, &bundle);
> >       }
> > -    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
> > +    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> > +                    binding->header_.uuid.parts[0],
> >                       match, ofpacts_p, &binding->header_.uuid);
> >   }
> >
> > @@ -406,6 +408,7 @@ struct remote_chassis_mac {
> >       struct hmap_node hmap_node;
> >       char *chassis_mac;
> >       char *chassis_id;
> > +    uint32_t chassis_sb_cookie;
> >   };
> >
> >   static void
> > @@ -446,6 +449,8 @@ populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
> >                           hash_string(chassis_mac_bridge, 0));
> >               remote_chassis_mac->chassis_mac = xstrdup(chassis_mac_str);
> >               remote_chassis_mac->chassis_id = xstrdup(chassis->name);
> > +            remote_chassis_mac->chassis_sb_cookie =
> > +                chassis->header_.uuid.parts[0];
> >           }
> >           free(tokstr);
> >       }
> > @@ -496,7 +501,8 @@ put_chassis_mac_conj_id_flow(const struct sbrec_chassis_table *chassis_table,
> >           conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
> >           conj->n_clauses = 2;
> >           conj->clause = 0;
> > -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> > +                        mac->chassis_sb_cookie,
> >                           &match, ofpacts_p, hc_uuid);
> >       }
> >
> > @@ -568,8 +574,9 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
> >
> >           /* Resubmit to first logical ingress pipeline table. */
> >           put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> > -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> > -                        180, 0, &match, ofpacts_p, hc_uuid);
> > +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> > +                        rport_binding->header_.uuid.parts[0],
> > +                        &match, ofpacts_p, hc_uuid);
> >
> >           /* Provide second search criteria, i.e localnet port's
> >            * vlan ID for conjunction flow */
> > @@ -587,8 +594,9 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
> >           conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
> >           conj->n_clauses = 2;
> >           conj->clause = 1;
> > -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0, &match,
> > -                        ofpacts_p, hc_uuid);
> > +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> > +                        rport_binding->header_.uuid.parts[0],
> > +                        &match, ofpacts_p, hc_uuid);
> >       }
> >   }
> >
> > @@ -687,7 +695,8 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index
> >
> >           ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
> >
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150,
> > +                        localnet_port->header_.uuid.parts[0],
> >                           &match, ofpacts_p, &localnet_port->header_.uuid);
> >       }
> >   }
> > @@ -902,7 +911,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >           ofpacts_p->header = clone;
> >           ofpact_finish_CLONE(ofpacts_p, &clone);
> >
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> > +                        binding->header_.uuid.parts[0],
> >                           &match, ofpacts_p, &binding->header_.uuid);
> >           return;
> >       }
> > @@ -971,7 +981,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >               put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
> >           }
> >
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > +                        binding->header_.uuid.parts[0],
> >                           &match, ofpacts_p, &binding->header_.uuid);
> >
> >           goto out;
> > @@ -1124,8 +1135,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >           /* Resubmit to first logical ingress pipeline table. */
> >           put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> >           ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> > -                        tag ? 150 : 100, 0, &match, ofpacts_p,
> > -                        &binding->header_.uuid);
> > +                        tag ? 150 : 100, binding->header_.uuid.parts[0],
> > +                        &match, ofpacts_p, &binding->header_.uuid);
> >
> >           if (!tag && (!strcmp(binding->type, "localnet")
> >                        || !strcmp(binding->type, "l2gateway"))) {
> > @@ -1135,7 +1146,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >                * action. */
> >               ofpbuf_pull(ofpacts_p, ofpacts_orig_size);
> >               match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
> > -            ofctrl_add_flow(flow_table, 0, 100, 0, &match, ofpacts_p,
> > +            ofctrl_add_flow(flow_table, 0, 100,
> > +                            binding->header_.uuid.parts[0], &match, ofpacts_p,
> >                               &binding->header_.uuid);
> >           }
> >
> > @@ -1168,7 +1180,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >                * switch will also contain the tag. */
> >               ofpact_put_STRIP_VLAN(ofpacts_p);
> >           }
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> > +                        binding->header_.uuid.parts[0],
> >                           &match, ofpacts_p, &binding->header_.uuid);
> >
> >           if (!strcmp(binding->type, "localnet")) {
> > @@ -1199,7 +1212,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >
> >           /* Resubmit to table 33. */
> >           put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > +                        binding->header_.uuid.parts[0],
> >                           &match, ofpacts_p, &binding->header_.uuid);
> >       } else {
> >
> > @@ -1322,7 +1336,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
> >            * group as the logical output port. */
> >           put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
> >
> > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > +                        mc->header_.uuid.parts[0],
> >                           &match, &ofpacts, &mc->header_.uuid);
> >       }
> >
> > @@ -1360,7 +1375,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
> >               if (local_ports) {
> >                   put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts);
> >               }
> > -            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
> > +            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> > +                            mc->header_.uuid.parts[0],
> >                               &match, &remote_ofpacts, &mc->header_.uuid);
> >           }
> >       }
> > @@ -1672,8 +1688,9 @@ physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >               put_load(1, MFF_LOG_FLAGS, MLF_RCV_FROM_VXLAN_BIT, 1, &ofpacts);
> >               put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
> >
> > -            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, 0, &match,
> > -                            &ofpacts, hc_uuid);
> > +            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100,
> > +                            binding->header_.uuid.parts[0],
> > +                            &match, &ofpacts, hc_uuid);
> >           }
> >       }
> >
> > @@ -1730,7 +1747,8 @@ physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >           if (pb && !strcmp(pb->type, "localport")) {
> >               match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, pb->tunnel_key);
> >               match_set_metadata(&match, htonll(pb->datapath->tunnel_key));
> > -            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, 0,
> > +            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150,
> > +                            pb->header_.uuid.parts[0],
> >                               &match, &ofpacts, hc_uuid);
> >           }
> >       }
> > diff --git a/utilities/ovn-sbctl.8.in b/utilities/ovn-sbctl.8.in
> > index 644f944..d9fc3a5 100644
> > --- a/utilities/ovn-sbctl.8.in
> > +++ b/utilities/ovn-sbctl.8.in
> > @@ -167,7 +167,7 @@ to unbind logical port that is not bound has no effect.
> >   .
> >   .SS "Logical Flow Commands"
> >   .
> > -.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
> > +.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] [\fB\-\-vflows\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
> >   List logical flows.  If \fIlogical-datapath\fR is specified, only list
> >   flows for that logical datapath.  The \fIlogical-datapath\fR may be
> >   given as a UUID or as a datapath name (reporting an error if multiple
> > @@ -197,6 +197,12 @@ for more information about the OpenFlow flow output.
> >   By default, OpenFlow flow output includes only match and actions.  Add
> >   \fB\-\-stats\fR to include all OpenFlow information, such as packet
> >   and byte counters, duration, and timeouts.
> > +.IP
> > +If \fB\-\-vflows\fR is included, other southbound database records directly
> > +used for generating OpenFlow flows are also listed. This includes:
> > +\fIport-bindings\fR, \fImac-bindings\fR, \fImulticast-groups\fR,
> > +\fIchassis\fR.  The \fB\-\-ovs\fR and \fB\-\-stats\fR can also be used in
> > +conjunction with \fB\-\-vflows\fR.
> >   .
> >   .IP "[\fB\-\-uuid\fR] \fBdump\-flows\fR [\fIlogical-datapath\fR]"
> >   Alias for \fBlflow\-list\fB.
> > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> > index ffcaee2..4facdb1 100644
> > --- a/utilities/ovn-sbctl.c
> > +++ b/utilities/ovn-sbctl.c
> > @@ -516,7 +516,9 @@ pre_get_info(struct ctl_context *ctx)
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip);
> >
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key);
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath);
> >
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_logical_datapath);
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_pipeline);
> > @@ -530,6 +532,16 @@ pre_get_info(struct ctl_context *ctx)
> >
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_datapath);
> >       ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_seq_no);
> > +
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_name);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_datapath);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_tunnel_key);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_ports);
> > +
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_datapath);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_logical_port);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_ip);
> > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
> >   }
> >
> >   static struct cmd_show_table cmd_show_tables[] = {
> > @@ -844,6 +856,161 @@ sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats)
> >   }
> >
> >   static void
> > +print_datapath_name(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) {
> > +        printf("\"%s\" aka \"%s\"", name, name2);
> > +    } else if (name || name2) {
> > +        printf("\"%s\"", name ? name : name2);
> > +    }
> > +}
> > +
> > +static void
> > +print_vflow_datapath_name(const struct sbrec_datapath_binding *dp,
> > +                          bool do_print)
> > +{
> > +    if (!do_print) {
> > +        return;
> > +    }
> > +    printf("datapath=");
> > +    print_datapath_name(dp);
> > +    printf(", ");
> > +}
> > +
> > +static void
> > +print_uuid_part(const struct uuid *uuid, bool do_print)
> > +{
> > +    if (!do_print) {
> > +        return;
> > +    }
> > +    printf("uuid=0x%08"PRIx32", ", uuid->parts[0]);
> > +}
> > +
> > +static void
> > +cmd_lflow_list_port_bindings(struct ctl_context *ctx, struct vconn *vconn,
> > +                             const struct sbrec_datapath_binding *datapath,
> > +                             bool stats, bool print_uuid)
> > +{
> > +    const struct sbrec_port_binding *pb;
> > +    const struct sbrec_port_binding *pb_prev = NULL;
> > +    SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) {
> > +
> > +        if (datapath && pb->datapath != datapath) {
> > +            continue;
> > +        }
> > +
> > +        if (!pb_prev) {
> > +            printf("\nPort Bindings:\n");
> > +        }
> > +
> > +        printf("  ");
> > +        print_uuid_part(&pb->header_.uuid, print_uuid);
> > +        print_vflow_datapath_name(pb->datapath, !datapath);
> > +        printf("logical_port=%s, tunnel_key=%-5"PRId64"\n",
> > +               pb->logical_port, pb->tunnel_key);
> > +        if (vconn) {
> > +            sbctl_dump_openflow(vconn, &pb->header_.uuid, stats);
> > +        }
> > +
> > +        pb_prev = pb;
> > +    }
> > +}
> > +
> > +static void
> > +cmd_lflow_list_mac_bindings(struct ctl_context *ctx, struct vconn *vconn,
> > +                            const struct sbrec_datapath_binding *datapath,
> > +                            bool stats, bool print_uuid)
> > +{
> > +    const struct sbrec_mac_binding *mb;
> > +    const struct sbrec_mac_binding *mb_prev = NULL;
> > +    SBREC_MAC_BINDING_FOR_EACH (mb, ctx->idl) {
> > +        if (datapath && mb->datapath != datapath) {
> > +            continue;
> > +        }
> > +
> > +        if (!mb_prev) {
> > +            printf("\nMAC Bindings:\n");
> > +        }
> > +
> > +        printf("  ");
> > +        print_uuid_part(&mb->header_.uuid, print_uuid);
> > +        print_vflow_datapath_name(mb->datapath, !datapath);
> > +
> > +        printf("logical_port=%s, ip=%s, mac=%s\n",
> > +               mb->logical_port, mb->ip, mb->mac);
> > +        if (vconn) {
> > +            sbctl_dump_openflow(vconn, &mb->header_.uuid, stats);
> > +        }
> > +
> > +        mb_prev = mb;
> > +    }
> > +}
> > +
> > +static void
> > +cmd_lflow_list_mc_groups(struct ctl_context *ctx, struct vconn *vconn,
> > +                         const struct sbrec_datapath_binding *datapath,
> > +                         bool stats, bool print_uuid)
> > +{
> > +    const struct sbrec_multicast_group *mc;
> > +    const struct sbrec_multicast_group *mc_prev = NULL;
> > +    SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->idl) {
> > +        if (datapath && mc->datapath != datapath) {
> > +            continue;
> > +        }
> > +
> > +        if (!mc_prev) {
> > +            printf("\nMC Groups:\n");
> > +        }
> > +
> > +        printf("  ");
> > +        print_uuid_part(&mc->header_.uuid, print_uuid);
> > +        print_vflow_datapath_name(mc->datapath, !datapath);
> > +
> > +        printf("name=%s, tunnel_key=%-5"PRId64", ports=(",
> > +               mc->name, mc->tunnel_key);
> > +        for (size_t i = 0; i < mc->n_ports; i++) {
> > +            printf("%s", mc->ports[i]->logical_port);
> > +            if (i != mc->n_ports - 1) {
> > +                printf(", ");
> > +            }
> > +        }
> > +        printf(")\n");
> > +
> > +        if (vconn) {
> > +            sbctl_dump_openflow(vconn, &mc->header_.uuid, stats);
> > +        }
> > +
> > +        mc_prev = mc;
> > +    }
> > +}
> > +
> > +static void
> > +cmd_lflow_list_chassis(struct ctl_context *ctx, struct vconn *vconn,
> > +                       bool stats, bool print_uuid)
> > +{
> > +    const struct sbrec_chassis *chassis;
> > +    const struct sbrec_chassis *chassis_prev = NULL;
> > +    SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) {
> > +        if (!chassis_prev) {
> > +            printf("\nChassis:\n");
> > +        }
> > +
> > +        printf("  ");
> > +        print_uuid_part(&chassis->header_.uuid, print_uuid);
> > +
> > +        printf("name=%s\n", chassis->name);
> > +        if (vconn) {
> > +            sbctl_dump_openflow(vconn, &chassis->header_.uuid, stats);
> > +        }
> > +
> > +        chassis_prev = chassis;
> > +    }
> > +}
> > +
> > +static void
> >   cmd_lflow_list(struct ctl_context *ctx)
> >   {
> >       const struct sbrec_datapath_binding *datapath = NULL;
> > @@ -925,16 +1092,8 @@ cmd_lflow_list(struct ctl_context *ctx)
> >           if (!prev
> >               || prev->logical_datapath != lflow->logical_datapath
> >               || strcmp(prev->pipeline, lflow->pipeline)) {
> > -            printf("Datapath:");
> > -
> > -            const struct smap *ids = &lflow->logical_datapath->external_ids;
> > -            const char *name = smap_get(ids, "name");
> > -            const char *name2 = smap_get(ids, "name2");
> > -            if (name && name2) {
> > -                printf(" \"%s\" aka \"%s\"", name, name2);
> > -            } else if (name || name2) {
> > -                printf(" \"%s\"", name ? name : name2);
> > -            }
> > +            printf("Datapath: ");
> > +            print_datapath_name(lflow->logical_datapath);
> >               printf(" ("UUID_FMT")  Pipeline: %s\n",
> >                      UUID_ARGS(&lflow->logical_datapath->header_.uuid),
> >                      lflow->pipeline);
> > @@ -942,9 +1101,7 @@ cmd_lflow_list(struct ctl_context *ctx)
> >
> >           /* Print the flow. */
> >           printf("  ");
> > -        if (print_uuid) {
> > -            printf("uuid=0x%08"PRIx32", ", lflow->header_.uuid.parts[0]);
> > -        }
> > +        print_uuid_part(&lflow->header_.uuid, print_uuid);
> >           printf("table=%-2"PRId64"(%-19s), priority=%-5"PRId64
> >                  ", match=(%s), action=(%s)\n",
> >                  lflow->table_id,
> > @@ -956,6 +1113,14 @@ cmd_lflow_list(struct ctl_context *ctx)
> >           prev = lflow;
> >       }
> >
> > +    bool vflows = shash_find(&ctx->options, "--vflows") != NULL;
> > +    if (vflows) {
> > +        cmd_lflow_list_port_bindings(ctx, vconn, datapath, stats, print_uuid);
> > +        cmd_lflow_list_mac_bindings(ctx, vconn, datapath, stats, print_uuid);
> > +        cmd_lflow_list_mc_groups(ctx, vconn, datapath, stats, print_uuid);
> > +        cmd_lflow_list_chassis(ctx, vconn, stats, print_uuid);
> > +    }
> > +
> >       vconn_close(vconn);
> >       free(lflows);
> >   }
> > @@ -1509,10 +1674,11 @@ static const struct ctl_command_syntax sbctl_commands[] = {
> >       /* Logical flow commands */
> >       {"lflow-list", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
> >        pre_get_info, cmd_lflow_list, NULL,
> > -     "--uuid,--ovs?,--stats", RO},
> > +     "--uuid,--ovs?,--stats,--vflows?", RO},
> >       {"dump-flows", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
> >        pre_get_info, cmd_lflow_list, NULL,
> > -     "--uuid,--ovs?,--stats", RO}, /* Friendly alias for lflow-list */
> > +     "--uuid,--ovs?,--stats,--vflows?",
> > +     RO}, /* Friendly alias for lflow-list */
> >
> >       /* IP multicast commands. */
> >       {"ip-multicast-flush", 0, 1, "SWITCH",
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Dumitru Ceara Nov. 4, 2019, 11:54 a.m. UTC | #4
On Mon, Nov 4, 2019 at 12:49 PM Numan Siddique <numans@ovn.org> wrote:
>
> Thanks for the patch. It would be really helpful in debugging.
>
> I applied this to master.
>
> Thanks
> Numan

Thanks for the reviews and for merging the patch.

Regards,
Dumitru

>
> On Sat, Nov 2, 2019 at 2:39 AM Mark Michelson <mmichels@redhat.com> wrote:
> >
> > Great addition, Dumitru!
> >
> > Acked-by: Mark Michelson <mmichels@redhat.com>
> >
> > On 11/1/19 8:05 AM, Dumitru Ceara wrote:
> > > Until now, when translating SB database contents to OpenFlow flows,
> > > ovn-controller stored (part of) the SB record UUID in the cookie field
> > > of the OpenFlow entry only when translating logical flows.
> > >
> > > Extend this behavior to the following SB Database table records too:
> > > - Port_Binding
> > > - Mac_Binding
> > > - Multicast_Group
> > > - Chassis
> > >
> > > This makes debugging easier by allowing the user to trace back the
> > > original SB entry that generated the OpenFlow entry. Also, the
> > > "ovn-sbctl lflow-list" command now supports an additional "--vflows"
> > > argument (virtual flows). When present this will instruct ovn-sbctl
> > > to also pretty print the contents of the above mentioned tables for
> > > the given datapath (or all datapaths if none is specified).
> > >
> > > Combined with the "--ovs" and "--stats" arguments it allows the
> > > user to have a more complete view of how forwarding in the
> > > logical network is translated to forwarding in OVS.
> > >
> > > A usage example:
> > > $ ovn-sbctl --ovs --vflows lflow-list
> > > [...]
> > > Port Bindings:
> > >    datapath="ls", logical_port=vm2, tunnel_key=4
> > >      cookie=0xeaee10a0, duration=2122.949s, table=65, n_packets=0,
> > >        n_bytes=0, priority=100,reg15=0x4,metadata=0x2 actions=output:2
> > >    datapath="rtr", logical_port=rtr-ls, tunnel_key=1
> > >      cookie=0x640b6337, duration=2122.945s, table=65, n_packets=0,
> > >        n_bytes=0, priority=100,reg15=0x1,metadata=0x1
> > >        actions=clone(ct_clear,set_field:0->reg11,set_field:0->reg12,
> > >          set_field:0->reg13,set_field:0x4->reg11,set_field:0x3->reg12,
> > >          set_field:0x2->metadata,set_field:0x3->reg14,set_field:0->reg10,
> > >          set_field:0->reg15,set_field:0->reg0,set_field:0->reg1,
> > >          set_field:0->reg2,set_field:0->reg3,set_field:0->reg4,
> > >          set_field:0->reg5,set_field:0->reg6,set_field:0->reg7,
> > >          set_field:0->reg8,set_field:0->reg9,set_field:0->in_port,
> > >          resubmit(,8))
> > >
> > > MAC Bindings:
> > >    datapath="rtr", logical_port=rtr-ls, ip=10::254, mac=00:00:00:00:00:01
> > >      cookie=0x1c2b771d, duration=2122.946s, table=66, n_packets=0,
> > >        n_bytes=0,priority=100,reg0=0x100000,reg1=0,reg2=0,reg3=0x254,
> > >        reg15=0x1,metadata=0x1
> > >        actions=set_field:00:00:00:00:00:01->eth_dst
> > >      cookie=0x1c2b771d, duration=2122.945s, table=67, n_packets=0,
> > >        n_bytes=0,priority=100,icmp6,reg0=0x100000,reg1=0,reg2=0,
> > >        reg3=0x254,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:00:01,icmp_code=0
> > >        actions=load:0x1->NXM_NX_REG10[6]
> > >
> > > MC Groups:
> > >    datapath="ls", name=_MC_flood, tunnel_key=32768, ports=(vm1, ls-rtr, vm2)
> > >      cookie=0x4a196492, duration=2122.949s, table=33, n_packets=0,
> > >        n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
> > >        actions=set_field:0x1->reg13,set_field:0x2->reg15,resubmit(,34),
> > >        set_field:0xc->reg13,set_field:0x4->reg15,resubmit(,34),
> > >        set_field:0x8000->reg15
> > >      cookie=0x4a196492, duration=2122.947s, table=32, n_packets=0,
> > >        n_bytes=0, priority=100,reg15=0x8000,metadata=0x2
> > >        actions=set_field:0x3->reg15,resubmit(,34),set_field:0x8000->reg15,
> > >        resubmit(,33)
> > > [..]
> > >
> > > Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> > > ---
> > >   controller/lflow.c       |   6 +-
> > >   controller/physical.c    |  58 +++++++++-----
> > >   utilities/ovn-sbctl.8.in |   8 +-
> > >   utilities/ovn-sbctl.c    | 196 +++++++++++++++++++++++++++++++++++++++++++----
> > >   4 files changed, 230 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/controller/lflow.c b/controller/lflow.c
> > > index f34abce..36150bd 100644
> > > --- a/controller/lflow.c
> > > +++ b/controller/lflow.c
> > > @@ -817,7 +817,8 @@ consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >       uint64_t stub[1024 / 8];
> > >       struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
> > >       put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
> > > -    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, 0, &get_arp_match,
> > > +    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
> > > +                    b->header_.uuid.parts[0], &get_arp_match,
> > >                       &ofpacts, &b->header_.uuid);
> > >
> > >       ofpbuf_clear(&ofpacts);
> > > @@ -825,7 +826,8 @@ consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >       put_load(&value, sizeof value, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1,
> > >                &ofpacts);
> > >       match_set_dl_src(&lookup_arp_match, mac);
> > > -    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100, 0, &lookup_arp_match,
> > > +    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100,
> > > +                    b->header_.uuid.parts[0], &lookup_arp_match,
> > >                       &ofpacts, &b->header_.uuid);
> > >
> > >       ofpbuf_uninit(&ofpacts);
> > > diff --git a/controller/physical.c b/controller/physical.c
> > > index 6e606d3..500d419 100644
> > > --- a/controller/physical.c
> > > +++ b/controller/physical.c
> > > @@ -298,7 +298,8 @@ put_remote_port_redirect_bridged(const struct
> > >                                &value, NULL);
> > >
> > >           put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p);
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > > +                        binding->header_.uuid.parts[0],
> > >                           match, ofpacts_p, &binding->header_.uuid);
> > >
> > >   }
> > > @@ -393,7 +394,8 @@ put_remote_port_redirect_overlay(const struct
> > >           bundle->fields = NX_HASH_FIELDS_ETH_SRC;
> > >           ofpact_finish_BUNDLE(ofpacts_p, &bundle);
> > >       }
> > > -    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
> > > +    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> > > +                    binding->header_.uuid.parts[0],
> > >                       match, ofpacts_p, &binding->header_.uuid);
> > >   }
> > >
> > > @@ -406,6 +408,7 @@ struct remote_chassis_mac {
> > >       struct hmap_node hmap_node;
> > >       char *chassis_mac;
> > >       char *chassis_id;
> > > +    uint32_t chassis_sb_cookie;
> > >   };
> > >
> > >   static void
> > > @@ -446,6 +449,8 @@ populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
> > >                           hash_string(chassis_mac_bridge, 0));
> > >               remote_chassis_mac->chassis_mac = xstrdup(chassis_mac_str);
> > >               remote_chassis_mac->chassis_id = xstrdup(chassis->name);
> > > +            remote_chassis_mac->chassis_sb_cookie =
> > > +                chassis->header_.uuid.parts[0];
> > >           }
> > >           free(tokstr);
> > >       }
> > > @@ -496,7 +501,8 @@ put_chassis_mac_conj_id_flow(const struct sbrec_chassis_table *chassis_table,
> > >           conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
> > >           conj->n_clauses = 2;
> > >           conj->clause = 0;
> > > -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> > > +                        mac->chassis_sb_cookie,
> > >                           &match, ofpacts_p, hc_uuid);
> > >       }
> > >
> > > @@ -568,8 +574,9 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
> > >
> > >           /* Resubmit to first logical ingress pipeline table. */
> > >           put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> > > -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> > > -                        180, 0, &match, ofpacts_p, hc_uuid);
> > > +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> > > +                        rport_binding->header_.uuid.parts[0],
> > > +                        &match, ofpacts_p, hc_uuid);
> > >
> > >           /* Provide second search criteria, i.e localnet port's
> > >            * vlan ID for conjunction flow */
> > > @@ -587,8 +594,9 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
> > >           conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
> > >           conj->n_clauses = 2;
> > >           conj->clause = 1;
> > > -        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0, &match,
> > > -                        ofpacts_p, hc_uuid);
> > > +        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
> > > +                        rport_binding->header_.uuid.parts[0],
> > > +                        &match, ofpacts_p, hc_uuid);
> > >       }
> > >   }
> > >
> > > @@ -687,7 +695,8 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index
> > >
> > >           ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
> > >
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150,
> > > +                        localnet_port->header_.uuid.parts[0],
> > >                           &match, ofpacts_p, &localnet_port->header_.uuid);
> > >       }
> > >   }
> > > @@ -902,7 +911,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >           ofpacts_p->header = clone;
> > >           ofpact_finish_CLONE(ofpacts_p, &clone);
> > >
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> > > +                        binding->header_.uuid.parts[0],
> > >                           &match, ofpacts_p, &binding->header_.uuid);
> > >           return;
> > >       }
> > > @@ -971,7 +981,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >               put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
> > >           }
> > >
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > > +                        binding->header_.uuid.parts[0],
> > >                           &match, ofpacts_p, &binding->header_.uuid);
> > >
> > >           goto out;
> > > @@ -1124,8 +1135,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >           /* Resubmit to first logical ingress pipeline table. */
> > >           put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> > >           ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> > > -                        tag ? 150 : 100, 0, &match, ofpacts_p,
> > > -                        &binding->header_.uuid);
> > > +                        tag ? 150 : 100, binding->header_.uuid.parts[0],
> > > +                        &match, ofpacts_p, &binding->header_.uuid);
> > >
> > >           if (!tag && (!strcmp(binding->type, "localnet")
> > >                        || !strcmp(binding->type, "l2gateway"))) {
> > > @@ -1135,7 +1146,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >                * action. */
> > >               ofpbuf_pull(ofpacts_p, ofpacts_orig_size);
> > >               match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
> > > -            ofctrl_add_flow(flow_table, 0, 100, 0, &match, ofpacts_p,
> > > +            ofctrl_add_flow(flow_table, 0, 100,
> > > +                            binding->header_.uuid.parts[0], &match, ofpacts_p,
> > >                               &binding->header_.uuid);
> > >           }
> > >
> > > @@ -1168,7 +1180,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >                * switch will also contain the tag. */
> > >               ofpact_put_STRIP_VLAN(ofpacts_p);
> > >           }
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> > > +                        binding->header_.uuid.parts[0],
> > >                           &match, ofpacts_p, &binding->header_.uuid);
> > >
> > >           if (!strcmp(binding->type, "localnet")) {
> > > @@ -1199,7 +1212,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >
> > >           /* Resubmit to table 33. */
> > >           put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > > +                        binding->header_.uuid.parts[0],
> > >                           &match, ofpacts_p, &binding->header_.uuid);
> > >       } else {
> > >
> > > @@ -1322,7 +1336,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
> > >            * group as the logical output port. */
> > >           put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
> > >
> > > -        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> > > +        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
> > > +                        mc->header_.uuid.parts[0],
> > >                           &match, &ofpacts, &mc->header_.uuid);
> > >       }
> > >
> > > @@ -1360,7 +1375,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
> > >               if (local_ports) {
> > >                   put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts);
> > >               }
> > > -            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
> > > +            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> > > +                            mc->header_.uuid.parts[0],
> > >                               &match, &remote_ofpacts, &mc->header_.uuid);
> > >           }
> > >       }
> > > @@ -1672,8 +1688,9 @@ physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >               put_load(1, MFF_LOG_FLAGS, MLF_RCV_FROM_VXLAN_BIT, 1, &ofpacts);
> > >               put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
> > >
> > > -            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, 0, &match,
> > > -                            &ofpacts, hc_uuid);
> > > +            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100,
> > > +                            binding->header_.uuid.parts[0],
> > > +                            &match, &ofpacts, hc_uuid);
> > >           }
> > >       }
> > >
> > > @@ -1730,7 +1747,8 @@ physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > >           if (pb && !strcmp(pb->type, "localport")) {
> > >               match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, pb->tunnel_key);
> > >               match_set_metadata(&match, htonll(pb->datapath->tunnel_key));
> > > -            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, 0,
> > > +            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150,
> > > +                            pb->header_.uuid.parts[0],
> > >                               &match, &ofpacts, hc_uuid);
> > >           }
> > >       }
> > > diff --git a/utilities/ovn-sbctl.8.in b/utilities/ovn-sbctl.8.in
> > > index 644f944..d9fc3a5 100644
> > > --- a/utilities/ovn-sbctl.8.in
> > > +++ b/utilities/ovn-sbctl.8.in
> > > @@ -167,7 +167,7 @@ to unbind logical port that is not bound has no effect.
> > >   .
> > >   .SS "Logical Flow Commands"
> > >   .
> > > -.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
> > > +.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] [\fB\-\-vflows\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
> > >   List logical flows.  If \fIlogical-datapath\fR is specified, only list
> > >   flows for that logical datapath.  The \fIlogical-datapath\fR may be
> > >   given as a UUID or as a datapath name (reporting an error if multiple
> > > @@ -197,6 +197,12 @@ for more information about the OpenFlow flow output.
> > >   By default, OpenFlow flow output includes only match and actions.  Add
> > >   \fB\-\-stats\fR to include all OpenFlow information, such as packet
> > >   and byte counters, duration, and timeouts.
> > > +.IP
> > > +If \fB\-\-vflows\fR is included, other southbound database records directly
> > > +used for generating OpenFlow flows are also listed. This includes:
> > > +\fIport-bindings\fR, \fImac-bindings\fR, \fImulticast-groups\fR,
> > > +\fIchassis\fR.  The \fB\-\-ovs\fR and \fB\-\-stats\fR can also be used in
> > > +conjunction with \fB\-\-vflows\fR.
> > >   .
> > >   .IP "[\fB\-\-uuid\fR] \fBdump\-flows\fR [\fIlogical-datapath\fR]"
> > >   Alias for \fBlflow\-list\fB.
> > > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> > > index ffcaee2..4facdb1 100644
> > > --- a/utilities/ovn-sbctl.c
> > > +++ b/utilities/ovn-sbctl.c
> > > @@ -516,7 +516,9 @@ pre_get_info(struct ctl_context *ctx)
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip);
> > >
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key);
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath);
> > >
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_logical_datapath);
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_pipeline);
> > > @@ -530,6 +532,16 @@ pre_get_info(struct ctl_context *ctx)
> > >
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_datapath);
> > >       ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_seq_no);
> > > +
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_name);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_datapath);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_tunnel_key);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_ports);
> > > +
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_datapath);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_logical_port);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_ip);
> > > +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
> > >   }
> > >
> > >   static struct cmd_show_table cmd_show_tables[] = {
> > > @@ -844,6 +856,161 @@ sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats)
> > >   }
> > >
> > >   static void
> > > +print_datapath_name(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) {
> > > +        printf("\"%s\" aka \"%s\"", name, name2);
> > > +    } else if (name || name2) {
> > > +        printf("\"%s\"", name ? name : name2);
> > > +    }
> > > +}
> > > +
> > > +static void
> > > +print_vflow_datapath_name(const struct sbrec_datapath_binding *dp,
> > > +                          bool do_print)
> > > +{
> > > +    if (!do_print) {
> > > +        return;
> > > +    }
> > > +    printf("datapath=");
> > > +    print_datapath_name(dp);
> > > +    printf(", ");
> > > +}
> > > +
> > > +static void
> > > +print_uuid_part(const struct uuid *uuid, bool do_print)
> > > +{
> > > +    if (!do_print) {
> > > +        return;
> > > +    }
> > > +    printf("uuid=0x%08"PRIx32", ", uuid->parts[0]);
> > > +}
> > > +
> > > +static void
> > > +cmd_lflow_list_port_bindings(struct ctl_context *ctx, struct vconn *vconn,
> > > +                             const struct sbrec_datapath_binding *datapath,
> > > +                             bool stats, bool print_uuid)
> > > +{
> > > +    const struct sbrec_port_binding *pb;
> > > +    const struct sbrec_port_binding *pb_prev = NULL;
> > > +    SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) {
> > > +
> > > +        if (datapath && pb->datapath != datapath) {
> > > +            continue;
> > > +        }
> > > +
> > > +        if (!pb_prev) {
> > > +            printf("\nPort Bindings:\n");
> > > +        }
> > > +
> > > +        printf("  ");
> > > +        print_uuid_part(&pb->header_.uuid, print_uuid);
> > > +        print_vflow_datapath_name(pb->datapath, !datapath);
> > > +        printf("logical_port=%s, tunnel_key=%-5"PRId64"\n",
> > > +               pb->logical_port, pb->tunnel_key);
> > > +        if (vconn) {
> > > +            sbctl_dump_openflow(vconn, &pb->header_.uuid, stats);
> > > +        }
> > > +
> > > +        pb_prev = pb;
> > > +    }
> > > +}
> > > +
> > > +static void
> > > +cmd_lflow_list_mac_bindings(struct ctl_context *ctx, struct vconn *vconn,
> > > +                            const struct sbrec_datapath_binding *datapath,
> > > +                            bool stats, bool print_uuid)
> > > +{
> > > +    const struct sbrec_mac_binding *mb;
> > > +    const struct sbrec_mac_binding *mb_prev = NULL;
> > > +    SBREC_MAC_BINDING_FOR_EACH (mb, ctx->idl) {
> > > +        if (datapath && mb->datapath != datapath) {
> > > +            continue;
> > > +        }
> > > +
> > > +        if (!mb_prev) {
> > > +            printf("\nMAC Bindings:\n");
> > > +        }
> > > +
> > > +        printf("  ");
> > > +        print_uuid_part(&mb->header_.uuid, print_uuid);
> > > +        print_vflow_datapath_name(mb->datapath, !datapath);
> > > +
> > > +        printf("logical_port=%s, ip=%s, mac=%s\n",
> > > +               mb->logical_port, mb->ip, mb->mac);
> > > +        if (vconn) {
> > > +            sbctl_dump_openflow(vconn, &mb->header_.uuid, stats);
> > > +        }
> > > +
> > > +        mb_prev = mb;
> > > +    }
> > > +}
> > > +
> > > +static void
> > > +cmd_lflow_list_mc_groups(struct ctl_context *ctx, struct vconn *vconn,
> > > +                         const struct sbrec_datapath_binding *datapath,
> > > +                         bool stats, bool print_uuid)
> > > +{
> > > +    const struct sbrec_multicast_group *mc;
> > > +    const struct sbrec_multicast_group *mc_prev = NULL;
> > > +    SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->idl) {
> > > +        if (datapath && mc->datapath != datapath) {
> > > +            continue;
> > > +        }
> > > +
> > > +        if (!mc_prev) {
> > > +            printf("\nMC Groups:\n");
> > > +        }
> > > +
> > > +        printf("  ");
> > > +        print_uuid_part(&mc->header_.uuid, print_uuid);
> > > +        print_vflow_datapath_name(mc->datapath, !datapath);
> > > +
> > > +        printf("name=%s, tunnel_key=%-5"PRId64", ports=(",
> > > +               mc->name, mc->tunnel_key);
> > > +        for (size_t i = 0; i < mc->n_ports; i++) {
> > > +            printf("%s", mc->ports[i]->logical_port);
> > > +            if (i != mc->n_ports - 1) {
> > > +                printf(", ");
> > > +            }
> > > +        }
> > > +        printf(")\n");
> > > +
> > > +        if (vconn) {
> > > +            sbctl_dump_openflow(vconn, &mc->header_.uuid, stats);
> > > +        }
> > > +
> > > +        mc_prev = mc;
> > > +    }
> > > +}
> > > +
> > > +static void
> > > +cmd_lflow_list_chassis(struct ctl_context *ctx, struct vconn *vconn,
> > > +                       bool stats, bool print_uuid)
> > > +{
> > > +    const struct sbrec_chassis *chassis;
> > > +    const struct sbrec_chassis *chassis_prev = NULL;
> > > +    SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) {
> > > +        if (!chassis_prev) {
> > > +            printf("\nChassis:\n");
> > > +        }
> > > +
> > > +        printf("  ");
> > > +        print_uuid_part(&chassis->header_.uuid, print_uuid);
> > > +
> > > +        printf("name=%s\n", chassis->name);
> > > +        if (vconn) {
> > > +            sbctl_dump_openflow(vconn, &chassis->header_.uuid, stats);
> > > +        }
> > > +
> > > +        chassis_prev = chassis;
> > > +    }
> > > +}
> > > +
> > > +static void
> > >   cmd_lflow_list(struct ctl_context *ctx)
> > >   {
> > >       const struct sbrec_datapath_binding *datapath = NULL;
> > > @@ -925,16 +1092,8 @@ cmd_lflow_list(struct ctl_context *ctx)
> > >           if (!prev
> > >               || prev->logical_datapath != lflow->logical_datapath
> > >               || strcmp(prev->pipeline, lflow->pipeline)) {
> > > -            printf("Datapath:");
> > > -
> > > -            const struct smap *ids = &lflow->logical_datapath->external_ids;
> > > -            const char *name = smap_get(ids, "name");
> > > -            const char *name2 = smap_get(ids, "name2");
> > > -            if (name && name2) {
> > > -                printf(" \"%s\" aka \"%s\"", name, name2);
> > > -            } else if (name || name2) {
> > > -                printf(" \"%s\"", name ? name : name2);
> > > -            }
> > > +            printf("Datapath: ");
> > > +            print_datapath_name(lflow->logical_datapath);
> > >               printf(" ("UUID_FMT")  Pipeline: %s\n",
> > >                      UUID_ARGS(&lflow->logical_datapath->header_.uuid),
> > >                      lflow->pipeline);
> > > @@ -942,9 +1101,7 @@ cmd_lflow_list(struct ctl_context *ctx)
> > >
> > >           /* Print the flow. */
> > >           printf("  ");
> > > -        if (print_uuid) {
> > > -            printf("uuid=0x%08"PRIx32", ", lflow->header_.uuid.parts[0]);
> > > -        }
> > > +        print_uuid_part(&lflow->header_.uuid, print_uuid);
> > >           printf("table=%-2"PRId64"(%-19s), priority=%-5"PRId64
> > >                  ", match=(%s), action=(%s)\n",
> > >                  lflow->table_id,
> > > @@ -956,6 +1113,14 @@ cmd_lflow_list(struct ctl_context *ctx)
> > >           prev = lflow;
> > >       }
> > >
> > > +    bool vflows = shash_find(&ctx->options, "--vflows") != NULL;
> > > +    if (vflows) {
> > > +        cmd_lflow_list_port_bindings(ctx, vconn, datapath, stats, print_uuid);
> > > +        cmd_lflow_list_mac_bindings(ctx, vconn, datapath, stats, print_uuid);
> > > +        cmd_lflow_list_mc_groups(ctx, vconn, datapath, stats, print_uuid);
> > > +        cmd_lflow_list_chassis(ctx, vconn, stats, print_uuid);
> > > +    }
> > > +
> > >       vconn_close(vconn);
> > >       free(lflows);
> > >   }
> > > @@ -1509,10 +1674,11 @@ static const struct ctl_command_syntax sbctl_commands[] = {
> > >       /* Logical flow commands */
> > >       {"lflow-list", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
> > >        pre_get_info, cmd_lflow_list, NULL,
> > > -     "--uuid,--ovs?,--stats", RO},
> > > +     "--uuid,--ovs?,--stats,--vflows?", RO},
> > >       {"dump-flows", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
> > >        pre_get_info, cmd_lflow_list, NULL,
> > > -     "--uuid,--ovs?,--stats", RO}, /* Friendly alias for lflow-list */
> > > +     "--uuid,--ovs?,--stats,--vflows?",
> > > +     RO}, /* Friendly alias for lflow-list */
> > >
> > >       /* IP multicast commands. */
> > >       {"ip-multicast-flush", 0, 1, "SWITCH",
> > >
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/controller/lflow.c b/controller/lflow.c
index f34abce..36150bd 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -817,7 +817,8 @@  consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
     uint64_t stub[1024 / 8];
     struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
     put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
-    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, 0, &get_arp_match,
+    ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
+                    b->header_.uuid.parts[0], &get_arp_match,
                     &ofpacts, &b->header_.uuid);
 
     ofpbuf_clear(&ofpacts);
@@ -825,7 +826,8 @@  consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name,
     put_load(&value, sizeof value, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1,
              &ofpacts);
     match_set_dl_src(&lookup_arp_match, mac);
-    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100, 0, &lookup_arp_match,
+    ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, 100,
+                    b->header_.uuid.parts[0], &lookup_arp_match,
                     &ofpacts, &b->header_.uuid);
 
     ofpbuf_uninit(&ofpacts);
diff --git a/controller/physical.c b/controller/physical.c
index 6e606d3..500d419 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -298,7 +298,8 @@  put_remote_port_redirect_bridged(const struct
                              &value, NULL);
 
         put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p);
-        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
+                        binding->header_.uuid.parts[0],
                         match, ofpacts_p, &binding->header_.uuid);
 
 }
@@ -393,7 +394,8 @@  put_remote_port_redirect_overlay(const struct
         bundle->fields = NX_HASH_FIELDS_ETH_SRC;
         ofpact_finish_BUNDLE(ofpacts_p, &bundle);
     }
-    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
+    ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
+                    binding->header_.uuid.parts[0],
                     match, ofpacts_p, &binding->header_.uuid);
 }
 
@@ -406,6 +408,7 @@  struct remote_chassis_mac {
     struct hmap_node hmap_node;
     char *chassis_mac;
     char *chassis_id;
+    uint32_t chassis_sb_cookie;
 };
 
 static void
@@ -446,6 +449,8 @@  populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
                         hash_string(chassis_mac_bridge, 0));
             remote_chassis_mac->chassis_mac = xstrdup(chassis_mac_str);
             remote_chassis_mac->chassis_id = xstrdup(chassis->name);
+            remote_chassis_mac->chassis_sb_cookie =
+                chassis->header_.uuid.parts[0];
         }
         free(tokstr);
     }
@@ -496,7 +501,8 @@  put_chassis_mac_conj_id_flow(const struct sbrec_chassis_table *chassis_table,
         conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
         conj->n_clauses = 2;
         conj->clause = 0;
-        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
+                        mac->chassis_sb_cookie,
                         &match, ofpacts_p, hc_uuid);
     }
 
@@ -568,8 +574,9 @@  put_replace_chassis_mac_flows(const struct simap *ct_zones,
 
         /* Resubmit to first logical ingress pipeline table. */
         put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
-        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
-                        180, 0, &match, ofpacts_p, hc_uuid);
+        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
+                        rport_binding->header_.uuid.parts[0],
+                        &match, ofpacts_p, hc_uuid);
 
         /* Provide second search criteria, i.e localnet port's
          * vlan ID for conjunction flow */
@@ -587,8 +594,9 @@  put_replace_chassis_mac_flows(const struct simap *ct_zones,
         conj->id = CHASSIS_MAC_TO_ROUTER_MAC_CONJID;
         conj->n_clauses = 2;
         conj->clause = 1;
-        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, 0, &match,
-                        ofpacts_p, hc_uuid);
+        ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180,
+                        rport_binding->header_.uuid.parts[0],
+                        &match, ofpacts_p, hc_uuid);
     }
 }
 
@@ -687,7 +695,8 @@  put_replace_router_port_mac_flows(struct ovsdb_idl_index
 
         ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
 
-        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150,
+                        localnet_port->header_.uuid.parts[0],
                         &match, ofpacts_p, &localnet_port->header_.uuid);
     }
 }
@@ -902,7 +911,8 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         ofpacts_p->header = clone;
         ofpact_finish_CLONE(ofpacts_p, &clone);
 
-        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
+                        binding->header_.uuid.parts[0],
                         &match, ofpacts_p, &binding->header_.uuid);
         return;
     }
@@ -971,7 +981,8 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
             put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
         }
 
-        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
+                        binding->header_.uuid.parts[0],
                         &match, ofpacts_p, &binding->header_.uuid);
 
         goto out;
@@ -1124,8 +1135,8 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         /* Resubmit to first logical ingress pipeline table. */
         put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
         ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
-                        tag ? 150 : 100, 0, &match, ofpacts_p,
-                        &binding->header_.uuid);
+                        tag ? 150 : 100, binding->header_.uuid.parts[0],
+                        &match, ofpacts_p, &binding->header_.uuid);
 
         if (!tag && (!strcmp(binding->type, "localnet")
                      || !strcmp(binding->type, "l2gateway"))) {
@@ -1135,7 +1146,8 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
              * action. */
             ofpbuf_pull(ofpacts_p, ofpacts_orig_size);
             match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
-            ofctrl_add_flow(flow_table, 0, 100, 0, &match, ofpacts_p,
+            ofctrl_add_flow(flow_table, 0, 100,
+                            binding->header_.uuid.parts[0], &match, ofpacts_p,
                             &binding->header_.uuid);
         }
 
@@ -1168,7 +1180,8 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
              * switch will also contain the tag. */
             ofpact_put_STRIP_VLAN(ofpacts_p);
         }
-        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
+                        binding->header_.uuid.parts[0],
                         &match, ofpacts_p, &binding->header_.uuid);
 
         if (!strcmp(binding->type, "localnet")) {
@@ -1199,7 +1212,8 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
 
         /* Resubmit to table 33. */
         put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
-        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
+                        binding->header_.uuid.parts[0],
                         &match, ofpacts_p, &binding->header_.uuid);
     } else {
 
@@ -1322,7 +1336,8 @@  consider_mc_group(enum mf_field_id mff_ovn_geneve,
          * group as the logical output port. */
         put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
 
-        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
+        ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
+                        mc->header_.uuid.parts[0],
                         &match, &ofpacts, &mc->header_.uuid);
     }
 
@@ -1360,7 +1375,8 @@  consider_mc_group(enum mf_field_id mff_ovn_geneve,
             if (local_ports) {
                 put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts);
             }
-            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0,
+            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
+                            mc->header_.uuid.parts[0],
                             &match, &remote_ofpacts, &mc->header_.uuid);
         }
     }
@@ -1672,8 +1688,9 @@  physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
             put_load(1, MFF_LOG_FLAGS, MLF_RCV_FROM_VXLAN_BIT, 1, &ofpacts);
             put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
 
-            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, 0, &match,
-                            &ofpacts, hc_uuid);
+            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100,
+                            binding->header_.uuid.parts[0],
+                            &match, &ofpacts, hc_uuid);
         }
     }
 
@@ -1730,7 +1747,8 @@  physical_run(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         if (pb && !strcmp(pb->type, "localport")) {
             match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, pb->tunnel_key);
             match_set_metadata(&match, htonll(pb->datapath->tunnel_key));
-            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, 0,
+            ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150,
+                            pb->header_.uuid.parts[0],
                             &match, &ofpacts, hc_uuid);
         }
     }
diff --git a/utilities/ovn-sbctl.8.in b/utilities/ovn-sbctl.8.in
index 644f944..d9fc3a5 100644
--- a/utilities/ovn-sbctl.8.in
+++ b/utilities/ovn-sbctl.8.in
@@ -167,7 +167,7 @@  to unbind logical port that is not bound has no effect.
 .
 .SS "Logical Flow Commands"
 .
-.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
+.IP "[\fB\-\-uuid\fR] [\fB\-\-ovs\fR[\fB=\fIremote\fR]] [\fB\-\-stats\fR] [\fB\-\-vflows\fR] \fBlflow\-list\fR [\fIlogical-datapath\fR] [\fIlflow\fR...]"
 List logical flows.  If \fIlogical-datapath\fR is specified, only list
 flows for that logical datapath.  The \fIlogical-datapath\fR may be
 given as a UUID or as a datapath name (reporting an error if multiple
@@ -197,6 +197,12 @@  for more information about the OpenFlow flow output.
 By default, OpenFlow flow output includes only match and actions.  Add
 \fB\-\-stats\fR to include all OpenFlow information, such as packet
 and byte counters, duration, and timeouts.
+.IP
+If \fB\-\-vflows\fR is included, other southbound database records directly
+used for generating OpenFlow flows are also listed. This includes:
+\fIport-bindings\fR, \fImac-bindings\fR, \fImulticast-groups\fR,
+\fIchassis\fR.  The \fB\-\-ovs\fR and \fB\-\-stats\fR can also be used in
+conjunction with \fB\-\-vflows\fR.
 .
 .IP "[\fB\-\-uuid\fR] \fBdump\-flows\fR [\fIlogical-datapath\fR]"
 Alias for \fBlflow\-list\fB.
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index ffcaee2..4facdb1 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -516,7 +516,9 @@  pre_get_info(struct ctl_context *ctx)
     ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip);
 
     ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key);
     ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath);
 
     ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_logical_datapath);
     ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_pipeline);
@@ -530,6 +532,16 @@  pre_get_info(struct ctl_context *ctx)
 
     ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_datapath);
     ovsdb_idl_add_column(ctx->idl, &sbrec_ip_multicast_col_seq_no);
+
+    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_name);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_datapath);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_tunnel_key);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_multicast_group_col_ports);
+
+    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_datapath);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_logical_port);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_ip);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
 }
 
 static struct cmd_show_table cmd_show_tables[] = {
@@ -844,6 +856,161 @@  sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats)
 }
 
 static void
+print_datapath_name(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) {
+        printf("\"%s\" aka \"%s\"", name, name2);
+    } else if (name || name2) {
+        printf("\"%s\"", name ? name : name2);
+    }
+}
+
+static void
+print_vflow_datapath_name(const struct sbrec_datapath_binding *dp,
+                          bool do_print)
+{
+    if (!do_print) {
+        return;
+    }
+    printf("datapath=");
+    print_datapath_name(dp);
+    printf(", ");
+}
+
+static void
+print_uuid_part(const struct uuid *uuid, bool do_print)
+{
+    if (!do_print) {
+        return;
+    }
+    printf("uuid=0x%08"PRIx32", ", uuid->parts[0]);
+}
+
+static void
+cmd_lflow_list_port_bindings(struct ctl_context *ctx, struct vconn *vconn,
+                             const struct sbrec_datapath_binding *datapath,
+                             bool stats, bool print_uuid)
+{
+    const struct sbrec_port_binding *pb;
+    const struct sbrec_port_binding *pb_prev = NULL;
+    SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) {
+
+        if (datapath && pb->datapath != datapath) {
+            continue;
+        }
+
+        if (!pb_prev) {
+            printf("\nPort Bindings:\n");
+        }
+
+        printf("  ");
+        print_uuid_part(&pb->header_.uuid, print_uuid);
+        print_vflow_datapath_name(pb->datapath, !datapath);
+        printf("logical_port=%s, tunnel_key=%-5"PRId64"\n",
+               pb->logical_port, pb->tunnel_key);
+        if (vconn) {
+            sbctl_dump_openflow(vconn, &pb->header_.uuid, stats);
+        }
+
+        pb_prev = pb;
+    }
+}
+
+static void
+cmd_lflow_list_mac_bindings(struct ctl_context *ctx, struct vconn *vconn,
+                            const struct sbrec_datapath_binding *datapath,
+                            bool stats, bool print_uuid)
+{
+    const struct sbrec_mac_binding *mb;
+    const struct sbrec_mac_binding *mb_prev = NULL;
+    SBREC_MAC_BINDING_FOR_EACH (mb, ctx->idl) {
+        if (datapath && mb->datapath != datapath) {
+            continue;
+        }
+
+        if (!mb_prev) {
+            printf("\nMAC Bindings:\n");
+        }
+
+        printf("  ");
+        print_uuid_part(&mb->header_.uuid, print_uuid);
+        print_vflow_datapath_name(mb->datapath, !datapath);
+
+        printf("logical_port=%s, ip=%s, mac=%s\n",
+               mb->logical_port, mb->ip, mb->mac);
+        if (vconn) {
+            sbctl_dump_openflow(vconn, &mb->header_.uuid, stats);
+        }
+
+        mb_prev = mb;
+    }
+}
+
+static void
+cmd_lflow_list_mc_groups(struct ctl_context *ctx, struct vconn *vconn,
+                         const struct sbrec_datapath_binding *datapath,
+                         bool stats, bool print_uuid)
+{
+    const struct sbrec_multicast_group *mc;
+    const struct sbrec_multicast_group *mc_prev = NULL;
+    SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->idl) {
+        if (datapath && mc->datapath != datapath) {
+            continue;
+        }
+
+        if (!mc_prev) {
+            printf("\nMC Groups:\n");
+        }
+
+        printf("  ");
+        print_uuid_part(&mc->header_.uuid, print_uuid);
+        print_vflow_datapath_name(mc->datapath, !datapath);
+
+        printf("name=%s, tunnel_key=%-5"PRId64", ports=(",
+               mc->name, mc->tunnel_key);
+        for (size_t i = 0; i < mc->n_ports; i++) {
+            printf("%s", mc->ports[i]->logical_port);
+            if (i != mc->n_ports - 1) {
+                printf(", ");
+            }
+        }
+        printf(")\n");
+
+        if (vconn) {
+            sbctl_dump_openflow(vconn, &mc->header_.uuid, stats);
+        }
+
+        mc_prev = mc;
+    }
+}
+
+static void
+cmd_lflow_list_chassis(struct ctl_context *ctx, struct vconn *vconn,
+                       bool stats, bool print_uuid)
+{
+    const struct sbrec_chassis *chassis;
+    const struct sbrec_chassis *chassis_prev = NULL;
+    SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) {
+        if (!chassis_prev) {
+            printf("\nChassis:\n");
+        }
+
+        printf("  ");
+        print_uuid_part(&chassis->header_.uuid, print_uuid);
+
+        printf("name=%s\n", chassis->name);
+        if (vconn) {
+            sbctl_dump_openflow(vconn, &chassis->header_.uuid, stats);
+        }
+
+        chassis_prev = chassis;
+    }
+}
+
+static void
 cmd_lflow_list(struct ctl_context *ctx)
 {
     const struct sbrec_datapath_binding *datapath = NULL;
@@ -925,16 +1092,8 @@  cmd_lflow_list(struct ctl_context *ctx)
         if (!prev
             || prev->logical_datapath != lflow->logical_datapath
             || strcmp(prev->pipeline, lflow->pipeline)) {
-            printf("Datapath:");
-
-            const struct smap *ids = &lflow->logical_datapath->external_ids;
-            const char *name = smap_get(ids, "name");
-            const char *name2 = smap_get(ids, "name2");
-            if (name && name2) {
-                printf(" \"%s\" aka \"%s\"", name, name2);
-            } else if (name || name2) {
-                printf(" \"%s\"", name ? name : name2);
-            }
+            printf("Datapath: ");
+            print_datapath_name(lflow->logical_datapath);
             printf(" ("UUID_FMT")  Pipeline: %s\n",
                    UUID_ARGS(&lflow->logical_datapath->header_.uuid),
                    lflow->pipeline);
@@ -942,9 +1101,7 @@  cmd_lflow_list(struct ctl_context *ctx)
 
         /* Print the flow. */
         printf("  ");
-        if (print_uuid) {
-            printf("uuid=0x%08"PRIx32", ", lflow->header_.uuid.parts[0]);
-        }
+        print_uuid_part(&lflow->header_.uuid, print_uuid);
         printf("table=%-2"PRId64"(%-19s), priority=%-5"PRId64
                ", match=(%s), action=(%s)\n",
                lflow->table_id,
@@ -956,6 +1113,14 @@  cmd_lflow_list(struct ctl_context *ctx)
         prev = lflow;
     }
 
+    bool vflows = shash_find(&ctx->options, "--vflows") != NULL;
+    if (vflows) {
+        cmd_lflow_list_port_bindings(ctx, vconn, datapath, stats, print_uuid);
+        cmd_lflow_list_mac_bindings(ctx, vconn, datapath, stats, print_uuid);
+        cmd_lflow_list_mc_groups(ctx, vconn, datapath, stats, print_uuid);
+        cmd_lflow_list_chassis(ctx, vconn, stats, print_uuid);
+    }
+
     vconn_close(vconn);
     free(lflows);
 }
@@ -1509,10 +1674,11 @@  static const struct ctl_command_syntax sbctl_commands[] = {
     /* Logical flow commands */
     {"lflow-list", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
      pre_get_info, cmd_lflow_list, NULL,
-     "--uuid,--ovs?,--stats", RO},
+     "--uuid,--ovs?,--stats,--vflows?", RO},
     {"dump-flows", 0, INT_MAX, "[DATAPATH] [LFLOW...]",
      pre_get_info, cmd_lflow_list, NULL,
-     "--uuid,--ovs?,--stats", RO}, /* Friendly alias for lflow-list */
+     "--uuid,--ovs?,--stats,--vflows?",
+     RO}, /* Friendly alias for lflow-list */
 
     /* IP multicast commands. */
     {"ip-multicast-flush", 0, 1, "SWITCH",