irmd: Always use reg_destroy_proc

Destroying a process will now always be done with reg_destroy_proc,
regardless of whether it was an IPCP or spawned. This makes it easier
to keep the registry consistent and avoid races.

Also improves some logs and updates some default settings.

Signed-off-by: Dimitri Staessens <dimitri@ouroboros.rocks>
Signed-off-by: Sander Vrijders <sander@ouroboros.rocks>

6 files changed, 94 insertions, 125 deletions

diff --git a/src/irmd/CMakeLists.txt b/src/irmd/CMakeLists.txt
index 7eaa0ce6..c9c2e553 100644
--- a/src/irmd/CMakeLists.txt
+++ b/src/irmd/CMakeLists.txt
@@ -43,15 +43,15 @@ set(IRMD_REQ_ARR_TIMEOUT 1000 CACHE STRING
 
 set(BOOTSTRAP_TIMEOUT 5000 CACHE STRING
   "Timeout for an IPCP to bootstrap (ms)")
-set(ENROLL_TIMEOUT 60000 CACHE STRING
+set(ENROLL_TIMEOUT 20000 CACHE STRING
   "Timeout for an IPCP to enroll (ms)")
-set(REG_TIMEOUT 60000 CACHE STRING
+set(REG_TIMEOUT 20000 CACHE STRING
   "Timeout for registering a name (ms)")
-set(QUERY_TIMEOUT 60000 CACHE STRING
+set(QUERY_TIMEOUT 20000 CACHE STRING
   "Timeout to query a name with an IPCP (ms)")
-set(CONNECT_TIMEOUT 60000 CACHE STRING
+set(CONNECT_TIMEOUT 20000 CACHE STRING
   "Timeout to connect an IPCP to another IPCP (ms)")
-set(FLOW_ALLOC_TIMEOUT 5000 CACHE STRING
+set(FLOW_ALLOC_TIMEOUT 20000 CACHE STRING
   "Timeout for a flow allocation response (ms)")
 set(IRMD_MIN_THREADS 8 CACHE STRING
   "Minimum number of worker threads in the IRMd")
diff --git a/src/irmd/ipcp.c b/src/irmd/ipcp.c
index c8055aa1..d7abe15d 100644
--- a/src/irmd/ipcp.c
+++ b/src/irmd/ipcp.c
@@ -47,6 +47,37 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 
+static char * str_ipcp_cmd(int code)
+{
+        switch (code) {
+        case IPCP_MSG_CODE__IPCP_BOOTSTRAP:
+                return "bootstrap";
+        case IPCP_MSG_CODE__IPCP_ENROLL:
+                return "enroll";
+        case IPCP_MSG_CODE__IPCP_CONNECT:
+                return "connect";
+        case IPCP_MSG_CODE__IPCP_DISCONNECT:
+                return "disconnect";
+        case IPCP_MSG_CODE__IPCP_REG:
+                return "reg";
+        case IPCP_MSG_CODE__IPCP_UNREG:
+                return "unreg";
+        case IPCP_MSG_CODE__IPCP_QUERY:
+                return "query";
+        case IPCP_MSG_CODE__IPCP_FLOW_JOIN:
+                return "join";
+        case IPCP_MSG_CODE__IPCP_FLOW_ALLOC:
+                return "alloc";
+        case IPCP_MSG_CODE__IPCP_FLOW_ALLOC_RESP:
+                return "alloc_resp";
+        case IPCP_MSG_CODE__IPCP_FLOW_DEALLOC:
+                return "dealloc";
+        default:
+                assert(false);
+                return "unknown";
+        }
+}
+
 ipcp_msg_t * send_recv_ipcp_msg(pid_t        pid,
                                 ipcp_msg_t * msg)
 {
@@ -139,7 +170,8 @@ ipcp_msg_t * send_recv_ipcp_msg(pid_t        pid,
         else {
                 if (errno == EAGAIN && !dealloc) {
                         int diff = ts_diff_ms(&tic, &toc);
-                        log_warn("IPCP command timed out after %d ms.", diff);
+                        log_warn("IPCP %s timed out after %d ms.",
+                                 str_ipcp_cmd(msg->code), diff);
                 }
                 return NULL;
         }
diff --git a/src/irmd/main.c b/src/irmd/main.c
index 32f41ab2..e2503d0c 100644
--- a/src/irmd/main.c
+++ b/src/irmd/main.c
@@ -268,7 +268,7 @@ int create_ipcp(struct ipcp_info * info)
 
  fail_boot:
         waitpid(info->pid, &status, 0);
-        reg_destroy_ipcp(info->pid);
+        reg_destroy_proc(info->pid);
         return -1;
 
  fail_reg_ipcp:
@@ -289,7 +289,7 @@ static int destroy_ipcp(pid_t pid)
                 goto fail;
         }
 
-        if (reg_destroy_ipcp(pid)) {
+        if (reg_destroy_proc(pid)) {
                 log_err("Failed to remove IPCP from registry.");
                 goto fail;
         }
@@ -741,9 +741,6 @@ static int proc_announce(const struct proc_info * info)
 
 static int proc_exit(pid_t pid)
 {
-        if (reg_has_ipcp(pid) && reg_destroy_ipcp(pid) < 0)
-                log_warn("Failed to remove IPCP %d.", pid);
-
         if (reg_destroy_proc(pid) < 0)
                 log_err("Failed to remove process %d.", pid);
 
@@ -1023,12 +1020,12 @@ static int flow_alloc(struct flow_info * flow,
         uint8_t * s = NULL;
         buffer_t  hash;
         int       err;
-
         /* piggyback of user data not yet implemented */
         assert(data != NULL && data->len == 0 && data->data == NULL);
 
         log_info("Allocating flow for %d to %s.", flow->n_pid, dst);
 
+
         if (flow->qs.cypher_s > 0) {
                 ssize_t key_len;
 
@@ -2025,8 +2022,8 @@ static void kill_all_spawned(void)
                         waitpid(pid, &s, 0);
                 else {
                         log_warn("Child process %d died.", pid);
-                        reg_destroy_spawned(pid);
                         cleanup_pid(pid);
+                        reg_destroy_proc(pid);
                 }
                 pid = reg_first_spawned();
         }
diff --git a/src/irmd/reg/reg.c b/src/irmd/reg/reg.c
index 87641f1a..0a0a1786 100644
--- a/src/irmd/reg/reg.c
+++ b/src/irmd/reg/reg.c
@@ -798,41 +798,6 @@ int reg_create_ipcp(const struct ipcp_info * info)
 
 }
 
-int reg_destroy_ipcp(pid_t pid)
-{
-        struct reg_ipcp *  ipcp;
-        struct pid_entry * entry;
-
-        pthread_mutex_lock(&reg.mtx);
-
-        ipcp = __reg_get_ipcp(pid);
-        if (ipcp == NULL) {
-                log_err("IPCP %d does not exist.", pid);
-                goto no_ipcp;
-        }
-
-        list_del(&ipcp->next);
-
-        reg.n_ipcps--;
-
-        entry = __reg_get_spawned(pid);
-        assert(entry != NULL);
-
-        list_del(&entry->next);
-        free(entry);
-        reg.n_spawned--;
-
-        pthread_mutex_unlock(&reg.mtx);
-
-        reg_ipcp_destroy(ipcp);
-
-        return 0;
-
- no_ipcp:
-        pthread_mutex_unlock(&reg.mtx);
-        return -1;
-}
-
 int reg_update_ipcp(struct ipcp_info * info)
 {
         struct reg_ipcp * ipcp;
@@ -1134,33 +1099,38 @@ int reg_create_proc(const struct proc_info * info)
 
 int reg_destroy_proc(pid_t pid)
 {
-        struct reg_proc * proc;
+        struct reg_proc *  proc;
+        struct pid_entry * spawn;
+        struct reg_ipcp *  ipcp;
 
         pthread_mutex_lock(&reg.mtx);
 
         proc = __reg_get_proc(pid);
-        if (proc == NULL) {
-                log_err("Process %d does not exist.", pid);
-                goto no_proc;
+        if (proc != NULL) {
+                list_del(&proc->next);
+                reg.n_procs--;
+                reg_proc_destroy(proc);
+                __reg_del_proc_from_names(pid);
+                __reg_cancel_flows_for_proc(pid);
         }
 
-        __reg_del_proc_from_names(pid);
-
-        list_del(&proc->next);
-
-        reg.n_procs--;
+        spawn = __reg_get_spawned(pid);
+        if (spawn != NULL) {
+                list_del(&spawn->next);
+                reg.n_spawned--;
+                free(spawn);
+        }
 
-        __reg_cancel_flows_for_proc(pid);
+        ipcp = __reg_get_ipcp(pid);
+        if (ipcp != NULL) {
+                list_del(&ipcp->next);
+                reg.n_ipcps--;
+                reg_ipcp_destroy(ipcp);
+        }
 
         pthread_mutex_unlock(&reg.mtx);
 
-        reg_proc_destroy(proc);
-
         return 0;
-
- no_proc:
-        pthread_mutex_unlock(&reg.mtx);
-        return -1;
 }
 
 bool reg_has_proc(pid_t pid)
@@ -1229,33 +1199,6 @@ int reg_create_spawned(pid_t pid)
         return -1;
 }
 
-int  reg_destroy_spawned(pid_t pid)
-{
-        struct pid_entry * entry;
-
-        pthread_mutex_lock(&reg.mtx);
-
-        entry = __reg_get_spawned(pid);
-        if (entry == NULL) {
-                log_err("Spawned process %d does not exist.", pid);
-                goto no_proc;
-        }
-
-        list_del(&entry->next);
-
-        reg.n_spawned--;
-
-        pthread_mutex_unlock(&reg.mtx);
-
-        free(entry);
-
-        return 0;
-
- no_proc:
-        pthread_mutex_unlock(&reg.mtx);
-        return -1;
-}
-
 bool reg_has_spawned(pid_t pid)
 {
         bool ret;
diff --git a/src/irmd/reg/reg.h b/src/irmd/reg/reg.h
index e6deb8e4..17dfcc32 100644
--- a/src/irmd/reg/reg.h
+++ b/src/irmd/reg/reg.h
@@ -43,9 +43,32 @@ int   reg_destroy_flow(int flow_id);
 
 bool  reg_has_flow(int flow_id);
 
-int   reg_create_ipcp(const struct ipcp_info * info);
+int   reg_create_proc(const struct proc_info * info);
+
+/* Use this for all processes, including ipcps */
+int   reg_destroy_proc(pid_t pid);
 
-int   reg_destroy_ipcp(pid_t pid);
+bool  reg_has_proc(pid_t pid);
+
+void  reg_kill_all_proc(int signal);
+
+pid_t reg_get_dead_proc(void);
+
+int   reg_create_spawned(pid_t pid);
+
+bool  reg_has_spawned(pid_t pid);
+
+void  reg_kill_all_spawned(int signal);
+
+int   reg_first_spawned(void);
+
+int   reg_bind_proc(const char * name,
+                    pid_t        proc);
+
+int   reg_unbind_proc(const char * name,
+                      pid_t        proc);
+
+int   reg_create_ipcp(const struct ipcp_info * info);
 
 bool  reg_has_ipcp(pid_t pid);
 
@@ -70,32 +93,6 @@ bool  reg_has_name(const char * name);
 /* TODO don't rely on protobuf here */
 int   reg_list_names(name_info_msg_t *** names);
 
-int   reg_create_proc(const struct proc_info * info);
-
-int   reg_destroy_proc(pid_t pid);
-
-bool  reg_has_proc(pid_t pid);
-
-void  reg_kill_all_proc(int signal);
-
-pid_t reg_get_dead_proc(void);
-
-int   reg_create_spawned(pid_t pid);
-
-int   reg_destroy_spawned(pid_t pid);
-
-bool  reg_has_spawned(pid_t pid);
-
-void  reg_kill_all_spawned(int signal);
-
-int   reg_first_spawned(void);
-
-int   reg_bind_proc(const char * name,
-                    pid_t        proc);
-
-int   reg_unbind_proc(const char * name,
-                      pid_t        proc);
-
 int   reg_create_prog(const struct prog_info * info);
 
 int   reg_destroy_prog(const char * name);
diff --git a/src/irmd/reg/tests/reg_test.c b/src/irmd/reg/tests/reg_test.c
index f840a3e0..c341c297 100644
--- a/src/irmd/reg/tests/reg_test.c
+++ b/src/irmd/reg/tests/reg_test.c
@@ -538,7 +538,7 @@ static int test_reg_create_ipcp(void)
                 goto fail;
         }
 
-        if (reg_destroy_ipcp(info.pid) < 0) {
+        if (reg_destroy_proc(info.pid) < 0) {
                 printf("Failed to destroy ipcp.\n");
                 goto fail;
         }
@@ -605,7 +605,7 @@ static int test_set_layer(void)
                 goto fail;
         }
 
-        if (reg_destroy_ipcp(info.pid) < 0) {
+        if (reg_destroy_proc(info.pid) < 0) {
                 printf("Failed to destroy ipcp.\n");
                 goto fail;
         }
@@ -771,7 +771,7 @@ static int test_reg_spawned(void)
                 goto fail;
         }
 
-        if (reg_destroy_spawned(TEST_PID) < 0) {
+        if (reg_destroy_proc(TEST_PID) < 0) {
                 printf("Failed to destroy spawned.\n");
                 goto fail;
         }
@@ -1291,7 +1291,7 @@ static int test_wait_ipcp_boot_timeout(void)
                 goto fail;
         }
 
-        if (reg_destroy_ipcp(info.pid) < 0) {
+        if (reg_destroy_proc(info.pid) < 0) {
                 printf("Failed to destroy ipcp.\n");
                 goto fail;
         }
@@ -1357,7 +1357,7 @@ static int test_wait_ipcp_boot_fail(void)
 
         pthread_join(thr, NULL);
 
-        if (reg_destroy_ipcp(info.pid) < 0) {
+        if (reg_destroy_proc(info.pid) < 0) {
                 printf("Failed to destroy ipcp.\n");
                 goto fail;
         }
@@ -1424,7 +1424,7 @@ static int test_wait_ipcp_boot_success(void)
                 goto fail;
         }
 
-        if (reg_destroy_ipcp(info.pid) < 0) {
+        if (reg_destroy_proc(info.pid) < 0) {
                 printf("Failed to destroy ipcp.\n");
                 goto fail;
         }