[strongSwan-dev] [PATCH 6/8] trap-manager: Changed how acquires we acted on are tracked

[Timo Teräs]

From: Tobias Brunner <tobias at strongswan.org>

---
 src/libcharon/sa/trap_manager.c | 115 ++++++++++++++++++++++++++++------------
 1 file changed, 80 insertions(+), 35 deletions(-)

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index eeb188e..a002231 100644
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -18,10 +18,10 @@
 
 #include <hydra.h>
 #include <daemon.h>
+#include <threading/mutex.h>
 #include <threading/rwlock.h>
 #include <collections/linked_list.h>
 
-
 typedef struct private_trap_manager_t private_trap_manager_t;
 typedef struct trap_listener_t trap_listener_t;
 
@@ -70,6 +70,16 @@ struct private_trap_manager_t {
 	 * Whether to ignore traffic selectors from acquires
 	 */
 	bool ignore_acquire_ts;
+
+	/**
+	 * list of acquires we currently handle
+	 */
+	linked_list_t *acquires;
+
+	/**
+	 * mutex for list of acquires
+	 */
+	mutex_t *mutex;
 };
 
 /**
@@ -80,23 +90,45 @@ typedef struct {
 	char *name;
 	/** ref to peer_cfg to initiate */
 	peer_cfg_t *peer_cfg;
-	/** ref to instanciated CHILD_SA */
+	/** ref to instantiated CHILD_SA (i.e the trap policy) */
 	child_sa_t *child_sa;
-	/** TRUE if an acquire is pending */
-	bool pending;
+} entry_t;
+
+/**
+ * A handled acquire
+ */
+typedef struct {
 	/** pending IKE_SA connecting upon acquire */
 	ike_sa_t *ike_sa;
-} entry_t;
+	/** reqid of pending trap policy */
+	u_int32_t reqid;
+} acquire_t;
 
 /**
  * actually uninstall and destroy an installed entry
  */
-static void destroy_entry(entry_t *entry)
+static void destroy_entry(entry_t *this)
 {
-	entry->child_sa->destroy(entry->child_sa);
-	entry->peer_cfg->destroy(entry->peer_cfg);
-	free(entry->name);
-	free(entry);
+	this->child_sa->destroy(this->child_sa);
+	this->peer_cfg->destroy(this->peer_cfg);
+	free(this->name);
+	free(this);
+}
+
+/**
+ * destroy a cached acquire entry
+ */
+static void destroy_acquire(acquire_t *this)
+{
+	free(this);
+}
+
+/**
+ * match an acquire entry by reqid
+ */
+static bool acquire_by_reqid(acquire_t *this, u_int32_t *reqid)
+{
+	return this->reqid == *reqid;
 }
 
 METHOD(trap_manager_t, install, u_int32_t,
@@ -314,6 +346,7 @@ METHOD(trap_manager_t, acquire, void,
 {
 	enumerator_t *enumerator;
 	entry_t *entry, *found = NULL;
+	acquire_t *acquire;
 	peer_cfg_t *peer;
 	child_cfg_t *child;
 	ike_sa_t *ike_sa;
@@ -337,16 +370,29 @@ METHOD(trap_manager_t, acquire, void,
 		this->lock->unlock(this->lock);
 		return;
 	}
-	if (!cas_bool(&found->pending, FALSE, TRUE))
+
+	this->mutex->lock(this->mutex);
+	reqid = found->child_sa->get_reqid(found->child_sa);
+	if (this->acquires->find_first(this->acquires, (void*)acquire_by_reqid,
+								  (void**)&acquire, &reqid) == SUCCESS)
 	{
 		DBG1(DBG_CFG, "ignoring acquire, connection attempt pending");
+		this->mutex->unlock(this->mutex);
 		this->lock->unlock(this->lock);
 		return;
 	}
+	else
+	{
+		INIT(acquire,
+			.reqid = reqid,
+		);
+		this->acquires->insert_last(this->acquires, acquire);
+	}
+	this->mutex->unlock(this->mutex);
+
 	peer = found->peer_cfg->get_ref(found->peer_cfg);
 	child = found->child_sa->get_config(found->child_sa);
 	child = child->get_ref(child);
-	reqid = found->child_sa->get_reqid(found->child_sa);
 	/* don't hold the lock while checking out the IKE_SA */
 	this->lock->unlock(this->lock);
 
@@ -366,14 +412,9 @@ METHOD(trap_manager_t, acquire, void,
 		}
 		if (ike_sa->initiate(ike_sa, child, reqid, src, dst) != DESTROY_ME)
 		{
-			/* make sure the entry is still there */
-			this->lock->read_lock(this->lock);
-			if (this->traps->find_first(this->traps, NULL,
-										(void**)&found) == SUCCESS)
-			{
-				found->ike_sa = ike_sa;
-			}
-			this->lock->unlock(this->lock);
+			this->mutex->lock(this->mutex);
+			acquire->ike_sa = ike_sa;
+			this->mutex->unlock(this->mutex);
 			charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa);
 		}
 		else
@@ -392,26 +433,25 @@ static void complete(private_trap_manager_t *this, ike_sa_t *ike_sa,
 					 child_sa_t *child_sa)
 {
 	enumerator_t *enumerator;
-	entry_t *entry;
+	acquire_t *acquire;
 
-	this->lock->read_lock(this->lock);
-	enumerator = this->traps->create_enumerator(this->traps);
-	while (enumerator->enumerate(enumerator, &entry))
+	this->mutex->lock(this->mutex);
+	enumerator = this->acquires->create_enumerator(this->acquires);
+	while (enumerator->enumerate(enumerator, &acquire))
 	{
-		if (entry->ike_sa != ike_sa)
+		if (acquire->ike_sa != ike_sa)
 		{
 			continue;
 		}
-		if (child_sa && child_sa->get_reqid(child_sa) !=
-									entry->child_sa->get_reqid(entry->child_sa))
+		if (child_sa && child_sa->get_reqid(child_sa) != acquire->reqid)
 		{
 			continue;
 		}
-		entry->ike_sa = NULL;
-		entry->pending = FALSE;
+		this->acquires->remove_at(this->acquires, enumerator);
+		destroy_acquire(acquire);
 	}
 	enumerator->destroy(enumerator);
-	this->lock->unlock(this->lock);
+	this->mutex->unlock(this->mutex);
 }
 
 METHOD(listener_t, ike_state_change, bool,
@@ -445,14 +485,15 @@ METHOD(listener_t, child_state_change, bool,
 METHOD(trap_manager_t, flush, void,
 	private_trap_manager_t *this)
 {
-	linked_list_t *traps;
-	/* since destroying the CHILD_SA results in events which require a read
-	 * lock we cannot destroy the list while holding the write lock */
 	this->lock->write_lock(this->lock);
-	traps = this->traps;
+	this->traps->destroy_function(this->traps, (void*)destroy_entry);
 	this->traps = linked_list_create();
 	this->lock->unlock(this->lock);
-	traps->destroy_function(traps, (void*)destroy_entry);
+
+	this->mutex->lock(this->mutex);
+	this->acquires->destroy_function(this->acquires, (void*)destroy_acquire);
+	this->acquires = linked_list_create();
+	this->mutex->unlock(this->mutex);
 }
 
 METHOD(trap_manager_t, destroy, void,
@@ -460,6 +501,8 @@ METHOD(trap_manager_t, destroy, void,
 {
 	charon->bus->remove_listener(charon->bus, &this->listener.listener);
 	this->traps->destroy_function(this->traps, (void*)destroy_entry);
+	this->acquires->destroy_function(this->acquires, (void*)destroy_acquire);
+	this->mutex->destroy(this->mutex);
 	this->lock->destroy(this->lock);
 	free(this);
 }
@@ -489,6 +532,8 @@ trap_manager_t *trap_manager_create(void)
 			},
 		},
 		.traps = linked_list_create(),
+		.acquires = linked_list_create(),
+		.mutex = mutex_create(MUTEX_TYPE_DEFAULT),
 		.lock = rwlock_create(RWLOCK_TYPE_DEFAULT),
 		.ignore_acquire_ts = lib->settings->get_bool(lib->settings,
 										"%s.ignore_acquire_ts", FALSE, lib->ns),
-- 
2.3.6