]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
kernel-netlink: Release lock while doing Netlink NEW/DELADDR operations
authorMartin Willi <martin@revosec.ch>
Wed, 16 Jul 2014 09:59:59 +0000 (11:59 +0200)
committerMartin Willi <martin@revosec.ch>
Fri, 21 Nov 2014 09:55:45 +0000 (10:55 +0100)
Besides that it can improve throughput, it avoids a deadlock situation. If
all threads are busy, watcher will invoke the FD notification for NEWADDR
events itself. If the lock is held, it gets locked up. As watcher is not
dispatching anymore, it can't signal Netlink socket send() completion, and
the send() operation does not return and keeps the lock.

src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c

index 9d9f15974fedfd24a124aae68d6d572c574d3f39..3c1a3f87dd0a5d7a330a6259825477f395bd2894 100644 (file)
@@ -1975,6 +1975,8 @@ METHOD(kernel_net_t, add_ip, status_t,
        if (iface)
        {
                addr_entry_t *addr;
+               char *ifname;
+               int ifi;
 
                INIT(addr,
                        .ip = virtual_ip->clone(virtual_ip),
@@ -1983,26 +1985,30 @@ METHOD(kernel_net_t, add_ip, status_t,
                );
                iface->addrs->insert_last(iface->addrs, addr);
                addr_map_entry_add(this->vips, addr, iface);
+               ifi = iface->ifindex;
+               this->lock->unlock(this->lock);
                if (manage_ipaddr(this, RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL,
-                                                 iface->ifindex, virtual_ip, prefix) == SUCCESS)
+                                                 ifi, virtual_ip, prefix) == SUCCESS)
                {
+                       this->lock->write_lock(this->lock);
                        while (!is_vip_installed_or_gone(this, virtual_ip, &entry))
                        {       /* wait until address appears */
                                this->condvar->wait(this->condvar, this->lock);
                        }
                        if (entry)
                        {       /* we fail if the interface got deleted in the meantime */
-                               DBG2(DBG_KNL, "virtual IP %H installed on %s", virtual_ip,
-                                        entry->iface->ifname);
+                               ifname = strdup(entry->iface->ifname);
                                this->lock->unlock(this->lock);
+                               DBG2(DBG_KNL, "virtual IP %H installed on %s",
+                                        virtual_ip, ifname);
                                /* during IKEv1 reauthentication, children get moved from
                                 * old the new SA before the virtual IP is available. This
                                 * kills the route for our virtual IP, reinstall. */
-                               queue_route_reinstall(this, strdup(entry->iface->ifname));
+                               queue_route_reinstall(this, ifname);
                                return SUCCESS;
                        }
+                       this->lock->unlock(this->lock);
                }
-               this->lock->unlock(this->lock);
                DBG1(DBG_KNL, "adding virtual IP %H failed", virtual_ip);
                return FAILED;
        }
@@ -2048,20 +2054,23 @@ METHOD(kernel_net_t, del_ip, status_t,
        if (entry->addr->refcount == 1)
        {
                status_t status;
+               int ifi;
 
                /* we set this flag so that threads calling add_ip will block and wait
                 * until the entry is gone, also so we can wait below */
                entry->addr->installed = FALSE;
-               status = manage_ipaddr(this, RTM_DELADDR, 0, entry->iface->ifindex,
-                                                          virtual_ip, prefix);
+               ifi = entry->iface->ifindex;
+               this->lock->unlock(this->lock);
+               status = manage_ipaddr(this, RTM_DELADDR, 0, ifi, virtual_ip, prefix);
                if (status == SUCCESS && wait)
                {       /* wait until the address is really gone */
+                       this->lock->write_lock(this->lock);
                        while (is_known_vip(this, virtual_ip))
                        {
                                this->condvar->wait(this->condvar, this->lock);
                        }
+                       this->lock->unlock(this->lock);
                }
-               this->lock->unlock(this->lock);
                return status;
        }
        else