Merge branch 'ikev1-adopt-child-tasks'

Makes sure to adopt active and queued Quick Mode tasks if the peer
reauthenticates the IKE_SA while creating lots of CHILD_SAs.

Closes strongswan/strongswan#117.

src/libcharon/processing/jobs/adopt_children_job.c

@@ -53,6 +53,36 @@ METHOD(job_t, destroy, void,
 	free(this);
 }
 
+METHOD(adopt_children_job_t, queue_task, void,
+	private_adopt_children_job_t *this, task_t *task)
+{
+	array_insert_create(&this->tasks, ARRAY_TAIL, task);
+}
+
+/**
+ * Adopt child-creating tasks from the given IKE_SA
+ */
+static u_int adopt_child_tasks(private_adopt_children_job_t *this,
+							   ike_sa_t *ike_sa, task_queue_t queue)
+{
+	enumerator_t *tasks;
+	task_t *task;
+	u_int count = 0;
+
+	tasks = ike_sa->create_task_enumerator(ike_sa, queue);
+	while (tasks->enumerate(tasks, &task))
+	{
+		if (task->get_type(task) == TASK_QUICK_MODE)
+		{
+			ike_sa->remove_task(ike_sa, tasks);
+			queue_task(this, task);
+			count++;
+		}
+	}
+	tasks->destroy(tasks);
+	return count;
+}
+
 METHOD(job_t, execute, job_requeue_t,
 	private_adopt_children_job_t *this)
 {
@@ -65,6 +95,7 @@ METHOD(job_t, execute, job_requeue_t,
 	ike_sa_t *ike_sa;
 	child_sa_t *child_sa;
 	uint32_t unique;
+	u_int tasks = 0;
 
 	ike_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, this->id);
 	if (ike_sa)
@@ -127,11 +158,17 @@ METHOD(job_t, execute, job_requeue_t,
 					 * it does trigger an assign_vips(FALSE) event, so we also
 					 * trigger one below */
 					ike_sa->clear_virtual_ips(ike_sa, FALSE);
-					if (children->get_count(children) || vips->get_count(vips))
+
+					tasks += adopt_child_tasks(this, ike_sa, TASK_QUEUE_ACTIVE);
+					tasks += adopt_child_tasks(this, ike_sa, TASK_QUEUE_QUEUED);
+
+					if (children->get_count(children) || tasks ||
+						vips->get_count(vips))
 					{
 						DBG1(DBG_IKE, "detected reauth of existing IKE_SA, "
-							 "adopting %d children and %d virtual IPs",
-							 children->get_count(children), vips->get_count(vips));
+							 "adopting %d children, %d child tasks, and %d "
+							 "virtual IPs", children->get_count(children),
+							 tasks, vips->get_count(vips));
 					}
 					if (ike_sa->get_state(ike_sa) == IKE_PASSIVE)
 					{
@@ -152,7 +189,8 @@ METHOD(job_t, execute, job_requeue_t,
 					charon->ike_sa_manager->checkin(
 											charon->ike_sa_manager, ike_sa);
 				}
-				if (children->get_count(children) || vips->get_count(vips))
+				if (children->get_count(children) || tasks ||
+					vips->get_count(vips))
 				{
 					break;
 				}
@@ -237,12 +275,6 @@ METHOD(job_t, get_priority, job_priority_t,
 	return JOB_PRIO_HIGH;
 }
 
-METHOD(adopt_children_job_t, queue_task, void,
-	private_adopt_children_job_t *this, task_t *task)
-{
-	array_insert_create(&this->tasks, ARRAY_TAIL, task);
-}
-
 /**
  * See header
  */

src/libcharon/sa/ike_sa.c

@@ -1996,8 +1996,7 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new,
 	/* adopt any active or queued CHILD-creating tasks */
 	if (status != DESTROY_ME)
 	{
-		task_manager_t *other_tasks = ((private_ike_sa_t*)new)->task_manager;
-		other_tasks->adopt_child_tasks(other_tasks, this->task_manager);
+		new->adopt_child_tasks(new, &this->public);
 		if (new->get_state(new) == IKE_CREATED)
 		{
 			status = new->initiate(new, NULL, 0, NULL, NULL);
@@ -2721,6 +2720,12 @@ METHOD(ike_sa_t, create_task_enumerator, enumerator_t*,
 	return this->task_manager->create_task_enumerator(this->task_manager, queue);
 }
 
+METHOD(ike_sa_t, remove_task, void,
+	private_ike_sa_t *this, enumerator_t *enumerator)
+{
+	return this->task_manager->remove_task(this->task_manager, enumerator);
+}
+
 METHOD(ike_sa_t, flush_queue, void,
 	private_ike_sa_t *this, task_queue_t queue)
 {
@@ -2739,6 +2744,36 @@ METHOD(ike_sa_t, queue_task_delayed, void,
 	this->task_manager->queue_task_delayed(this->task_manager, task, delay);
 }
 
+/**
+ * Migrate and queue child-creating tasks from another IKE_SA
+ */
+static void migrate_child_tasks(private_ike_sa_t *this, ike_sa_t *other,
+								task_queue_t queue)
+{
+	enumerator_t *enumerator;
+	task_t *task;
+
+	enumerator = other->create_task_enumerator(other, queue);
+	while (enumerator->enumerate(enumerator, &task))
+	{
+		if (task->get_type(task) == TASK_CHILD_CREATE ||
+			task->get_type(task) == TASK_QUICK_MODE)
+		{
+			other->remove_task(other, enumerator);
+			task->migrate(task, &this->public);
+			queue_task(this, task);
+		}
+	}
+	enumerator->destroy(enumerator);
+}
+
+METHOD(ike_sa_t, adopt_child_tasks, void,
+	private_ike_sa_t *this, ike_sa_t *other)
+{
+	migrate_child_tasks(this, other, TASK_QUEUE_ACTIVE);
+	migrate_child_tasks(this, other, TASK_QUEUE_QUEUED);
+}
+
 METHOD(ike_sa_t, inherit_pre, void,
 	private_ike_sa_t *this, ike_sa_t *other_public)
 {
@@ -3054,9 +3089,11 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id, bool initiator,
 			.create_attribute_enumerator = _create_attribute_enumerator,
 			.set_kmaddress = _set_kmaddress,
 			.create_task_enumerator = _create_task_enumerator,
+			.remove_task = _remove_task,
 			.flush_queue = _flush_queue,
 			.queue_task = _queue_task,
 			.queue_task_delayed = _queue_task_delayed,
+			.adopt_child_tasks = _adopt_child_tasks,
 #ifdef ME
 			.act_as_mediation_server = _act_as_mediation_server,
 			.get_server_reflexive_host = _get_server_reflexive_host,

src/libcharon/sa/ike_sa.h

@@ -1124,6 +1124,16 @@ struct ike_sa_t {
 	 */
 	enumerator_t* (*create_task_enumerator)(ike_sa_t *this, task_queue_t queue);
 
+	/**
+	 * Remove the task the given enumerator points to.
+	 *
+	 * @note This should be used with caution, in partciular, for tasks in the
+	 * active and passive queues.
+	 *
+	 * @param enumerator	enumerator created with the method above
+	 */
+	void (*remove_task)(ike_sa_t *this, enumerator_t *enumerator);
+
 	/**
 	 * Flush a task queue, cancelling all tasks in it.
 	 *
@@ -1147,6 +1157,13 @@ struct ike_sa_t {
 	 */
 	void (*queue_task_delayed)(ike_sa_t *this, task_t *task, uint32_t delay);
 
+	/**
+	 * Adopt child creating tasks from the given IKE_SA.
+	 *
+	 * @param other			other IKE_SA to adopt tasks from
+	 */
+	void (*adopt_child_tasks)(ike_sa_t *this, ike_sa_t *other);
+
 	/**
 	 * Inherit required attributes to new SA before rekeying.
 	 *

src/libcharon/sa/ike_sa_manager.c

@@ -1967,6 +1967,8 @@ static void adopt_children_and_vips(ike_sa_t *old, ike_sa_t *new)
 	}
 	enumerator->destroy(enumerator);
 
+	new->adopt_child_tasks(new, old);
+
 	enumerator = old->create_virtual_ip_enumerator(old, FALSE);
 	while (enumerator->enumerate(enumerator, &vip))
 	{

src/libcharon/sa/ikev1/task_manager_v1.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2016 Tobias Brunner
+ * Copyright (C) 2007-2018 Tobias Brunner
  * Copyright (C) 2007-2011 Martin Willi
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -1891,39 +1891,6 @@ METHOD(task_manager_t, adopt_tasks, void,
 	}
 }
 
-/**
- * Migrates child-creating tasks from src to dst
- */
-static void migrate_child_tasks(private_task_manager_t *this,
-								linked_list_t *src, linked_list_t *dst)
-{
-	enumerator_t *enumerator;
-	task_t *task;
-
-	enumerator = src->create_enumerator(src);
-	while (enumerator->enumerate(enumerator, &task))
-	{
-		if (task->get_type(task) == TASK_QUICK_MODE)
-		{
-			src->remove_at(src, enumerator);
-			task->migrate(task, this->ike_sa);
-			dst->insert_last(dst, task);
-		}
-	}
-	enumerator->destroy(enumerator);
-}
-
-METHOD(task_manager_t, adopt_child_tasks, void,
-	private_task_manager_t *this, task_manager_t *other_public)
-{
-	private_task_manager_t *other = (private_task_manager_t*)other_public;
-
-	/* move active child tasks from other to this */
-	migrate_child_tasks(this, other->active_tasks, this->queued_tasks);
-	/* do the same for queued tasks */
-	migrate_child_tasks(this, other->queued_tasks, this->queued_tasks);
-}
-
 METHOD(task_manager_t, busy, bool,
 	private_task_manager_t *this)
 {
@@ -1984,19 +1951,86 @@ METHOD(task_manager_t, reset, void,
 	}
 }
 
+/**
+ * Data for a task queue enumerator
+ */
+typedef struct {
+	enumerator_t public;
+	task_queue_t queue;
+	enumerator_t *inner;
+} task_enumerator_t;
+
+METHOD(enumerator_t, task_enumerator_destroy, void,
+	task_enumerator_t *this)
+{
+	this->inner->destroy(this->inner);
+	free(this);
+}
+
+METHOD(enumerator_t, task_enumerator_enumerate, bool,
+	task_enumerator_t *this, va_list args)
+{
+	task_t **task;
+
+	VA_ARGS_VGET(args, task);
+	return this->inner->enumerate(this->inner, task);
+}
+
 METHOD(task_manager_t, create_task_enumerator, enumerator_t*,
 	private_task_manager_t *this, task_queue_t queue)
 {
+	task_enumerator_t *enumerator;
+
+	INIT(enumerator,
+		.public = {
+			.enumerate = enumerator_enumerate_default,
+			.venumerate = _task_enumerator_enumerate,
+			.destroy = _task_enumerator_destroy,
+		},
+		.queue = queue,
+	);
 	switch (queue)
 	{
 		case TASK_QUEUE_ACTIVE:
-			return this->active_tasks->create_enumerator(this->active_tasks);
+			enumerator->inner = this->active_tasks->create_enumerator(
+														this->active_tasks);
+			break;
 		case TASK_QUEUE_PASSIVE:
-			return this->passive_tasks->create_enumerator(this->passive_tasks);
+			enumerator->inner = this->passive_tasks->create_enumerator(
+														this->passive_tasks);
+			break;
 		case TASK_QUEUE_QUEUED:
-			return this->queued_tasks->create_enumerator(this->queued_tasks);
+			enumerator->inner = this->queued_tasks->create_enumerator(
+														this->queued_tasks);
+			break;
 		default:
-			return enumerator_create_empty();
+			enumerator->inner = enumerator_create_empty();
+			break;
+	}
+	return &enumerator->public;
+}
+
+METHOD(task_manager_t, remove_task, void,
+	private_task_manager_t *this, enumerator_t *enumerator_public)
+{
+	task_enumerator_t *enumerator = (task_enumerator_t*)enumerator_public;
+
+	switch (enumerator->queue)
+	{
+		case TASK_QUEUE_ACTIVE:
+			this->active_tasks->remove_at(this->active_tasks,
+										  enumerator->inner);
+			break;
+		case TASK_QUEUE_PASSIVE:
+			this->passive_tasks->remove_at(this->passive_tasks,
+										   enumerator->inner);
+			break;
+		case TASK_QUEUE_QUEUED:
+			this->queued_tasks->remove_at(this->queued_tasks,
+										  enumerator->inner);
+			break;
+		default:
+			break;
 	}
 }
 
@@ -2047,9 +2081,9 @@ task_manager_v1_t *task_manager_v1_create(ike_sa_t *ike_sa)
 				.get_mid = _get_mid,
 				.reset = _reset,
 				.adopt_tasks = _adopt_tasks,
-				.adopt_child_tasks = _adopt_child_tasks,
 				.busy = _busy,
 				.create_task_enumerator = _create_task_enumerator,
+				.remove_task = _remove_task,
 				.flush = _flush,
 				.flush_queue = _flush_queue,
 				.destroy = _destroy,

src/libcharon/sa/ikev2/task_manager_v2.c

@@ -2076,61 +2076,6 @@ METHOD(task_manager_t, adopt_tasks, void,
 	}
 }
 
-/**
- * Migrates child-creating tasks from other to this
- */
-static void migrate_child_tasks(private_task_manager_t *this,
-								private_task_manager_t *other,
-								task_queue_t queue)
-{
-	enumerator_t *enumerator;
-	array_t *array;
-	task_t *task;
-
-	switch (queue)
-	{
-		case TASK_QUEUE_ACTIVE:
-			array = other->active_tasks;
-			break;
-		case TASK_QUEUE_QUEUED:
-			array = other->queued_tasks;
-			break;
-		default:
-			return;
-	}
-
-	enumerator = array_create_enumerator(array);
-	while (enumerator->enumerate(enumerator, &task))
-	{
-		queued_task_t *queued = NULL;
-
-		if (queue == TASK_QUEUE_QUEUED)
-		{
-			queued = (queued_task_t*)task;
-			task = queued->task;
-		}
-		if (task->get_type(task) == TASK_CHILD_CREATE)
-		{
-			array_remove_at(array, enumerator);
-			task->migrate(task, this->ike_sa);
-			queue_task(this, task);
-			free(queued);
-		}
-	}
-	enumerator->destroy(enumerator);
-}
-
-METHOD(task_manager_t, adopt_child_tasks, void,
-	private_task_manager_t *this, task_manager_t *other_public)
-{
-	private_task_manager_t *other = (private_task_manager_t*)other_public;
-
-	/* move active child tasks from other to this */
-	migrate_child_tasks(this, other, TASK_QUEUE_ACTIVE);
-	/* do the same for queued tasks */
-	migrate_child_tasks(this, other, TASK_QUEUE_QUEUED);
-}
-
 METHOD(task_manager_t, busy, bool,
 	private_task_manager_t *this)
 {
@@ -2186,17 +2131,39 @@ METHOD(task_manager_t, reset, void,
 	this->reset = TRUE;
 }
 
-CALLBACK(filter_queued, bool,
-	void *unused, enumerator_t *orig, va_list args)
-{
+/**
+ * Data for a task queue enumerator
+ */
+typedef struct {
+	enumerator_t public;
+	task_queue_t queue;
+	enumerator_t *inner;
 	queued_task_t *queued;
+} task_enumerator_t;
+
+METHOD(enumerator_t, task_enumerator_destroy, void,
+	task_enumerator_t *this)
+{
+	this->inner->destroy(this->inner);
+	free(this);
+}
+
+METHOD(enumerator_t, task_enumerator_enumerate, bool,
+	task_enumerator_t *this, va_list args)
+{
 	task_t **task;
 
 	VA_ARGS_VGET(args, task);
-
-	if (orig->enumerate(orig, &queued))
+	if (this->queue == TASK_QUEUE_QUEUED)
+	{
+		if (this->inner->enumerate(this->inner, &this->queued))
+		{
+			*task = this->queued->task;
+			return TRUE;
+		}
+	}
+	else if (this->inner->enumerate(this->inner, task))
 	{
-		*task = queued->task;
 		return TRUE;
 	}
 	return FALSE;
@@ -2205,18 +2172,54 @@ CALLBACK(filter_queued, bool,
 METHOD(task_manager_t, create_task_enumerator, enumerator_t*,
 	private_task_manager_t *this, task_queue_t queue)
 {
+	task_enumerator_t *enumerator;
+
+	INIT(enumerator,
+		.public = {
+			.enumerate = enumerator_enumerate_default,
+			.venumerate = _task_enumerator_enumerate,
+			.destroy = _task_enumerator_destroy,
+		},
+		.queue = queue,
+	);
 	switch (queue)
 	{
 		case TASK_QUEUE_ACTIVE:
-			return array_create_enumerator(this->active_tasks);
+			enumerator->inner = array_create_enumerator(this->active_tasks);
+			break;
 		case TASK_QUEUE_PASSIVE:
-			return array_create_enumerator(this->passive_tasks);
+			enumerator->inner = array_create_enumerator(this->passive_tasks);
+			break;
 		case TASK_QUEUE_QUEUED:
-			return enumerator_create_filter(
-									array_create_enumerator(this->queued_tasks),
-									filter_queued, NULL, NULL);
+			enumerator->inner = array_create_enumerator(this->queued_tasks);
+			break;
 		default:
-			return enumerator_create_empty();
+			enumerator->inner = enumerator_create_empty();
+			break;
+	}
+	return &enumerator->public;
+}
+
+METHOD(task_manager_t, remove_task, void,
+	private_task_manager_t *this, enumerator_t *enumerator_public)
+{
+	task_enumerator_t *enumerator = (task_enumerator_t*)enumerator_public;
+
+	switch (enumerator->queue)
+	{
+		case TASK_QUEUE_ACTIVE:
+			array_remove_at(this->active_tasks, enumerator->inner);
+			break;
+		case TASK_QUEUE_PASSIVE:
+			array_remove_at(this->passive_tasks, enumerator->inner);
+			break;
+		case TASK_QUEUE_QUEUED:
+			array_remove_at(this->queued_tasks, enumerator->inner);
+			free(enumerator->queued);
+			enumerator->queued = NULL;
+			break;
+		default:
+			break;
 	}
 }
 
@@ -2266,9 +2269,9 @@ task_manager_v2_t *task_manager_v2_create(ike_sa_t *ike_sa)
 				.get_mid = _get_mid,
 				.reset = _reset,
 				.adopt_tasks = _adopt_tasks,
-				.adopt_child_tasks = _adopt_child_tasks,
 				.busy = _busy,
 				.create_task_enumerator = _create_task_enumerator,
+				.remove_task = _remove_task,
 				.flush = _flush,
 				.flush_queue = _flush_queue,
 				.destroy = _destroy,

src/libcharon/sa/task_manager.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2016 Tobias Brunner
+ * Copyright (C) 2013-2018 Tobias Brunner
  * Copyright (C) 2006 Martin Willi
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -227,13 +227,6 @@ struct task_manager_t {
 	 */
 	void (*adopt_tasks) (task_manager_t *this, task_manager_t *other);
 
-	/**
-	 * Migrate all active or queued CHILD_SA-creating tasks from other to this.
-	 *
-	 * @param other			manager which gives away its tasks
-	 */
-	void (*adopt_child_tasks) (task_manager_t *this, task_manager_t *other);
-
 	/**
 	 * Increment a message ID counter, in- or outbound.
 	 *
@@ -284,6 +277,16 @@ struct task_manager_t {
 	enumerator_t* (*create_task_enumerator)(task_manager_t *this,
 											task_queue_t queue);
 
+	/**
+	 * Remove the task the given enumerator points to.
+	 *
+	 * @note This should be used with caution, in partciular, for tasks in the
+	 * active and passive queues.
+	 *
+	 * @param enumerator	enumerator created with the method above
+	 */
+	void (*remove_task)(task_manager_t *this, enumerator_t *enumerator);
+
 	/**
 	 * Flush all tasks, regardless of the queue.
 	 */