On Fri, Jul 11, 2025 at 10:54 AM Ilya Maximets <i.maxim...@ovn.org> 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='168 -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. This also makes it necessary to build the OVS submodule > with --enable-usdt-probes flag, otherwise retis may refuse to start > when it can't find any probes in the running ovs-vswitchd. > > Only adding support for kernel test suite 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. > > This change is ported from the corresponding OVS change: > > https://github.com/openvswitch/ovs/commit/22732c0e6770597031d1eb0a61ec922ea296c8b1 > The environment variable name starts with OVS_ for that reason. But > we already have the OVS_PAUSE_TEST, so just following the pattern. > > The DAEMONIZE() macro is changed to use 'on_exit' instead of appending > commands directly to the 'cleanup' file. That ensures the correct > ordering between stopping the daemon and running the 'retis sort', as > 'on_exit' ensures that cleanup commands are executed in the reverse > order. This makes the macro similar to OVS_DAEMONIZE in the OVS repo. > In the future, a similar change can be made in the NETNS_DAEMONIZE() > as well to ensure the proper ordering, but it's not necessary for now. > > Link: https://github.com/retis-org/retis > Signed-off-by: Ilya Maximets <i.maxim...@ovn.org> > --- > Documentation/topics/testing.rst | 24 ++++++++++++++++++++++++ > NEWS | 3 +++ > tests/system-common-macros.at | 17 ++++++++++++++--- > tests/system-kmod-macros.at | 3 +++ > 4 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/Documentation/topics/testing.rst > b/Documentation/topics/testing.rst > index d85dc50c6..cc928ef64 100644 > --- a/Documentation/topics/testing.rst > +++ b/Documentation/topics/testing.rst > @@ -220,6 +220,30 @@ All the features documented under `Unit Tests`_ are > available for the > datapath testsuites, except that the datapath testsuites do not > support running tests in parallel. > > +It is also possible to run `retis`_ capture along with the `check-kernel` > 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 168 under `retis`:: > + > + $ make check-kernel OVS_TEST_WITH_RETIS=yes TESTSUITEFLAGS='168 -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. And this also requires OVS > submodule to > +be built with `--enable-usdt-probes`. > + > +.. _retis: https://github.com/retis-org/retis > + > + > Performance testing > ~~~~~~~~~~~~~~~~~~~ > > diff --git a/NEWS b/NEWS > index 8fa0c3f26..365134d61 100644 > --- a/NEWS > +++ b/NEWS > @@ -36,6 +36,9 @@ Post v25.03.0 > - Added a new ACL option "--all" to "acl-list" command. When set, > "acl-list" command will also list port groups ACLs associated with > each port of the target logical switch. > + - Added support for running tests from the 'check-kernel' system test > target > + under retis by setting OVS_TEST_WITH_RETIS=yes. See the 'Testing' > section > + of the documentation for more details. > > OVN v25.03.0 - 07 Mar 2025 > -------------------------- > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 93979ff62..a49776f38 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -244,9 +244,20 @@ m4_define([FORMAT_CT], > # > m4_define([DAEMONIZE], > [$1 & echo $! > $2 > - echo "kill \`cat $2\`" >> cleanup > - ] > -) > + on_exit "kill \`cat $2\`" > + ]) > + > +# RETIS_CHECK_AND_RUN() > +# > +# 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' > + DAEMONIZE([retis -p ifdump collect --utc --allow-system-changes \ > + --ovs-track --out --print 2>retis.err > 1>retis.log], > + [retis.pid]) > + OVS_WAIT_UNTIL([grep -q 'loaded' retis.err]) > + fi]) > > # NETNS_DAEMONIZE([namespace], [command], [pidfile]) > # > diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at > index 41a8b7914..1ada0077e 100644 > --- a/tests/system-kmod-macros.at > +++ b/tests/system-kmod-macros.at > @@ -30,6 +30,9 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START], > if test OVN_MONITOR_ALL = yes; then > ovs-vsctl set open . external_ids:ovn-monitor-all=true > fi > + > + dnl Start retis capture if requested. > + RETIS_CHECK_AND_RUN() > ]) > > # OVS_TRAFFIC_VSWITCHD_STOP([ALLOWLIST], [extra_cmds]) > -- > 2.49.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thank you Ilya, I went ahead and merged this into main. Regards, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
