[ovs-dev] [PATCH] Document map members as separate columns

Ben Pfaff blp at nicira.com
Tue Oct 4 17:47:33 UTC 2011


On Mon, Oct 03, 2011 at 11:49:38PM -0700, Justin Pettit wrote:
> - Visually, I liked having the box around the summaries in the old
> version; it made it easier to spot new tables. Did you see how it
> looks by keeping the box?

I liked the box too and did not intend to remove it, but "tbl" threw
up all over when some of the summaries started exceeding a page.

> - On some strings, the summary contains the values that are allowed
> (e.g., "fail_mode") on others (e.g.,
> "other_config:disable-in-band"), they're not.

The summary reports constraints from the database schema.  The schema
format doesn't allow constraining the values of individual keys like
other-config:disable-in-band, so the summary doesn't say that.

That said, it's not too hard to extend the documentation format to
allow them to be specified in vswitch.xml, so I did that.  I'll send
out a separate patch.

> - Sometimes commas are in large numbers and sometimes they're not.
> For example, "set of up to 4096 integers, in range 0 to 4,095".

Not hard to fix, so I wrote up another patch.

> - In the description of "gre" type, it says "See Tunnel Options for
> information on configuring GRE tunnels."  I think you can safely
> drop that, since it's covered at the bottom of the section and is
> inconsistent with the other tunnel types.

OK, done.

> - Only the "ipsec_gre"-specific options are broken out in the summary.
> It seems like all the breakdowns should be there or none.  I can see
> an argument both for and against having that breakdown in the
> summary.  Regardless, it seems like it should be consistent.

The only option that isn't broken out is header_cache.  The others in
the Tunnel Options section apply to every kind of tunnel, or at least
they lack text that says they don't.

I've added a separate subgroup for header_cache now.

> - The Interface "statistics" items aren't broken out like Open vSwitch ones.

I wasn't sure it was worthwhile.  Based on your feedback, I broke them
out now.

> - Some of the items that are filled out by OVS are listed as
> "optional", but they're not optional from the user's perspective.
> As we've discussed, it would be really useful (in a future commit)
> to indicate whether the user or OVS is going to fill out a
> column/value.  If we go down that road, I think a word other than
> "optional" would be good for fields OVS writes.  (I'm too tired to
> come up with that word right now.  :) )

Yes, I agree, this is the right direction.



More information about the dev mailing list