Hi，Eelco, Which kernel version you have tested on? I've also tested the #50 for several hundreds times, but only get 0 packets once.
I have put your patches upon on commits 49efc63ad: ofproto-dpif-xlate: Fix error messages for nonexistent ports/recirc_ids. not sure if it's related to the modification on master branch. Eelco Chaudron <[email protected]> 于2022年10月7日周五 18:54写道： > Hi hepeng, > > I tried your changes, but I also do get a lot of zero packet failures on > test 50. Not sure what happened, maybe it was related to syncing to the > latest master. > > However I focused on test 49, and I can replicate it every x test, but > within 40 tries: > > ## ------------------------------ ## > ## openvswitch 3.0.90 test suite. ## > ## ------------------------------ ## > 49: datapath - truncate and output to gre tunnel by simulated packets > FAILED (ovs-macros.at:247) > > ... > ... > system-traffic.at:1663: waiting until ovs-ofctl dump-flows br-underlay | > grep "in_port=LOCAL" | sed -n 's/.*\(n\_bytes=[0-9]*\).*/\1/p' | grep > "n_bytes=138"... > system-traffic.at:1663: wait failed after 30 seconds > n_bytes=18446744073707612670 > ./ovs-macros.at:247: hard failure > > I’ll try to dig into this later once I get some time... > > //Eelco > > On 2 Oct 2022, at 8:40, Peng He wrote: > > Hi, > > I have tried test #50 for 200 times, the overflow as well as stats of 0 > did not show up. > > the command line is: > for i in {1..100}; do echo $i; make check-offloads TESTSUITEFLAGS="-v 50" > || break; done > > I am using Debian 10, and my kernel version is > root@ubuntu:~/ovs# uname -r > 5.4.0-121-generic > > I doubt the overflow might be related to kernel datapath. > IIRC, the kernel datapath does not support some IPv6 ND/NA match. > > In the logs, there are some warning saying that "failed to get/del/put > some datapath flow", which is all > related the ICMPv6. It seems to prove the missing of the support of NA/ND > match in datapath. > > since the ICMPv6 is not related to the test itself. I did some > modification to the test to drop all IPv6 traffic. > > Below is my complete diff to the #50: > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index dfbc30e47..b759c4bb9 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -1762,7 +1762,7 @@ on_exit 'rm -f payload200.bin' > > AT_CHECK([ovs-ofctl del-flows br0]) > AT_DATA([flows.txt], [dnl > > -priority=99,in_port=1,actions=output(port=2,max_len=100),output(port=3,max_len=100) > > +priority=99,in_port=1,ip,actions=output(port=2,max_len=100),output(port=3,max_len=100) > priority=99,in_port=2,udp,actions=output(port=1,max_len=100) > priority=1,in_port=4,ip,actions=drop > priority=1,actions=drop > @@ -1771,6 +1771,8 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > AT_CHECK([ovs-ofctl del-flows br-underlay]) > AT_DATA([flows-underlay.txt], [dnl > +priority=99,arp,in_port=1,actions=LOCAL > +priority=99,arp,in_port=LOCAL,actions=1 > priority=99,dl_type=0x0800,nw_proto=47,in_port=1,actions=LOCAL > priority=99,dl_type=0x0800,nw_proto=47,in_port=LOCAL,ip_dst= > 172.31.1.1/24,actions=1 > priority=1,actions=drop > @@ -1787,7 +1789,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep > "in_port=2" | sed -n 's/.*\(n\_bytes=[ > n_bytes=242 > ]) > dnl After truncation = outer ETH(14) + outer IP(20) + GRE(4) + 100 = 138B > -AT_CHECK([ovs-ofctl dump-flows br-underlay | grep "in_port=LOCAL" | sed > -n 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl > +AT_CHECK([ovs-ofctl dump-flows br-underlay | grep "ip,in_port=LOCAL" | > sed -n 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl > n_bytes=138 > ]) > > @@ -1822,7 +1824,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep > "in_port=2" | sed -n 's/.*\(n\_bytes=[ > n_bytes=242 > ]) > dnl After truncation = outer ETH(14) + outer IP(20) + GRE(4) + 100 = 138B > -AT_CHECK([ovs-ofctl dump-flows br-underlay | grep "in_port=LOCAL" | sed > -n 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl > +AT_CHECK([ovs-ofctl dump-flows br-underlay | grep "ip,in_port=LOCAL" | > sed -n 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl > n_bytes=138 > ]) > > @@ -1834,7 +1836,11 @@ 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 port.*/d > +/.failed to flow_get.*/d > +/.failed to flow_del.*/d > +/.*Failed to acquire udpif_key corresponding to unexpected flow.*/d"]) > AT_CLEANUP > > > It seems that the overflow here is not related to the race in > revalidators, but it is related to the design of > the testsuite and also the missing of the support of NA/ND in kernel > datapath. > > > > Eelco Chaudron <[email protected]> 于2022年9月30日周五 23:47写道： > >> 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 >> >> > > -- > hepeng > > -- hepeng _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
