Ilya Maximets <i.maxim...@ovn.org> writes: > On 8/5/22 23:49, Paolo Valerio wrote: >> Ilya Maximets <i.maxim...@ovn.org> 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. >
In general, the idea seems a good one to me aside from this specific issue. Yes, no APIs spotted, and I agree that all those modifications don't sound particularly fun :) > For now, I confirmed that the fix is working on my setup. > Applied and backported down to 2.13. > Thank you Ilya! > 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 <pvale...@redhat.com> >>>> --- >>>> 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
