conntrack: Fix the icmp conntrack new state.

The same icmp packet may traverse conntrack module more than once.
Or same icmp packets traverse contranck module in orderly.

Don't change state to CS_ESTABLISHED before receiving reply or related packets.

Fixes: a867c01 ("conntrack: Fix conntrack new state")
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

lib/conntrack-icmp.c

@@ -51,13 +51,16 @@ icmp_conn_update(struct conntrack *ct, struct conn *conn_,
                  struct dp_packet *pkt OVS_UNUSED, bool reply, long long now)
 {
     struct conn_icmp *conn = conn_icmp_cast(conn_);
+    enum ct_update_res ret = CT_UPDATE_VALID;
 
     if (reply && conn->state == ICMPS_FIRST) {
        conn->state = ICMPS_REPLY;
+    } else if (conn->state == ICMPS_FIRST) {
+        ret = CT_UPDATE_VALID_NEW;
     }
 
     conn_update_expiration(ct, &conn->up, icmp_timeouts[conn->state], now);
-    return CT_UPDATE_VALID;
+    return ret;
 }
 
 static bool

tests/system-common-macros.at

@@ -275,6 +275,12 @@ m4_define([OVS_START_L7],
    ]
 )
 
+# OFPROTO_CLEAR_DURATION_IDLE([])
+#
+# Clear the duration from the piped input which would differ from test to test
+#
+m4_define([OFPROTO_CLEAR_DURATION_IDLE], [[sed -e 's/duration=.*s,/duration=<cleared>,/g' -e 's/idle_age=[0-9]*,/idle_age=<cleared>,/g']])
+
 # OVS_CHECK_VXLAN()
 #
 # Do basic check for vxlan functionality, skip the test if it's not there.

tests/system-traffic.at

@@ -5927,6 +5927,50 @@ ovs-appctl dpif/dump-flows br0
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+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  ip,icmp,ct_state=-trk action=ct(zone=1,table=1)
+table=0,priority=0   action=drop
+table=1,priority=10  ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 action=ct(commit,table=2)
+table=1,priority=10  ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2)
+table=1,priority=0   action=drop
+table=2,priority=10  ct_state=+trk+new,in_port=1 action=drop
+table=2,priority=10  ct_state=+trk+est action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+# sending icmp pkts, first and second
+NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 f0 00 00 01 01 01 08 00 45 00 00 1c 00 01 00 00 40 01 64 dc 0a 01 01 01 0a 01 01 02 08 00 f7 ff ff ff ff ff > /dev/null])
+
+NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 f0 00 00 01 01 01 08 00 45 00 00 1c 00 01 00 00 40 01 64 dc 0a 01 01 01 0a 01 01 02 08 00 f7 ff ff ff ff ff > /dev/null])
+
+sleep 1
+
+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_IDLE],
+         [0], [dnl
+ cookie=0x0, duration=<cleared>, table=2, n_packets=2, n_bytes=84, idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 actions=drop
+ cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, idle_age=<cleared>, priority=10,ct_state=+est+trk actions=drop
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_BANNER([802.1ad])
 
 AT_SETUP([802.1ad - vlan_limit])