[ovs-dev] [PATCH 2/5] ofproto: Add support for OF1.3 port description multipart message.

Ben Pfaff blp at nicira.com
Mon May 7 23:54:02 UTC 2012


On Mon, May 07, 2012 at 11:56:49AM -0700, Justin Pettit wrote:
> OpenFlow 1.0 is limited to displaying 1364 ports in the Features Reply
> message, and there is no other way to get consolidated port information.
> OpenFlow 1.3 adds a new port description multipart message
> (OFPMP_PORT_DESC) that is not limited by size.  This commit adds support
> through the OpenFlow 1.0 stats mechanism, since they have complimentary
> enum values.
> 
> Bug #11040
> 
> Signed-off-by: Justin Pettit <jpettit at nicira.com>

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.

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?

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])

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.

Thanks,

Ben.



More information about the dev mailing list