Thanks for your testing. Add the ignore msg "failed to offload flow" to OVS_TRAFFIC_VSWITCHD_STOP can pass the fail test. I'll post an update later.
From: Simon Horman <[email protected]> Date: 2023-03-29 23:22:17 To: Faicker Mo <[email protected]> Cc: [email protected],Eelco Chaudron <[email protected]>,Ilya Maximets <[email protected]> Subject: Re: [ovs-dev] [PATCH v9] netdev-offload-tc: del ufid mapping if device not exist>On Wed, Mar 15, 2023 at 11:26:55AM +0800, Faicker Mo wrote: >> The device may be deleted and added with ifindex changed. >> The tc rules on the device will be deleted if the device is deleted. >> The func tc_del_filter will fail when flow del. The mapping of >> ufid to tc will not be deleted. >> The traffic will trigger the same flow(with same ufid) to put to tc >> on the new device. Duplicated ufid mapping will be added. >> If the hashmap is expanded, the old mapping entry will be the first entry, >> and now the dp flow can't be deleted. >> >> >> Signed-off-by: Faicker Mo <[email protected]> > >Hi, > >I have run the test added by this patch without the code changes of this >patch, and verified that it fails reliably. > >I have also run the test with the code changes in a for loop on on a low-end >Ubuntu 22.04 VM 34,500k times (for about 2 days). > >Of those test runs, 17 failed, and the rest succeeded: >a success rate of 99.95% :) > >As it's not entirely clear how reliable such a setup is >(or our expectations for it) [1], I think I am ok with this patch. > >Reviewed-by: Simon Horman <[email protected]> > >[1] https://mail.openvswitch.org/pipermail/ovs-dev/2023-March/403164.html > >For the record, of the failures, 8 looked like this: > >./system-offloads-traffic.at:803: check_logs "/could not open network device >ovs-p0/d >/on nonexistent port/d >/failed to flow_get/d >/Failed to acquire udpif_key/d >/No such device/d >" >--- /dev/null 2023-03-27 12:47:20.572000000 +0000 >+++ /home/horms/ovs/tests/system-offloads-testsuite.dir/at-groups/12/stdout > 2023-03-28 06:23:59.071853684 +0000 >@@ -0,0 +1,8 @@ >+2023-03-28T06:23:58.360Z|00001|dpif_netlink(revalidator1)|ERR|failed to >offload flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.360Z|00002|dpif_netlink(revalidator1)|ERR|failed to >offload flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.388Z|00003|dpif_netlink(revalidator1)|ERR|failed to >offload flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.388Z|00004|dpif_netlink(revalidator1)|ERR|failed to >offload flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.603Z|00001|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.611Z|00002|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.819Z|00003|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >+2023-03-28T06:23:58.823Z|00004|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >ovsdb-server.log: > >Another 8 looked like this: > >./system-offloads-traffic.at:803: check_logs "/could not open network device >ovs-p0/d >/on nonexistent port/d >/failed to flow_get/d >/Failed to acquire udpif_key/d >/No such device/d >" >--- /dev/null 2023-03-27 12:47:20.572000000 +0000 >+++ /home/horms/ovs/tests/system-offloads-testsuite.dir/at-groups/12/stdout > 2023-03-28 07:38:19.457642442 +0000 >@@ -0,0 +1 @@ >+2023-03-28T07:38:18.587Z|00001|dpif_netlink(revalidator1)|ERR|failed to >offload flow: Invalid argument: ovs-p0 >ovsdb-server.log: >> 2023-03-28T07:38:17.713Z|00001|vlog|INFO|opened log file >> /home/horms/ovs/tests/system-offloads-testsuite.dir/012/ovsdb-server.log >> 2023-03-28T07:38:17.736Z|00002|ovsdb_server|INFO|ovsdb-server (Open vSwitch) >> 3.1.90 >ovs-vswitchd.log: > >And 1 looked like this: > >./system-offloads-traffic.at:803: check_logs "/could not open network device >ovs-p0/d >/on nonexistent port/d >/failed to flow_get/d >/Failed to acquire udpif_key/d >/No such device/d >" >--- /dev/null 2023-03-27 12:47:20.572000000 +0000 >+++ /home/horms/ovs/tests/system-offloads-testsuite.dir/at-groups/12/stdout > 2023-03-28 01:59:56.526587195 +0000 >@@ -0,0 +1,4 @@ >+2023-03-28T01:59:56.050Z|00001|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >+2023-03-28T01:59:56.055Z|00002|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >+2023-03-28T01:59:56.262Z|00003|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >+2023-03-28T01:59:56.267Z|00004|dpif_netlink(handler2)|ERR|failed to offload >flow: Invalid argument: ovs-p0 >ovsdb-server.log: >> 2023-03-28T01:59:54.878Z|00001|vlog|INFO|opened log file >> /home/horms/ovs/tests/system-offloads-testsuite.dir/012/ovsdb-server.log >> 2023-03-28T01:59:54.895Z|00002|ovsdb_server|INFO|ovsdb-server (Open vSwitch) >> 3.1.90 >ovs-vswitchd.log: > > >> --- >> v2: >> - Add tc offload test case >> v3: >> - No change >> v4: >> - No change >> v5: >> - No change >> v6: >> - No change >> v7: >> - Minor fix for test case and rebased >> v8: >> - Shorten the time of the test case >> v9: >> - Remove sleep in the test case >> --- >> lib/netdev-offload-tc.c | 3 +- >> tests/system-offloads-traffic.at | 54 ++++++++++++++++++++++++++++++++ >> 2 files changed, 56 insertions(+), 1 deletion(-) >> >> >> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c >> index 4fb9d9f21..dd2020cad 100644 >> --- a/lib/netdev-offload-tc.c >> +++ b/lib/netdev-offload-tc.c >> @@ -276,8 +276,9 @@ del_filter_and_ufid_mapping(struct tcf_id *id, const >> ovs_u128 *ufid, >> } >> >> err = tc_del_flower_filter(id); >> - if (!err) { >> + if (!err || err == ENODEV) { >> del_ufid_tc_mapping(ufid); >> + return 0; >> } >> return err; >> } >> diff --git a/tests/system-offloads-traffic.at >> b/tests/system-offloads-traffic.at >> index eb331d6ce..19dd7a966 100644 >> --- a/tests/system-offloads-traffic.at >> +++ b/tests/system-offloads-traffic.at >> @@ -750,3 +750,57 @@ AT_CHECK([ovs-appctl coverage/read-counter >> ukey_invalid_stat_reset], [0], [dnl >> >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> + >> +AT_SETUP([offloads - delete ufid mapping if device not exist - offloads >> enabled]) >> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . >> other_config:hw-offload=true]) >> + >> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) >> + >> +ADD_NAMESPACES(at_ns0, at_ns1, at_ns2) >> + >> +dnl Disable IPv6 to skip unexpected flow >> +AT_CHECK([sysctl -w net.ipv6.conf.br0.disable_ipv6=1], [0], [ignore]) >> +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >> [ignore]) >> +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >> [ignore]) >> +NS_CHECK_EXEC([at_ns2], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >> [ignore]) >> + >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "aa:1a:54:e9:c5:56") >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") >> + >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 2 -i 0.2 10.1.1.2 | FORMAT_PING], [0], >> [dnl >> +2 packets transmitted, 2 received, 0% packet loss, time 0ms >> +]) >> + >> +dnl Delete and add interface ovs-p0/p0 >> +AT_CHECK([ip link del dev ovs-p0]) >> +AT_CHECK([ip link add p0 type veth peer name ovs-p0 || return 77]) >> +AT_CHECK([ip link set p0 netns at_ns0]) >> +AT_CHECK([ip link set dev ovs-p0 up]) >> +NS_CHECK_EXEC([at_ns0], [ip addr add dev p0 "10.1.1.1/24"]) >> +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 up]) >> +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address "aa:1a:54:e9:c5:56"]) >> + >> +AT_CHECK([ovs-appctl revalidator/purge], [0]) >> + >> +dnl Generate flows to trigger the hmap expand once >> +ADD_VETH(p2, at_ns2, br0, "10.1.1.3/24") >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 2 -i 0.2 10.1.1.2 | FORMAT_PING], [0], >> [dnl >> +2 packets transmitted, 2 received, 0% packet loss, time 0ms >> +]) >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 2 -i 0.2 10.1.1.3 | FORMAT_PING], [0], >> [dnl >> +2 packets transmitted, 2 received, 0% packet loss, time 0ms >> +]) >> + >> +AT_CHECK([ovs-appctl revalidator/purge], [0]) >> +dnl Fix purge fail occasionally >> +AT_CHECK([ovs-appctl revalidator/purge], [0]) >> + >> +AT_CHECK([test $(ovs-appctl dpctl/dump-flows | grep -c "eth_type(0x0800)") >> -eq 0], [0], [ignore]) >> + >> +OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device ovs-p0/d >> +/on nonexistent port/d >> +/failed to flow_get/d >> +/Failed to acquire udpif_key/d >> +/No such device/d >> +"]) >> +AT_CLEANUP >> -- >> 2.31.1 >> >> >> >> >> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
