Re: [ovs-dev] [PATCH v8 00/15] tests: Add system-traffic.at tests to check-offloads.
On Thu, Jan 26, 2023 at 06:29:44PM +0100, Eelco Chaudron wrote: > > > On 26 Jan 2023, at 17:52, Simon Horman wrote: > > > On Tue, Jan 24, 2023 at 01:42:57PM +0100, Eelco Chaudron wrote: > >> This series makes it possible to include system-traffic.at tests into > >> "make check-offloads" tests. > >> > >> The last patch of the series explains which tests are still not passing > >> and might need some more work. > >> > >> I'll try to work on the remaining failing test cases or find someone > >> who can work on them. > > > > Hi Eelco, > > > > This looks really nice :) > > > > Perhaps unwisely I tried running this on my laptop, > > which does not have an entirely new kernel. > > > > $ uname -spv > > Linux #1 SMP Debian 5.10.158-2 (2022-12-13) unknown > > > > This presented several problems. > > > > 1. Some of the original system-offload test fail/are unreliable. > >I'll see about following up on that. > > > >I don't think this should effect this patchset, > >the problems seem unrelated to it. > > > > 2. The tests covered in patch 14/15 fail with absurdly large > >byte counts. I'll reply to that patch with a log of the test run. > > I replied to your patch. > > > 3. Sharing of recirc_ids seems to be a kernel feature > >I'll describe my observations in the form of a patch. > > I do not see all the tests you mention below fail in my setup with that > error, guess I do not have the recirc_id_shared_with_tc feature. Yes, probably. > I’m ok with your changes, and I can add it to the series. What do you think? Sure, I think that is a good idea. But looking at some failures that came up after I sent my email, let me test them a bit more. Also, it did occur to me that it might be a bit more robust and a whole lot less verbose to override OVS_TRAFFIC_VSWITCHD_STOP(). In any case, old kernels may not be so important. So I don't think this has to block your patchset. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v8 00/15] tests: Add system-traffic.at tests to check-offloads.
On 26 Jan 2023, at 17:52, Simon Horman wrote: > On Tue, Jan 24, 2023 at 01:42:57PM +0100, Eelco Chaudron wrote: >> This series makes it possible to include system-traffic.at tests into >> "make check-offloads" tests. >> >> The last patch of the series explains which tests are still not passing >> and might need some more work. >> >> I'll try to work on the remaining failing test cases or find someone >> who can work on them. > > Hi Eelco, > > This looks really nice :) > > Perhaps unwisely I tried running this on my laptop, > which does not have an entirely new kernel. > > $ uname -spv > Linux #1 SMP Debian 5.10.158-2 (2022-12-13) unknown > > This presented several problems. > > 1. Some of the original system-offload test fail/are unreliable. >I'll see about following up on that. > >I don't think this should effect this patchset, >the problems seem unrelated to it. > > 2. The tests covered in patch 14/15 fail with absurdly large >byte counts. I'll reply to that patch with a log of the test run. I replied to your patch. > 3. Sharing of recirc_ids seems to be a kernel feature >I'll describe my observations in the form of a patch. I do not see all the tests you mention below fail in my setup with that error, guess I do not have the recirc_id_shared_with_tc feature. I’m ok with your changes, and I can add it to the series. What do you think? //Eelco > --- > tests: ignore absence of recirc_id sharing support > > In the case of the following tests when used with TC offload, > recirculation id sharing is needed for flows to be added to the TC > datapath. > > datapath - mpls actions > datapath - multiple mpls label pop > datapath - encap decap mpls actions > datapath - encap decap mpls_mc actions > datapath - multiple encap decap mpls actions > datapath - multiple encap decap mpls_mc actions > datapath - encap mpls pop mpls actions > mpls - decap header dp-support > mpls - decap header slow-path > conntrack - IPv4 ping > conntrack - IPv6 ping > conntrack - invalid > conntrack - preserve registers > conntrack - zones > conntrack - zones from field > conntrack - zones from other field > conntrack - zones from other field, more tests > conntrack - multiple bridges > conntrack - multiple zones > conntrack - ct_mark > conntrack - ct_mark bit-fiddling > conntrack - ct_mark from register > conntrack - ct_label > conntrack - ct_label bit-fiddling > conntrack - new connections > conntrack - IPv4 fragmentation [*] > conntrack - IPv4 fragmentation + vlan [*] > conntrack - IPv6 fragmentation > conntrack - IPv6 fragmentation expiry > conntrack - IPv6 fragmentation + vlan > conntrack - IPv4 Fragmentation + NAT > conntrack - resubmit to ct multiple times > conntrack - IPv4 HTTP > conntrack - IPv6 HTTP > conntrack - commit, recirc > conntrack - SNAT with ct_mark change on reply > conntrack - SNAT with port range using ICMP > conntrack - IPv6 HTTP with SNAT > conntrack - IPv6 ICMP6 Related with SNAT > conntrack - floating IP > conntrack - negative test for recirculation optimization > conntrack - Multiple ICMP traverse > IGMP - forward with ICMP > nsh - decap header > > However, if not supported, the test can still run, with > flows being added to the OVS kernel datapath instead. > > Allow the test to pass in such cases by ignoring log messages like > the following: > > ...|ERR|flow_put: recirc_id sharing not supported > > An alternate approach would be to probe for recirc_id sharing support. > > [*] These tests fail, presumably mixing TC and OVS kernel datapath > flows breaks things. This feels like a bug. > As a work-around this patch disables these tests via the > OVS_TEST_SKIP_LIST mechanism. > > Signed-off-by: Simon Horman > --- > tests/system-offloads.at | 2 + > tests/system-traffic.at | 91 > 2 files changed, 47 insertions(+), 46 deletions(-) > > diff --git a/tests/system-offloads.at b/tests/system-offloads.at > index cc45f662acf3..6fab1e25932a 100644 > --- a/tests/system-offloads.at > +++ b/tests/system-offloads.at > @@ -66,6 +66,8 @@ conntrack - multiple namespaces, internal ports > conntrack - ct metadata, multiple zones > conntrack - ICMP related > conntrack - ICMP related to original direction > +conntrack - IPv4 fragmentation > +conntrack - IPv4 fragmentation + vlan > conntrack - IPv4 fragmentation + cvlan > conntrack - IPv4 fragmentation with fragments specified > conntrack - IPv6 fragmentation + cvlan > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index b140a1e26092..066d11f7a170 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -1232,7 +1232,7 @@ NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 > 10.1.1.1 | FORMAT_PING], [0], > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > > -OVS_TRAFFIC_VSWITCHD_STOP > +OVS_TRAFFIC_VSWITCHD_STOP(["/recirc_id sharing not supported/d"]) > AT_CLEANUP > > AT_SETUP([datapath -
Re: [ovs-dev] [PATCH v8 00/15] tests: Add system-traffic.at tests to check-offloads.
On Tue, Jan 24, 2023 at 01:42:57PM +0100, Eelco Chaudron wrote: > This series makes it possible to include system-traffic.at tests into > "make check-offloads" tests. > > The last patch of the series explains which tests are still not passing > and might need some more work. > > I'll try to work on the remaining failing test cases or find someone > who can work on them. Hi Eelco, This looks really nice :) Perhaps unwisely I tried running this on my laptop, which does not have an entirely new kernel. $ uname -spv Linux #1 SMP Debian 5.10.158-2 (2022-12-13) unknown This presented several problems. 1. Some of the original system-offload test fail/are unreliable. I'll see about following up on that. I don't think this should effect this patchset, the problems seem unrelated to it. 2. The tests covered in patch 14/15 fail with absurdly large byte counts. I'll reply to that patch with a log of the test run. 3. Sharing of recirc_ids seems to be a kernel feature I'll describe my observations in the form of a patch. --- tests: ignore absence of recirc_id sharing support In the case of the following tests when used with TC offload, recirculation id sharing is needed for flows to be added to the TC datapath. datapath - mpls actions datapath - multiple mpls label pop datapath - encap decap mpls actions datapath - encap decap mpls_mc actions datapath - multiple encap decap mpls actions datapath - multiple encap decap mpls_mc actions datapath - encap mpls pop mpls actions mpls - decap header dp-support mpls - decap header slow-path conntrack - IPv4 ping conntrack - IPv6 ping conntrack - invalid conntrack - preserve registers conntrack - zones conntrack - zones from field conntrack - zones from other field conntrack - zones from other field, more tests conntrack - multiple bridges conntrack - multiple zones conntrack - ct_mark conntrack - ct_mark bit-fiddling conntrack - ct_mark from register conntrack - ct_label conntrack - ct_label bit-fiddling conntrack - new connections conntrack - IPv4 fragmentation [*] conntrack - IPv4 fragmentation + vlan [*] conntrack - IPv6 fragmentation conntrack - IPv6 fragmentation expiry conntrack - IPv6 fragmentation + vlan conntrack - IPv4 Fragmentation + NAT conntrack - resubmit to ct multiple times conntrack - IPv4 HTTP conntrack - IPv6 HTTP conntrack - commit, recirc conntrack - SNAT with ct_mark change on reply conntrack - SNAT with port range using ICMP conntrack - IPv6 HTTP with SNAT conntrack - IPv6 ICMP6 Related with SNAT conntrack - floating IP conntrack - negative test for recirculation optimization conntrack - Multiple ICMP traverse IGMP - forward with ICMP nsh - decap header However, if not supported, the test can still run, with flows being added to the OVS kernel datapath instead. Allow the test to pass in such cases by ignoring log messages like the following: ...|ERR|flow_put: recirc_id sharing not supported An alternate approach would be to probe for recirc_id sharing support. [*] These tests fail, presumably mixing TC and OVS kernel datapath flows breaks things. This feels like a bug. As a work-around this patch disables these tests via the OVS_TEST_SKIP_LIST mechanism. Signed-off-by: Simon Horman --- tests/system-offloads.at | 2 + tests/system-traffic.at | 91 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/tests/system-offloads.at b/tests/system-offloads.at index cc45f662acf3..6fab1e25932a 100644 --- a/tests/system-offloads.at +++ b/tests/system-offloads.at @@ -66,6 +66,8 @@ conntrack - multiple namespaces, internal ports conntrack - ct metadata, multiple zones conntrack - ICMP related conntrack - ICMP related to original direction +conntrack - IPv4 fragmentation +conntrack - IPv4 fragmentation + vlan conntrack - IPv4 fragmentation + cvlan conntrack - IPv4 fragmentation with fragments specified conntrack - IPv6 fragmentation + cvlan diff --git a/tests/system-traffic.at b/tests/system-traffic.at index b140a1e26092..066d11f7a170 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -1232,7 +1232,7 @@ NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], [0], 3 packets transmitted, 3 received, 0% packet loss, time 0ms ]) -OVS_TRAFFIC_VSWITCHD_STOP +OVS_TRAFFIC_VSWITCHD_STOP(["/recirc_id sharing not supported/d"]) AT_CLEANUP AT_SETUP([datapath - multiple mpls label pop]) @@ -1270,7 +1270,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], [0], [dnl 3 packets transmitted, 3 received, 0% packet loss, time 0ms ]) -OVS_TRAFFIC_VSWITCHD_STOP +OVS_TRAFFIC_VSWITCHD_STOP(["/recirc_id sharing not supported/d"]) AT_CLEANUP AT_SETUP([datapath - encap decap mpls actions]) @@ -1310,7 +1310,7 @@ NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 10.1.1.1 | FORMAT_PING],
[ovs-dev] [PATCH v8 00/15] tests: Add system-traffic.at tests to check-offloads.
This series makes it possible to include system-traffic.at tests into "make check-offloads" tests. The last patch of the series explains which tests are still not passing and might need some more work. I'll try to work on the remaining failing test cases or find someone who can work on them. v8: - Re-based on top of latest OVS master. v7: - Removed left over merge comment, and re-run all tests. v6: - Added ACKs from v5 - Changed 'netdev-offload-tc: If the flow has not been used, report it as such.' to also work on hardware offloaded flows. v5: - Include all patches, v4 went out with missing two patches :( v4: - Fix rename from system-traffic.at to sym-traffic.at in patch 11 v3: - Fixed missing MACRO's in patches 4, 6 and 10. v2: - Fix commit message on last patch - Moved handling of system-traffic.at tests to a separate file system-offloads.at - Re-based to the latest ovs master branch - Added Roi's ACKs Eelco Chaudron (15): tests: Allow system-traffic tests to be skipped based on a list. tests: Include working system-traffic tests into the system-offloads-testsuite. test: Do not use MPLS implicit null label in test cases. test: Add delay on revalidator flush for offload test cases. netdev-offload-tc: Fix tc conntrack force commit support. tests: Add delay to dump-conntrack for tc test cases. test: Fix "conntrack - floating IP" test for TC. test: Flush datapath when changing rules on the fly. netdev-offload-tc: Conntrack ALGs are not supported with tc. test: tc does not support conntrack timeout, skip the related test. test: Fix 'conntrack - Multiple ICMP traverse' for tc case. odp-util: Make odp_flow_key_from_flow__ nlattr order the same as the kernel. netdev-offload-tc: If the flow has not been used, report it as such. tests: Fix reading of OpenFlow byte counters in GRE test cases. tests: Comment currently failing TC system-traffic tests. Documentation/howto/tc-offload.rst | 11 ++ lib/netdev-offload-tc.c| 17 +- lib/odp-util.c | 21 ++- lib/tc.c | 14 +- tests/automake.mk | 1 + tests/dpif-netdev.at | 28 +-- tests/mcast-snooping.at| 4 +- tests/nsh.at | 10 +- tests/odp.at | 84 - tests/ofproto-dpif.at | 30 +-- tests/ofproto-macros.at| 5 +- tests/ovs-macros.at| 7 + tests/packet-type-aware.at | 22 +-- tests/pmd.at | 2 +- tests/system-common-macros.at | 7 + tests/system-offloads-testsuite.at | 1 + tests/system-offloads.at | 110 +++ tests/system-traffic.at| 283 +++-- tests/tunnel-push-pop-ipv6.at | 2 +- tests/tunnel-push-pop.at | 2 +- tests/tunnel.at| 2 +- 21 files changed, 415 insertions(+), 248 deletions(-) create mode 100644 tests/system-offloads.at ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
