On Oct 22, 2012, at 3:54 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Thu, Oct 18, 2012 at 12:51:55PM -0700, Justin Pettit wrote:
>> These commands will be useful in a future commit that makes multiple
>> bridges share a single backing datapath. The ovs-dpctl commands will
>> show information about the backing datapath, so it will be difficult to
>> determine which information belongs to which bridge. The new "dpif/*"
>> ovs-appctl commands return information about the bridge--regardless of
>> how the backing datapath is configured.
>>
>> Signed-off-by: Justin Pettit <jpet...@nicira.com>
>
> Users are already confused enough (and rightly so) about the
> difference between "ovs-ofctl dump-flows" and "ovs-dpctl dump-flows"
> (and other uses of these programs). Adding a third way is going to
> make their heads spin. I suggest adding some explanation to the
> manpage text for each program/fragment, and perhaps a FAQ entry giving
> an overview.
Don't forget "ovs-appctl bridge/dump-flows". :-)
I added a reference to "ovs-ofctl dump-flows" in the "ovs-dpctl dump-flows" and
"ovs-appctl dpif/dump-flows", since that's what people usually want to see.
Unless you object, I'll add a FAQ entry in the final patch, since that makes
this new dump-flows actually useful, and we'd need to rewrite then, anyway.
> dpif/dump-dps and dpif/show both need a sorted list of ofprotos,
> perhaps they could team up. It might be wise to sort the ofprotos on
> type@name instead of just name.
I created a new get_ofprotos() function they can double team. It sorts them
how you suggested.
> For consistency with ovs-dpctl it might be nice if dpif/show would
> accept multiple names at once.
Okay.
> "ovs-dpctl show" prints a colon on the line that introduces a datapath
> name, "dpif/show" omits it.
Fixed.
> I'd drop the following change from vswitchd/ovs-vswitchd.8.in:
>
> @@ -122,10 +122,11 @@ interfaces if none is given) to be \fIstatus\fR.
> \fIstatus\fR can be
> "true", "false", or "normal" which reverts to the standard behavior.
> .IP "\fBstp/tcn\fR [\fIbridge\fR]"
> Forces a topology change event on \fIbridge\fR if it's running STP. This
> may cause it to send Topology Change Notifications to its peers and flush
> its MAC table.. If no \fIbridge\fR is given, forces a topology change
> +.
> event on all bridges.
> .SS "BRIDGE COMMANDS"
> These commands manage bridges.
> .IP "\fBfdb/flush\fR [\fIbridge\fR]"
> Flushes \fIbridge\fR MAC address learning table, or all learning tables
Weird. Removed.
I've added an incremental to the end of this message.
--Justin
-=-=-=-=-=-=-=-=-=-=-
diff --git a/ofproto/ofproto-dpif-unixctl.man b/ofproto/ofproto-dpif-unixctl.man
index 1d9054d..daacd3a 100644
--- a/ofproto/ofproto-dpif-unixctl.man
+++ b/ofproto/ofproto-dpif-unixctl.man
@@ -5,15 +5,15 @@ equivalent \fBovs\-dpctl\fR commands.
.IP "\fBdpif/dump\-dps\fR"
Prints the name of each configured datapath on a separate line.
.
-.IP "\fBdpif/show\fR [\fIdp\fR]"
+.IP "\fBdpif/show\fR [\fIdp\fR...]"
Prints a summary of configured datapaths, including statistics and a
list of connected ports. The port information includes the OpenFlow
port number, datapath port number, and the type. (The local port is
identified as OpenFlow port 65534.)
.IP
-If datapath \fIdp\fR is specified, information on only that datapath is
-displayed. Otherwise, information about all configured datapaths are
-shown.
+If one or more datapaths are specified, information on only those
+datapaths are displayed. Otherwise, information about all configured
+datapaths are shown.
.
.IP "\fBdpif/dump\-flows \fIdp\fR"
Prints to the console all flow entries in datapath \fIdp\fR's
@@ -22,7 +22,8 @@ flow table.
This command is primarily useful for debugging Open vSwitch. The flow
table entries that it displays are not OpenFlow flow entries. Instead,
they are different and considerably simpler flows maintained by the
-datapath module.
+datapath module. If you wish to see the OpenFlow flow entries, use
+\fBovs\-ofctl dump\-flows\fR.
.
.IP "\fBdpif/del\-flows \fIdp\fR"
Deletes all flow entries from datapath \fIdp\fR's flow table and
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c1cbb7e..65e560e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -7044,27 +7044,37 @@ ofproto_dpif_self_check(struct unixctl_conn *conn,
ds_destroy(&reply);
}
+/* Store the current ofprotos in 'ofproto_shash'. Returns a sorted list
+ * of the 'ofproto_shash' nodes. It is the responsibility of the caller
+ * to destroy 'ofproto_shash' and free the returned value. */
+static const struct shash_node **
+get_ofprotos(struct shash *ofproto_shash)
+{
+ const struct ofproto_dpif *ofproto;
+
+ HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+ char *name = xasprintf("%s@%s", ofproto->up.type, ofproto->up.name);
+ shash_add_nocopy(ofproto_shash, name, ofproto);
+ }
+
+ return shash_sort(ofproto_shash);
+}
+
static void
ofproto_unixctl_dpif_dump_dps(struct unixctl_conn *conn, int argc OVS_UNUSED,
const char *argv[] OVS_UNUSED,
void *aux OVS_UNUSED)
{
struct ds ds = DS_EMPTY_INITIALIZER;
- const struct ofproto_dpif *ofproto;
- struct shash ofproto_shash = SHASH_INITIALIZER(&ofproto_shash);
+ struct shash ofproto_shash;
const struct shash_node **sorted_ofprotos;
int i;
- HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
- shash_add(&ofproto_shash, ofproto->up.name, ofproto);
- }
-
- sorted_ofprotos = shash_sort(&ofproto_shash);
+ shash_init(&ofproto_shash);
+ sorted_ofprotos = get_ofprotos(&ofproto_shash);
for (i = 0; i < shash_count(&ofproto_shash); i++) {
const struct shash_node *node = sorted_ofprotos[i];
-
- ofproto = node->data;
- ds_put_format(&ds, "%s@%s\n", ofproto->up.type, ofproto->up.name);
+ ds_put_format(&ds, "%s\n", node->name);
}
shash_destroy(&ofproto_shash);
@@ -7083,7 +7093,7 @@ show_dp_format(const struct ofproto_dpif *ofproto, struct
dpif_get_dp_stats(ofproto->dpif, &s);
- ds_put_format(ds, "%s@%s\n", ofproto->up.type, ofproto->up.name);
+ ds_put_format(ds, "%s@%s:\n", ofproto->up.type, ofproto->up.name);
ds_put_format(ds,
"\tlookups: hit:%"PRIu64" missed:%"PRIu64" lost:%"PRIu64"\n",
s.n_hit, s.n_missed, s.n_lost);
@@ -7141,23 +7151,24 @@ ofproto_unixctl_dpif_show(struct unixctl_conn *conn, int
const struct ofproto_dpif *ofproto;
if (argc > 1) {
- ofproto = ofproto_dpif_lookup(argv[1]);
- if (!ofproto) {
- unixctl_command_reply_error(conn, "Unknown bridge (use "
- "dpif/dump-dps for help)");
- return;
+ int i;
+ for (i = 1; i < argc; i++) {
+ ofproto = ofproto_dpif_lookup(argv[i]);
+ if (!ofproto) {
+ ds_put_format(&ds, "Unknown bridge %s (use dpif/dump-dps "
+ "for help)", argv[i]);
+ unixctl_command_reply_error(conn, ds_cstr(&ds));
+ return;
+ }
+ show_dp_format(ofproto, &ds);
}
- show_dp_format(ofproto, &ds);
} else {
- struct shash ofproto_shash = SHASH_INITIALIZER(&ofproto_shash);
+ struct shash ofproto_shash;
const struct shash_node **sorted_ofprotos;
int i;
- HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
- shash_add(&ofproto_shash, ofproto->up.name, ofproto);
- }
-
- sorted_ofprotos = shash_sort(&ofproto_shash);
+ shash_init(&ofproto_shash);
+ sorted_ofprotos = get_ofprotos(&ofproto_shash);
for (i = 0; i < shash_count(&ofproto_shash); i++) {
const struct shash_node *node = sorted_ofprotos[i];
show_dp_format(node->data, &ds);
@@ -7260,7 +7271,7 @@ ofproto_dpif_unixctl_init(void)
ofproto_dpif_self_check, NULL);
unixctl_command_register("dpif/dump-dps", "", 0, 0,
ofproto_unixctl_dpif_dump_dps, NULL);
- unixctl_command_register("dpif/show", "[bridge]", 0, 1,
+ unixctl_command_register("dpif/show", "[bridge]", 0, INT_MAX,
ofproto_unixctl_dpif_show, NULL);
unixctl_command_register("dpif/dump-flows", "bridge", 1, 1,
ofproto_unixctl_dpif_dump_flows, NULL);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6a2e1a7..fb4cb73 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1209,13 +1209,13 @@ ADD_OF_PORTS([br0], [1], [2])
ADD_OF_PORTS([br1], [3])
AT_CHECK([ovs-appctl dpif/show], [0], [dnl
-dummy@br0
+dummy@br0:
lookups: hit:0 missed:0 lost:0
flows: 0
br0 65534/100: (dummy)
p1 1/1: (dummy)
p2 2/2: (dummy)
-dummy@br1
+dummy@br1:
lookups: hit:0 missed:0 lost:0
flows: 0
br1 65534/101: (dummy)
@@ -1223,7 +1223,7 @@ dummy@br1
])
AT_CHECK([ovs-appctl dpif/show br0], [0], [dnl
-dummy@br0
+dummy@br0:
lookups: hit:0 missed:0 lost:0
flows: 0
br0 65534/100: (dummy)
diff --git a/utilities/ovs-dpctl.8.in b/utilities/ovs-dpctl.8.in
index 042bcd5..d031ea4 100644
--- a/utilities/ovs-dpctl.8.in
+++ b/utilities/ovs-dpctl.8.in
@@ -112,7 +112,8 @@ flow table.
This command is primarily useful for debugging Open vSwitch. The flow
table entries that it displays are not
OpenFlow flow entries. Instead, they are different and considerably
-simpler flows maintained by the Open vSwitch kernel module.
+simpler flows maintained by the Open vSwitch kernel module. If you wish
+to see the OpenFlow flow entries, use \fBovs\-ofctl dump\-flows\fR.
.IP "\fBdel\-flows \fIdp\fR"
Deletes all flow entries from datapath \fIdp\fR's flow table.
.IP
diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index 7ddaf20..6bb08c7 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -124,7 +124,6 @@ interfaces if none is given) to be \fIstatus\fR. \fIstatus\
Forces a topology change event on \fIbridge\fR if it's running STP. This
may cause it to send Topology Change Notifications to its peers and flush
its MAC table.. If no \fIbridge\fR is given, forces a topology change
-.
event on all bridges.
.SS "BRIDGE COMMANDS"
These commands manage bridges.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
