On May 7, 2012, at 4:54 PM, Ben Pfaff wrote:
> One change between OF1.0 and OF1.3 that I like is putting "= <value>"
> explicitly on all the enum values. It'd be nice to add these to all
> the existing OFPST_* constants. I guess I should have noticed this
> for the previous patch; perhaps you could fold it in there.
Done.
> I try to make sure that every OpenFlow message has a corresponding
> test in tests/ofp-print.at. Would you mind adding one for
> OFPST_PORT_DESC_REQUEST and one for OFPST_PORT_DESC_REPLY?
Yes. I actually did it for both 1.0 and 1.1, so there are four new tests. It
caught the fact that we weren't processing OF1.1 stats requests, so I added
that support to the patch.
> I've got a review request out to use the new simplified style for -v
> options, so can you change -vANY:ANY:WARN to just -vwarn here?
>> +AT_CHECK([ovs-ofctl -vANY:ANY:WARN dump-ports-desc br0], [0], [stdout])
Sure.
> Will you document dump-ports-desc in the ovs-ofctl manpage?
> Alternatively, if you think it's only useful for unit testing, you
> could move its implementation and the entry in all_commands[] to the
> /* Undocumented commands for unit testing. */
> section, and then drop it from usage.
Whoops. That was an oversight. I've added one.
Below, you should find an incremental.
--Justin
---
lib/ofp-util.c | 14 ++++++++++++
tests/ofp-print.at | 53 ++++++++++++++++++++++++++++++++++++++++++++++
tests/ofproto.at | 2 +-
utilities/ovs-ofctl.8.in | 5 ++++
4 files changed, 73 insertions(+), 1 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6007147..cd743b8 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -590,6 +590,9 @@ ofputil_decode_ofpst_request(const struct ofp_header *oh, si
ze_t length,
{ OFPUTIL_OFPST_PORT_DESC_REQUEST, OFP10_VERSION,
OFPST_PORT_DESC, "OFPST_PORT_DESC request",
sizeof(struct ofp_stats_msg), 0 },
+ { OFPUTIL_OFPST_PORT_DESC_REQUEST, OFP11_VERSION,
+ OFPST_PORT_DESC, "OFPST_PORT_DESC request",
+ sizeof(struct ofp_stats_msg), 0 },
{ 0, 0,
OFPST_VENDOR, "OFPST_VENDOR request",
@@ -651,6 +654,9 @@ ofputil_decode_ofpst_reply(const struct ofp_header *oh, size
_t length,
{ OFPUTIL_OFPST_PORT_DESC_REPLY, OFP10_VERSION,
OFPST_PORT_DESC, "OFPST_PORT_DESC reply",
sizeof(struct ofp_stats_msg), sizeof(struct ofp10_phy_port) },
+ { OFPUTIL_OFPST_PORT_DESC_REPLY, OFP11_VERSION,
+ OFPST_PORT_DESC, "OFPST_PORT_DESC reply",
+ sizeof(struct ofp_stats_msg), sizeof(struct ofp11_port) },
{ 0, 0,
OFPST_VENDOR, "OFPST_VENDOR reply",
@@ -756,10 +762,16 @@ ofputil_decode_msg_type__(const struct ofp_header *oh, siz
e_t length,
{ 0, OFP10_VERSION,
OFPT10_STATS_REQUEST, "OFPT_STATS_REQUEST",
sizeof(struct ofp_stats_msg), 1 },
+ { 0, OFP11_VERSION,
+ OFPT11_STATS_REQUEST, "OFPT_STATS_REQUEST",
+ sizeof(struct ofp_stats_msg), 1 },
{ 0, OFP10_VERSION,
OFPT10_STATS_REPLY, "OFPT_STATS_REPLY",
sizeof(struct ofp_stats_msg), 1 },
+ { 0, OFP11_VERSION,
+ OFPT11_STATS_REPLY, "OFPT_STATS_REPLY",
+ sizeof(struct ofp_stats_msg), 1 },
{ OFPUTIL_OFPT_BARRIER_REQUEST, OFP10_VERSION,
OFPT10_BARRIER_REQUEST, "OFPT_BARRIER_REQUEST",
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 2b172d4..1714307 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -688,6 +688,59 @@ OFPST_QUEUE reply (xid=0x1): 6 queues
])
AT_CLEANUP
+AT_SETUP([OFPST_PORT_DESC request - OF1.0])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "0110000c00000001000d0000"], [0], [dnl
+OFPST_PORT_DESC request (xid=0x1):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT_DESC reply - OF1.0])
+AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+01 11 00 3c 00 00 00 00 00 0d 00 00 00 03 50 54 \
+00 00 00 01 65 74 68 30 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 01 00 00 00 01 00 00 02 08 \
+00 00 02 8f 00 00 02 8f 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT_DESC reply (xid=0x0): 1 ports
+ 3(eth0): addr:50:54:00:00:00:01
+ config: PORT_DOWN
+ state: LINK_DOWN
+ current: 100MB-FD AUTO_NEG
+ advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ supported: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ speed: 100 Mbps now, 100 Mbps max
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT_DESC request - OF1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "0212000c00000001000d0000"], [0], [dnl
+OFPST_PORT_DESC request (OF1.1) (xid=0x1):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT_DESC reply - OF1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+02 13 00 4c 00 00 00 00 00 0d 00 00 00 00 00 03 \
+00 00 00 00 50 54 00 00 00 01 00 00 65 74 68 30 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 20 08 00 00 28 0f 00 00 28 0f \
+00 00 00 00 00 01 86 a0 00 01 86 a0 \
+"], [0], [dnl
+OFPST_PORT_DESC reply (OF1.1) (xid=0x0): 1 ports
+ 3(eth0): addr:50:54:00:00:00:01
+ config: 0
+ state: 0
+ current: 100MB-FD AUTO_NEG
+ advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ supported: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ speed: 100 Mbps now, 100 Mbps max
+])
+AT_CLEANUP
+
AT_SETUP([OFPT_BARRIER_REQUEST])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print '01 12 00 08 00 00 00 01'], [0], [dnl
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 0d9d4ff..7db7f18 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -40,7 +40,7 @@ dnl This is really bare-bones.
dnl It at least checks request and reply serialization and deserialization.
AT_SETUP([ofproto - port-desc stats])
OVS_VSWITCHD_START
-AT_CHECK([ovs-ofctl -vANY:ANY:WARN dump-ports-desc br0], [0], [stdout])
+AT_CHECK([ovs-ofctl -vwarn dump-ports-desc br0], [0], [stdout])
AT_CHECK([STRIP_XIDS stdout], [0], [dnl
OFPST_PORT_DESC reply: 1 ports
LOCAL(br0): addr:aa:55:aa:55:00:00
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index fdaa037..d1b3bc6 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -66,6 +66,11 @@ associated with that device will be printed. \fInetdev\fR ca
n be an
OpenFlow assigned port number or device name, e.g. \fBeth0\fR.
.
.TP
+\fBdump\-ports\-desc \fIswitch\fR
+Prints to the console detailed information about network devices
+associated with \fIswitch\fR.
+.
+.TP
\fBmod\-port \fIswitch\fR \fInetdev\fR \fIaction\fR
Modify characteristics of an interface monitored by \fIswitch\fR.
\fInetdev\fR can be referred to by its OpenFlow assigned port number or
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
