On Tue, Jan 13, 2015 at 05:28:06PM -0800, Andy Zhou wrote:
> This patch adds a basic infrastructure for developing and running
> kernel module unit tests. Currently OVS contains thousands
> of useful unit tests for user space programs. It is desirable to
> have corresponding kernel module unit tests.
>
> This commit adds basic framework for adding kernel module tests. Like
> user space unit tests, Kmod tests are based autotest framework, thus
> are similar to existing unit tests. For references, kmod-traffic.at
> contains a simple ping test.
>
> "make check-kmod" can be invoked on any build machine as a root
> user. Since kernel testing can potentially crash the kernel, it is
> not recommended to run those tests directly on a development machine,
> but rather a testing VM, such as ones can be launched by vagrant.
>
> Signed-off-by: Andy Zhou <az...@nicira.com>
We've been waiting a long time for someone to write some kernel module
tests. Thanks a lot for stepping forward to add the infrastructure and
getting started on the tests.
I have a few comments.
I think that KMOD_TESTSUITE_AT should include the .at files from the
regular testsuite that the kmod testsuite uses, so that kmod-testsuite
gets rebuilt when those change. For example, I think that
kmod-testsuite includes ofproto-macros.at, which means that
kmod-testsuite should be rebuild when ofproto-macros.at changes, so I
think that KMOD_TESTSUITE_AT should list ofproto-macros.at
> +m4_define([OVS_KMOD_VSWITCHD_STOP],
> + [AT_CHECK([ovs-vsctl del-br br0])
I think that $1 should be quoted below, e.g. ([$1]):
> + OVS_SWITCHD_STOP($1)
> + AT_CHECK([modprobe -r openvswitch])
> + ])
I think that as written this will expand literally without any spacing,
e.g. DEL_NAMESPACES([a], [b], [c]) will become:
ip netns del aip netns del bip netns del c
> +m4_define([DEL_NAMESPACES],
> + [m4_foreach([ns], [$@],
> + [ip netns del ns])
> + ]
> +)
While there's no correctness problem with extra spaces inside [] here,
it doesn't match the style we've used elsewhere:
> +m4_define([ADD_VETH],
> + [ ovs-vsctl del-port $3 ovs-$1
> + ip netns exec $2 ip link del $1
> + AT_CHECK([ ip link add $1 type veth peer name ovs-$1 ])
> + AT_CHECK([ ip link set $1 netns $2 ])
> + AT_CHECK([ ovs-vsctl add-port $3 ovs-$1 ])
> + AT_CHECK([ ip link set dev ovs-$1 up ])
> + AT_CHECK([ ip netns exec $2 ifconfig $1 $4 netmask $5 up ])
> + ]
> +)
kmod-testsuite.at has a lot in common with testsuite.at. Could the
common macros be factored out into a mutually included .at?
I find myself wondering whether we should adopt some naming convention
for namespaces created by the testsuite, such as a prefix. That would
make it safe(r) to delete all of the namespaces created by the tests.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
