On 26 Jan 2023, at 21:03, Ilya Maximets wrote:
> 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?
I’m fine with this. How about a macro called CHECK_NO_TC_OFFLOAD()?
>>
>> 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 ?
I’ll take another look and see if I can fix this 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
Will fix all these typos :)
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
