Ilya Maximets <[email protected]> writes: > On 8/9/26 3:03 PM, Eli Britstein wrote: >> RFC 791 permits zero-payload last fragments, but common stacks >> including the Linux kernel discard them. Align the reassembly test >> with that policy: use valid one-byte fragments, and update expectations >> for duplicate handling (recirc flow and expiry coverage count). >> >> Assisted-by: composer-2.5-fast, Cursor >> Signed-off-by: Eli Britstein <[email protected]> >> --- >> v2-v1: >> - Fixed non robust allocated recirc_id. >> >> tests/ofproto-dpif.at | 18 ++++++++++-------- >> 1 file changed, 10 insertions(+), 8 deletions(-) >> >> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at >> index ee6ac873d..644b7b7fb 100644 >> --- a/tests/ofproto-dpif.at >> +++ b/tests/ofproto-dpif.at >> @@ -5536,23 +5536,24 @@ table=2 tcp actions=3 >> AT_CHECK([ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt]) >> >> dnl Test frag expiry. >> -AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0014 0001 8192 40 06 3316 ac11370d ac11370b"]) >> +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0015 0001 8192 40 06 3315 ac11370d ac11370b 00"]) >> ovs-appctl time/warp 10000 >> -AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0014 0001 8192 40 06 3316 ac11370d ac11370b"]) >> +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0015 0001 8192 40 06 3315 ac11370d ac11370b 00"]) >> >> dnl Test that no packets flow. > > So, this is not true anymore, the second packet is now delivered to > the destination, hence the extra datapath flow below. It seems to > be not an intended behavior of this test. What is also not clear to > me is that the test seems to assume that expiration timeout is 10 > seconds, not 15, which may be the root of the issue here. By the > time the second packet arrives the previous one is not expired yet, > though the comments like "Test frag expiry" and "Expire second frag" > suggest that "time/warp 10000" supposed to expire the fragment. > > Should we change the warp instead of the expected flows?
Agreed - it probably should time/warp by at least 15000 if I'm reading it correctly. > CC: Mike. > >> -AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\) | sed >> s'/recirc(.*)/recirc(X)/'], [0], [dnl >> +AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\) | sed >> 's/recirc[[^)]]*)/recirc()/g' | sort], [0], [dnl > > nit: Should probably use strip_recirc helper here. And we should keep > the same pattern for all other calls within the test, so it is consistent. > >> 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(),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() >> +recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=later), >> packets:0, bytes:0, used:never, actions:2 >> ]) >> >> dnl Expire second frag. >> ovs-appctl time/warp 10000 >> >> dnl Test frag purge >> -AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0014 0002 8192 40 06 3315 ac11370d ac11370b"]) >> +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0015 0002 8192 40 06 3314 ac11370d ac11370b 00"]) >> ovs-appctl time/warp 33000 >> -AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0014 0003 8192 40 06 3314 ac11370d ac11370b"]) >> +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 >> 0800 4500 0015 0003 8192 40 06 3313 ac11370d ac11370b 00"]) >> >> dnl Test that no packets flow. >> AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\) | sed >> s'/recirc(.*)/recirc(X)/'], [0], [dnl >> @@ -5563,8 +5564,9 @@ >> recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth(src=00:26:b9:8c:b0:f9),eth_t >> dnl Purge second frag >> ovs-appctl time/warp 33000 >> >> -dnl Make sure all four packets are counted properly in the coverage. >> -AT_CHECK([ovs-appctl coverage/show | grep -c "^ipf.*total: 2"], [0], [2 >> +dnl Make sure expiry and purge counters reflect the four packets sent. >> +dnl The duplicate second fragment is counted as overlap, not a second >> expiry. >> +AT_CHECK([ovs-appctl coverage/show | grep -c "^ipf.*total: 2"], [0], [1 >> ]) >> >> zero1208=$(printf '%0*d' 2416 0) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
