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
