On 8/5/22 23:49, Paolo Valerio wrote: > Ilya Maximets <[email protected]> writes: > >> On 8/5/22 17:08, Paolo Valerio wrote: >>> The following test sequence: >>> >>> conntrack - IPv4 fragmentation incomplete reassembled packet >>> conntrack - IPv4 fragmentation with fragments specified >>> >>> leads to a systematic failure of the latter test on the kernel >>> datapath (linux). Multiple executions of the former may also lead to >>> multiple failures. >>> This is due to the fact that fragments not yet reassembled are kept in >>> a queue for /proc/sys/net/ipv4/ipfrag_time seconds, and if the >>> kernel receives a fragment already present in the queue, it returns >>> -EINVAL. >> >> Thanks for the patch! I've been looking at the issue earlier >> this week. One thing I don't understand is that we're reloading >> all the netfilter modules between tests, shouldn't this clear >> all the pending queues? Or this re-assembly is happening outside >> of the conntrack? >> > > That's a fair point. > AFAICT, queues and the pending fragments sit in a per netns fragment > queue directory. In the case of the kernel dp ovs_dp_get_net(dp). If my > reading is correct, IPv4 pending fragments should be removed when the > netns is destroyed.
Hmm, ok. Thanks for the explanation. I tried to prototype some change to run all tests in a separate namespace that gets removed after each test, but the integration with autotest doesn't work well this way. I guess, we either need a way to put current shell (not the forked one) into a new namespace, for which I didn't find any supported APIs, or we'll have to heavily modify all the tests and macros, which doesn't sound like a lot of fun. For now, I confirmed that the fix is working on my setup. Applied and backported down to 2.13. Best regards, Ilya Maximets. > >>> >>> Below the related log message: >>> |00058|dpif|WARN|system@ovs-system: execute ct(commit) failed (Invalid >>> argument) >>> on packet >>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a, >>> >>> nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,nw_frag=first,tp_src=1, >>> tp_dst=2 udp_csum:0 >>> >>> Fix the sequence by sending the second fragment in "conntrack - IPv4 >>> fragmentation incomplete reassembled packet", once the checks are >>> done. >>> >>> IPv6 tests are not affected as the defrag kernel code path pretends to >>> add the duplicate fragment to the queue returning -EINPROGRESS, when a >>> duplicate is detected. >>> >>> Signed-off-by: Paolo Valerio <[email protected]> >>> --- >>> tests/system-traffic.at | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >>> index 1a864057c..8497b4d9e 100644 >>> --- a/tests/system-traffic.at >>> +++ b/tests/system-traffic.at >>> @@ -3452,6 +3452,11 @@ AT_CHECK([ovs-ofctl bundle br0 bundle.txt]) >>> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl >>> ]) >>> >>> +dnl Send the second fragment in order to avoid keeping the first fragment >>> +dnl in the queue until the expiration occurs. Fragments already queued, if >>> resent, >>> +dnl may lead to failures on the kernel datapath. >>> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1, >>> packet=50540000000a505400000009080045000030000100320011a4860a0101010a01010200010002000800000010203040506070809000010203040506070809, >>> actions=ct(commit)"]) >>> + >>> OVS_TRAFFIC_VSWITCHD_STOP >>> AT_CLEANUP >>> >>> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
