On 05/08/2015 18:44, "Joe Stringer" <joestrin...@nicira.com> wrote:
>Thanks for this Daniele. > >A couple of general things: >- "userspace testsuite" as a name, while accurate, is also perhaps >misleading as the unit tests are also userspace tests (and don't >require any special root privileges). I guess this class of tests >should be "system", in which case it would make sense to name them >something like "system-userspace-testsuite.at" and >"system-kmod-testsuite.at". I feel that those are much better names. Should I rename also the make targets? >- We should have a new vagrant provision target in VagrantFile to run >these tests. This could be mentioned in the vagrant section of >INSTALL.md. Sure, I can do that. >- Seems like we've omitted a NEWS item for the kernel version of these >tests, but perhaps these should get a mention - check-ryu and >check-oftest have mentions there. I can add that >- All of this is linux-centric. I don't think we have to address this >today, as my main concern is to increase the body of these tests to >get more assurance on datapath behaviour. It might be a good thing to >keep in mind though when designing tests - Try to keep >platform-specific stuff inside the macros, and generic stuff inside >the test files. I've thought about it, but I'm not sure how easy it would be in practice to support other OS (especially for namespaces). I can move some more linux specific code into the macros (like the ip netns exec commands), but my goal for now would be to be able to test the userspace datapath as well as the kernel module. > >On 4 August 2015 at 11:00, Daniele Di Proietto <diproiet...@vmware.com> >wrote: >> @@ -32,10 +32,11 @@ m4_define([ADD_NAMESPACES], >> # The existing 'port' or 'ovs-port' will be removed before new ones >>are added. >> # >> m4_define([ADD_VETH], >> - [ AT_CHECK([ip link add $1 type veth peer name ovs-$1]) >> + [ ip link del ovs-$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([ovs-vsctl add-port $3 ovs-$1]) >> AT_CHECK([ip netns exec $2 ip addr add $4 dev $1]) >> AT_CHECK([ip netns exec $2 ip link set dev $1 up]) >> ] > >ovs-$1 should be cleaned up properly before running the test. I think >it practice this just means that we should add an ON_EXIT([ip link del >ovs-$1]) at the end of this macro. That's better, I'll add the ON_EXIT. Thanks for taking a look at this and for all your suggestions! I'll post a v2 soon Daniele _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
