On 30 Sep 2022, at 17:41, Peng He wrote: > It's so easy to reproduce ? > > Thanks! then I should have to dig it again.
It’s been on my to-do for a while but did not get to it. Sometimes it reproduced easily, and sometimes it takes 100+ runs :( If you apply the following series: https://patchwork.ozlabs.org/project/openvswitch/list/?series=316861 And then the diff below you can run something like: sudo bash -c 'for i in {1..100}; do make check-offloads TESTSUITEFLAGS="49 50" 49: datapath - truncate and output to gre tunnel by simulated packets ok 50: datapath - truncate and output to gre tunnel ok And after a while, it will fail with the error (sometimes also 0 bytes, I hope it’s the same issue :): system-traffic.at:1663: wait failed after 30 seconds n_bytes=18446744073709393054 $ git diff diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 57f94df54..52d61d0a4 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1868,6 +1868,7 @@ try_ukey_replace(struct umap *umap, struct udpif_key *old_ukey, ovs_mutex_lock(&new_ukey->mutex); cmap_replace(&umap->cmap, &old_ukey->cmap_node, &new_ukey->cmap_node, new_ukey->hash); + new_ukey->dump_seq = old_ukey->dump_seq; ovsrcu_postpone(ukey_delete__, old_ukey); transition_ukey(old_ukey, UKEY_DELETED); transition_ukey(new_ukey, UKEY_VISIBLE); diff --git a/tests/system-offloads-testsuite.at b/tests/system-offloads-testsuite.at index 318e6d1e6..4d546011d 100644 --- a/tests/system-offloads-testsuite.at +++ b/tests/system-offloads-testsuite.at @@ -114,8 +114,8 @@ conntrack - DNAT load balancing with NC # Occasionalt we fail with extreme high byte counters, i.e. # n_bytes=18446744073705804134 -datapath - truncate and output to gre tunnel by simulated packets -datapath - truncate and output to gre tunnel +#datapath - truncate and output to gre tunnel by simulated packets +#datapath - truncate and output to gre tunnel " echo "$ovs_test_skip_list" | sed "s/<SPC>/ /g"]) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 528d2ca64..e86699d1d 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -1708,7 +1708,10 @@ OVS_REVALIDATOR_PURGE() OVS_WAIT_UNTIL([ovs-ofctl dump-flows br0 | grep "in_port=4" | ofctl_strip | grep "n_packets=1, n_bytes=100, priority=1,ip,in_port=4 actions=drop"], [ovs-ofctl dump-flows br0 | grep "in_port=4" | ofctl_strip]) -OVS_TRAFFIC_VSWITCHD_STOP +OVS_TRAFFIC_VSWITCHD_STOP(["dnl +/.*lost packet on handler.*/d +/.failed to flow_get.*/d +/.*Failed to acquire udpif_key corresponding to unexpected flow.*/d"]) AT_CLEANUP dnl Create 2 bridges and 2 namespaces to test truncate over @@ -1834,7 +1837,10 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep "in_port=4" | ofctl_strip], [0], [dnl n_packets=1, n_bytes=100, priority=1,ip,in_port=4 actions=drop ]) -OVS_TRAFFIC_VSWITCHD_STOP +OVS_TRAFFIC_VSWITCHD_STOP(["dnl +/.*lost packet on handler.*/d +/.failed to flow_get.*/d +/.*Failed to acquire udpif_key corresponding to unexpected flow.*/d"]) AT_CLEANUP AT_SETUP([datapath - configure cache size]) > Eelco Chaudron <[email protected]> 于2022年9月30日周五 23:39写道： > >> >> >> On 30 Sep 2022, at 17:26, Peng He wrote: >> >>> Eelco Chaudron <[email protected]> 于2022年9月30日周五 23:01写道： >>> >>>> >>>> >>>> On 23 Sep 2022, at 18:29, Peng He wrote: >>>> >>>>> The userspace datapath mananges all the magaflows by a cmap. The cmap >>>>> data structrue will grow/shrink during the datapath processing and it >>>>> will re-position megaflows. This might result in two revalidator >> threads >>>>> might process a same megaflow during one dump stage. >>>>> >>>>> Consider a situation that, revalidator 1 processes a megaflow A, and >>>>> decides to delete it from the datapath, at the mean time, this megaflow >>>>> A is also queued in the process batch of revalidator 2. Normally it's >> ok >>>>> for revalidators to process the same megaflow multiple times, as the >>>>> dump_seq shows it's already dumped and the stats will not be >> contributed >>>>> twice. >>>>> >>>>> Assume that right after A is deleted, a PMD thread generates again >>>>> a new megaflow B which has the same match and action of A. The ukey >>>>> of megaflow B will replace the one of megaflow A. Now the ukey B is >>>>> new to the revalidator system and its dump seq is 0. >>>>> >>>>> Now since the dump seq of ukey B is 0, when processing megaflow A, >>>>> the revalidator 2 will not identify this megaflow A has already been >>>>> dumped by revalidator 1 and will contribute the old megaflow A's stats >>>>> again, this results in an inconsistent stats between ukeys and >> megaflows. >>>>> >>>>> To fix this, the newly generated the ukey B should take the dump_seq >>>>> of the replaced ukey A to avoid a same megaflow being revalidated >>>>> twice in one dump stage. >>>>> >>>>> We observe in the production environment, the OpenFlow rules' stats >>>>> sometimes are amplified compared to the actual value. I believe this >>>>> is also the reason that why somtimes there is mismatch between the >>>>> ukey and megaflow in stats value. The Eelco's patch >>>>> [ovs-dev] [PATCH v2 09/10] revalidator: Fix datapath statistics update >>>>> tried to fix it in the past. >>>> >>>> This sounds plausible, are your statistics extremely elevated? >>>> Mine are in the likes of n_bytes=18446744073705804134 where it should be >>>> around 100. >>>> >>>> It looks more like an overflow. >>> I just get the report from another team, I need to ask them, we will >> have a >>> 7 days off due to the national day. >>> so it will take time to get the value. :( >> >> Enjoy your time off!! This fix is not solving my problem; >> >> Still get the error once out of X runs, n_bytes=18446744073709393054 >> >>> I’ll try to get my old setup up and run it continuously over the weekend >>>> and see if it’s replicated again. >>>> >>>> thanks! >>> >>> >>>>> Signed-off-by: Peng He <[email protected]> >>>>> --- >>>>> ofproto/ofproto-dpif-upcall.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/ofproto/ofproto-dpif-upcall.c >>>> b/ofproto/ofproto-dpif-upcall.c >>>>> index e8bbcfeaf..89fad1bdf 100644 >>>>> --- a/ofproto/ofproto-dpif-upcall.c >>>>> +++ b/ofproto/ofproto-dpif-upcall.c >>>>> @@ -1877,6 +1877,7 @@ try_ukey_replace(struct umap *umap, struct >>>> udpif_key *old_ukey, >>>>> ovs_mutex_lock(&new_ukey->mutex); >>>>> cmap_replace(&umap->cmap, &old_ukey->cmap_node, >>>>> &new_ukey->cmap_node, new_ukey->hash); >>>>> + new_ukey->dump_seq = old_ukey->dump_seq; >>>>> ovsrcu_postpone(ukey_delete__, old_ukey); >>>>> transition_ukey(old_ukey, UKEY_DELETED); >>>>> transition_ukey(new_ukey, UKEY_VISIBLE); >>>>> -- >>>>> 2.25.1 >>>>> >>>>> _______________________________________________ >>>>> dev mailing list >>>>> [email protected] >>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>> >>>> >>> >>> -- >>> hepeng >> >> > > -- > hepeng _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
