On 19 April 2016 at 16:53, Joe Stringer <j...@ovn.org> wrote: > On 15 April 2016 at 17:02, Daniele Di Proietto <diproiet...@vmware.com> wrote: >> The userspace connection tracker doesn't support ALGs, frag reassembly >> or NAT yet, so skip those tests. >> >> Also, connection tracking state input from a local port is not possible >> in userspace. >> >> Finally, the userspace datapath pads all frames with 0, to make them at >> least 64 bytes. >> >> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com> > > Having three prongs to your commit message like this usually indicates > that there are three separate patches folded inside ;-) > > FWIW the LOCAL tests should work whether the local stack provides > conntrack state input or not - they just exercise slightly different > codepaths in the kernel, once the packet leaves OVS, by going out a > different netdev type (with a different destination netns). Probably > the class of bugs these tests pick up are not applicable for userspace > datapath, so it seems fine to skip the conntrack-related ones. I > wouldn't mind seeing a basic sanity test that sends packets between > the local stack and another namespace, though - which would run for > both testsuites, and both with IPv4 and IPv6 traffic.
I was mistaken here. I looked back over and apparently some of the tests do rely on this behaviour, and they're passing on the latest net-next kernels. They can be skipped for userspace. > A few comments on the comments below, but LGTM. > > Acked-by: Joe Stringer <j...@ovn.org> > >> --- >> tests/system-kmod-macros.at | 28 +++++++++++++++++++++++ >> tests/system-traffic.at | 49 >> ++++++++++++++++++++++++++++++---------- >> tests/system-userspace-macros.at | 45 +++++++++++++++++++++++++++++++++--- >> 3 files changed, 107 insertions(+), 15 deletions(-) >> >> diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at >> index 8e60929..4cecc23 100644 >> --- a/tests/system-kmod-macros.at >> +++ b/tests/system-kmod-macros.at >> @@ -65,3 +65,31 @@ m4_define([CHECK_CONNTRACK], >> on_exit 'ovstest test-netlink-conntrack flush' >> ] >> ) >> + >> +# CHECK_CONNTRACK_ALG() >> +# >> +# Perform requirements checks for running conntrack ALG tests. The kernel >> +# always supports ALG, so no check is needed. >> +# >> +m4_define([CHECK_CONNTRACK_ALG]) > > Saying the kernel "always" supports X is a little misleading, as these > features can all be turned off in the CONFIG.. of course, all the > major distros provide kernels with them turned on so it's not such a > big deal. Maybe drop the 'always'. > >> +# CHECK_CONNTRACK_FRAG() >> +# >> +# Perform requirements checks for running conntrack fragmentations tests. >> +# The kernel always supports fragmentation, so no check is needed. >> +m4_define([CHECK_CONNTRACK_FRAG]) >> + >> +# CHECK_CONNTRACK_LOCAL_STACK() >> +# >> +# Perform requirements checks for running conntrack tests with local stack. >> +# The kernel always supports reading the connection state of an skb coming >> +# from an internal port, without an explicit ct() action, so no check is >> +# needed. >> +m4_define([CHECK_CONNTRACK_LOCAL_STACK]) > > While the kernel module supports this, the skb is not guaranteed to > have conntrack state when coming from an internal port, depending on > the kernel version and whether iptables rules are installed in the > host network namespace. Looks like I got a little mixed up - netfilter hooks are not installed in all namespaces by default on the latest kernels, but I'd guess that they're still set up in the root netns so it should be fairly reliable there (at least for the moment). > <snip> > >> +# CHECK_CONNTRACK_LOCAL_STACK() >> +# >> +# Perform requirements checks for running conntrack tests with local stack. >> +# While the kernel connection tracker automatically passes all the >> connection >> +# tracking state from an internal port to the OpenvSwitch kernel module, >> there >> +# is simply no way of doing that with the userspace, so skip the tests. >> +m4_define([CHECK_CONNTRACK_LOCAL_STACK], >> +[ >> + AT_SKIP_IF([:]) >> +]) > > Same comment here, packets may not have conntrack state associated > when arriving on internal port. > >> +# CHECK_CONNTRACK_NAT() >> +# >> +# Perform requirements checks for running conntrack NAT tests. The userspace >> +# doesn't support NATs yet, so skip the tests >> +# >> +m4_define([CHECK_CONNTRACK_NAT], >> +[ >> + AT_SKIP_IF([:]) >> +]) >> -- >> 2.1.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
