On Mon, Jun 30, 2025 at 07:09:57PM +0200, Ilya Maximets wrote:
> Retis is very useful for debugging our system tests or debugging
> kernel issues through our system tests.  This change adds a convenient
> way to run any kernel system test with the retis capture on the
> background.  E.g.:
>
>   make check-kernel OVS_TEST_WITH_RETIS=yes TESTSUITEFLAGS='167 -d'
>
> Retis 1.5 is required, since we're using ifdump profile, and it also
> will mount debugfs for us in case of running in a different namespace.
> It should be available in $PATH.
>
> In addition to just capturing the retis.data, we're also running the
> capture with --print to print all the events as they appear, and
> producing the sorted output in the end.  This makes it easier to work
> across systems with different versions of retis and saves time for
> running the sort manually.  The raw data is still available for
> advanced processing, if needed.
>
> Not specifying any particular collector, capturing everything that's
> enabled by default.  OVS tracking is turned on by default.
>
> Since OVS tracking is used, it's required to start retis after the
> kernel datapath is created, otherwise it will fail to obtain the map
> of upcall PIDs.  That's why we need to start it after the bridge is
> created.
>
> Only adding support for kernel-related test suites for now.  For
> userspace test suites it may also be useful at some point, but
> currently that requires running without --ovs-track and isn't too
> important.
>
> Startup of the retis capture adds significant amount of time to each
> test, so not running it by default.

That will hopefully be improved in 1.6 :-).

>
> Link: https://github.com/retis-org/retis
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
>  Documentation/topics/testing.rst          | 22 ++++++++++++++++++++++
>  NEWS                                      |  3 +++
>  tests/ovs-macros.at                       | 10 ++++++++++
>  tests/system-kmod-macros.at               |  3 +++
>  tests/system-offloads-testsuite-macros.at |  3 +++
>  5 files changed, 41 insertions(+)
>
> diff --git a/Documentation/topics/testing.rst 
> b/Documentation/topics/testing.rst
> index b97bf32a9..78f38ecf5 100644
> --- a/Documentation/topics/testing.rst
> +++ b/Documentation/topics/testing.rst
> @@ -455,6 +455,28 @@ datapath testsuite.
>
>    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git
>
> +It is also possible to run `retis`_ capture along with the tests by setting
> +`OVS_TEST_WITH_RETIS` environment variable to 'yes'.  This can be useful for
> +debugging the test cases.  For example, the following command can be used to
> +run the test 167 under retis::
> +
> +    $ make check-kernel OVS_TEST_WITH_RETIS=yes TESTSUITEFLAGS='167 -d'
> +
> +After the test is completed, the following data will be available in the test
> +directory:
> +
> +* `retis.err` - standard error stream of the `retis collect`.
> +* `retis.log` - standard output of the `retis collect`, contains all captured
> +  events in the order they appeared.
> +* `retis.data` - raw events collected by retis, `retis sort` or other 
> commands
> +  can be used on this file for further analysis.
> +* `retis.sorted` - text file containing the output of `retis sort` executed 
> on
> +  the `retis.data`, for convenience.
> +
> +Requires retis version 1.5 or newer.
> +
> +.. _retis: https://github.com/retis-org/retis
> +

It might be worth mentioning it requires compiling with
--enable-usdt-probes (--ovs-track will fail otherwise).

>  .. _testing-static-analysis:
>
>  Static Code Analysis
> diff --git a/NEWS b/NEWS
> index d7231fabc..3439a70d6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -24,6 +24,9 @@ Post-v3.5.0
>     - ovs-tcpdump:
>       * Update the --mirror-to option, adding support for specifying an
>         existing port as a mirror interface.
> +   - Added support for running tests from 'check-kernel' and 'check-offloads'
> +     system test targets under retis by setting OVS_TEST_WITH_RETIS=yes.
> +     See the 'Testing' section of the documentation for more details.
>     - Updated documentation to remove most documentation of kernel module
>       that was previously part of the OVS distribution. It was removed from
>       the OVS distribution in the 3.0 release and is no longer present in
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 738cda2e4..132a00541 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -360,6 +360,16 @@ m4_ifndef([AT_FAIL_IF],
>      [AT_CHECK([($1) \
>      && exit 99 || exit 0], [0], [ignore], [ignore])])])
>
> +dnl Start retis to track all the traffic passing through OVS.
> +m4_define([RETIS_CHECK_AND_RUN],
> +  [if test "$OVS_TEST_WITH_RETIS" = yes && retis --version > /dev/null; then
> +       on_exit 'retis sort --utc retis.data > retis.sorted'

IIUC, this will run "retis sort" before "retis collect" is done, which
could lead to some issues because retis json output is buffered. It
would be safer to first kill collection.

> +       OVS_DAEMONIZE([retis -p ifdump collect --utc --allow-system-changes \
> +                          --ovs-track --out --print 2>retis.err 1>retis.log],
> +                     [retis.pid])

Capturing on ifdump without a filter can fill the event file of a lot of
unwanted traffic.
Providing a useful global filter (e.g: OVS_TEST_RETIS_FILTER) would be
difficult for anything but one or two related tests.

Adding a filter per test would be nice since each test knows what
traffic is expected but that'd mean ... well ... modifying each test.

We could do something like "retis collect -p ovs_vport_receive" to only
catch packets going inside OVS. This could go into a profile file that is
easy to modify. WDYT?

> +       OVS_WAIT_UNTIL([grep -q 'loaded' retis.err])
> +   fi])
> +
>  dnl Add a rule to always accept the traffic.
>  dnl The first argument to this macro should be the command to run:
>  dnl iptables or ip6tables
> diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
> index f7e9ff689..1de0e616b 100644
> --- a/tests/system-kmod-macros.at
> +++ b/tests/system-kmod-macros.at
> @@ -29,6 +29,9 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
>     _OVS_VSWITCHD_START([], [$3])
>     dnl Add bridges, ports, etc.
>     AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
> uuidfilt])], [0], [$2])
> +
> +   dnl Start retis capture if requested.
> +   RETIS_CHECK_AND_RUN()
>  ])
>
>  # OVS_TRAFFIC_VSWITCHD_STOP([ALLOWLIST], [extra_cmds])
> diff --git a/tests/system-offloads-testsuite-macros.at 
> b/tests/system-offloads-testsuite-macros.at
> index e6d044d21..c80538fd6 100644
> --- a/tests/system-offloads-testsuite-macros.at
> +++ b/tests/system-offloads-testsuite-macros.at
> @@ -27,6 +27,9 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
>     _OVS_VSWITCHD_START([], [-- set Open_vSwitch . 
> other_config:hw-offload=true $3])
>     dnl Add bridges, ports, etc.
>     AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
> uuidfilt])], [0], [$2])
> +
> +   dnl Start retis capture if requested.
> +   RETIS_CHECK_AND_RUN()
>  ])
>
>  # Macro to exclude tests that will fail with TC offload enabled.
> --
> 2.49.0
>

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

Reply via email to