[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