[ovs-dev] [PATCH v2] tests: Add check for correct l3l4 conntrack frag reassembly

[Darrell Ball]

Thanks for the patch

This approach will not work for the userspace datapath

Few issues off the top of my head:

1/ packet-out frees the packet (which is a fragment in this case) after
completion
   hence multiple packet-outs need to be part of a single Openflow bundle
command as in other similar tests
   This test involves 2 fragments for the first 2 packets.

2/ Userpsace datapath checks UDP checksums; for V6 packets, they need to
always be correct and they are not presently

3/ UDP header lengths cannot be larger than the memory allocated for the
packet, else sanity checking will filter
    out the packet

Alternatively, if you want to use this simplified approach, you can disable
the test for the userspace datapath.

Darrell


On Thu, Oct 3, 2019 at 10:46 AM Greg Rose <gvrose8192 at gmail.com> wrote:

> Two commits recently fixed an issue with setting the corrrect l3 and l4
> flow information when conntrack reassembles packet fragments.
>
> c98f776 datapath: Clear the L4 portion of the key for "later" fragments
> 2609173 datapath: Properly set L4 keys on "later" IP fragments
>
> This test checks for regressions that might break this feature.  It
> counts on the fact that when the bug is present the udp src port
> will not be correct.  It will either be zero or else some other
> garbage value.  So the test feeds some fragments through for
> reassembly and then checks to make sure that the udp srce port
> is actually the correct value of 5001.
>
> Tested by reverting the above commits and observing that the test
> then fails.
>
> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
> ---
>
> V2 - Break up long lines with dnl to help with email formatting
> ---
>  tests/system-traffic.at | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index bfc6bb5..9afd818 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -3245,6 +3245,32 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack |
> FORMAT_CT(10.1.1.2)], [0], [dnl
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
> +AT_SETUP([conntrack - fragment reassembly test])
> +CHECK_CONNTRACK()
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +AT_DATA([flows.txt], [dnl
> +action=normal
> +])
> +
> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +
> +AT_CHECK([ovs-ofctl packet-out br0
> "packet=52540003287c525400444ab586dd6006f70605b02c4020010001000000000000000000000020200100010000000000000000000000101100000134e88deb13891389080803136161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616"dnl
>
> +"16161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161"dnl
>
> +"61616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616"dnl
> +"1616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161,
> actions=ct(table=1)"])
> +
> +AT_CHECK([ovs-ofctl packet-out br0
> "packet=52540003287c525400444ab586dd6006f70602682c402001000100000000000000000000002020010001000000000000000000000010110005a834e88deb6161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616"dnl
> +"161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161,
> actions=ct(table=1)"])
> +
> +AT_CHECK([ovs-ofctl packet-out br0
> "packet=52540003287c525400444ab586dd6006f706033d1140200100010000000000000000000000202001000100000000000000000000001013891389033d923861616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616"dnl
> +"1616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161610a,
> actions=ct(table=1)"])
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | head -2 | tail -1 | grep -q -e
> ["]udp[(]src=5001["]])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_BANNER([conntrack - L7])
>
>  AT_SETUP([conntrack - IPv4 HTTP])
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>