improved MOBIKE:

prefer address family already used
  do not change address implicit when mobike supported
  handle multiple simultaneous roaming requests more properly
  proper enabling/disabling of UDP encapsulation

src/charon/sa/child_sa.c

@@ -776,20 +776,22 @@ static status_t update_hosts(private_child_sa_t *this,
 							 host_t *me, host_t *other, bool encap) 
 {
 	/* anything changed at all? */
-	if (me->equals(me, this->me.addr) && other->equals(other, this->other.addr))
+	if (me->equals(me, this->me.addr) && 
+		other->equals(other, this->other.addr) && this->encap == encap)
 	{
 		return SUCCESS;
 	}
-	
 	/* run updown script to remove iptables rules */
 	updown(this, FALSE);
 	
+	this->encap = encap;
+	
 	/* update our (initator) SAs */
 	charon->kernel_interface->update_sa(charon->kernel_interface, this->me.spi,
-				this->protocol, this->other.addr, this->me.addr, other, me);
+			this->protocol, this->other.addr, this->me.addr, other, me, encap);
 	/* update his (responder) SAs */
 	charon->kernel_interface->update_sa(charon->kernel_interface, this->other.spi, 
-				this->protocol, this->me.addr, this->other.addr, me, other);
+			this->protocol, this->me.addr, this->other.addr, me, other, encap);
 	
 	/* update policies */
 	if (!me->ip_equals(me, this->me.addr) ||

src/charon/sa/ike_sa.c

@@ -219,6 +219,11 @@ struct private_ike_sa_t {
 	 * list of peers additional addresses, transmitted via MOBIKE
 	 */
 	linked_list_t *additional_addresses;
+	
+	/**
+	 * number pending UPDATE_SA_ADDRESS (MOBIKE)
+	 */
+	u_int32_t pending_updates;
 
 	/**
 	 * Timestamps for this IKE_SA
@@ -715,6 +720,22 @@ static iterator_t* create_additional_address_iterator(private_ike_sa_t *this)
 	return this->additional_addresses->create_iterator(
 											this->additional_addresses, TRUE);
 }
+	
+/**
+ * Implementation of ike_sa_t.set_pending_updates.
+ */
+static void set_pending_updates(private_ike_sa_t *this, u_int32_t updates)
+{
+	this->pending_updates = updates;
+}
+
+/**
+ * Implementation of ike_sa_t.get_pending_updates.
+ */
+static u_int32_t get_pending_updates(private_ike_sa_t *this)
+{
+	return this->pending_updates;
+}
 
 /**
  * Update hosts, as addresses may change (NAT)
@@ -723,6 +744,11 @@ static void update_hosts(private_ike_sa_t *this, host_t *me, host_t *other)
 {
 	bool update = FALSE;
 	
+	if (supports_extension(this, EXT_MOBIKE))
+	{	/* if peer speaks mobike, address updates are explicit only */
+		return;
+	}
+	
 	if (me == NULL)
 	{
 		me = this->my_host;
@@ -1727,16 +1753,23 @@ static int get_path_prio(host_t *me, host_t *other)
 /**
  * Implementation of ike_sa_t.roam.
  */
-static status_t roam(private_ike_sa_t *this)
+static status_t roam(private_ike_sa_t *this, bool address)
 {
 	iterator_t *iterator;
 	host_t *me, *other, *cand_me, *cand_other;
 	ike_mobike_t *mobike;
-	int prio, best = 0;
+	int prio, best4 = 0, best6 = 0;
 	
-	/* only initiator handles address updated actively */
+	/* responder just updates the peer about changed address config */
 	if (!this->ike_sa_id->is_initiator(this->ike_sa_id))
 	{
+		if (supports_extension(this, EXT_MOBIKE) && address)
+		{
+			DBG1(DBG_IKE, "sending address list update using MOBIKE");
+			mobike = ike_mobike_create(&this->public, TRUE);
+			this->task_manager->queue_task(this->task_manager, (task_t*)mobike);
+			return this->task_manager->initiate(this->task_manager);
+		}
 		return SUCCESS;
 	}
 	
@@ -1746,11 +1779,20 @@ static status_t roam(private_ike_sa_t *this)
 												   other);
 	if (me)
 	{
-		best = get_path_prio(me, other);
+		if (me->get_family(me) == AF_INET)
+		{
+			best4 = get_path_prio(me, other);
+		}
+		else
+		{
+			best6 = get_path_prio(me, other);
+		}
 	}
 	iterator = create_additional_address_iterator(this);
 	while (iterator->iterate(iterator, (void**)&cand_other))
 	{
+		bool better = FALSE;
+	
 		cand_me = charon->kernel_interface->get_source_addr(
 										charon->kernel_interface, cand_other);
 		if (!cand_me)
@@ -1763,10 +1805,28 @@ static status_t roam(private_ike_sa_t *this)
 			cand_me->destroy(cand_me);
 			continue;
 		}
+		
 		prio = get_path_prio(cand_me, cand_other);
-		if (prio > best)
+		if (cand_me->get_family(cand_me) == AF_INET)
+		{
+			if (prio > best4 && (best6 == 0 ||
+						this->my_host->get_family(this->my_host) == AF_INET))
+			{
+				best4 = prio;
+				better = TRUE;
+			}
+		}
+		else
+		{
+			if (prio > best6 && (best4 == 0 ||
+						this->my_host->get_family(this->my_host) == AF_INET6))
+			{
+				best6 = prio;
+				better = TRUE;
+			}
+		}
+		if (better)
 		{
-			best = prio;
 			DESTROY_IF(me);
 			me = cand_me;
 			other = cand_other;
@@ -1797,19 +1857,20 @@ static status_t roam(private_ike_sa_t *this)
 	me->set_port(me, this->my_host->get_port(this->my_host));
 	other = other->clone(other);
 	other->set_port(other, this->other_host->get_port(this->other_host));
+	set_my_host(this, me);
+	set_other_host(this, other);
 	
 	/* update addresses with mobike, if supported ... */
 	if (supports_extension(this, EXT_MOBIKE))
 	{
 		DBG1(DBG_IKE, "requesting address change using MOBIKE");
 		mobike = ike_mobike_create(&this->public, TRUE);
-		mobike->roam(mobike, me, other);
+		mobike->roam(mobike, address);
 		this->task_manager->queue_task(this->task_manager, (task_t*)mobike);
 		return this->task_manager->initiate(this->task_manager);
 	}
 	DBG1(DBG_IKE, "reestablishing IKE_SA due address change");
 	/* ... reestablish if not */
-	set_my_host(this, me);
 	return reestablish(this);
 }
 
@@ -2078,6 +2139,8 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->public.supports_extension = (bool(*)(ike_sa_t*, ike_extension_t extension))supports_extension;
 	this->public.set_condition = (void (*)(ike_sa_t*, ike_condition_t,bool)) set_condition;
 	this->public.has_condition = (bool (*)(ike_sa_t*,ike_condition_t)) has_condition;
+	this->public.set_pending_updates = (void(*)(ike_sa_t*, u_int32_t updates))set_pending_updates;
+	this->public.get_pending_updates = (u_int32_t(*)(ike_sa_t*))get_pending_updates;
 	this->public.create_additional_address_iterator = (iterator_t*(*)(ike_sa_t*))create_additional_address_iterator;
 	this->public.add_additional_address = (void(*)(ike_sa_t*, host_t *host))add_additional_address;
 	this->public.retransmit = (status_t (*)(ike_sa_t *, u_int32_t)) retransmit;
@@ -2098,7 +2161,7 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->public.destroy_child_sa = (status_t (*)(ike_sa_t*,protocol_id_t,u_int32_t))destroy_child_sa;
 	this->public.rekey = (status_t (*)(ike_sa_t*))rekey;
 	this->public.reestablish = (status_t (*)(ike_sa_t*))reestablish;
-	this->public.roam = (status_t(*)(ike_sa_t*))roam;
+	this->public.roam = (status_t(*)(ike_sa_t*,bool))roam;
 	this->public.inherit = (status_t (*)(ike_sa_t*,ike_sa_t*))inherit;
 	this->public.generate_message = (status_t (*)(ike_sa_t*,message_t*,packet_t**))generate_message;
 	this->public.reset = (void (*)(ike_sa_t*))reset;
@@ -2138,6 +2201,7 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->other_virtual_ip = NULL;
 	this->dns_servers = linked_list_create();
 	this->additional_addresses = linked_list_create();
+	this->pending_updates = 0;
 	this->keyingtry = 0;
 	
 	return &this->public;

src/charon/sa/ike_sa.h

@@ -432,6 +432,22 @@ struct ike_sa_t {
 	 */
 	bool (*has_condition) (ike_sa_t *this, ike_condition_t condition);
 	
+	/**
+	 * @brief Get the number of queued MOBIKE address updates.
+	 *
+	 * @param this			calling object
+	 * @return				number of pending updates
+	 */
+	u_int32_t (*get_pending_updates)(ike_sa_t *this);
+	
+	/**
+	 * @brief Set the number of queued MOBIKE address updates.
+	 *
+	 * @param this			calling object
+	 * @param updates		number of pending updates
+	 */
+	void (*set_pending_updates)(ike_sa_t *this, u_int32_t updates);
+	
 	/**
 	 * @brief Initiate a new connection.
 	 *
@@ -510,10 +526,11 @@ struct ike_sa_t {
 	 * If MOBIKE is supported, addresses are updated; If not, the tunnel is
 	 * restarted.
 	 *
-	 * @param 
-	 * @return
+	 * @param this			calling object
+	 * @param address		TRUE if address list changed, FALSE otherwise
+	 * @return				SUCCESS, FAILED, DESTROY_ME
 	 */
-	status_t (*roam)(ike_sa_t *this);
+	status_t (*roam)(ike_sa_t *this, bool address);
 	
 	/**
 	 * @brief Processes a incoming IKEv2-Message.

src/charon/sa/task_manager.c

@@ -719,6 +719,10 @@ static status_t process_request(private_task_manager_t *this,
 					default:
 						break;
 				}
+				if (task)
+				{
+					break;
+				}
 			}
 			iterator->destroy(iterator);
 			

src/charon/sa/tasks/ike_mobike.c

@@ -51,16 +51,6 @@ struct private_ike_mobike_t {
 	 */
 	bool initiator;
 	
-	/**
-	 * local host to roam to
-	 */
-	host_t *me;
-	
-	/**
-	 * remote host to roam to
-	 */
-	host_t *other;
-	
 	/**
 	 * cookie2 value to verify new addresses
 	 */
@@ -70,6 +60,16 @@ struct private_ike_mobike_t {
 	 * NAT discovery reusing the IKE_NATD task
 	 */
 	ike_natd_t *natd;
+	
+	/**
+	 * use task to update addresses
+	 */
+	bool roam;
+	
+	/**
+	 * include address list update
+	 */
+	bool address;
 };
 
 /**
@@ -138,6 +138,11 @@ static void process_payloads(private_ike_mobike_t *this, message_t *message)
 				this->ike_sa->add_additional_address(this->ike_sa, host);
 				break;
 			}
+			case UPDATE_SA_ADDRESSES:
+			{
+				this->roam = TRUE;
+				break;
+			}
 			case NO_ADDITIONAL_ADDRESSES:
 			{
 				flush_additional_addresses(this);
@@ -200,6 +205,25 @@ static void build_address_list(private_ike_mobike_t *this, message_t *message)
 	iterator->destroy(iterator);
 }
 
+/**
+ * update addresses of associated CHILD_SAs
+ */
+static void update_children(private_ike_mobike_t *this)
+{
+	iterator_t *iterator;
+	child_sa_t *child_sa;
+	
+	iterator = this->ike_sa->create_child_sa_iterator(this->ike_sa);
+	while (iterator->iterate(iterator, (void**)&child_sa))
+	{
+		child_sa->update_hosts(child_sa,
+						this->ike_sa->get_my_host(this->ike_sa), 
+						this->ike_sa->get_other_host(this->ike_sa),
+						this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY));
+	}
+	iterator->destroy(iterator);
+}
+
 /**
  * Implementation of task_t.process for initiator
  */
@@ -211,16 +235,20 @@ static status_t build_i(private_ike_mobike_t *this, message_t *message)
 		message->add_notify(message, FALSE, MOBIKE_SUPPORTED, chunk_empty);
 		build_address_list(this, message);
 	}
-	else if (this->me || this->other)
-	{	/* address change */
-		message->add_notify(message, FALSE, UPDATE_SA_ADDRESSES, chunk_empty);
-		build_address_list(this, message);
-		/* set new addresses */
-		this->ike_sa->update_hosts(this->ike_sa, this->me, this->other);
-		if (this->natd)
+	else 
+	{
+		if (this->roam)
 		{
-			this->natd->task.build(&this->natd->task, message);
+			message->add_notify(message, FALSE, UPDATE_SA_ADDRESSES, chunk_empty);
 		}
+		if (this->address)
+		{
+			build_address_list(this, message);
+		}
+		
+		this->natd = ike_natd_create(this->ike_sa, this->initiator);
+		this->natd->task.build(&this->natd->task, message);
+		update_children(this);
 	}
 	
 	return NEED_MORE;
@@ -239,6 +267,16 @@ static status_t process_r(private_ike_mobike_t *this, message_t *message)
 	else if (message->get_exchange_type(message) == INFORMATIONAL)
 	{
 		process_payloads(this, message);
+		if (this->roam)
+		{
+			host_t *me, *other;
+			
+			me = message->get_destination(message);
+			other = message->get_source(message);
+			this->ike_sa->set_my_host(this->ike_sa, me->clone(me));
+			this->ike_sa->set_other_host(this->ike_sa, other->clone(other));
+		}
+		
 		if (this->natd)
 		{
 			this->natd->task.process(&this->natd->task, message);
@@ -268,6 +306,10 @@ static status_t build_r(private_ike_mobike_t *this, message_t *message)
 		{
 			this->natd->task.build(&this->natd->task, message);
 		}
+		if (this->roam)
+		{
+			update_children(this);
+		}
 		return SUCCESS;
 	}
 	return NEED_MORE;
@@ -287,11 +329,23 @@ static status_t process_i(private_ike_mobike_t *this, message_t *message)
 	}
 	else if (message->get_exchange_type(message) == INFORMATIONAL)
 	{
+		u_int32_t updates = this->ike_sa->get_pending_updates(this->ike_sa) - 1;
+		this->ike_sa->set_pending_updates(this->ike_sa, updates);
+		if (updates > 0)
+		{
+			/* newer update queued, ignore this one */
+			return SUCCESS;
+		}
 		process_payloads(this, message);
 		if (this->natd)
 		{
 			this->natd->task.process(&this->natd->task, message);
 		}
+		if (this->roam)
+		{
+			/* update again, as NAT state may have changed */
+			update_children(this);
+		}
 		return SUCCESS;
 	}
 	return NEED_MORE;
@@ -300,13 +354,12 @@ static status_t process_i(private_ike_mobike_t *this, message_t *message)
 /**
  * Implementation of ike_mobike_t.roam.
  */
-static void roam(private_ike_mobike_t *this, host_t *me, host_t *other)
+static void roam(private_ike_mobike_t *this, bool address)
 {
-	this->me = me;
-	this->other = other;
-	
-	/* include NAT detection when roaming */
-	this->natd = ike_natd_create(this->ike_sa, this->initiator);
+	this->roam = TRUE;
+	this->address = address;
+	this->ike_sa->set_pending_updates(this->ike_sa, 
+							this->ike_sa->get_pending_updates(this->ike_sa) + 1);
 }
 
 /**
@@ -322,12 +375,8 @@ static task_type_t get_type(private_ike_mobike_t *this)
  */
 static void migrate(private_ike_mobike_t *this, ike_sa_t *ike_sa)
 {
-	DESTROY_IF(this->me);
-	DESTROY_IF(this->other);
 	chunk_free(&this->cookie2);
 	this->ike_sa = ike_sa;
-	this->me = NULL;
-	this->other = NULL;
 	if (this->natd)
 	{
 		this->natd->task.migrate(&this->natd->task, ike_sa);
@@ -339,8 +388,6 @@ static void migrate(private_ike_mobike_t *this, ike_sa_t *ike_sa)
  */
 static void destroy(private_ike_mobike_t *this)
 {
-	DESTROY_IF(this->me);
-	DESTROY_IF(this->other);
 	chunk_free(&this->cookie2);
 	if (this->natd)
 	{
@@ -356,7 +403,7 @@ ike_mobike_t *ike_mobike_create(ike_sa_t *ike_sa, bool initiator)
 {
 	private_ike_mobike_t *this = malloc_thing(private_ike_mobike_t);
 
-	this->public.roam = (void(*)(ike_mobike_t*, host_t *, host_t *))roam;
+	this->public.roam = (void(*)(ike_mobike_t*,bool))roam;
 	this->public.task.get_type = (task_type_t(*)(task_t*))get_type;
 	this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate;
 	this->public.task.destroy = (void(*)(task_t*))destroy;
@@ -374,8 +421,8 @@ ike_mobike_t *ike_mobike_create(ike_sa_t *ike_sa, bool initiator)
 	
 	this->ike_sa = ike_sa;
 	this->initiator = initiator;
-	this->me = NULL;
-	this->other = NULL;
+	this->roam = FALSE;
+	this->address = TRUE;
 	this->cookie2 = chunk_empty;
 	this->natd = NULL;
 	

src/charon/sa/tasks/ike_mobike.h

@@ -54,13 +54,10 @@ struct ike_mobike_t {
 	/**
 	 * @brief Use the task to roam to other addresses.
 	 *
-	 * Supplied hosts may be NULL to reuse existing IKE_SA hosts.
-	 *
 	 * @param this			calling object
-	 * @param me			local host to roam to, or NULL
-	 * @param other			remote host to roam to, or NULL
+	 * @param address		TRUE to include address list update
 	 */
-	void (*roam)(ike_mobike_t *this, host_t *me, host_t *other);
+	void (*roam)(ike_mobike_t *this, bool address);
 };
 
 /**