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 > 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 > + > + > -- Best regards, Tonghao _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
