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

Ben Pfaff blp at nicira.com
Tue May 8 13:44:03 UTC 2012


On Mon, May 07, 2012 at 11:34:18PM -0700, Justin Pettit wrote:
> 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.

OK.

> >>> 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.

Sure.  Thanks.



More information about the dev mailing list