[ovs-dev,4/6] doc: Convert ovs-vlan-test to rST

Message ID 20170414044319.11074-5-blp@ovn.org
State Accepted
Headers show

Commit Message

Ben Pfaff April 14, 2017, 4:43 a.m.
From: Stephen Finucane <stephen@that.guru>

Let's start with a simple one that lets us focus on setting up most of
the required "infrastructure" for building man pages using Sphinx.

This changes the 'check-htmldocs' target to 'check-docs' as its now
responsible for building man page docs too.

Other than that, hurrah for (mostly) legible syntaxes.

[1] http://www.tldp.org/HOWTO/Man-Page/q2.html

Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 Documentation/automake.mk                          |  84 +++++++++++++--
 Documentation/conf.py                              |   5 +-
 .../internals/contributing/documentation-style.rst |   3 +-
 Documentation/intro/install/documentation.rst      |   4 +-
 Documentation/ref/index.rst                        |  16 ++-
 Documentation/ref/ovs-vlan-test.8.rst              | 115 +++++++++++++++++++++
 debian/openvswitch-switch.manpages                 |   1 -
 manpages.mk                                        |  10 --
 utilities/automake.mk                              |   3 -
 utilities/ovs-vlan-test.8.in                       |  96 -----------------
 10 files changed, 211 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/ref/ovs-vlan-test.8.rst
 delete mode 100644 utilities/ovs-vlan-test.8.in

Comments

Stephen Finucane April 18, 2017, 10:58 a.m. | #1
On Thu, 2017-04-13 at 21:43 -0700, Ben Pfaff wrote:
> From: Stephen Finucane <stephen@that.guru>
> 
> Let's start with a simple one that lets us focus on setting up most
> of
> the required "infrastructure" for building man pages using Sphinx.
> 
> This changes the 'check-htmldocs' target to 'check-docs' as its now
> responsible for building man page docs too.
> 
> Other than that, hurrah for (mostly) legible syntaxes.
> 
> [1] http://www.tldp.org/HOWTO/Man-Page/q2.html

This mostly looks good to me. Some comments below but nothing I'd block
on.

Acked-by: Stephen Finucane <stephen@that.guru>

> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>  Documentation/automake.mk                          |  84
> +++++++++++++--
>  Documentation/conf.py                              |   5 +-
>  .../internals/contributing/documentation-style.rst |   3 +-
>  Documentation/intro/install/documentation.rst      |   4 +-
>  Documentation/ref/index.rst                        |  16 ++-
>  Documentation/ref/ovs-vlan-test.8.rst              | 115
> +++++++++++++++++++++
>  debian/openvswitch-switch.manpages                 |   1 -
>  manpages.mk                                        |  10 --
>  utilities/automake.mk                              |   3 -
>  utilities/ovs-vlan-test.8.in                       |  96 ---------
> --------
>  10 files changed, 211 insertions(+), 126 deletions(-)
>  create mode 100644 Documentation/ref/ovs-vlan-test.8.rst
>  delete mode 100644 utilities/ovs-vlan-test.8.in
> 
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 9911668c1ca9..762255277102 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -90,7 +90,8 @@ DOC_SOURCE = \
>  	Documentation/internals/contributing/documentation-style.rst 
> \
>  	Documentation/internals/contributing/libopenvswitch-abi.rst
> \
>  	Documentation/internals/contributing/submitting-patches.rst
> \
> -	Documentation/requirements.txt
> +	Documentation/requirements.txt \
> +	$(addprefix Documentation/ref/,$(RST_MANPAGES))
>  
>  EXTRA_DIST += $(DOC_SOURCE)
>  
> @@ -110,20 +111,89 @@ sphinx_verbose_ = $(sphinx_verbose_@AM_DEFAULT_
> V@)
>  sphinx_verbose_0 = -q
>  
>  if HAVE_SPHINX
> -htmldocs-check: $(DOC_SOURCE)
> +docs-check: $(DOC_SOURCE)
>  	$(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b html
> $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html && touch $@
> -ALL_LOCAL += htmldocs-check
> -CLEANFILES += htmldocs-check
> +	$(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b man
> $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/man && touch $@
> +ALL_LOCAL += docs-check
> +CLEANFILES += docs-check
>  
>  check-docs:
>  	$(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS)
> $(SPHINXBUILDDIR)/linkcheck
>  
>  clean-docs:
> -	rm -rf $(SPHINXBUILDDIR)/doctrees
> -	rm -rf $(SPHINXBUILDDIR)/html
> -	rm -rf $(SPHINXBUILDDIR)/linkcheck
> +	rm -rf $(SPHINXBUILDDIR)
>  	rm -f docs-check
>  CLEAN_LOCAL += clean-docs
>  endif
>  .PHONY: check-docs
>  .PHONY: clean-docs
> +

I might have done something funky applying this, but I can see a '^L'
character (linefeed?) on this line. Might be worth watching for.

> +# Installing manpages based on rST.
> +#
> +# The docs-check target converts the rST files listed in
> RST_MANPAGES
> +# into nroff manpages in Documentation/_build/man.  The easiest way
> to
> +# get these installed by "make install" is to write our own helper
> +# rules.
> +
> +# rST formatted manpages under Documentation/ref.
> +RST_MANPAGES = \
> +	ovs-vlan-test.8.rst
> +
> +# The GNU standards say that these variables should control
> +# installation directories for manpages in each section.  Automake
> +# will define them for us only if it sees that a manpage in the
> +# appropriate section is to be installed through its built-in
> feature.
> +# Since we're working independently, for best safety, we need to
> +# define them ourselves.
> +man1dir = $(mandir)/man1
> +man2dir = $(mandir)/man2
> +man3dir = $(mandir)/man3
> +man4dir = $(mandir)/man4
> +man5dir = $(mandir)/man5
> +man6dir = $(mandir)/man6
> +man7dir = $(mandir)/man7
> +man8dir = $(mandir)/man8
> +man9dir = $(mandir)/man9
> +
> +# Set a shell variable for each manpage directory.
> +set_mandirs = \
> +	man1dir='$(man1dir)' \
> +	man2dir='$(man2dir)' \
> +	man3dir='$(man3dir)' \
> +	man4dir='$(man4dir)' \
> +	man5dir='$(man5dir)' \
> +	man6dir='$(man6dir)' \
> +	man7dir='$(man7dir)' \
> +	man8dir='$(man8dir)' \
> +	man9dir='$(man9dir)'
> +
> +# Given an $rst of "ovs-vlan-test.8.rst", sets $stem to
> +# "ovs-vlan-test", $section to "8", and $mandir to $man8dir.
> +extract_stem_and_section = \
> +	stem=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-
> 9]\).rst$$/\1/p'`; \
> +	section=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-
> 9]\).rst$$/\2/p'`; \
> +	test -n "$$section" || { echo "$$rst: cannot infer manpage
> section from filename" 2>&1; continue; }; \
> +	eval "mandir=\$$man$${section}dir"; \
> +	test -n "$$mandir" || { echo "unknown directory for manpage
> section $$section"; continue; }
> +
> +if HAVE_SPHINX
> +INSTALL_DATA_LOCAL += install-man-rst
> +install-man-rst: docs-check
> +	@$(set_mandirs); \
> +	for rst in $(RST_MANPAGES); do \
> +	    $(extract_stem_and_section); \
> +	    echo " $(MKDIR_P) '$(DESTDIR)'\"$$mandir\""; \
> +	    $(MKDIR_P) '$(DESTDIR)'"$$mandir"; \
> +	    echo " $(INSTALL_DATA)
> $(SPHINXBUILDDIR)/man/$$stem.$$section
> '$(DESTDIR)'\"$$mandir/$$stem.$$section\""; \
> +	    $(INSTALL_DATA) $(SPHINXBUILDDIR)/man/$$stem.$$section
> '$(DESTDIR)'"$$mandir/$$stem.$$section"; \
> +	done
> +endif
> +
> +UNINSTALL_LOCAL += uninstall-man-rst
> +uninstall-man-rst:
> +	@$(set_mandirs); \
> +	for rst in $(RST_MANPAGES); do \
> +	    $(extract_stem_and_section); \
> +	    echo "rm -f '$(DESTDIR)'\"$$mandir/$$stem.$$section\"";
> \
> +	    rm -f '$(DESTDIR)'"$$mandir/$$stem.$$section"; \
> +	done

This whole section is still mostly noise to me and I would love to
simplify it somehow. I'm not ready to do that yet though and so long as
I don't have to maintain this (:)), I can live with it as-is. Might be
good to get someone else with more autotools know-how to look at it
though?

> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index bfd7f33d88ba..8671a097d17e 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -328,8 +328,9 @@ latex_documents = [
>  # One entry per manual page. List of tuples
>  # (source start file, name, description, authors, manual section).
>  man_pages = [
> -    (master_doc, 'openvswitch', u'Open vSwitch Documentation',
> -     [author], 1)
> +    ('ref/ovs-vlan-test.8', 'ovs-vlan-test',
> +     u'Check Linux drivers for problems with vlan traffic',
> +     [author], 8)
>  ]
>  
>  # If true, show URL addresses after external links.
> diff --git a/Documentation/internals/contributing/documentation-
> style.rst b/Documentation/internals/contributing/documentation-
> style.rst
> index 0ba5e544f14e..0e561ec23f10 100644
> --- a/Documentation/internals/contributing/documentation-style.rst
> +++ b/Documentation/internals/contributing/documentation-style.rst
> @@ -347,7 +347,8 @@ In addition to the above, man pages have some
> specific requirements:
>  - The man page must be included in the list of man page documents
> found in
>    `conf.py`__
>  
> -Refer to existing man pages for worked examples.
> +Refer to existing man pages, such as :doc:`/ref/ovs-vlan-test.8` for
> a worked
> +example.
>  
>  __ http://www.sphinx-doc.org/en/stable/domains.html#directive-progra
> m
>  __ http://www.sphinx-doc.org/en/stable/domains.html#directive-option
> diff --git a/Documentation/intro/install/documentation.rst
> b/Documentation/intro/install/documentation.rst
> index 94af950afc3e..0eeeab15226f 100644
> --- a/Documentation/intro/install/documentation.rst
> +++ b/Documentation/intro/install/documentation.rst
> @@ -74,11 +74,11 @@ Building
>  Once Sphinx installed, the documentation can be built using the
> provided
>  Makefile targets::
>  
> -    $ make htmldocs-check
> +    $ make docs-check
>  
>  .. important::
>  
> -   The ``htmldocs-check`` target will fail if there are any syntax
> errors.
> +   The ``docs-check`` target will fail if there are any syntax
> errors.
>     However, it won't catch more succint issues such as style or
> grammar issues.
>     As a result, you should always inspect changes visually to ensure
> the result
>     is as intended.
> diff --git a/Documentation/ref/index.rst
> b/Documentation/ref/index.rst
> index 49121b7599fd..6b368809c11b 100644
> --- a/Documentation/ref/index.rst
> +++ b/Documentation/ref/index.rst
> @@ -30,10 +30,18 @@ Reference Guide
>  Man Pages
>  ---------
>  
> -.. TODO(stephenfin): Investigate some way to get the manpages into
> rST format.
> -   The most viable option seems to be writing them all in rST,
> converting them
> -   to roff format and storing both the rST and roff formatted docs
> in version
> -   control.
> +.. TODO(stephenfin): Remove the below notice once everything is
> converted to
> +   rST
> +
> +The following man pages are written in rST and converted to roff at
> compile
> +time:
> +
> +.. toctree::
> +   :maxdepth: 3
> +
> +   ovs-vlan-test.8
> +
> +The remainder are still in roff format can be found below:
>  
>  .. list-table::
>  
> diff --git a/Documentation/ref/ovs-vlan-test.8.rst
> b/Documentation/ref/ovs-vlan-test.8.rst
> new file mode 100644
> index 000000000000..c3066917f00d
> --- /dev/null
> +++ b/Documentation/ref/ovs-vlan-test.8.rst
> @@ -0,0 +1,115 @@
> +=============
> +ovs-vlan-test
> +=============
> +
> +Synopsis
> +========
> +
> +**ovs-vlan-test** [**-s** | **--server**] *control_ip* *vlan_ip*

This results in better man page output at the expense of the HTML
output (which is no longer monospaced as you'd expect). The 'code-
block' directive would probably help here, but that causes issues for
some users. Guess we'll just live with it for now.

> +Description
> +===========
> +
> +.. TODO(stephenfin): Add the `:program:` prefixes to `ovs-test` once
> that doc
> +   is converted
> +
> +The :program:`ovs-vlan-test` utility has some limitations, for
> example, it does
> +not use TCP in its tests. Also it does not take into account MTU to
> detect
> +potential edge cases. To overcome those limitations a new tool was
> developed -
> +`ovs-test`. `ovs-test` is currently supported only on Debian so, if
> possible,
> +try to use that on instead of :program:`ovs-vlan-test`.
> +
> +The :program:`ovs-vlan-test` program may be used to check for
> problems sending
> +802.1Q traffic which may occur when running Open vSwitch. These
> problems can
> +occur when Open vSwitch is used to send 802.1Q traffic through
> physical
> +interfaces running certain drivers of certain Linux kernel versions.
> To run a
> +test, configure Open vSwitch to tag traffic originating from
> `vlan_ip` and
> +forward it out the target interface. Then run the :program:`ovs-
> vlan-test` in
> +client mode connecting to an :program:`ovs-vlan-test` server.
> +:program:`ovs-vlan-test` will display "OK" if it did not detect
> problems.
> +
> +Some examples of the types of problems that may be encountered are:
> +
> +- When NICs use VLAN stripping on receive they must pass a pointer
> to a
> +  `vlan_group` when reporting the stripped tag to the networking
> core. If no
> +  `vlan_group` is in use then some drivers just drop the extracted
> tag.
> +  Drivers are supposed to only enable stripping if a `vlan_group` is
> registered
> +  but not all of them do that.
> +
> +- On receive, some drivers handle priority tagged packets specially
> and don't
> +  pass the tag onto the network stack at all, so Open vSwitch never
> has a
> +  chance to see it.
> +
> +- Some drivers size their receive buffers based on whether a
> `vlan_group` is
> +  enabled, meaning that a maximum size packet with a VLAN tag will
> not fit if
> +  no `vlan_group` is configured.
> +
> +- On transmit, some drivers expect that VLAN acceleration will be
> used if it is
> +  available, which can only be done if a `vlan_group` is configured.
> In these
> +  cases, the driver may fail to parse the packet and correctly setup
> checksum
> +  offloading or TSO.
> +
> +Client Mode
> +  An :program:`ovs-vlan-test` client may be run on a host to check
> for VLAN
> +  connectivity problems. The client must be able to establish HTTP
> connections
> +  with an :program:`ovs-vlan-test` server located at the specified
> `control_ip`
> +  address. UDP traffic sourced at `vlan_ip` should be tagged and
> directed out
> +  the interface whose connectivity is being tested.
> +
> +Server Mode
> +  To conduct tests, an :program:`ovs-vlan-test` server must be
> running on a
> +  host known not to have VLAN connectivity problems. The server must
> have a
> +  `control_ip` on a non-VLAN network which clients can establish
> connectivity
> +  with. It must also have a `vlan_ip` address on a VLAN network
> which clients
> +  will use to test their VLAN connectivity. Multiple clients may
> test against a
> +  single :program:`ovs-vlan-test` server concurrently.
> +
> +Options
> +=======
> +
> +.. program:: ovs-vlan-test
> +
> +.. option:: -s, --server
> +
> +    Run in server mode.
> +
> +.. option:: -h, --help
> +
> +    Prints a brief help message to the console.
> +
> +.. option:: -V, --version
> +
> +    Prints version information to the console.
> +
> +Examples
> +========
> +
> +Display the Linux kernel version and driver of `eth1`::
> +
> +   uname -r
> +   ethtool -i eth1
> +
> +Set up a bridge which forwards traffic originating from `1.2.3.4`
> out `eth1`
> +with VLAN tag 10::
> +
> +    ovs-vsctl -- add-br vlan-br \
> +      -- add-port vlan-br eth1 \
> +      -- add-port vlan-br vlan-br-tag tag=10 \
> +      -- set Interface vlan-br-tag type=internal
> +    ifconfig vlan-br-tag up 1.2.3.4
> +
> +Run an :program:`ovs-vlan-test` server listening for client control
> traffic on
> +`172.16.0.142` port `8080` and VLAN traffic on the default port of
> `1.2.3.3`::
> +
> +    ovs-vlan-test -s 172.16.0.142:8080 1.2.3.3
> +
> +Run an :program:`ovs-vlan-test` client with a control server located
> at
> +`172.16.0.142` port `8080` and a local VLAN IP of `1.2.3.4`::
> +
> +    ovs-vlan-test 172.16.0.142:8080 1.2.3.4
> +
> +See Also
> +========
> +
> +`ovs-vswitchd(8)`, `ovs-ofctl(8)`, `ovs-vsctl(8)`, `ovs-test(8)`,
> `ethtool(8)`,
> +`uname(1)`
> diff --git a/debian/openvswitch-switch.manpages b/debian/openvswitch-
> switch.manpages
> index a3aae642d8d1..a2f661a3e58c 100644
> --- a/debian/openvswitch-switch.manpages
> +++ b/debian/openvswitch-switch.manpages
> @@ -5,7 +5,6 @@ utilities/ovs-dpctl.8
>  utilities/ovs-pcap.1
>  utilities/ovs-tcpdump.8
>  utilities/ovs-tcpundump.1
> -utilities/ovs-vlan-test.8
>  utilities/ovs-vsctl.8
>  vswitchd/ovs-vswitchd.8
>  vswitchd/ovs-vswitchd.conf.db.5
> diff --git a/manpages.mk b/manpages.mk
> index 742bd66cdc32..372d06cad0a4 100644
> --- a/manpages.mk
> +++ b/manpages.mk
> @@ -244,16 +244,6 @@ utilities/ovs-vlan-bug-workaround.8.in:
>  lib/common.man:
>  utilities/ovs-vlan-bugs.man:
>  
> -utilities/ovs-vlan-test.8: \
> -	utilities/ovs-vlan-test.8.in \
> -	lib/common-syn.man \
> -	lib/common.man \
> -	utilities/ovs-vlan-bugs.man
> -utilities/ovs-vlan-test.8.in:
> -lib/common-syn.man:
> -lib/common.man:
> -utilities/ovs-vlan-bugs.man:
> -
>  utilities/ovs-vsctl.8: \
>  	utilities/ovs-vsctl.8.in \
>  	lib/common.man \
> diff --git a/utilities/automake.mk b/utilities/automake.mk
> index 169388fda5ec..02531f6dd3d6 100644
> --- a/utilities/automake.mk
> +++ b/utilities/automake.mk
> @@ -72,7 +72,6 @@ MAN_ROOTS += \
>  	utilities/ovs-tcpundump.1.in \
>  	utilities/ovs-vlan-bug-workaround.8.in \
>  	utilities/ovs-test.8.in \
> -	utilities/ovs-vlan-test.8.in \
>  	utilities/ovs-vsctl.8.in
>  MAN_FRAGMENTS += utilities/ovs-vlan-bugs.man
>  CLEANFILES += \
> @@ -101,7 +100,6 @@ CLEANFILES += \
>  	utilities/ovs-test \
>  	utilities/ovs-test.8 \
>  	utilities/ovs-vlan-test \
> -	utilities/ovs-vlan-test.8 \
>  	utilities/ovs-vlan-bug-workaround.8 \
>  	utilities/ovs-vsctl.8
>  
> @@ -120,7 +118,6 @@ man_MANS += \
>  	utilities/ovs-tcpundump.1 \
>  	utilities/ovs-vlan-bug-workaround.8 \
>  	utilities/ovs-test.8 \
> -	utilities/ovs-vlan-test.8 \
>  	utilities/ovs-vsctl.8
>  
>  utilities_ovs_appctl_SOURCES = utilities/ovs-appctl.c
> diff --git a/utilities/ovs-vlan-test.8.in b/utilities/ovs-vlan-
> test.8.in
> deleted file mode 100644
> index 2cdeff832104..000000000000
> --- a/utilities/ovs-vlan-test.8.in
> +++ /dev/null
> @@ -1,96 +0,0 @@
> -.de IQ
> -.  br
> -.  ns
> -.  IP "\\$1"
> -..
> -.TH ovs\-vlan\-test 1 "@VERSION@" "Open vSwitch" "Open vSwitch
> Manual"
> -.
> -.SH NAME
> -\fBovs\-vlan\-test\fR \- check Linux drivers for problems with vlan
> traffic
> -.
> -.SH SYNOPSIS
> -\fBovs\-vlan\-test\fR [\fB\-s\fR | \fB\-\-server\fR]
> \fIcontrol_ip\fR \fIvlan_ip\fR
> -.so lib/common-syn.man
> -.
> -.SH DESCRIPTION
> -The \fBovs\-vlan\-test\fR utility has some limitations, for example,
> it does
> -not use TCP in its tests. Also it does not take into account MTU to
> detect
> -potential edge cases. To overcome those limitations a new tool was
> -developed \- \fBovs\-test\fR. \fBovs\-test\fR is currently supported
> only
> -on Debian so, if possible try to use that on instead of \fBovs\-
> vlan\-test\fR.
> -.PP
> -The \fBovs\-vlan\-test\fR program may be used to check for problems
> sending
> -802.1Q traffic which may occur when running Open vSwitch. These
> problems can
> -occur when Open vSwitch is used to send 802.1Q traffic through
> physical
> -interfaces running certain drivers of certain Linux kernel versions.
> To run a
> -test, configure Open vSwitch to tag traffic originating from
> \fIvlan_ip\fR and
> -forward it out the target interface. Then run the \fBovs\-vlan\-
> test\fR in
> -client mode connecting to an \fBovs\-vlan\-test\fR server.
> -\fBovs\-vlan\-test\fR will display "OK" if it did not detect
> problems.
> -.PP
> -Some examples of the types of problems that may be encountered are:
> -.so utilities/ovs-vlan-bugs.man
> -.
> -.SS "Client Mode"
> -An \fBovs\-vlan\-test\fR client may be run on a host to check for
> VLAN
> -connectivity problems.  The client must be able to establish HTTP
> connections
> -with an \fBovs\-vlan\-test\fR server located at the specified
> \fIcontrol_ip\fR
> -address.  UDP traffic sourced at \fIvlan_ip\fR should be tagged and
> directed out
> -the interface whose connectivity is being tested.
> -.
> -.SS "Server Mode"
> -To conduct tests, an \fBovs\-vlan\-test\fR server must be running on
> a host
> -known not to have VLAN connectivity problems.  The server must have
> a
> -\fIcontrol_ip\fR on a non\-VLAN network which clients can establish
> -connectivity with.  It must also have a \fIvlan_ip\fR address on a
> VLAN network
> -which clients will use to test their VLAN connectivity.  Multiple
> clients may
> -test against a single \fBovs\-vlan\-test\fR server concurrently.
> -.
> -.SH OPTIONS
> -.
> -.IP "\fB\-s\fR"
> -.IQ "\fB\-\-server\fR"
> -Run in server mode.
> -.
> -.so lib/common.man
> -.SH EXAMPLES
> -Display the Linux kernel version and driver of \fBeth1\fR.
> -.IP
> -.B uname \-r
> -.IP
> -.B ethtool \-i eth1
> -.
> -.PP
> -Set up a bridge which forwards traffic originating from
> \fB1.2.3.4\fR out
> -\fBeth1\fR with VLAN tag 10.
> -.IP
> -.B ovs\-vsctl \-\- add\-br vlan\-br \(rs
> -.IP
> -.B \-\- add\-port vlan\-br eth1 \(rs
> -.IP
> -.B \-\- add\-port vlan\-br vlan\-br\-tag tag=10 \(rs
> -.IP
> -.B \-\- set Interface vlan\-br\-tag type=internal
> -.IP
> -.B ifconfig vlan\-br\-tag up 1.2.3.4
> -.
> -.PP
> -Run an \fBovs\-vlan\-test\fR server listening for client control
> traffic on
> -172.16.0.142 port 8080 and VLAN traffic on the default port of
> 1.2.3.3.
> -.IP
> -.B ovs\-vlan\-test \-s 172.16.0.142:8080 1.2.3.3
> -.
> -.PP
> -Run an \fBovs\-vlan\-test\fR client with a control server located at
> -172.16.0.142 port 8080 and a local VLAN ip of 1.2.3.4.
> -.IP
> -.B ovs\-vlan\-test 172.16.0.142:8080 1.2.3.4
> -.
> -.SH SEE ALSO
> -.
> -.BR ovs\-vswitchd (8),
> -.BR ovs\-ofctl (8),
> -.BR ovs\-vsctl (8),
> -.BR ovs\-test (8),
> -.BR ethtool (8),
> -.BR uname (1)

This is all fine by me.
Ben Pfaff April 21, 2017, 9:26 p.m. | #2
On Tue, Apr 18, 2017 at 11:58:27AM +0100, Stephen Finucane wrote:
> On Thu, 2017-04-13 at 21:43 -0700, Ben Pfaff wrote:
> > From: Stephen Finucane <stephen@that.guru>
> > 
> > Let's start with a simple one that lets us focus on setting up most
> > of
> > the required "infrastructure" for building man pages using Sphinx.
> > 
> > This changes the 'check-htmldocs' target to 'check-docs' as its now
> > responsible for building man page docs too.
> > 
> > Other than that, hurrah for (mostly) legible syntaxes.
> > 
> > [1] http://www.tldp.org/HOWTO/Man-Page/q2.html
> 
> This mostly looks good to me. Some comments below but nothing I'd block
> on.
> 
> Acked-by: Stephen Finucane <stephen@that.guru>

Thanks!
> > +
> 
> I might have done something funky applying this, but I can see a '^L'
> character (linefeed?) on this line. Might be worth watching for.

^L is a form-feed.  We often use it to divide files into logical
sections, mostly in C files but there are some other examples in the
repo in tests/automake.mk, in some .at files, and in some Python files.

(I originally picked up this habit decades ago from reading the GNU
coding standards.)

> > +UNINSTALL_LOCAL += uninstall-man-rst
> > +uninstall-man-rst:
> > +	@$(set_mandirs); \
> > +	for rst in $(RST_MANPAGES); do \
> > +	    $(extract_stem_and_section); \
> > +	    echo "rm -f '$(DESTDIR)'\"$$mandir/$$stem.$$section\"";
> > \
> > +	    rm -f '$(DESTDIR)'"$$mandir/$$stem.$$section"; \
> > +	done
> 
> This whole section is still mostly noise to me and I would love to
> simplify it somehow. I'm not ready to do that yet though and so long as
> I don't have to maintain this (:)), I can live with it as-is. Might be
> good to get someone else with more autotools know-how to look at it
> though?

Possibly that's a good idea but it's hard to get anyone to look at this
kind of thing.

I'm willing to maintain it.

> > --- /dev/null
> > +++ b/Documentation/ref/ovs-vlan-test.8.rst
> > @@ -0,0 +1,115 @@
> > +=============
> > +ovs-vlan-test
> > +=============
> > +
> > +Synopsis
> > +========
> > +
> > +**ovs-vlan-test** [**-s** | **--server**] *control_ip* *vlan_ip*
> 
> This results in better man page output at the expense of the HTML
> output (which is no longer monospaced as you'd expect). The 'code-
> block' directive would probably help here, but that causes issues for
> some users. Guess we'll just live with it for now.

Oh, that's an interesting point.  The reason I moved it to the left
margin was that it looked like it was indented wrong in the output
anyway.

Let's keep this in mind for the future.

Patch

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 9911668c1ca9..762255277102 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -90,7 +90,8 @@  DOC_SOURCE = \
 	Documentation/internals/contributing/documentation-style.rst \
 	Documentation/internals/contributing/libopenvswitch-abi.rst \
 	Documentation/internals/contributing/submitting-patches.rst \
-	Documentation/requirements.txt
+	Documentation/requirements.txt \
+	$(addprefix Documentation/ref/,$(RST_MANPAGES))
 
 EXTRA_DIST += $(DOC_SOURCE)
 
@@ -110,20 +111,89 @@  sphinx_verbose_ = $(sphinx_verbose_@AM_DEFAULT_V@)
 sphinx_verbose_0 = -q
 
 if HAVE_SPHINX
-htmldocs-check: $(DOC_SOURCE)
+docs-check: $(DOC_SOURCE)
 	$(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b html $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html && touch $@
-ALL_LOCAL += htmldocs-check
-CLEANFILES += htmldocs-check
+	$(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b man $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/man && touch $@
+ALL_LOCAL += docs-check
+CLEANFILES += docs-check
 
 check-docs:
 	$(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
 
 clean-docs:
-	rm -rf $(SPHINXBUILDDIR)/doctrees
-	rm -rf $(SPHINXBUILDDIR)/html
-	rm -rf $(SPHINXBUILDDIR)/linkcheck
+	rm -rf $(SPHINXBUILDDIR)
 	rm -f docs-check
 CLEAN_LOCAL += clean-docs
 endif
 .PHONY: check-docs
 .PHONY: clean-docs
+
+# Installing manpages based on rST.
+#
+# The docs-check target converts the rST files listed in RST_MANPAGES
+# into nroff manpages in Documentation/_build/man.  The easiest way to
+# get these installed by "make install" is to write our own helper
+# rules.
+
+# rST formatted manpages under Documentation/ref.
+RST_MANPAGES = \
+	ovs-vlan-test.8.rst
+
+# The GNU standards say that these variables should control
+# installation directories for manpages in each section.  Automake
+# will define them for us only if it sees that a manpage in the
+# appropriate section is to be installed through its built-in feature.
+# Since we're working independently, for best safety, we need to
+# define them ourselves.
+man1dir = $(mandir)/man1
+man2dir = $(mandir)/man2
+man3dir = $(mandir)/man3
+man4dir = $(mandir)/man4
+man5dir = $(mandir)/man5
+man6dir = $(mandir)/man6
+man7dir = $(mandir)/man7
+man8dir = $(mandir)/man8
+man9dir = $(mandir)/man9
+
+# Set a shell variable for each manpage directory.
+set_mandirs = \
+	man1dir='$(man1dir)' \
+	man2dir='$(man2dir)' \
+	man3dir='$(man3dir)' \
+	man4dir='$(man4dir)' \
+	man5dir='$(man5dir)' \
+	man6dir='$(man6dir)' \
+	man7dir='$(man7dir)' \
+	man8dir='$(man8dir)' \
+	man9dir='$(man9dir)'
+
+# Given an $rst of "ovs-vlan-test.8.rst", sets $stem to
+# "ovs-vlan-test", $section to "8", and $mandir to $man8dir.
+extract_stem_and_section = \
+	stem=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-9]\).rst$$/\1/p'`; \
+	section=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-9]\).rst$$/\2/p'`; \
+	test -n "$$section" || { echo "$$rst: cannot infer manpage section from filename" 2>&1; continue; }; \
+	eval "mandir=\$$man$${section}dir"; \
+	test -n "$$mandir" || { echo "unknown directory for manpage section $$section"; continue; }
+
+if HAVE_SPHINX
+INSTALL_DATA_LOCAL += install-man-rst
+install-man-rst: docs-check
+	@$(set_mandirs); \
+	for rst in $(RST_MANPAGES); do \
+	    $(extract_stem_and_section); \
+	    echo " $(MKDIR_P) '$(DESTDIR)'\"$$mandir\""; \
+	    $(MKDIR_P) '$(DESTDIR)'"$$mandir"; \
+	    echo " $(INSTALL_DATA) $(SPHINXBUILDDIR)/man/$$stem.$$section '$(DESTDIR)'\"$$mandir/$$stem.$$section\""; \
+	    $(INSTALL_DATA) $(SPHINXBUILDDIR)/man/$$stem.$$section '$(DESTDIR)'"$$mandir/$$stem.$$section"; \
+	done
+endif
+
+UNINSTALL_LOCAL += uninstall-man-rst
+uninstall-man-rst:
+	@$(set_mandirs); \
+	for rst in $(RST_MANPAGES); do \
+	    $(extract_stem_and_section); \
+	    echo "rm -f '$(DESTDIR)'\"$$mandir/$$stem.$$section\""; \
+	    rm -f '$(DESTDIR)'"$$mandir/$$stem.$$section"; \
+	done
diff --git a/Documentation/conf.py b/Documentation/conf.py
index bfd7f33d88ba..8671a097d17e 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -328,8 +328,9 @@  latex_documents = [
 # One entry per manual page. List of tuples
 # (source start file, name, description, authors, manual section).
 man_pages = [
-    (master_doc, 'openvswitch', u'Open vSwitch Documentation',
-     [author], 1)
+    ('ref/ovs-vlan-test.8', 'ovs-vlan-test',
+     u'Check Linux drivers for problems with vlan traffic',
+     [author], 8)
 ]
 
 # If true, show URL addresses after external links.
diff --git a/Documentation/internals/contributing/documentation-style.rst b/Documentation/internals/contributing/documentation-style.rst
index 0ba5e544f14e..0e561ec23f10 100644
--- a/Documentation/internals/contributing/documentation-style.rst
+++ b/Documentation/internals/contributing/documentation-style.rst
@@ -347,7 +347,8 @@  In addition to the above, man pages have some specific requirements:
 - The man page must be included in the list of man page documents found in
   `conf.py`__
 
-Refer to existing man pages for worked examples.
+Refer to existing man pages, such as :doc:`/ref/ovs-vlan-test.8` for a worked
+example.
 
 __ http://www.sphinx-doc.org/en/stable/domains.html#directive-program
 __ http://www.sphinx-doc.org/en/stable/domains.html#directive-option
diff --git a/Documentation/intro/install/documentation.rst b/Documentation/intro/install/documentation.rst
index 94af950afc3e..0eeeab15226f 100644
--- a/Documentation/intro/install/documentation.rst
+++ b/Documentation/intro/install/documentation.rst
@@ -74,11 +74,11 @@  Building
 Once Sphinx installed, the documentation can be built using the provided
 Makefile targets::
 
-    $ make htmldocs-check
+    $ make docs-check
 
 .. important::
 
-   The ``htmldocs-check`` target will fail if there are any syntax errors.
+   The ``docs-check`` target will fail if there are any syntax errors.
    However, it won't catch more succint issues such as style or grammar issues.
    As a result, you should always inspect changes visually to ensure the result
    is as intended.
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 49121b7599fd..6b368809c11b 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -30,10 +30,18 @@  Reference Guide
 Man Pages
 ---------
 
-.. TODO(stephenfin): Investigate some way to get the manpages into rST format.
-   The most viable option seems to be writing them all in rST, converting them
-   to roff format and storing both the rST and roff formatted docs in version
-   control.
+.. TODO(stephenfin): Remove the below notice once everything is converted to
+   rST
+
+The following man pages are written in rST and converted to roff at compile
+time:
+
+.. toctree::
+   :maxdepth: 3
+
+   ovs-vlan-test.8
+
+The remainder are still in roff format can be found below:
 
 .. list-table::
 
diff --git a/Documentation/ref/ovs-vlan-test.8.rst b/Documentation/ref/ovs-vlan-test.8.rst
new file mode 100644
index 000000000000..c3066917f00d
--- /dev/null
+++ b/Documentation/ref/ovs-vlan-test.8.rst
@@ -0,0 +1,115 @@ 
+=============
+ovs-vlan-test
+=============
+
+Synopsis
+========
+
+**ovs-vlan-test** [**-s** | **--server**] *control_ip* *vlan_ip*
+
+Description
+===========
+
+.. TODO(stephenfin): Add the `:program:` prefixes to `ovs-test` once that doc
+   is converted
+
+The :program:`ovs-vlan-test` utility has some limitations, for example, it does
+not use TCP in its tests. Also it does not take into account MTU to detect
+potential edge cases. To overcome those limitations a new tool was developed -
+`ovs-test`. `ovs-test` is currently supported only on Debian so, if possible,
+try to use that on instead of :program:`ovs-vlan-test`.
+
+The :program:`ovs-vlan-test` program may be used to check for problems sending
+802.1Q traffic which may occur when running Open vSwitch. These problems can
+occur when Open vSwitch is used to send 802.1Q traffic through physical
+interfaces running certain drivers of certain Linux kernel versions. To run a
+test, configure Open vSwitch to tag traffic originating from `vlan_ip` and
+forward it out the target interface. Then run the :program:`ovs-vlan-test` in
+client mode connecting to an :program:`ovs-vlan-test` server.
+:program:`ovs-vlan-test` will display "OK" if it did not detect problems.
+
+Some examples of the types of problems that may be encountered are:
+
+- When NICs use VLAN stripping on receive they must pass a pointer to a
+  `vlan_group` when reporting the stripped tag to the networking core. If no
+  `vlan_group` is in use then some drivers just drop the extracted tag.
+  Drivers are supposed to only enable stripping if a `vlan_group` is registered
+  but not all of them do that.
+
+- On receive, some drivers handle priority tagged packets specially and don't
+  pass the tag onto the network stack at all, so Open vSwitch never has a
+  chance to see it.
+
+- Some drivers size their receive buffers based on whether a `vlan_group` is
+  enabled, meaning that a maximum size packet with a VLAN tag will not fit if
+  no `vlan_group` is configured.
+
+- On transmit, some drivers expect that VLAN acceleration will be used if it is
+  available, which can only be done if a `vlan_group` is configured. In these
+  cases, the driver may fail to parse the packet and correctly setup checksum
+  offloading or TSO.
+
+Client Mode
+  An :program:`ovs-vlan-test` client may be run on a host to check for VLAN
+  connectivity problems. The client must be able to establish HTTP connections
+  with an :program:`ovs-vlan-test` server located at the specified `control_ip`
+  address. UDP traffic sourced at `vlan_ip` should be tagged and directed out
+  the interface whose connectivity is being tested.
+
+Server Mode
+  To conduct tests, an :program:`ovs-vlan-test` server must be running on a
+  host known not to have VLAN connectivity problems. The server must have a
+  `control_ip` on a non-VLAN network which clients can establish connectivity
+  with. It must also have a `vlan_ip` address on a VLAN network which clients
+  will use to test their VLAN connectivity. Multiple clients may test against a
+  single :program:`ovs-vlan-test` server concurrently.
+
+Options
+=======
+
+.. program:: ovs-vlan-test
+
+.. option:: -s, --server
+
+    Run in server mode.
+
+.. option:: -h, --help
+
+    Prints a brief help message to the console.
+
+.. option:: -V, --version
+
+    Prints version information to the console.
+
+Examples
+========
+
+Display the Linux kernel version and driver of `eth1`::
+
+   uname -r
+   ethtool -i eth1
+
+Set up a bridge which forwards traffic originating from `1.2.3.4` out `eth1`
+with VLAN tag 10::
+
+    ovs-vsctl -- add-br vlan-br \
+      -- add-port vlan-br eth1 \
+      -- add-port vlan-br vlan-br-tag tag=10 \
+      -- set Interface vlan-br-tag type=internal
+    ifconfig vlan-br-tag up 1.2.3.4
+
+Run an :program:`ovs-vlan-test` server listening for client control traffic on
+`172.16.0.142` port `8080` and VLAN traffic on the default port of `1.2.3.3`::
+
+    ovs-vlan-test -s 172.16.0.142:8080 1.2.3.3
+
+Run an :program:`ovs-vlan-test` client with a control server located at
+`172.16.0.142` port `8080` and a local VLAN IP of `1.2.3.4`::
+
+    ovs-vlan-test 172.16.0.142:8080 1.2.3.4
+
+See Also
+========
+
+`ovs-vswitchd(8)`, `ovs-ofctl(8)`, `ovs-vsctl(8)`, `ovs-test(8)`, `ethtool(8)`,
+`uname(1)`
diff --git a/debian/openvswitch-switch.manpages b/debian/openvswitch-switch.manpages
index a3aae642d8d1..a2f661a3e58c 100644
--- a/debian/openvswitch-switch.manpages
+++ b/debian/openvswitch-switch.manpages
@@ -5,7 +5,6 @@  utilities/ovs-dpctl.8
 utilities/ovs-pcap.1
 utilities/ovs-tcpdump.8
 utilities/ovs-tcpundump.1
-utilities/ovs-vlan-test.8
 utilities/ovs-vsctl.8
 vswitchd/ovs-vswitchd.8
 vswitchd/ovs-vswitchd.conf.db.5
diff --git a/manpages.mk b/manpages.mk
index 742bd66cdc32..372d06cad0a4 100644
--- a/manpages.mk
+++ b/manpages.mk
@@ -244,16 +244,6 @@  utilities/ovs-vlan-bug-workaround.8.in:
 lib/common.man:
 utilities/ovs-vlan-bugs.man:
 
-utilities/ovs-vlan-test.8: \
-	utilities/ovs-vlan-test.8.in \
-	lib/common-syn.man \
-	lib/common.man \
-	utilities/ovs-vlan-bugs.man
-utilities/ovs-vlan-test.8.in:
-lib/common-syn.man:
-lib/common.man:
-utilities/ovs-vlan-bugs.man:
-
 utilities/ovs-vsctl.8: \
 	utilities/ovs-vsctl.8.in \
 	lib/common.man \
diff --git a/utilities/automake.mk b/utilities/automake.mk
index 169388fda5ec..02531f6dd3d6 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -72,7 +72,6 @@  MAN_ROOTS += \
 	utilities/ovs-tcpundump.1.in \
 	utilities/ovs-vlan-bug-workaround.8.in \
 	utilities/ovs-test.8.in \
-	utilities/ovs-vlan-test.8.in \
 	utilities/ovs-vsctl.8.in
 MAN_FRAGMENTS += utilities/ovs-vlan-bugs.man
 CLEANFILES += \
@@ -101,7 +100,6 @@  CLEANFILES += \
 	utilities/ovs-test \
 	utilities/ovs-test.8 \
 	utilities/ovs-vlan-test \
-	utilities/ovs-vlan-test.8 \
 	utilities/ovs-vlan-bug-workaround.8 \
 	utilities/ovs-vsctl.8
 
@@ -120,7 +118,6 @@  man_MANS += \
 	utilities/ovs-tcpundump.1 \
 	utilities/ovs-vlan-bug-workaround.8 \
 	utilities/ovs-test.8 \
-	utilities/ovs-vlan-test.8 \
 	utilities/ovs-vsctl.8
 
 utilities_ovs_appctl_SOURCES = utilities/ovs-appctl.c
diff --git a/utilities/ovs-vlan-test.8.in b/utilities/ovs-vlan-test.8.in
deleted file mode 100644
index 2cdeff832104..000000000000
--- a/utilities/ovs-vlan-test.8.in
+++ /dev/null
@@ -1,96 +0,0 @@ 
-.de IQ
-.  br
-.  ns
-.  IP "\\$1"
-..
-.TH ovs\-vlan\-test 1 "@VERSION@" "Open vSwitch" "Open vSwitch Manual"
-.
-.SH NAME
-\fBovs\-vlan\-test\fR \- check Linux drivers for problems with vlan traffic
-.
-.SH SYNOPSIS
-\fBovs\-vlan\-test\fR [\fB\-s\fR | \fB\-\-server\fR] \fIcontrol_ip\fR \fIvlan_ip\fR
-.so lib/common-syn.man
-.
-.SH DESCRIPTION
-The \fBovs\-vlan\-test\fR utility has some limitations, for example, it does
-not use TCP in its tests. Also it does not take into account MTU to detect
-potential edge cases. To overcome those limitations a new tool was
-developed \- \fBovs\-test\fR. \fBovs\-test\fR is currently supported only
-on Debian so, if possible try to use that on instead of \fBovs\-vlan\-test\fR.
-.PP
-The \fBovs\-vlan\-test\fR program may be used to check for problems sending
-802.1Q traffic which may occur when running Open vSwitch. These problems can
-occur when Open vSwitch is used to send 802.1Q traffic through physical
-interfaces running certain drivers of certain Linux kernel versions. To run a
-test, configure Open vSwitch to tag traffic originating from \fIvlan_ip\fR and
-forward it out the target interface. Then run the \fBovs\-vlan\-test\fR in
-client mode connecting to an \fBovs\-vlan\-test\fR server.
-\fBovs\-vlan\-test\fR will display "OK" if it did not detect problems.
-.PP
-Some examples of the types of problems that may be encountered are:
-.so utilities/ovs-vlan-bugs.man
-.
-.SS "Client Mode"
-An \fBovs\-vlan\-test\fR client may be run on a host to check for VLAN
-connectivity problems.  The client must be able to establish HTTP connections
-with an \fBovs\-vlan\-test\fR server located at the specified \fIcontrol_ip\fR
-address.  UDP traffic sourced at \fIvlan_ip\fR should be tagged and directed out
-the interface whose connectivity is being tested.
-.
-.SS "Server Mode"
-To conduct tests, an \fBovs\-vlan\-test\fR server must be running on a host
-known not to have VLAN connectivity problems.  The server must have a
-\fIcontrol_ip\fR on a non\-VLAN network which clients can establish
-connectivity with.  It must also have a \fIvlan_ip\fR address on a VLAN network
-which clients will use to test their VLAN connectivity.  Multiple clients may
-test against a single \fBovs\-vlan\-test\fR server concurrently.
-.
-.SH OPTIONS
-.
-.IP "\fB\-s\fR"
-.IQ "\fB\-\-server\fR"
-Run in server mode.
-.
-.so lib/common.man
-.SH EXAMPLES
-Display the Linux kernel version and driver of \fBeth1\fR.
-.IP
-.B uname \-r
-.IP
-.B ethtool \-i eth1
-.
-.PP
-Set up a bridge which forwards traffic originating from \fB1.2.3.4\fR out
-\fBeth1\fR with VLAN tag 10.
-.IP
-.B ovs\-vsctl \-\- add\-br vlan\-br \(rs
-.IP
-.B \-\- add\-port vlan\-br eth1 \(rs
-.IP
-.B \-\- add\-port vlan\-br vlan\-br\-tag tag=10 \(rs
-.IP
-.B \-\- set Interface vlan\-br\-tag type=internal
-.IP
-.B ifconfig vlan\-br\-tag up 1.2.3.4
-.
-.PP
-Run an \fBovs\-vlan\-test\fR server listening for client control traffic on
-172.16.0.142 port 8080 and VLAN traffic on the default port of 1.2.3.3.
-.IP
-.B ovs\-vlan\-test \-s 172.16.0.142:8080 1.2.3.3
-.
-.PP
-Run an \fBovs\-vlan\-test\fR client with a control server located at
-172.16.0.142 port 8080 and a local VLAN ip of 1.2.3.4.
-.IP
-.B ovs\-vlan\-test 172.16.0.142:8080 1.2.3.4
-.
-.SH SEE ALSO
-.
-.BR ovs\-vswitchd (8),
-.BR ovs\-ofctl (8),
-.BR ovs\-vsctl (8),
-.BR ovs\-test (8),
-.BR ethtool (8),
-.BR uname (1)