On 1/31/23 10:58, Eelco Chaudron wrote:
>
>
> 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()?
Doesn't sound great, but I'm not sure what a better name would be.
So, should be OK. Naming is hard. :)
>
>>>
>>> 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
