Hi Tonghao Xhang,

[email protected] writes:

> From: Tonghao Zhang <[email protected]>
>
> The same icmp packet may traverse conntrack module more than once.
> Or same icmp packets traverse contranck module in orderly.
>
> Don't change stats to CS_ESTABLISHED before receiving reply or related 
> packets.
>
> Fixes: b269a1229df2 ("conntrack: Track ICMP type and code.")
> Cc: Daniele Di Proietto <[email protected]>
> Signed-off-by: Tonghao Zhang <[email protected]>

I'm not quite sure what you're seeing from the state machine side, based
on the commit message.  I've included a test case to try and reproduce
your issue but it doesn't seem like I can based on your description.

Can you modify it to show exactly what you are fixing, and include that
with v2?  As it stands, if I use this test, it passes and so doesn't
seem like the packet metadata is changed on a second call through ct() -
did I miss something?  Maybe I'm looking at a wrong output somewhere?

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 2824f879c1..767efb2ed3 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5898,6 +5898,59 @@ ovs-appctl dpif/dump-flows br0
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+# OFPROTO_CLEAR_DURATION([])
+#
+# Clear the duration from the piped input which would differ from test to test
+#
+m4_define([OFPROTO_CLEAR_DURATION], [sed -e 
's/duration=.*s,/duration=<cleared>,/g'])
+
+# OFPROTO_CLEAR_DURATION_PKTS([])
+#
+# Clear the duration from the piped input which would differ from test to test
+#
+m4_define([OFPROTO_CLEAR_DURATION_PKTS], [sed -e 
's/duration=.*s,/duration=<cleared>,/g' -e 
's/n_packets=.*,/n_packets=<cleared>,/g'])
+
+
+AT_SETUP([conntrack - Multiple ICMP traverse])
+dnl This tracks sending ICMP packets via conntrack multiple times for the
+dnl same packet
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+OVS_CHECK_CT_CLEAR()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02")
+dnl setup ct flows
+AT_DATA([flows.txt], [dnl
+table=0,priority=10  arp action=normal
+table=0,priority=10  ip,icmp,ct_state=-trk action=ct(table=1)
+table=0,priority=0   action=drop
+table=1,priority=10  ct_state=+trk+new,ip,in_port=1 action=ct(commit,table=2)
+table=1,priority=10  in_port=2 action=1
+table=2,priority=10  ct_state=+trk+new,in_port=1 action=2
+table=2,priority=10  ct_state=+trk+est action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+NS_EXEC([at_ns0], [ping 10.1.1.2 -c 1 -q])
+
+dnl ensure CT picked up the packet
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0)
+])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION],
+         [0], [dnl
+ cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=98, idle_age=0, 
priority=10,ct_state=+new+trk,in_port=1 actions=output:2
+ cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, idle_age=0, 
priority=10,ct_state=+est+trk actions=drop
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
+

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to