Ilya Maximets <i.maxim...@ovn.org> writes:
> On 3/13/24 12:08, Paolo Valerio wrote:
>> Recent kernels introduced a mechanism that allows to evict colliding
>> entries in a closing state whereas they were previously considered as
>> parts of a non-recoverable clash.
>> This new behavior makes "conntrack - SNAT with port range with
>> exhaustion test" fail, as it relies on the previous assumptions.
>>
>> Fix it by creating and not advancing the first entry in SYN_SENT to
>> avoid early eviction.
>>
>> Suggested-by: Ilya Maximets <i.maxim...@ovn.org>
>> Reported-at: https://issues.redhat.com/browse/FDP-486
>> Signed-off-by: Paolo Valerio <pvale...@redhat.com>
>> ---
>
> Hi, Paolo. Thanks for the fix!
>
Hi Ilya,
Thanks for the feedback!
> Some small comments inline.
>
>> tests/system-traffic.at | 21 ++++++++++++++-------
>> 1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index 2d12d558e..04559f5e8 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -6388,7 +6388,6 @@ OVS_TRAFFIC_VSWITCHD_STOP
>> AT_CLEANUP
>>
>> AT_SETUP([conntrack - SNAT with port range with exhaustion])
>> -OVS_CHECK_GITHUB_ACTION()
>> CHECK_CONNTRACK()
>> CHECK_CONNTRACK_NAT()
>> OVS_TRAFFIC_VSWITCHD_START()
>> @@ -6398,11 +6397,11 @@ ADD_NAMESPACES(at_ns0, at_ns1)
>> ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
>> NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
>> ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
>> +NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address 80:89:89:89:89:89])
>>
>> dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from
>> ns1->ns0.
>> AT_DATA([flows.txt], [dnl
>> -in_port=1,tcp,action=ct(commit,zone=1,nat(src=10.1.1.240:34568,random)),2
>> -in_port=2,ct_state=-trk,tcp,tp_dst=34567,action=ct(table=0,zone=1,nat)
>
> Do you know why this flow was there in the first place?
>
AFAICT, this seemed to me part of C/P ("conntrack - SNAT with port
range").
While at it, I preferred to clean it a bit as this (along with a couple
of minor things) was not required.
>> +in_port=1,tcp,action=ct(commit,zone=1,nat(src=10.1.1.240:34568)),2
>> in_port=2,ct_state=-trk,tcp,tp_dst=34568,action=ct(table=0,zone=1,nat)
>> in_port=2,ct_state=+trk,ct_zone=1,tcp,action=1
>> dnl
>> @@ -6426,17 +6425,25 @@ AT_CHECK([ovs-ofctl --bundle add-flows br0
>> flows.txt])
>>
>> dnl HTTP requests from p0->p1 should work fine.
>> OVS_START_L7([at_ns1], [http])
>> -NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 1 -T 1 --retry-connrefused -v -o
>> wget0.log])
>> +
>> +dnl Send a valid SYN to make conntrack pick it up.
>> +dnl The source port used is 123 to prevent unwanted reuse in the next HTTP
>> request.
>> +AT_CHECK([ovs-ofctl packet-out br0
>> "packet=80898989898980888888888808004500002800010000400664cb0a0101010a010102007b005000000000000000005002200079130000
>> actions=ct(commit,zone=1,nat(src=10.1.1.240:34568))"])
>
> Can we use 'ovs-ofctl compose-packet --bare' instead of open-coding bytes?
>
sure, I'll send a v2.
> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
