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

Darrell Ball dlu998 at gmail.com
Fri Oct 4 14:44:38 UTC 2019


On Fri, Oct 4, 2019 at 7:05 AM Gregory Rose <gvrose8192 at gmail.com> wrote:

>
> On 10/3/2019 6:56 PM, Darrell Ball wrote:
> > 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.
>
> There's no reason that I know of to run this test in the userspace
> datapath.


yep


> It should be disabled.


> How do you do that?
>

You need to add a macro to check applicability
for kernel, it will do nothing for check-kmod case; for full check-kernel
support, 'if desired', you need to check for versions that have the fix
for userspace, it will unconditionally skip the test, although a comment
explaining why would be helpful
see the example for CHECK_CT_DPIF_PER_ZONE_LIMIT which handles check-kmod
vs check-system-userspace

I noticed a couple other issues:

The test is labeled "fragment reassembly test":

a/ All the conntrack fragmentation tests include a reassembly aspect so a
better
name describing the special purpose of the test might be helpful along with
a
comment explaining the special purpose for the test.

b/ The "test" part of the name is redundant


>
> - Greg
>
>
>


More information about the dev mailing list