diff mbox series

[ovs-dev,v2] conntrack: Fix SNAT with exhaustion system test.

Message ID 20240328161846.252508-1-pvalerio@redhat.com
State Accepted
Commit b674e7e04e1d37400c3946f1d3ebe81c99501f7e
Delegated to: Ilya Maximets
Headers show
Series [ovs-dev,v2] conntrack: Fix SNAT with exhaustion system test. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Paolo Valerio March 28, 2024, 4:18 p.m. UTC 
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.maximets@ovn.org>
Reported-at: https://issues.redhat.com/browse/FDP-486
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
v2:
- replaced open-coded bytes with
  'ovs-ofctl compose-packet --bare' (Ilya)
---
 tests/system-traffic.at | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

Comments

Ilya Maximets April 3, 2024, 10:48 a.m. UTC | #1 
On 3/28/24 17:18, 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.maximets@ovn.org>
> Reported-at: https://issues.redhat.com/browse/FDP-486
> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
> ---
> v2:
> - replaced open-coded bytes with
>   'ovs-ofctl compose-packet --bare' (Ilya)
> ---
>  tests/system-traffic.at | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)

Thanks!  Applied and backported down to 2.17.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 2d12d558e..20b011b7e 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)
+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,28 @@  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.
+syn_pkt=$(ovs-ofctl compose-packet --bare "eth_src=80:88:88:88:88:88,eth_dst=80:89:89:89:89:89,\
+  dl_type=0x0800,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=6,nw_ttl=64,nw_frag=no,tcp_flags=syn,\
+  tcp_src=123,tcp_dst=80")
+AT_CHECK([ovs-ofctl packet-out br0 "packet=${syn_pkt} actions=ct(commit,zone=1,nat(src=10.1.1.240:34568))"])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | uniq], [0], [dnl
+tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.240,sport=<cleared>,dport=<cleared>),zone=1,protoinfo=(state=<cleared>)
+])
 
 NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 1 -T 1 --retry-connrefused -v -o wget0.log], [4])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e 's/dst=10.1.1.2[[45]][[0-9]]/dst=10.1.1.2XX/' | uniq], [0], [dnl
-tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.2XX,sport=<cleared>,dport=<cleared>),zone=1,protoinfo=(state=<cleared>)
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | uniq], [0], [dnl
+tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.240,sport=<cleared>,dport=<cleared>),zone=1,protoinfo=(state=<cleared>)
 ])
 
 OVS_TRAFFIC_VSWITCHD_STOP(["dnl
 /Unable to NAT due to tuple space exhaustion - if DoS attack, use firewalling and\/or zone partitioning./d
-/Dropped .* log messages in last .* seconds \(most recently, .* seconds ago\) due to excessive rate/d"])
+/Dropped .* log messages in last .* seconds \(most recently, .* seconds ago\) due to excessive rate/d
+/|WARN|.* execute ct.* failed/d"])
 AT_CLEANUP
 
 AT_SETUP([conntrack - more complex SNAT])