| Message ID | 1564097054-72663-13-git-send-email-yihung.wei@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support zone-based conntrack timeout policy | expand |
Thanks for the patch Comments inline On Thu, Jul 25, 2019 at 4:31 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote: > This patch adds a system traffic test to verify the zone-based conntrack > timeout feature. The test uses ovs-vsctl commands to configure > the customized ICMP and UDP timeout on zone 5 to a shorter period. > It then injects ICMP and UDP traffic to conntrack, and checks if the > corresponding conntrack entry expires after the predefined timeout. > > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > --- > tests/system-kmod-macros.at | 9 ++++++ > tests/system-traffic.at | 65 > ++++++++++++++++++++++++++++++++++++++++ > tests/system-userspace-macros.at | 10 +++++++ > 3 files changed, 84 insertions(+) > > diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at > index 554a61e9bd95..f3c68277be65 100644 > --- a/tests/system-kmod-macros.at > +++ b/tests/system-kmod-macros.at > @@ -100,6 +100,15 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP], > # > m4_define([CHECK_CONNTRACK_NAT]) > > +# CHECK_CONNTRACK_TIMEOUT() > +# > +# Perform requirements checks for running conntrack customized timeout > tests. > +# > +m4_define([CHECK_CONNTRACK_TIMEOUT], > +[ > + AT_SKIP_IF([! cat /boot/config-$(uname -r) | grep > NF_CONNTRACK_TIMEOUT | grep '=y' > /dev/null]) > +]) > + > # CHECK_CT_DPIF_PER_ZONE_LIMIT() > # > # Perform requirements checks for running ovs-dpctl > ct-[set|get|del]-limits per > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 1a04199dcfe9..6699466dbf4f 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -3179,6 +3179,71 @@ NXST_FLOW reply: > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([conntrack - zone-based timeout policy]) > +CHECK_CONNTRACK() > +CHECK_CONNTRACK_TIMEOUT() > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > + > +AT_DATA([flows.txt], [dnl > +priority=1,action=drop > +priority=10,arp,action=normal > +priority=100,in_port=1,ip,action=ct(zone=5, table=1) > +priority=100,in_port=2,ip,action=ct(zone=5, table=1) > +table=1,in_port=2,ip,ct_state=+trk+est,action=1 > +table=1,in_port=1,ip,ct_state=+trk+new,action=ct(commit,zone=5),2 > +table=1,in_port=1,ip,ct_state=+trk+est,action=2 > +]) > + > +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > + > +dnl Add customized timeout > +dnl Note that the default ICMP timeout is 30 seconds. > +dnl The default timeout for unreplied UDP is 30 seconds, and > +dnl 180 seconds for replied UDP connection. > Might be good to confirm that the conntrack entries are not removed quickly before reduced timeouts are configured Then flush the entries Then configure lower timeouts The send the packets again Then confirm they are removed quickly > +AT_CHECK([ovs-vsctl add-dp system]) > +AT_CHECK([ovs-vsctl add-zone-tp system zone=5 udp_first=3 icmp_first=3]) > + > +dnl ICMP traffic > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | > FORMAT_PING], [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [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),zone=5 > +]) > + > +dnl Wait until ICMP timeout expire. > +dnl We intend to wait a bit longer, because conntrack does not recycle > the entry right after it is expired. > +sleep 4 > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], > [dnl > +]) > + > +dnl Send out an UDP packet from port 1 > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 > actions=resubmit(,0)"]) > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "dst=10\.1\.1\.2,"], > [0], [dnl > > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),zone=5 > +]) > + > +dnl Wait until UDP timeout expire > +sleep 4 > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], > [dnl > +]) > + > +AT_CHECK([ovs-ofctl del-flows br0]) > +AT_CHECK([ovs-appctl revalidator/wait]) > Hopefully, the above 2 lines are NOT required to allow 'del-zone' to work - can you remove them ? I will test later today to confirm. This patch logically makes sense as part of Patch 11. > +AT_CHECK([ovs-vsctl del-zone-tp system zone=5]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > AT_BANNER([conntrack - L7]) > > AT_SETUP([conntrack - IPv4 HTTP]) > diff --git a/tests/system-userspace-macros.at b/tests/ > system-userspace-macros.at > index 9d5f3bf419d3..ceabad7499d8 100644 > --- a/tests/system-userspace-macros.at > +++ b/tests/system-userspace-macros.at > @@ -98,6 +98,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP]) > # > m4_define([CHECK_CONNTRACK_NAT]) > > +# CHECK_CONNTRACK_TIMEOUT() > +# > +# Perform requirements checks for running conntrack customized timeout > tests. > +* The userspace datapath does not support this feature yet. > +# > +m4_define([CHECK_CONNTRACK_TIMEOUT], > +[ > + AT_SKIP_IF([:]) > +]) > + > # CHECK_CT_DPIF_PER_ZONE_LIMIT() > # > # Perform requirements checks for running ovs-dpctl > ct-[set|get|del]-limits per > -- > 2.7.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
One more comment inline regarding to the cleanup part On Fri, Jul 26, 2019 at 9:27 AM Darrell Ball <dlu998@gmail.com> wrote: > Thanks for the patch > > Comments inline > > On Thu, Jul 25, 2019 at 4:31 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote: > >> This patch adds a system traffic test to verify the zone-based conntrack >> timeout feature. The test uses ovs-vsctl commands to configure >> the customized ICMP and UDP timeout on zone 5 to a shorter period. >> It then injects ICMP and UDP traffic to conntrack, and checks if the >> corresponding conntrack entry expires after the predefined timeout. >> >> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> >> --- >> tests/system-kmod-macros.at | 9 ++++++ >> tests/system-traffic.at | 65 >> ++++++++++++++++++++++++++++++++++++++++ >> tests/system-userspace-macros.at | 10 +++++++ >> 3 files changed, 84 insertions(+) >> >> diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at >> index 554a61e9bd95..f3c68277be65 100644 >> --- a/tests/system-kmod-macros.at >> +++ b/tests/system-kmod-macros.at >> @@ -100,6 +100,15 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP], >> # >> m4_define([CHECK_CONNTRACK_NAT]) >> >> +# CHECK_CONNTRACK_TIMEOUT() >> +# >> +# Perform requirements checks for running conntrack customized timeout >> tests. >> +# >> +m4_define([CHECK_CONNTRACK_TIMEOUT], >> +[ >> + AT_SKIP_IF([! cat /boot/config-$(uname -r) | grep >> NF_CONNTRACK_TIMEOUT | grep '=y' > /dev/null]) >> +]) >> + >> # CHECK_CT_DPIF_PER_ZONE_LIMIT() >> # >> # Perform requirements checks for running ovs-dpctl >> ct-[set|get|del]-limits per >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >> index 1a04199dcfe9..6699466dbf4f 100644 >> --- a/tests/system-traffic.at >> +++ b/tests/system-traffic.at >> @@ -3179,6 +3179,71 @@ NXST_FLOW reply: >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> >> +AT_SETUP([conntrack - zone-based timeout policy]) >> +CHECK_CONNTRACK() >> +CHECK_CONNTRACK_TIMEOUT() >> +OVS_TRAFFIC_VSWITCHD_START() >> + >> +ADD_NAMESPACES(at_ns0, at_ns1) >> + >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") >> + >> +AT_DATA([flows.txt], [dnl >> +priority=1,action=drop >> +priority=10,arp,action=normal >> +priority=100,in_port=1,ip,action=ct(zone=5, table=1) >> +priority=100,in_port=2,ip,action=ct(zone=5, table=1) >> +table=1,in_port=2,ip,ct_state=+trk+est,action=1 >> +table=1,in_port=1,ip,ct_state=+trk+new,action=ct(commit,zone=5),2 >> +table=1,in_port=1,ip,ct_state=+trk+est,action=2 >> +]) >> + >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) >> + >> +dnl Add customized timeout >> +dnl Note that the default ICMP timeout is 30 seconds. >> +dnl The default timeout for unreplied UDP is 30 seconds, and >> +dnl 180 seconds for replied UDP connection. >> > > Might be good to confirm that the conntrack entries are not removed > quickly before > reduced timeouts are configured > Then flush the entries > Then configure lower timeouts > The send the packets again > Then confirm they are removed quickly > > >> +AT_CHECK([ovs-vsctl add-dp system]) >> +AT_CHECK([ovs-vsctl add-zone-tp system zone=5 udp_first=3 icmp_first=3]) >> + >> +dnl ICMP traffic >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | >> FORMAT_PING], [0], [dnl >> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >> +]) >> + >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], >> [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),zone=5 >> +]) >> + >> +dnl Wait until ICMP timeout expire. >> +dnl We intend to wait a bit longer, because conntrack does not recycle >> the entry right after it is expired. >> +sleep 4 >> + >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], >> [dnl >> +]) >> + >> +dnl Send out an UDP packet from port 1 >> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 >> packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 >> actions=resubmit(,0)"]) >> + >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "dst=10\.1\.1\.2,"], >> [0], [dnl >> >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),zone=5 >> +]) >> + >> +dnl Wait until UDP timeout expire >> +sleep 4 >> + >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], >> [dnl >> +]) >> + >> +AT_CHECK([ovs-ofctl del-flows br0]) >> +AT_CHECK([ovs-appctl revalidator/wait]) >> > > Hopefully, the above 2 lines are NOT required to allow 'del-zone' to work > - can you remove them ? > I will test later today to confirm. > The above cleanup +AT_CHECK([ovs-ofctl del-flows br0]) +AT_CHECK([ovs-appctl revalidator/wait]) is not very realistic although without it, we might end up here: dball@ubuntu:~/openvswitch/ovs$ lsmod | grep nf nfnetlink_cttimeout 16384 1 and then not being able to rerun without intervention. This looks a little strange in a 'system' test, since a 'system' test is supposed to be closer to reality. However, more concerning is the possible non-test environment implications. I guess we need to check. > > This patch logically makes sense as part of Patch 11. > > > >> +AT_CHECK([ovs-vsctl del-zone-tp system zone=5]) >> + >> +OVS_TRAFFIC_VSWITCHD_STOP >> +AT_CLEANUP >> + >> AT_BANNER([conntrack - L7]) >> >> AT_SETUP([conntrack - IPv4 HTTP]) >> diff --git a/tests/system-userspace-macros.at b/tests/ >> system-userspace-macros.at >> index 9d5f3bf419d3..ceabad7499d8 100644 >> --- a/tests/system-userspace-macros.at >> +++ b/tests/system-userspace-macros.at >> @@ -98,6 +98,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP]) >> # >> m4_define([CHECK_CONNTRACK_NAT]) >> >> +# CHECK_CONNTRACK_TIMEOUT() >> +# >> +# Perform requirements checks for running conntrack customized timeout >> tests. >> +* The userspace datapath does not support this feature yet. >> +# >> +m4_define([CHECK_CONNTRACK_TIMEOUT], >> +[ >> + AT_SKIP_IF([:]) >> +]) >> + >> # CHECK_CT_DPIF_PER_ZONE_LIMIT() >> # >> # Perform requirements checks for running ovs-dpctl >> ct-[set|get|del]-limits per >> -- >> 2.7.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at index 554a61e9bd95..f3c68277be65 100644 --- a/tests/system-kmod-macros.at +++ b/tests/system-kmod-macros.at @@ -100,6 +100,15 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP], # m4_define([CHECK_CONNTRACK_NAT]) +# CHECK_CONNTRACK_TIMEOUT() +# +# Perform requirements checks for running conntrack customized timeout tests. +# +m4_define([CHECK_CONNTRACK_TIMEOUT], +[ + AT_SKIP_IF([! cat /boot/config-$(uname -r) | grep NF_CONNTRACK_TIMEOUT | grep '=y' > /dev/null]) +]) + # CHECK_CT_DPIF_PER_ZONE_LIMIT() # # Perform requirements checks for running ovs-dpctl ct-[set|get|del]-limits per diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 1a04199dcfe9..6699466dbf4f 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -3179,6 +3179,71 @@ NXST_FLOW reply: OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - zone-based timeout policy]) +CHECK_CONNTRACK() +CHECK_CONNTRACK_TIMEOUT() +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +AT_DATA([flows.txt], [dnl +priority=1,action=drop +priority=10,arp,action=normal +priority=100,in_port=1,ip,action=ct(zone=5, table=1) +priority=100,in_port=2,ip,action=ct(zone=5, table=1) +table=1,in_port=2,ip,ct_state=+trk+est,action=1 +table=1,in_port=1,ip,ct_state=+trk+new,action=ct(commit,zone=5),2 +table=1,in_port=1,ip,ct_state=+trk+est,action=2 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl Add customized timeout +dnl Note that the default ICMP timeout is 30 seconds. +dnl The default timeout for unreplied UDP is 30 seconds, and +dnl 180 seconds for replied UDP connection. +AT_CHECK([ovs-vsctl add-dp system]) +AT_CHECK([ovs-vsctl add-zone-tp system zone=5 udp_first=3 icmp_first=3]) + +dnl ICMP traffic +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [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),zone=5 +]) + +dnl Wait until ICMP timeout expire. +dnl We intend to wait a bit longer, because conntrack does not recycle the entry right after it is expired. +sleep 4 + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +]) + +dnl Send out an UDP packet from port 1 +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "dst=10\.1\.1\.2,"], [0], [dnl +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),zone=5 +]) + +dnl Wait until UDP timeout expire +sleep 4 + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +]) + +AT_CHECK([ovs-ofctl del-flows br0]) +AT_CHECK([ovs-appctl revalidator/wait]) +AT_CHECK([ovs-vsctl del-zone-tp system zone=5]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_BANNER([conntrack - L7]) AT_SETUP([conntrack - IPv4 HTTP]) diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at index 9d5f3bf419d3..ceabad7499d8 100644 --- a/tests/system-userspace-macros.at +++ b/tests/system-userspace-macros.at @@ -98,6 +98,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP]) # m4_define([CHECK_CONNTRACK_NAT]) +# CHECK_CONNTRACK_TIMEOUT() +# +# Perform requirements checks for running conntrack customized timeout tests. +* The userspace datapath does not support this feature yet. +# +m4_define([CHECK_CONNTRACK_TIMEOUT], +[ + AT_SKIP_IF([:]) +]) + # CHECK_CT_DPIF_PER_ZONE_LIMIT() # # Perform requirements checks for running ovs-dpctl ct-[set|get|del]-limits per
This patch adds a system traffic test to verify the zone-based conntrack timeout feature. The test uses ovs-vsctl commands to configure the customized ICMP and UDP timeout on zone 5 to a shorter period. It then injects ICMP and UDP traffic to conntrack, and checks if the corresponding conntrack entry expires after the predefined timeout. Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> --- tests/system-kmod-macros.at | 9 ++++++ tests/system-traffic.at | 65 ++++++++++++++++++++++++++++++++++++++++ tests/system-userspace-macros.at | 10 +++++++ 3 files changed, 84 insertions(+)