The error message when test failed,
> 2023-03-14T02:41:16.010Z|00077|unixctl|DBG|received request > revalidator/purge[], id=0 > 2023-03-14T02:41:16.021Z|00001|dpif(revalidator6)|WARN|system@ovs-system: > failed to flow_get (No such file or directory) > ufid:de773de7-f35f-4abd-bc98-b1e1ee80787a <empty>, packets:0, bytes:0, used:never > 2023-03-14T02:41:16.021Z|00002|ofproto_dpif_upcall(revalidator6)|WARN|Failed > to acquire udpif_key corresponding to unexpected flow (No such file or > directory): ufid:de773de7-f35f-4abd -bc98-b1e1ee80787a > 2023-03-14T02:41:16.041Z|00078|unixctl|DBG|replying with success, id=0: "" > 2023-03-14T02:41:16.045Z|00079|unixctl|DBG|received request > dpctl/dump-flows[], id=0 > 2023-03-14T02:41:16.045Z|00080|unixctl|DBG|replying with success, id=0: > "recirc_id(0),in_port(3),eth(src=96:3f:94:a0:e4:5a,dst=aa:1a:54:e9:c5:56),eth_type(0x0800),ipv4(frag=no), > packe ts:1, bytes:84, used:0.270s, actions:2 > " I run the test for 3000 loops. From: Eelco Chaudron <echau...@redhat.com> Date: 2023-03-13 18:31:24 To: Faicker Mo <faicker...@ucloud.cn>,i.maxim...@ovn.org Cc: d...@openvswitch.org,simon.hor...@corigine.com Subject: Re: [PATCH v8] netdev-offload-tc: del ufid mapping if device not exist> > >On 13 Mar 2023, at 8:15, Faicker Mo wrote: > >> Hi, >> The last sleep can't be removed in my env. There are errors occasionlly. >> The test procedure and the fail result are in my previous emails. >> But if I add revalidator/purge twice, the test is passed. No error >> complaints. > >Guess it would be interesting to find out why the flows are not yet deleted >(maybe not added) before the purge call. > >Maybe waiting for one revalidation round, before doing the purge might work, >i.e. ovs-appctl revalidator/wait? Ilya any other ideas? > >I tried to replicate your issue, but after 500 runs I do not see it. > >//Eelco > >> From: Eelco Chaudron <echau...@redhat.com> >> Date: 2023-03-08 15:28:05 >> To: Faicker Mo <faicker...@ucloud.cn> >> Cc: d...@openvswitch.org,i.maxim...@ovn.org,simon.hor...@corigine.com >> Subject: Re: [PATCH v8] netdev-offload-tc: del ufid mapping if device not >> exist> >>> >>> On 8 Mar 2023, at 3:34, Faicker Mo wrote: >>> >>>> Updated, >>>> Under my env, the sleep time before revalidator/purge can't be shorten to >>>> 0.2s or 0.5s. >>>> It's not reasonable there exists a dp flow after revalidator/purge. >>> >>> What about my suggested change of removing the sleeps completely? Does it >>> work for you, or is it not testing correctly? >>> >>>> >>>> From: Faicker Mo <faicker...@ucloud.cn> >>>> Date: 2023-03-06 12:22:48 >>>> To:Eelco Chaudron <echau...@redhat.com> >>>> cc: d...@openvswitch.org,i.maxim...@ovn.org,simon.hor...@corigine.com >>>> Subject: Re:Re: [PATCH v8] netdev-offload-tc: del ufid mapping if device >>>> not exist >>>> It failed at system-offloads-traffic.at:737 without the sleep before >>>> revalidator/purge. There was a flow, >>>>> 2023-03-06T02:17:13.245Z|00079|unixctl|DBG|received request >>>>> dpctl/dump-flows[], id=0 >>>>> 2023-03-06T02:17:13.245Z|00080|unixctl|DBG|replying with success, id=0: >>>>> "recirc_id(0),in_port(4),eth(src=f6:2d:a8:f7:d2:f7,dst=aa:1a:54:e9:c5:56),eth_type(0x0800),ipv4(frag=no), >>>>> packe >>>> ts:1, bytes:84, used:0.040s, actions:2 >>>>> " >>>> >>>> Tested, >>>> The first sleep can be removed with msg filters. >>>> The sleep before revalidator/purge can be shorten to 0.2s. >>>> >>>> >>>> >>>> >>>> >>>> >>>> From: Eelco Chaudron <echau...@redhat.com> >>>> Date: 2023-03-03 19:11:25 >>>> To: Faicker Mo <faicker...@ucloud.cn> >>>> Cc: d...@openvswitch.org,i.maxim...@ovn.org,simon.hor...@corigine.com >>>> Subject: Re: [PATCH v8] netdev-offload-tc: del ufid mapping if device not >>>> exist> >>>>> >>>>> On 2 Mar 2023, at 8:57, 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 <faicker...@ucloud.cn> >>>>>> --- >>>>>> 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 >>>>>> --- >>>>>> 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 7558812eb..ba18711cb 100644 >>>>>> --- a/tests/system-offloads-traffic.at >>>>>> +++ b/tests/system-offloads-traffic.at >>>>>> @@ -690,3 +690,57 @@ >>>>>> OVS_CHECK_ACTIONS([check_pkt_len(size=200,gt(5),le(check_pkt_len(size=100,gt(5), >>>>>> >>>>>> 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 >>>>>> +]) >>>>>> +sleep 1 >>>>>> + >>>>>> +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"]) >>>>>> + >>>>>> +sleep 1 >>>>>> +AT_CHECK([ovs-appctl revalidator/purge]) >>>>>> + >>>>>> +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 >>>>>> +]) >>>>>> + >>>>>> +sleep 1 >>>>>> +AT_CHECK([ovs-appctl revalidator/purge]) >>>>>> + >>>>>> +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 >>>>>> +"]) >>>>>> +AT_CLEANUP >>>>> >>>>> The changes look fine, however, you still have a 3-second delay in your >>>>> tests. Any specific reason for this? >>>>> >>>>> I tried removing them and I found that if I did I needed to add another >>>>> log messages exclusion. With this I was able to still make it fail >>>>> without your change, and get it to pass for 400 runs. >>>>> >>>>> Thought? >>>>> >>>>> //Eelco >>>>> >>>>> diff --git a/tests/system-offloads-traffic.at >>>>> b/tests/system-offloads-traffic.at >>>>> index ba18711cb..b2a402dec 100644 >>>>> --- a/tests/system-offloads-traffic.at >>>>> +++ b/tests/system-offloads-traffic.at >>>>> @@ -710,7 +710,7 @@ 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 >>>>> ]) >>>>> -sleep 1 >>>>> +#sleep 1 >>>>> >>>>> dnl Delete and add interface ovs-p0/p0 >>>>> AT_CHECK([ip link del dev ovs-p0]) >>>>> @@ -721,7 +721,7 @@ 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"]) >>>>> >>>>> -sleep 1 >>>>> +#sleep 1 >>>>> AT_CHECK([ovs-appctl revalidator/purge]) >>>>> >>>>> dnl Generate flows to trigger the hmap expand once >>>>> @@ -733,7 +733,7 @@ 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 >>>>> ]) >>>>> >>>>> -sleep 1 >>>>> +#sleep 1 >>>>> AT_CHECK([ovs-appctl revalidator/purge]) >>>>> >>>>> AT_CHECK([test $(ovs-appctl dpctl/dump-flows | grep -c >>>>> "eth_type(0x0800)") -eq 0], [0], [ignore]) >>>>> @@ -742,5 +742,6 @@ 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 >>>>> +/failed to get ifindex for ovs-p0: No such device/d >>>>> "]) >>>>> AT_CLEANUP >>>>> >>> > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev