[ovs-dev] [PATCH 12/12] system-traffic: Add zone-based conntrack timeout policy test

Darrell Ball dlu998 at gmail.com
Sat Jul 27 19:36:56 UTC 2019


One more comment inline regarding to the cleanup part

On Fri, Jul 26, 2019 at 9:27 AM Darrell Ball <dlu998 at gmail.com> wrote:

> Thanks for the patch
>
> Comments inline
>
> On Thu, Jul 25, 2019 at 4:31 PM Yi-Hung Wei <yihung.wei at 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 at 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 at 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 at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>


More information about the dev mailing list