Tonghao Zhang <[email protected]> writes:
> On Fri, Jan 15, 2021 at 5:20 AM Aaron Conole <[email protected]> wrote:
>>
>> 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?
> Hi Aaron
> I don't use the testsuite to reproduce it. but it's easy to reproduce
> it with dpdk, and scapy tools.
>
> The openflow rules as below:
> + /root/local/openvswitch-2.14/bin/ovs-ofctl add-flow br-int
> 'priority:50000,table=0,in_port=rep0,ip,ct_state=-trk
> action=set_field:1->xreg0,ct(table=0,zone=1)'
> + /root/local/openvswitch-2.14/bin/ovs-ofctl add-flow br-int
> 'priority:40000,table=0,in_port=rep0,ip,ct_state=+new-est-rel-inv+trk,ct_zone=1,icmp,ip_src=1.1.1.1
> action=ct(commit,zone=1,table=1)'
> + /root/local/openvswitch-2.14/bin/ovs-ofctl add-flow br-int
> 'priority:50000,table=0,in_port=rep0,ip,ct_state=-new+est-rel-inv+trk,ct_zone=1
> action=resubmit(,1)'
> + /root/local/openvswitch-2.14/bin/ovs-ofctl add-flow br-int
> 'priority:50000,table=1,xreg0=1,ip,ip_dst=1.1.1.2,ct_state=+new-est-rel-inv+trk,ct_zone=1,icmp
> action=ct(commit,zone=1,table=2)'
> + /root/local/openvswitch-2.14/bin/ovs-ofctl add-flow br-int
> 'priority:50000,table=1,xreg0=1,ip,ip_dst=1.1.1.2,ct_state=-new+est-rel-inv+trk,ct_zone=1,icmp
> action=resubmit(,2)'
>
> first packet:
> sendp(Ether(dst="00:11:22:33:44:52")/IP(src="1.1.1.1",
> dst="1.1.1.2")/ICMP(type=8, code=0,id=65535,seq=65535),
> iface="p4p1_0")
>
> $ ovs-appctl dpctl/dump-flows
> recirc_id(0xe),in_port(4),packet_type(ns=0,id=0),eth(dst=00:11:22:33:44:52),eth_type(0x0800),ipv4(dst=1.1.1.2,frag=no),
> packets:0, bytes:0, used:never, actions:5
> ct_state(+new-est-rel-inv+trk),ct_zone(0x1),recirc_id(0xd),in_port(4),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(dst=1.1.1.2,proto=1,frag=no),
> packets:0, bytes:0, used:never, actions:ct(commit,zone=1),recirc(0xe)
> ct_state(+new-est-rel-inv+trk),ct_zone(0x1),recirc_id(0xc),in_port(4),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=1.1.1.1,proto=1,frag=no),
> packets:0, bytes:0, used:never, actions:ct(commit,zone=1),recirc(0xd)
> ct_state(-trk),recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never, actions:ct(zone=1),recirc(0xc)
>
> the second
> sendp(Ether(dst="00:11:22:33:44:52")/IP(src="1.1.1.1",
> dst="1.1.1.2")/ICMP(type=8, code=0,id=65535,seq=65535),
> iface="p4p1_0")
>
> $ ovs-appctl dpctl/dump-flows
> recirc_id(0xe),in_port(4),packet_type(ns=0,id=0),eth(dst=00:11:22:33:44:52),eth_type(0x0800),ipv4(dst=1.1.1.2,frag=no),
> packets:0, bytes:0, used:never, actions:5
> ct_state(+new-est-rel-inv+trk),ct_zone(0x1),recirc_id(0xd),in_port(4),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(dst=1.1.1.2,proto=1,frag=no),
> packets:0, bytes:0, used:never, actions:ct(commit,zone=1),recirc(0xe)
> ct_state(+new-est-rel-inv+trk),ct_zone(0x1),recirc_id(0xc),in_port(4),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=1.1.1.1,proto=1,frag=no),
> packets:0, bytes:0, used:never, actions:ct(commit,zone=1),recirc(0xd)
> ct_state(-trk),recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:1, bytes:52, used:1.114s, actions:ct(zone=1),recirc(0xc)
>
> # the ct_state is +est, it is not right.
> ct_state(-new+est-rel-inv+trk),ct_zone(0x1),recirc_id(0xc),in_port(4),packet_type(ns=0,id=0),eth(dst=00:11:22:33:44:52),eth_type(0x0800),ipv4(dst=1.1.1.2,proto=1,frag=no),
> packets:0, bytes:0, used:never, actions:5
>
The fixes tag for this should be:
Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
Since that was the patch which attempted to make the behavior between
kernel-userspace consistent.
Please submit a v2 with the following diff (and run 'make
check-system-userspace' and 'make check-kernel' to ensure proper
coverage of the tests). This way, we don't break the behavior in the
future (and can guarantee consistency). I did try this out and it
worked for me.
122: conntrack - Multiple ICMP traverse ok
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5898,6 +5898,58 @@ ovs-appctl dpif/dump-flows br0
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+# 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']])
+
+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(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=0 ct_state=+est-new+trk,ct_zone=1,in_port=1
action=resubmit(,2)
+table=1,priority=10 in_port=2 action=1
+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
+
+
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
