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
