Before the cleanup option, the bridge_exit() call was fairly fast,
because it didn't include any particularly long operations. However,
with the cleanup flag, this function destroys a lot of datapath
resources freeing a lot of memory, waiting on RCU and talking to
the kernel. That may take a noticeable amount of time, especially
on a busy system or under profilers/sanitizers. However, the unixctl
'exit' command replies instantly without waiting for any work to
actually be done. This may cause system test failures or other
issues where scripts expect ovs-vswitchd to exit or destroy all the
datapath resources shortly after appctl call.
Fix that by waiting for the bridge_exit() before replying to the user.
At least, all the datapath resources will actually be destroyed by
the time ovs-appctl exits.
Also moving a structure from stack to global. Seems cleaner this way.
Fixes: fe13ccdca6a2 ("vswitchd: Add --cleanup option to the 'appctl exit'
command")
Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
---
vswitchd/ovs-vswitchd.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
index a244d2f70..55b437871 100644
--- a/vswitchd/ovs-vswitchd.c
+++ b/vswitchd/ovs-vswitchd.c
@@ -68,19 +68,18 @@ static unixctl_cb_func ovs_vswitchd_exit;
static char *parse_options(int argc, char *argv[], char **unixctl_path);
OVS_NO_RETURN static void usage(void);
-struct ovs_vswitchd_exit_args {
- bool *exiting;
- bool *cleanup;
-};
+static struct ovs_vswitchd_exit_args {
+ struct unixctl_conn *conn;
+ bool exiting;
+ bool cleanup;
+} exit_args;
int
main(int argc, char *argv[])
{
- char *unixctl_path = NULL;
struct unixctl_server *unixctl;
+ char *unixctl_path = NULL;
char *remote;
- bool exiting, cleanup;
- struct ovs_vswitchd_exit_args exit_args = {&exiting, &cleanup};
int retval;
set_program_name(argv[0]);
@@ -111,14 +110,12 @@ main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
unixctl_command_register("exit", "[--cleanup]", 0, 1,
- ovs_vswitchd_exit, &exit_args);
+ ovs_vswitchd_exit, NULL);
bridge_init(remote);
free(remote);
- exiting = false;
- cleanup = false;
- while (!exiting) {
+ while (!exit_args.exiting) {
OVS_USDT_PROBE(main, run_start);
memory_run();
if (memory_should_report()) {
@@ -137,16 +134,20 @@ main(int argc, char *argv[])
bridge_wait();
unixctl_server_wait(unixctl);
netdev_wait();
- if (exiting) {
+ if (exit_args.exiting) {
poll_immediate_wake();
}
OVS_USDT_PROBE(main, poll_block);
poll_block();
if (should_service_stop()) {
- exiting = true;
+ exit_args.exiting = true;
}
}
- bridge_exit(cleanup);
+ bridge_exit(exit_args.cleanup);
+
+ if (exit_args.conn) {
+ unixctl_command_reply(exit_args.conn, NULL);
+ }
unixctl_server_destroy(unixctl);
service_stop();
vlog_disable_async();
@@ -304,10 +305,9 @@ usage(void)
static void
ovs_vswitchd_exit(struct unixctl_conn *conn, int argc,
- const char *argv[], void *exit_args_)
+ const char *argv[], void *args OVS_UNUSED)
{
- struct ovs_vswitchd_exit_args *exit_args = exit_args_;
- *exit_args->exiting = true;
- *exit_args->cleanup = argc == 2 && !strcmp(argv[1], "--cleanup");
- unixctl_command_reply(conn, NULL);
+ exit_args.conn = conn;
+ exit_args.exiting = true;
+ exit_args.cleanup = argc == 2 && !strcmp(argv[1], "--cleanup");
}
--
2.40.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
