I have pushed this and patches 2 and 3 to main. Thanks!

On Fri, Aug 14, 2026 at 4:09 PM Mark Michelson <[email protected]> wrote:
>
> Hi Rosemarie, thanks for the update.
>
> Acked-by: Mark Michelson <[email protected]>
>
> I plan to make one small edit when merging this. See below:
>
> On Wed, Aug 12, 2026 at 6:26 PM Rosemarie O'Riorden via dev
> <[email protected]> wrote:
> >
> > Fix various typos, inaccuracies, and missing information in "Testing"
> > and "Test Development".
> >
> > Reported-at: https://redhat.atlassian.net/browse/FDP-3051
> > Assisted-by: Claude Opus 4.6, Claude Code
> > Signed-off-by: Rosemarie O'Riorden <[email protected]>
> > ---
> > v2 -> v3:
> >  - Update title and description.
> >  - Fix ovn_start args as Mark recommended.
> >  - Make "+++"s under titles the correct length.
> >  - Add "no_recompute_check" option to OVN_CLEANUP_SBOX
> >  - Remove list of supported versions of OVN.
> > ---
> >  Documentation/topics/test-development.rst | 74 +++++++++++------------
> >  Documentation/topics/testing.rst          | 12 ++--
> >  Documentation/tutorials/ovn-sandbox.rst   |  2 +-
> >  3 files changed, 41 insertions(+), 47 deletions(-)
> >
> > diff --git a/Documentation/topics/test-development.rst 
> > b/Documentation/topics/test-development.rst
> > index 823957983..0dbd76bec 100644
> > --- a/Documentation/topics/test-development.rst
> > +++ b/Documentation/topics/test-development.rst
> > @@ -61,7 +61,7 @@ check COMMAND...
> >
> >  Function to run COMMAND and check that it succeeds without any output. Also
> >  logs the COMMAND. Note that most ``ovn-nbctl`` and ``ovn-sbctl`` must be 
> > run
> > -withing ``check`` so that the return status is checked.
> > +within ``check`` so that the return status is checked.
> >
> >  OVN_CHECK_PACKETS([PCAP], [EXPECTED])
> >  +++++++++++++++++++++++++++++++++++++
> > @@ -102,8 +102,8 @@ uuid as output. It also fails if the output is empty.
> >  Daemon/Sandbox Management
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > -ovn_start [--backup-northd=none|paused] [AZ]
> > -++++++++++++++++++++++++++++++++++++++++++++
> > +ovn_start [--backup-northd[=paused] | --use-tcp-to-sb] [AZ]
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >
> >  Creates and initializes ovn-sb and ovn-nb databases and starts their
> >  ovsdb-server instance, sets appropriate environment variables so that 
> > ovn-sbctl
> > @@ -129,7 +129,8 @@ ovn_attach NETWORK BRIDGE IP [MASKLEN] [ENCAP]
> >
> >  First, this function attaches BRIDGE to interconnection network NETWORK.
> >  Second, it configures (simulated) address IP (with network mask length 
> > MASKLEN,
> > -which defaults to 24) on BRIDGE. Finally, it configures the Open vSwitch
> > +which defaults to 24) on BRIDGE. ENCAP specifies the tunnel encapsulation 
> > types
> > +(defaults to ``geneve,vxlan``). Finally, it configures the Open vSwitch
> >  database to work with OVN and starts ovn-controller.
> >
> >  sim_add SANDBOX
> > @@ -156,7 +157,7 @@ or::
> >  as [OVS_DIR] COMMAND
> >  ++++++++++++++++++++
> >
> > -``as $1`` sets the ``OVS_*DIR`` and ``OVN_*DIR*`` environment variables to
> > +``as $1`` sets the ``OVS_*DIR`` and ``OVN_*DIR`` environment variables to
> >  point to $ovs_base/$1.
> >
> >  ``as $1 COMMAND...`` sets those variables in a subshell and invokes COMMAND
> > @@ -187,36 +188,30 @@ OVN_POPULATE_ARP()
> >  ++++++++++++++++++
> >
> >  Macro to pre-populate the ARP tables of all of the OVN instances that have 
> > been
> > -started with ```ovn_attach()``. That means that packets sent from one
> > +started with ``ovn_attach()``. That means that packets sent from one
> >  hypervisor to another never get dropped or delayed by ARP resolution, which
> >  makes testing easier.
> >
> > -OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
> > -+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > +OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output])
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >
> > -Macro to create a database and starts ovsdb-server, starts ovs-vswitchd
> > -connected to that database, calls ovs-vsctl to create a bridge named br0 
> > with
> > -predictable settings, passing 'vsctl-args' as additional commands to
> > -ovs-vsctl. If 'vsctl-args' causes ovs-vsctl to provide output (e.g. 
> > because it
> > -includes "create" commands) then 'vsctl-output' specifies the expected 
> > output
> > -after filtering through uuidfilt.
> > -
> > -If a test needs to use "system" devices (as dummies), then specify
> > -``=override`` (literally) as the third argument. Otherwise, system devices
> > -won't work at all (which makes sense because tests should not access a 
> > system's
> > -real Ethernet devices).
> > +Macro to create a database and start ovsdb-server, start ovs-vswitchd 
> > connected
> > +to that database, and create a bridge named br0 with predictable settings.
> > +Additional ovs-vsctl commands can be passed via 'vsctl-args'. If those 
> > commands
> > +produce output (e.g. "create" commands), 'vsctl-output' specifies the 
> > expected
> > +output after filtering through uuidfilt.
> >
> >  OVS_TRAFFIC_VSWITCHD_STOP([WHITELIST], [extra_cmds])
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >
> > -Macro to gracefully stops ovs-vswitchd and ovsdb-server, checking their log
> > +Macro to gracefully stop ovs-vswitchd and ovsdb-server, checking their log
> >  files for messages with severity WARN or higher and signaling an error if 
> > any
> >  is present. The optional WHITELIST may contain shell-quoted "sed" commands 
> > to
> >  delete any warnings that are actually expected, e.g.::
> >
> >      OVS_TRAFFIC_VSWITCHD_STOP(["/expected error/d"])
> >
> > -'extra_cmds' are shell commands to be executed afte OVS_VSWITCHD_STOP() is
> > +'extra_cmds' are shell commands to be executed after OVS_VSWITCHD_STOP() is
> >  invoked. They can be used to perform additional cleanups such as name space
> >  removal.
> >
> > @@ -240,35 +235,34 @@ and after recompute are the same. Optional arguments 
> > may also contain
> >  acceptable ``related_ports`` differences, datapaths and tables on which 
> > flow
> >  differences are considered as acceptable.
> >
> > -OVN_CLEANUP_SBOX(sbox)
> > -++++++++++++++++++++++
> > +OVN_CLEANUP_SBOX(sbox[, error[, related_ports[, ignored_dp[, 
> > ignored_tables[, no_recompute_check]]]]])
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >
> >  Macro to gracefully terminate OVN daemons in the specified sandbox 
> > instance.
> >  The sandbox name ``vtep`` is treated as a special case, and is assumed to 
> > have
> >  ovn-controller-vtep and ovs-vtep daemons running instead of ovn-controller.
> >
> >  Also checks the log file for messages with severity WARN or
> > -higher and signals an error if any is present. Optional arguments may 
> > contain
> > -"acceptable" error messages.
> > +higher and signals an error if any is present. The 'error' argument may 
> > contain
> > +sed commands to delete acceptable error messages.
> >
> >  Before terminating the daemons, it also issues recomputes on 
> > ovn-controllers in
> > -listed sandboxes, and checks whether the related ports and the openflows 
> > before
> > -and after recompute are the same. Optional arguments may also contain
> > -acceptable ``related_ports`` differences, datapaths and tables on which 
> > flow
> > -differences are considered as acceptable.
> > +the sandbox, and checks whether the related ports and the openflows before
> > +and after recompute are the same. The 'related_ports', 'ignored_dp', and
> > +'ignored_tables' arguments specify acceptable differences.
> >
> > -OVN_CLEANUP_CONTROLLER(sbox)
> > -++++++++++++++++++++++++++++
> > +OVN_CLEANUP_CONTROLLER(hv[, sbox[, related_ports[, ignored_dp[, 
> > ignored_tables[, no_recompute_check]]]]])
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >
> >  Macro to gracefully terminate ovn-controller in the specified sandbox
> > -instance. The sandbox name ``vtep`` is treated as a special case, and is
> > -assumed to have ovn-controller-vtep and ovs-vtep daemons running instead of
> > -ovn-controller.
> > +instance. The 'hv' argument names the hypervisor; when it equals ``vtep``, 
> > the
> > +macro stops ovn-controller-vtep and ovs-vtep instead of ovn-controller. The
> > +'sbox' argument names the sandbox directory (defaults to 'hv' when 
> > omitted).
>
> This is a bit misleading. The 'sbox' argument does not have any sort
> of default value in OVN_CLEANUP_CONTROLLER. However, calling
> OVN_CLEANUP results in calling OVN_CLEANUP_CONTROLLER with both the
> 'hv' and 'sbox' arguments set to the same value. I plan to remove the
> parenthetical at the end mentioning default values.
>
> Interestingly, there are many places throughout our system tests where
> we directly call OVN_CLEANUP_CONTROLLER with only a single argument.
> This results in many cases where we call as() with no argument. I
> wonder if this could result in recompute checks not working as
> expected. That's potentially a separate issue though.
>
> >
> > -Issues recomputes on ovn-controllers in listed sandbox, and checks whether 
> > the
> > -related ports and the openflows before and after recompute are the same.
> > -Optional arguments may also contain acceptable ``related_ports`` 
> > differences,
> > -datapaths and tables on which flow differences are considered as 
> > acceptable.
> > +Unless 'no_recompute_check' is ``True``, issues recomputes on 
> > ovn-controller
> > +and checks whether the related ports and the openflows before and after
> > +recompute are the same. The 'related_ports', 'ignored_dp', and
> > +'ignored_tables' arguments specify acceptable differences.
> >
> >  OVN_CLEANUP_IC([az ...])
> >  ++++++++++++++++++++++++
> > @@ -314,7 +308,7 @@ RUN_OVN_NBCTL()
> >  +++++++++++++++
> >
> >  Macro to execute a list of commands built by the ``OVN_NBCTL`` macro. The 
> > list
> > -of commands is executed in a single invocation of ``ovn-nbctl``
> > +of commands is executed in a single invocation of ``ovn-nbctl``.
> >
> >  OVS_VSCTL(VSCTL_COMMAND)
> >  ++++++++++++++++++++++++
> > @@ -335,7 +329,7 @@ STDOUT on stdout, and prints STDERR on stderr. If this 
> > doesn't happen within a
> >  reasonable time limit, then the test fails.
> >
> >  There is an ``OVS_WAIT_FOR_OUTPUT_UNQUOTED`` version of this macro that 
> > expands
> > -shell ``$variables``, ``$(command)``, and so on.  The plain version does 
> > not
> > +shell ``$variables``, ``$(command)``, and so on.  The plain version does 
> > not.
> >
> >  OVS_WAIT_UNTIL(COMMAND[, IF-FAILED])
> >  ++++++++++++++++++++++++++++++++++++
> > diff --git a/Documentation/topics/testing.rst 
> > b/Documentation/topics/testing.rst
> > index 579422ca0..951133c29 100644
> > --- a/Documentation/topics/testing.rst
> > +++ b/Documentation/topics/testing.rst
> > @@ -36,7 +36,7 @@ validating basic functionality of OVN. Before running any 
> > of the tests
> >  described here, you must bootstrap, configure and build OVN as
> >  described in :doc:`/intro/install/general`. You do not need to install
> >  OVN, Open vSwitch or to build or load the kernel module to run these test
> > -suites.You do not need supervisor privilege to run these test suites.
> > +suites. You do not need superuser privilege to run these test suites.
> >
> >  Unit Tests
> >  ~~~~~~~~~~
> > @@ -78,7 +78,7 @@ To see a complete list of test options, run::
> >      $ make check TESTSUITEFLAGS=--help
> >
> >  The results of a testing run are reported in ``tests/testsuite.log``. 
> > Report
> > -report test failures as bugs and include the ``testsuite.log`` in your 
> > report.
> > +test failures as bugs and include the ``testsuite.log`` in your report.
> >
> >  .. note::
> >    Sometimes a few tests may fail on some runs but not others. This is 
> > usually a
> > @@ -266,7 +266,7 @@ cached. In order to force the tests to rebuild all 
> > these objects, run::
> >  A typical workflow for a developer trying to improve the performance of OVN
> >  would be the following:
> >
> > -0. Optional: Modify/add a performance test to buld the topology that you 
> > are
> > +0. Optional: Modify/add a performance test to build the topology that you 
> > are
> >     benchmarking, if required.
> >  1. Run ``make check-perf TESTSUITEFLAGS="--rebuild"`` to generate cached
> >     databases (and complete a test run). The results of each test run are
> > @@ -283,7 +283,7 @@ would be the following:
> >  2. Run ``make check-perf`` to measure the performance metric that you are
> >     benchmarking against. If you are only using one test, you can specify 
> > the
> >     test to run by adding the test number to the ``make`` command.
> > -   (e.g. ``make check-perf TESTSUITEFLAGS="--rebuild <test number>"``)
> > +   (e.g. ``make check-perf TESTSUITEFLAGS="<test number>"``)
> >  3. Modify OVN code to implement the change that you believe will improve 
> > the
> >     performance.
> >  4. Go to Step 2. to continue making improvements.
> > @@ -447,12 +447,12 @@ CI Integration
> >  Upgrade tests run automatically in GitHub Actions:
> >
> >  *On Schedule (Weekly)*
> > -  - Tests all supported versions (24.03, 24.09, 25.03, 25.09)
> > +  - Tests all supported versions.
> >
> >  Implementation Details
> >  ++++++++++++++++++++++
> >
> > -Test are run locally through ``check-upgrade`` Makefile target.
> > +Tests are run locally through ``check-upgrade`` Makefile target.
> >  The flow for make check-upgrade is:
> >
> >  - Makefile
> > diff --git a/Documentation/tutorials/ovn-sandbox.rst 
> > b/Documentation/tutorials/ovn-sandbox.rst
> > index 4acc22cdd..ff519c986 100644
> > --- a/Documentation/tutorials/ovn-sandbox.rst
> > +++ b/Documentation/tutorials/ovn-sandbox.rst
> > @@ -47,7 +47,7 @@ Running the sandbox does the following steps to the 
> > environment:
> >  1. Creates the ``OVN_Northbound`` and ``OVN_Southbound`` databases as 
> > described in
> >     `ovn-nb(5)`_ and `ovn-sb(5)`_.
> >
> > -2. Creates a backup server for ``OVN_Southbond`` database. Sandbox launch
> > +2. Creates a backup server for ``OVN_Southbound`` database. Sandbox launch
> >     screen provides the instructions on accessing the backup database.  
> > However
> >     access to the backup server is not required to go through the tutorial.
> >
> > --
> > 2.55.0
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to