On 30 July 2015 at 12:51, Joe Stringer <joestrin...@nicira.com> wrote: > On 30 July 2015 at 11:57, Andy Zhou <az...@nicira.com> wrote: >> On Wed, Jul 29, 2015 at 4:52 PM, Joe Stringer <joestrin...@nicira.com> wrote: >>> We already queue the removal of the kernel module in OVS_VSWITCHD_START, >>> via an ON_EXIT() call. This redundant removal also doesn't interact very >>> well with usage of vports: If the datapath still exists in the kernel, >>> we cannot remove a vport module; if we cannot remove a vport module, we >>> cannot remove the openvswitch module. Having the call here prevents >>> tunnel tests from manually cleaning up their datapath on exit, so this >>> patch removes it. >>> >> >> I don't understand this. If we consider each test as an independent >> sessions, should we be able >> to be add and remove kernel modules in between? Some kernel bugs only >> shows up during module >> clean ups. > > The issue is that as long as the datapath has a tunnel port configured, you > cannot remove the vport module (and you need to remove the vport > module before removing openvswitch module). The solution I use here is > to queue up three commands using ON_EXIT(): > - During OVS_VSWITCHD_START, ON_EXIT(modprobe -r openvswitch) > - At the beginning of vport tests, ON_EXIT(modprobe -r vport_vxlan) > - Following this, ON_EXIT(ovs-dpctl del-dp ovs-system). > > These should be executed in reverse order to remove tunnel ports, > remove tunnel modules, and remove the ovs module.
Andy, we spoke offline about this and as I understand your thought is that tests should begin with OVS_VSWITCHD_START and finish with OVS_VSWITCHD_STOP. I'm all for that, but I believe that the START operation (also, ADD_VETH, ADD_VLAN, etc.) should queue up cleanup of those devices where necessary so that whether it is a successful test run or a test failure, cleanup should occur. This will cause the minimum amount of disruption for subsequent tests. OVS_VSWITCHD_STOP still provides a way to safely cleanup the OVS processes and check logs for unexpected output, but it can leave out cleanup which is necessary to execute in both success and failure conditions. For cases where people want to debug a test in the middle, they can always clear the trap and nothing will be cleaned up, allowing debugging: trap ":" 0; AT_CHECK([fail]) Given that the trap that's set up by ON_EXIT() is always run (even if OVS_VSWITCHD_STOP occurs first), I believe that this patch has the correct behaviour. Do you have any remaining reservations? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
