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

[Justin Pettit]

On May 7, 2012, at 10:47 PM, Ben Pfaff wrote:

> On Mon, May 07, 2012 at 10:38:14PM -0700, Justin Pettit wrote:
>> On May 7, 2012, at 4:54 PM, Ben Pfaff wrote:
>>> 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.
> 
> At first glance, at least, the support you added for OF1.1 isn't right,
> because OF1.1 adds additional padding to stats requests and replies, so
> that a minimum-length request or reply is 16 bytes instead of 12.  (I
> actually thought about suggesting adding support, but then I remembered
> that change and decided not to.)
> 
> Unless you can think of a quick and easy way to work around that, I'd be
> inclined to just leave out that support for the moment.

D'oh.  Thanks for catching that.  I'll just drop OF1.1 support for now.

>>> 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.
> 
> Hmm.  Is it worth mentioning that it's only supported in OVS 1.7.0 and
> later?  It might also be worth mentioning that what it prints is a
> subset of what "ovs-ofctl show" prints.


Sure.  How about the following?

       dump-ports-desc switch
              Prints to the console detailed information about network devices
              associated with switch (version 1.7 or later).  This is a subset
              of the information provided by the show command.

--Justin