Thanks Ilya for reviewing the series. I applied the feedback to the best of my knowledge, see v2.
I wasn't sure if the trace is affected, thanks for confirming it's not. --- Side note: I have one more test failure in "ofproto-dpif - fragment handling - reassembly" where a packet is observed when not expected, even after I add revalidator wait: ./ofproto-dpif.at:5423: ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt ./ofproto-dpif.at:5424: ovs-appctl revalidator/wait ./ofproto-dpif.at:5427: ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 0014 0001 8192 40 06 3316 ac11370d ac11370b" warped ./ofproto-dpif.at:5429: ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 0014 0001 8192 40 06 3316 ac11370d ac11370b" ./ofproto-dpif.at:5432: ovs-appctl dpctl/dump-flows filter=in_port\(90\) | sed s'/recirc(.*)/recirc(X)/' --- - 2025-09-14 18:04:11.031737181 +0000 +++ /home/ihrachyshka/src/ovs/tests/testsuite.dir/at-groups/1235/stdout 2025-09-14 18:04:11.030029389 +0000 @@ -1,3 +1,3 @@ flow-dump from the main thread: -recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth(src=00:26:b9:8c:b0:f9),eth_type(0x0800),ipv4(src= 172.17.55.13/128.0.0.0,proto=6,frag=later), packets:0, bytes:0, used:never, actions:ct(commit,zone=10),recirc(X) +recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth(src=00:26:b9:8c:b0:f9),eth_type(0x0800),ipv4(src= 172.17.55.13/128.0.0.0,proto=6,frag=later), packets:1, bytes:34, used:0.001s, actions:ct(commit,zone=10),recirc(X) Does it mean the revalidator is not the culprit in this particular case? (Adding a `sleep 5` after `replace-flows` helps, so it's some timing issue.) Ihar On Wed, Sep 10, 2025 at 6:43 AM Ilya Maximets <[email protected]> wrote: > On 9/10/25 4:47 AM, Ihar Hrachyshka wrote: > > The `tunnel_push_pop - action` test case may fail due to a race > > condition between the revalidator modifying flows and dummy `receive` > > action. > > > > To fix the race for this test case and other test cases in the same > > file, this patch will call revalidator/wait every time a flow is > > modified. > > > > Signed-off-by: Ihar Hrachyshka <[email protected]> > > --- > > tests/tunnel-push-pop.at | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at > > index 64c41460b..b6fac1426 100644 > > --- a/tests/tunnel-push-pop.at > > +++ b/tests/tunnel-push-pop.at > > @@ -42,6 +42,7 @@ Cached: 2001:ca00::/24 dev br0 SRC 2001:cafe::88 local > > ]) > > > > AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > > +AT_CHECK([ovs-appctl revalidator/wait]) > > > > dnl Check ARP request > > AT_CHECK([ovs-vsctl -- set Interface p0 options:pcap=p0.pcap]) > > @@ -81,6 +82,7 @@ erspan_sys (3) ref_cnt=4 > > dnl Check ERSPAN v1 tunnel push > > AT_CHECK([ovs-vsctl -- set Interface br0 options:pcap=br0.pcap]) > > AT_CHECK([ovs-ofctl add-flow int-br action=2]) > > +AT_CHECK([ovs-appctl revalidator/wait]) > > I agree that we need these before running packets throught the datapath > with netdev-dummy/receive, but we should not need them for the trace. > The trace is happening on the ofproto level and revalidators do not > affect it in any way, at least they shouldn't. > > I see that this test does wait for traces in a few spots, but these should > not be needed. > > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
