From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 24 Apr 2020 13:16:26 +0000 (+0200)
Subject: vici: Store configs in a hashtable
X-Git-Tag: 5.9.0rc1~9^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=736fae4e6c98e52472b6373a4772bd7ea342995a;p=thirdparty%2Fstrongswan.git

vici: Store configs in a hashtable

This makes updates more efficient if many configs are loaded. Configs
still have to be enumerated to select them.
---

diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c
index 1eb7a24cd6..0f8113f929 100644
--- a/src/libcharon/plugins/vici/vici_config.c
+++ b/src/libcharon/plugins/vici/vici_config.c
@@ -48,6 +48,7 @@
 #include <threading/rwlock.h>
 #include <threading/rwlock_condvar.h>
 #include <collections/array.h>
+#include <collections/hashtable.h>
 #include <collections/linked_list.h>
 
 #include <pubkey_cert.h>
@@ -102,12 +103,12 @@ struct private_vici_config_t {
 	vici_dispatcher_t *dispatcher;
 
 	/**
-	 * List of loaded connections, as peer_cfg_t
+	 * Hashtable of loaded connections, as peer_cfg_t
 	 */
-	linked_list_t *conns;
+	hashtable_t *conns;
 
 	/**
-	 * Lock for conns list
+	 * Lock for conns table
 	 */
 	rwlock_t *lock;
 
@@ -133,12 +134,27 @@ struct private_vici_config_t {
 
 };
 
+CALLBACK(peer_filter, bool,
+	void *data, enumerator_t *orig, va_list args)
+{
+	peer_cfg_t **out;
+
+	VA_ARGS_VGET(args, out);
+
+	if (orig->enumerate(orig, NULL, out))
+	{
+		return TRUE;
+	}
+	return FALSE;
+}
+
 METHOD(backend_t, create_peer_cfg_enumerator, enumerator_t*,
 	private_vici_config_t *this, identification_t *me, identification_t *other)
 {
 	this->lock->read_lock(this->lock);
-	return enumerator_create_cleaner(this->conns->create_enumerator(this->conns),
-									 (void*)this->lock->unlock, this->lock);
+	return enumerator_create_filter(this->conns->create_enumerator(this->conns),
+									peer_filter, this->lock,
+									(void*)this->lock->unlock);
 }
 
 CALLBACK(ike_filter, bool,
@@ -149,7 +165,7 @@ CALLBACK(ike_filter, bool,
 
 	VA_ARGS_VGET(args, out);
 
-	if (orig->enumerate(orig, &cfg))
+	if (orig->enumerate(orig, NULL, &cfg))
 	{
 		*out = cfg->get_ike_cfg(cfg);
 		return TRUE;
@@ -169,23 +185,15 @@ METHOD(backend_t, create_ike_cfg_enumerator, enumerator_t*,
 METHOD(backend_t, get_peer_cfg_by_name, peer_cfg_t*,
 	private_vici_config_t *this, char *name)
 {
-	peer_cfg_t *current, *found = NULL;
-	enumerator_t *enumerator;
+	peer_cfg_t *found;
 
 	this->lock->read_lock(this->lock);
-	enumerator = this->conns->create_enumerator(this->conns);
-	while (enumerator->enumerate(enumerator, &current))
+	found = this->conns->get(this->conns, name);
+	if (found)
 	{
-		if (streq(current->get_name(current), name))
-		{
-			found = current;
-			found->get_ref(found);
-			break;
-		}
+		found->get_ref(found);
 	}
-	enumerator->destroy(enumerator);
 	this->lock->unlock(this->lock);
-
 	return found;
 }
 
@@ -2341,10 +2349,8 @@ static void replace_children(private_vici_config_t *this,
  */
 static void merge_config(private_vici_config_t *this, peer_cfg_t *peer_cfg)
 {
-	enumerator_t *enumerator;
-	peer_cfg_t *current;
+	peer_cfg_t *found;
 	ike_cfg_t *ike_cfg;
-	bool merged = FALSE;
 
 	this->lock->write_lock(this->lock);
 	while (this->handling_actions)
@@ -2352,40 +2358,33 @@ static void merge_config(private_vici_config_t *this, peer_cfg_t *peer_cfg)
 		this->condvar->wait(this->condvar, this->lock);
 	}
 
-	enumerator = this->conns->create_enumerator(this->conns);
-	while (enumerator->enumerate(enumerator, &current))
+	found = this->conns->get(this->conns, peer_cfg->get_name(peer_cfg));
+	if (found)
 	{
-		if (streq(peer_cfg->get_name(peer_cfg), current->get_name(current)))
+		ike_cfg = found->get_ike_cfg(found);
+		if (peer_cfg->equals(peer_cfg, found) &&
+			ike_cfg->equals(ike_cfg, peer_cfg->get_ike_cfg(peer_cfg)))
 		{
-			ike_cfg = current->get_ike_cfg(current);
-			if (peer_cfg->equals(peer_cfg, current) &&
-				ike_cfg->equals(ike_cfg, peer_cfg->get_ike_cfg(peer_cfg)))
-			{
-				DBG1(DBG_CFG, "updated vici connection: %s",
-					 peer_cfg->get_name(peer_cfg));
-				replace_children(this, peer_cfg, current);
-				peer_cfg->destroy(peer_cfg);
-			}
-			else
-			{
-				DBG1(DBG_CFG, "replaced vici connection: %s",
-					 peer_cfg->get_name(peer_cfg));
-				this->conns->insert_before(this->conns, enumerator, peer_cfg);
-				this->conns->remove_at(this->conns, enumerator);
-				handle_start_actions(this, current, TRUE);
-				handle_start_actions(this, peer_cfg, FALSE);
-				current->destroy(current);
-			}
-			merged = TRUE;
-			break;
+			DBG1(DBG_CFG, "updated vici connection: %s",
+				 peer_cfg->get_name(peer_cfg));
+			replace_children(this, peer_cfg, found);
+			peer_cfg->destroy(peer_cfg);
+		}
+		else
+		{
+			DBG1(DBG_CFG, "replaced vici connection: %s",
+				 peer_cfg->get_name(peer_cfg));
+			this->conns->put(this->conns, peer_cfg->get_name(peer_cfg),
+							 peer_cfg);
+			handle_start_actions(this, found, TRUE);
+			handle_start_actions(this, peer_cfg, FALSE);
+			found->destroy(found);
 		}
 	}
-	enumerator->destroy(enumerator);
-
-	if (!merged)
+	else
 	{
 		DBG1(DBG_CFG, "added vici connection: %s", peer_cfg->get_name(peer_cfg));
-		this->conns->insert_last(this->conns, peer_cfg);
+		this->conns->put(this->conns, peer_cfg->get_name(peer_cfg), peer_cfg);
 		handle_start_actions(this, peer_cfg, FALSE);
 	}
 	this->condvar->signal(this->condvar);
@@ -2659,10 +2658,8 @@ CALLBACK(load_conn, vici_message_t*,
 CALLBACK(unload_conn, vici_message_t*,
 	private_vici_config_t *this, char *name, u_int id, vici_message_t *message)
 {
-	enumerator_t *enumerator;
 	peer_cfg_t *cfg;
 	char *conn_name;
-	bool found = FALSE;
 
 	conn_name = message->get_str(message, NULL, "name");
 	if (!conn_name)
@@ -2675,23 +2672,16 @@ CALLBACK(unload_conn, vici_message_t*,
 	{
 		this->condvar->wait(this->condvar, this->lock);
 	}
-	enumerator = this->conns->create_enumerator(this->conns);
-	while (enumerator->enumerate(enumerator, &cfg))
+	cfg = this->conns->remove(this->conns, conn_name);
+	if (cfg)
 	{
-		if (streq(cfg->get_name(cfg), conn_name))
-		{
-			this->conns->remove_at(this->conns, enumerator);
-			handle_start_actions(this, cfg, TRUE);
-			cfg->destroy(cfg);
-			found = TRUE;
-			break;
-		}
+		handle_start_actions(this, cfg, TRUE);
+		cfg->destroy(cfg);
 	}
-	enumerator->destroy(enumerator);
 	this->condvar->signal(this->condvar);
 	this->lock->unlock(this->lock);
 
-	if (!found)
+	if (!cfg)
 	{
 		return create_reply("unload: connection '%s' not found", conn_name);
 	}
@@ -2710,7 +2700,7 @@ CALLBACK(get_conns, vici_message_t*,
 
 	this->lock->read_lock(this->lock);
 	enumerator = this->conns->create_enumerator(this->conns);
-	while (enumerator->enumerate(enumerator, &cfg))
+	while (enumerator->enumerate(enumerator, NULL, &cfg))
 	{
 		builder->add_li(builder, "%s", cfg->get_name(cfg));
 	}
@@ -2739,11 +2729,17 @@ static void manage_commands(private_vici_config_t *this, bool reg)
 	manage_command(this, "get-conns", get_conns, reg);
 }
 
+CALLBACK(destroy_conn, void,
+	peer_cfg_t *cfg, const void *key)
+{
+	cfg->destroy(cfg);
+}
+
 METHOD(vici_config_t, destroy, void,
 	private_vici_config_t *this)
 {
 	manage_commands(this, FALSE);
-	this->conns->destroy_offset(this->conns, offsetof(peer_cfg_t, destroy));
+	this->conns->destroy_function(this->conns, destroy_conn);
 	this->condvar->destroy(this->condvar);
 	this->lock->destroy(this->lock);
 	free(this);
@@ -2768,7 +2764,7 @@ vici_config_t *vici_config_create(vici_dispatcher_t *dispatcher,
 			.destroy = _destroy,
 		},
 		.dispatcher = dispatcher,
-		.conns = linked_list_create(),
+		.conns = hashtable_create(hashtable_hash_str, hashtable_equals_str, 32),
 		.lock = rwlock_create(RWLOCK_TYPE_DEFAULT),
 		.condvar = rwlock_condvar_create(),
 		.authority = authority,