ipcpd: Improve locking np1_flow_set in eth and udp

A flow_set is thread-safe and doesn't need to be protected by a lock.

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

2 files changed, 10 insertions, 13 deletions

diff --git a/src/ipcpd/eth/eth.c b/src/ipcpd/eth/eth.c
index 618ba44f..324c1ddc 100644
--- a/src/ipcpd/eth/eth.c
+++ b/src/ipcpd/eth/eth.c
@@ -1790,10 +1790,10 @@ static int eth_ipcp_flow_dealloc(int fd)
 #endif
         ipcp_flow_fini(fd);
 
-        pthread_rwlock_wrlock(&eth_data.flows_lock);
-
         fset_del(eth_data.np1_flows, fd);
 
+        pthread_rwlock_wrlock(&eth_data.flows_lock);
+
 #if defined(BUILD_ETH_DIX)
         eth_data.fd_to_ef[fd].r_eid = -1;
 #elif defined BUILD_ETH_LLC
diff --git a/src/ipcpd/udp/main.c b/src/ipcpd/udp/main.c
index c59ea36d..9e1900ef 100644
--- a/src/ipcpd/udp/main.c
+++ b/src/ipcpd/udp/main.c
@@ -1069,10 +1069,10 @@ static int ipcp_udp_flow_alloc(int             fd,
         udp_data.fd_to_uf[fd].d_eid = -1;
         udp_data.fd_to_uf[fd].skfd  = skfd;
 
-        fset_add(udp_data.np1_flows, fd);
-
         pthread_rwlock_unlock(&udp_data.flows_lock);
 
+        fset_add(udp_data.np1_flows, fd);
+
         log_dbg("Flow pending on fd %d, UDP src port %d, dst port %d.",
                 fd, ntohs(c_saddr.sin_port), ntohs(r_saddr.sin_port));
 
@@ -1118,18 +1118,15 @@ static int ipcp_udp_flow_alloc_resp(int          fd,
         skfd  = udp_data.fd_to_uf[fd].skfd;
         d_eid = udp_data.fd_to_uf[fd].d_eid;
 
-        fset_add(udp_data.np1_flows, fd);
-
         pthread_rwlock_unlock(&udp_data.flows_lock);
 
         if (ipcp_udp_port_alloc_resp(skfd, d_eid, fd, resp, data, len) < 0) {
-                pthread_rwlock_rdlock(&udp_data.flows_lock);
-                fset_del(udp_data.np1_flows, fd);
-                pthread_rwlock_unlock(&udp_data.flows_lock);
                 log_err("Failed to respond to flow request.");
                 return -1;
         }
 
+        fset_add(udp_data.np1_flows, fd);
+
         log_dbg("Accepted flow, fd %d on eid %d.",
                 fd, d_eid);
 
@@ -1142,19 +1139,19 @@ static int ipcp_udp_flow_dealloc(int fd)
 
         ipcp_flow_fini(fd);
 
-        pthread_rwlock_wrlock(&udp_data.flows_lock);
-
         fset_del(udp_data.np1_flows, fd);
 
+        pthread_rwlock_wrlock(&udp_data.flows_lock);
+
         skfd = udp_data.fd_to_uf[fd].skfd;
 
         udp_data.fd_to_uf[fd].d_eid = -1;
         udp_data.fd_to_uf[fd].skfd  = -1;
 
-        close(skfd);
-
         pthread_rwlock_unlock(&udp_data.flows_lock);
 
+        close(skfd);
+
         flow_dealloc(fd);
 
         log_dbg("Flow with fd %d deallocated.", fd);