On 1/24/23 14:01, Eelco Chaudron wrote: > The goal was to run 200 successful tc tests in a row. To do this the > following was run: > > for i in {1..200}; do make check-offloads || break; \ > echo "ALL_200_OK: $i"; done;
Hi, Eelco. Thanks for all the work! The patch set is fine in general, but I have a few comments I'll post for individual patches. One general design thought is that I'm not 100% certain that having a list of test names is a good way to approach test skipping. Names are not constant, and you're also using undocumented variable $at_desc to get them. We'll have to be very careful while changing tests and always consult with the list before adjusting names. The number is skipped tests also doesn't seem to be very high. If I'm not mistaken, we're skipping 21 test in the end. Only 7 of which actually have some issues with offloading. The bunch of unstable tests below contains known flaky tests that will not pass 200 runs in a row even without offloading. And the loop above doesn't seem entirely fair to these tests, as we usually run with RECHECK=yes. Some other tests from the list can potentially be fixed. Saying that, our usual way of skipping tests by adding a macro call to known failing tests might be a better option. What do you think? Best regards, Ilya Maximets. > > Unfortunately, a bunch of test cases showed occasional failures. > For now, they are excluded from the test cases and need further > investigation. They are: > > 802.1ad - vlan_limit > conntrack - DNAT load balancing > conntrack - DNAT load balancing with NC > conntrack - ICMP related > conntrack - ICMP related to original direction > conntrack - ICMP related with NAT > conntrack - IPv4 fragmentation with fragments specified > conntrack - multiple namespaces, internal ports > conntrack - zones from other field > conntrack - zones from other field, more tests > datapath - basic truncate action > datapath - multiple mpls label pop > datapath - truncate and output to gre tunnel > datapath - truncate and output to gre tunnel by simulated packets > > Some other test cases also fail due to what looks like problems > in the tc kernel conntrack implementation. For details see the > details in the system-offloads.at exclusion list definition. > > Signed-off-by: Eelco Chaudron <echau...@redhat.com> > Acked-by: Roi Dayan <r...@nvidia.com> > --- > tests/system-offloads.at | 43 +++++++++++++++++++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 6 deletions(-) > > diff --git a/tests/system-offloads.at b/tests/system-offloads.at > index 18e542aea..edffdd73b 100644 > --- a/tests/system-offloads.at > +++ b/tests/system-offloads.at > @@ -60,20 +60,51 @@ m4_define([CHECK_CONNTRACK_TIMEOUT], > # issue. > m4_define([OVS_TEST_SKIP_LIST], > [ovs_test_skip_list=" > +# TC does not support moving ports to a different namespace than vswitchd's > +# namespace, so we need to disable this test. > conntrack - multiple namespaces, internal ports > + > +# When moving through different zones, it can take up to ~8 seconds before > +# the conntrack state gets updated causing these tests to fail. Can we fix them with OVS_WAIT_UNTIL_EQUAL ? > conntrack - ct metadata, multiple zones > -conntrack - ICMP related > -conntrack - ICMP related to original direction > +conntrack - multiple zones, local > +conntrack - multi-stage pipeline, local > + > +# The kernel's tcf_ct_act() function does not seem to take care of any (QinQ) > +# VLAN headers causing commits to fail. However, if this is solved, we have > to > +# make sure conntrack does not break the VLAN boundary, i.e., putting > together > +# two packets with different CVLAN+SVLAN values. > conntrack - IPv4 fragmentation + cvlan > -conntrack - IPv4 fragmentation with fragments specified > conntrack - IPv6 fragmentation + cvlan > + > +# Fragmentation handling in ct zone 9 does not seem to work correctly. > +# When moving this test over to the default zone all works fine. > conntrack - Fragmentation over vxlan > conntrack - IPv6 Fragmentation over vxlan > -conntrack - multiple zones, local > -conntrack - multi-stage pipeline, local > + > +# Occasionaly we fail on the 'execute ct(commit) failed (Invalid argument) on * Occasionally > +# packet...' log message being present > +conntrack - zones from other field > +conntrack - zones from other field, more tests > +conntrack - multiple namespaces, internal ports > +conntrack - IPv4 fragmentation with fragments specified > + > +# Occasionaly we fail on the 'failed to flow_get/flow_del (No such file or > directory) ditto > +# ufid:..' log message being present. > +datapath - multiple mpls label pop > +datapath - basic truncate action > +conntrack - ICMP related > +conntrack - ICMP related to original direction > conntrack - ICMP related with NAT > conntrack - DNAT load balancing > -conntrack - DNAT load balancing with NC" > +conntrack - DNAT load balancing with NC > +802.1ad - vlan_limit > + > +# Occasionalt we fail with extreme high byte counters, i.e. Double typo. > +# n_bytes=18446744073705804134 > +datapath - truncate and output to gre tunnel by simulated packets > +datapath - truncate and output to gre tunnel > +" > echo "$ovs_test_skip_list" | sed "s/<SPC>/ /g"]) > > m4_include([tests/system-traffic.at]) _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev