On 7/14/23 09:19, Xavier Simonart wrote: > On Wed, Jul 12, 2023 at 8:20 PM Dumitru Ceara <[email protected]> wrote: > >> On 7/7/23 10:46, Ales Musil wrote: >>> On Wed, Jul 5, 2023 at 5:57 PM Xavier Simonart <[email protected]> >> wrote: >>> >>>> Usually, when cleaning up a system test we execute (for the ovn-vswitchd >>>> process): >>>> - OVS_TRAFFIC_VSWITCHD_STOP (i.e. ovs-appctl -t $1 exit --cleanup, wait >> up >>>> to 30 seconds >>>> and report failure if not properly cleaned up). >>>> - kill_ovs_vswitchd (i.e. if ovs-vsitwhd still running, >>>> ovs-appctl -t ovs-vswitchd exit --cleanup, wait 1 to 10 seconds and >> kill >>>> ovs-vswitvchd >>>> if not properly cleaned up). >>>> >>>> If we do not execute OVS_TRAFFIC_VSWITCHD_STOP, wait time is much >> smaller >>>> which might result in >>>> - clean up not done properly, ovs-vswitchd killed, ovs-netdev not >> deleted, >>>> and no error reported >>>> - next tests will fail >>>> >>>> Using this patch, we properly execute OVS_TRAFFIC_VSWITCHD_STOP, so the >>>> delay waiting for >>>> ovs-vswitchd to exit is longer, and, if by any chance this delay is >> still >>>> too short, >>>> the proper test will fail (i.e. the first test failing will be the one >>>> unable to clean). >>>> >>>> Note that, after this patch, some extra tests might fail such as >>>> "load-balancer template IPv4". >>>> This is due to the fact that, instead of killing ovn-controller at exit, >>>> we now properly exit and check the unexpected presence of warnings. In >>>> other words, we might now higlight some existing issues which were >>>> ignoring before. >> >> Hi Xavier, Ales, >> >> Ales shared this incremental privately which adds the "offending" >> warnings to the list of ignored ones. At the same time we opened a bug >> to track the reason why the warning was issued in the first place: >> >> https://bugzilla.redhat.com/show_bug.cgi?id=2222252 >> >> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at >> index f4ebdafc72..b0101330f5 100644 >> --- a/tests/ofproto-macros.at >> +++ b/tests/ofproto-macros.at >> @@ -287,6 +287,7 @@ check_logs () { >> /timeval.*context switches: [[0-9]]* voluntary, [[0-9]]* involuntary/d >> /ovs_rcu.*blocked [[0-9]]* ms waiting for .* to quiesce/d >> /Dropped [[0-9]]* log messages/d >> +/.*Trying to release unknown interface.*/d >> /|WARN|/p >> /|ERR|/p >> /|EMER|/p" ${logs} >> --- >> >> I had another look at this patch and I think we should also add the >> following change: >> >> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >> index 6b6181e8cc..97c6e433bb 100644 >> --- a/tests/system-common-macros.at >> +++ b/tests/system-common-macros.at >> @@ -488,7 +488,7 @@ AT_CHECK([ovn-nbctl get logical_router_port rp-sw0 >> ipv6_prefix | cut -c3-16], [0 >> [] >> ]) >> >> -kill $(pidof ovn-controller) >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> --- >> >> If this looks OK to you can fold both of them in and remove the >> note from the commit log. >> >> Please let me know what you think. >> >> Thanks, >> Dumitru >> >> Hi Ales, Dumitru > > Both look OK to me. Thanks, Dumitru and Ales >
Thanks, Xavier and Ales! I applied the patch to main and 23.06. It didn't apply cleanly to older branches but I don't think that's too big of a problem. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
