gsup_client: move ownership of the msg buffer into struct resp

The resp is already referencing data inside the msg buffer. It should
take the ownership of the incoming message and the caller must take care of it.

src/libcharon/plugins/osmo_epdg/gsup_client.c

@@ -25,6 +25,7 @@
 #include <threading/mutex.h>
 #include <threading/thread.h>
 #include <threading/condvar.h>
+#include <threading/rwlock.h>
 
 #include <osmocom/core/msgb.h>
 #include <osmocom/gsm/apn.h>
@@ -38,6 +39,27 @@
 #include "gsup_client.h"
 #include "osmo_epdg_utils.h"
 
+/* A GSUP client for osmocom.
+ *
+ * A request will block until it handled by the gsup or timeout.
+ * So all tx function will block with a timeout of 5 seconds.
+ *
+ * To allow multiple request in flight, request will flow through gsup_client:
+ * - send_auth_request() -> generate a gsup_request_t object
+ * - it will enqueue()d into the inqueue (a blocking queue).
+ * - the sender job (a differnet thread) will get a req out of inqueue, transmit it and enqueue it into **pending**.
+ * - the receveier job (also a different thread?) will receive a PDU and try to find a matching gsup_request_t.
+ * - if a matching gsup_request_t can be found, the thread of who is blocked in send_auth_request() will be woken and can work with the response.
+ *
+ * - if a timeout happen, the gsup_request_t can be at 3 different position.
+ * - a) still in the *inqueue*. the requester can remove it atomic
+ * - b) in the *pending* list, the requester can remove it atomic
+ * - c) neither in *inqueue* and *pending*, it is current in use by the sender/receiver. The requester will use the gsup_request_t->lock() to wait for the completion.
+ *
+ * The c) case is the most complex to ensure the request will be cleaned. If not synchronized, the requester look into the *pending* queue, can't find it there and the
+ * gsup_request_t will never been cleaned.
+ */
+
 typedef struct gsup_request_t gsup_request_t;
 struct gsup_request_t {
 	/**
@@ -50,8 +72,23 @@ struct gsup_request_t {
 	 */
 	condvar_t *condvar;
 
+	/**
+	 * Lock the object to allow working with it without an garbage collector.
+	 * When a request time out and at the same time the receiver or sender is using the object,
+	 * the object isn't part of the in-queue nor of the pending-list. Use the lock to synchronize
+	 * this small time.
+	 * After this lock is taken by enqueue(), the rx/tx gsup won't add it anymore to pending list and release it.
+	 */
+	rwlock_t *lock;
+
+	/**
+	 * refcounter to free the object
+	 */
+	refcount_t refcount;
+
 	struct msgb *msg;
 	enum osmo_gsup_message_type msg_type;
+	char *imsi;
 	osmo_epdg_gsup_response_t *resp;
 };
 
@@ -65,26 +102,25 @@ struct private_osmo_epdg_gsup_client_t {
 	osmo_epdg_ipa_client_t *ipa;
 
 	/**
-	 * List of all pending requests
+	 * List of all inqueue requests
+	 * The list "owns" a references by req->get().
 	 */
-	blocking_queue_t *pending;
+	blocking_queue_t *inqueue;
 
 	/**
-	 * Current request which isn't part of linked list
+	 * List of all pending requests (gsup_request_t).
+	 * The list "owns" a references by req->get().
 	 */
-	gsup_request_t *current_request;
-
-	/**
-	 * Mutex to protect current_request
-	 */
-	mutex_t *mutex;
+	linked_list_t *pending;
+	mutex_t *pending_mutex;
 
 	char *uri;
 
 	stream_t *stream;
 };
 
-static gsup_request_t *gsup_request_create(enum osmo_gsup_message_type msg_type, struct msgb *msg)
+/* TODO: move into own class? */
+static gsup_request_t *gsup_request_create(enum osmo_gsup_message_type msg_type, const char *imsi, struct msgb *msg)
 {
 	gsup_request_t *req = calloc(1, sizeof(gsup_request_t));
 	if (!req)
@@ -92,108 +128,64 @@ static gsup_request_t *gsup_request_create(enum osmo_gsup_message_type msg_type,
 		return NULL;
 	}
 
+	req->lock = rwlock_create(RWLOCK_TYPE_DEFAULT);
 	req->mutex = mutex_create(MUTEX_TYPE_DEFAULT);
 	req->condvar = condvar_create(CONDVAR_TYPE_DEFAULT);
 	req->msg_type = msg_type;
+	req->imsi = strdup(imsi);
 	req->msg = msg;
+	req->refcount = 1;
 
 	return req;
 }
 
-static void gsup_request_destroy(private_osmo_epdg_gsup_client_t *this, gsup_request_t *req)
+static void gsup_request_destroy(gsup_request_t *this)
 {
-	if (!req)
+	if (!this)
 	{
 		return;
 	}
 
-	if (req->mutex)
+	if (this->mutex)
 	{
-		req->mutex->destroy(req->mutex);
+		this->mutex->destroy(this->mutex);
 	}
 
-	if (req->condvar)
+	if (this->condvar)
 	{
-		req->condvar->destroy(req->condvar);
+		this->condvar->destroy(this->condvar);
 	}
 
-	if (req->msg)
+	DESTROY_IF(this->lock);
+
+	if (this->imsi)
 	{
-		free(req->msg);
+		free(this->imsi);
 	}
 
-	if (req->resp)
+	if (this->msg)
 	{
-		free(req->resp);
+		free(this->msg);
 	}
-	free(req);
+
+	if (this->resp)
+	{
+		free(this->resp);
+	}
+	free(this);
 }
 
-static struct msgb *encode_to_msgb(struct osmo_gsup_message *gsup_msg)
+static void gsup_request_get(gsup_request_t *this)
 {
-	chunk_t msg_chunk;
-	struct msgb *msg;
-	int ret;
-
-	msg_chunk = chunk_alloc(4000);
-	if (msg_chunk.ptr == NULL)
-	{
-		return NULL;
-	}
-
-	msg = chunk_to_msgb(&msg_chunk);
-	if (!msg)
-	{
-		goto free_msg;
-	}
-
-	/* reserve headroom */
-	msgb_reserve(msg, 64);
-	ret = osmo_gsup_encode(msg, gsup_msg);
-	if (ret)
-	{
-		DBG1(DBG_NET, "GSUP: couldn't encode gsup message %d.", ret);
-		goto free_msg;
-	}
-
-	return msg;
-
-free_msg:
-	chunk_free(&msg_chunk);
-	return NULL;
+	ref_get(&this->refcount);
 }
 
-/**
- * enqueue a message/request to be send out and wait for the response.
- * 
- * when exiting enqueue, it must be guaranteed the req isn't referenced by anything
- * @param timeout_ms A timeout in ms
- * @return TRUE is the request timed out.
- */
-static bool enqueue(private_osmo_epdg_gsup_client_t *this, gsup_request_t *req, u_int timeout_ms)
+static void gsup_request_put(gsup_request_t *this)
 {
-	bool ret = FALSE;
-
-	DBG1(DBG_NET, "Enqueuing message. Waiting %d ms for an answer", timeout_ms);
-	req->mutex->lock(req->mutex);
-	this->pending->enqueue(this->pending, req);
-	ret = req->condvar->timed_wait(req->condvar, req->mutex, timeout_ms);
-	if (ret)
+	if (ref_put(&this->refcount))
 	{
-		void *found = this->pending->remove(this->pending, req);
-		if (!found)
-		{
-			this->mutex->lock(this->mutex);
-			if (this->current_request == req)
-			{
-				this->current_request = NULL;
-			}
-			this->mutex->unlock(this->mutex);
-		}
-		DBG1(DBG_NET, "Message timedout!");
+		gsup_request_destroy(this);
 	}
-
-	return ret;
 }
 
 #define IMSI_LEN 15
@@ -213,14 +205,138 @@ int imsi_copy(void *dest, const char *imsi)
 	return 0;
 }
 
+static struct msgb *encode_to_msgb(struct osmo_gsup_message *gsup_msg)
+{
+	chunk_t msg_chunk;
+	struct msgb *msg;
+	int ret;
+
+	msg_chunk = chunk_alloc(4000);
+	if (msg_chunk.ptr == NULL)
+	{
+		return NULL;
+	}
+
+	msg = epdg_chunk_to_msgb(&msg_chunk);
+	if (!msg)
+	{
+		goto free_msg;
+	}
+
+	/* reserve headroom */
+	msgb_reserve(msg, 64);
+	ret = osmo_gsup_encode(msg, gsup_msg);
+	if (ret)
+	{
+		DBG1(DBG_NET, "epdg: gsupc: couldn't encode gsup message %d.", ret);
+		goto free_msg;
+	}
+
+	return msg;
+
+free_msg:
+	chunk_free(&msg_chunk);
+	return NULL;
+}
+
+static bool remove_pending(linked_list_t *list, gsup_request_t *req)
+{
+	enumerator_t *enumerator;
+	gsup_request_t *ele = NULL;
+	bool found = false;
+
+	enumerator = list->create_enumerator(list);
+	while (enumerator->enumerate(enumerator, (void **) &ele))
+	{
+		if (ele == req)
+		{
+			list->remove_at(list, enumerator);
+			found = true;
+			goto out;
+		}
+	}
+out:
+	enumerator->destroy(enumerator);
+	return found;
+}
+
+static gsup_request_t *get_pending(linked_list_t *list, const char *imsi, enum osmo_gsup_message_type message_type)
+{
+	enumerator_t *enumerator;
+	gsup_request_t *req = NULL;
+	message_type = message_type & ~0b11;
+	enumerator = list->create_enumerator(list);
+	while (enumerator->enumerate(enumerator, (void **) &req))
+	{
+		if (strncmp(imsi, req->imsi, IMSI_LEN) == 0 && req->msg_type == message_type)
+		{
+			list->remove_at(list, enumerator);
+			goto out;
+		}
+	}
+	req = NULL;
+out:
+	enumerator->destroy(enumerator);
+	return req;
+}
+
+
+/**
+ * enqueue a message/request to be send out and wait for the response.
+ *
+ * when exiting enqueue, it must be guaranteed the req isn't referenced by anything.
+ * The caller must hold a ref to req via get().
+ * @param timeout_ms A timeout in ms
+ * @return TRUE is the request timed out.
+ */
+static bool enqueue(private_osmo_epdg_gsup_client_t *this, gsup_request_t *req, u_int timeout_ms)
+{
+	bool ret = FALSE;
+	DBG1(DBG_NET, "epdg: gsupc: Enqueuing message. Waiting %d ms for an answer", timeout_ms);
+
+	req->mutex->lock(req->mutex);
+	/* take a ref to have for the in/pending queue */
+	gsup_request_get(req);
+	this->inqueue->enqueue(this->inqueue, req);
+	ret = req->condvar->timed_wait(req->condvar, req->mutex, timeout_ms);
+
+	/* take owner ship / allow garbage free release.
+	 * The owner ship isn't giving back. The rx/tx path will fail on try_write_lock() */
+	req->lock->write_lock(req->lock);
+	if (ret)
+	{
+		/* timed out */
+		DBG1(DBG_NET, "epdg: gsupc: %s/%d Message timedout!", req->imsi, req->msg_type);
+		void *found = this->inqueue->remove(this->inqueue, req);
+		if (found)
+		{
+			/* give back the ref we took for the pending queue */
+			gsup_request_put(req);
+			return ret;
+		}
+		this->pending_mutex->lock(this->pending_mutex);
+		bool found2 = remove_pending(this->pending, req);
+		this->pending_mutex->unlock(this->pending_mutex);
+		if (found2)
+		{
+			/* give back the ref we took for the pending queue */
+			gsup_request_put(req);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
 METHOD(osmo_epdg_gsup_client_t, tunnel_request, osmo_epdg_gsup_response_t*,
         private_osmo_epdg_gsup_client_t *this, const char *imsi)
 {
 	struct osmo_gsup_message gsup_msg = {0};
 	struct msgb *msg;
+	const char *pco = "\x80\x00\x0d\x00\x00\x0c\x00";
 	bool timedout;
 
-	DBG1(DBG_NET, "Tunnel Request Request for %s", imsi);
+	DBG1(DBG_NET, "epdg: gsupc: Tunnel Request Request for %s", imsi);
 	gsup_msg.message_type = OSMO_GSUP_MSGT_EPDG_TUNNEL_REQUEST;
 	gsup_msg.current_rat_type = OSMO_RAT_EUTRAN_SGS;
 	gsup_msg.message_class = OSMO_GSUP_MESSAGE_CLASS_IPSEC_EPDG;
@@ -230,25 +346,28 @@ METHOD(osmo_epdg_gsup_client_t, tunnel_request, osmo_epdg_gsup_response_t*,
 		return NULL;
 	}
 
+	gsup_msg.pco = pco;
+	gsup_msg.pco_len = 7;
+
 	msg = encode_to_msgb(&gsup_msg);
 	if (!msg)
 	{
-		DBG1(DBG_NET, "Couldn't alloc/encode gsup message.");
+		DBG1(DBG_NET, "epdg: gsupc: Couldn't alloc/encode gsup message.");
 		return NULL;
 	}
 
-	gsup_request_t *req = gsup_request_create(OSMO_GSUP_MSGT_EPDG_TUNNEL_REQUEST, msg);
+	gsup_request_t *req = gsup_request_create(OSMO_GSUP_MSGT_EPDG_TUNNEL_REQUEST, imsi, msg);
 	osmo_epdg_gsup_response_t *resp = NULL;
 	timedout = enqueue(this, req, 5000);
 	if (timedout)
 	{
-		gsup_request_destroy(this, req);
+		gsup_request_put(req);
 		return NULL;
 	}
 
 	resp = req->resp;
 	req->resp = NULL;
-	gsup_request_destroy(this, req);
+	gsup_request_put(req);
 	return resp;
 }
 
@@ -263,7 +382,7 @@ METHOD(osmo_epdg_gsup_client_t, send_auth_request, osmo_epdg_gsup_response_t*,
 	size_t apn_enc_len = 0;
 	int ret;
 
-	DBG1(DBG_NET, "GSUP: Send Auth Request for %s", imsi);
+	DBG1(DBG_NET, "epdg: gsupc: Send Auth Request for %s", imsi);
 	gsup_msg.message_type = OSMO_GSUP_MSGT_SEND_AUTH_INFO_REQUEST;
 	gsup_msg.message_class = OSMO_GSUP_MESSAGE_CLASS_IPSEC_EPDG;
 	gsup_msg.num_auth_vectors = 1;
@@ -272,13 +391,13 @@ METHOD(osmo_epdg_gsup_client_t, send_auth_request, osmo_epdg_gsup_response_t*,
 	if (imsi_copy(gsup_msg.imsi, imsi))
 	{
 		/* TODO: inval imsi! */
-		DBG1(DBG_NET, "GSUP: SAR: Invalid IMSI.");
+		DBG1(DBG_NET, "epdg: gsupc: SAR: Invalid IMSI.");
 		return NULL;
 	}
 
 	if (!apn || strlen(apn) == 0)
 	{
-		DBG1(DBG_NET, "GSUP: SAR: Invalid APN.");
+		DBG1(DBG_NET, "epdg: gsupc: SAR: Invalid APN.");
 		return NULL;
 	}
 
@@ -292,7 +411,7 @@ METHOD(osmo_epdg_gsup_client_t, send_auth_request, osmo_epdg_gsup_response_t*,
 			gsup_msg.cn_domain = cn_domain;
 			break;
 		default:
-			DBG1(DBG_NET, "GSUP: SAR: Ignoring invalid cn_domain message.");
+			DBG1(DBG_NET, "epdg: gsupc: SAR: Ignoring invalid cn_domain message.");
 			break;
 	}
 
@@ -326,7 +445,7 @@ METHOD(osmo_epdg_gsup_client_t, send_auth_request, osmo_epdg_gsup_response_t*,
 	ret = osmo_apn_from_str(apn_enc, APN_MAXLEN, apn);
 	if (ret < 0)
 	{
-		DBG1(DBG_NET, "GSUP: Couldn't encode APN %s!", apn);
+		DBG1(DBG_NET, "epdg: gsupc: Couldn't encode APN %s!", apn);
 		return NULL;
 	}
 	apn_enc_len = ret;
@@ -337,23 +456,23 @@ METHOD(osmo_epdg_gsup_client_t, send_auth_request, osmo_epdg_gsup_response_t*,
 	msg = encode_to_msgb(&gsup_msg);
 	if (!msg)
 	{
-		DBG1(DBG_NET, "GSUP: Couldn't alloc/encode gsup message.");
+		DBG1(DBG_NET, "epdg: gsupc: Couldn't alloc/encode gsup message.");
 		return NULL;
 	}
 
-	gsup_request_t *req = gsup_request_create(OSMO_GSUP_MSGT_SEND_AUTH_INFO_REQUEST, msg);
+	gsup_request_t *req = gsup_request_create(OSMO_GSUP_MSGT_SEND_AUTH_INFO_REQUEST, imsi, msg);
 	osmo_epdg_gsup_response_t *resp = NULL;
 	timedout = enqueue(this, req, 5000);
 	if (timedout)
 	{
-		DBG1(DBG_NET, "GSUP: Timeout request.");
-		gsup_request_destroy(this, req);
+		DBG1(DBG_NET, "epdg: gsupc: Timeout request.");
+		gsup_request_put(req);
 		return NULL;
 	}
 
 	resp = req->resp;
 	req->resp = NULL;
-	gsup_request_destroy(this, req);
+	gsup_request_put(req);
 
 	return resp;
 }
@@ -385,29 +504,29 @@ METHOD(osmo_epdg_gsup_client_t, update_location, osmo_epdg_gsup_response_t *,
 			gsup_msg.cn_domain = cn_domain;
 			break;
 		default:
-			DBG1(DBG_NET, "GSUP: ULR: Ignoring invalid cn_domain message.");
+			DBG1(DBG_NET, "epdg: gsupc: ULR: Ignoring invalid cn_domain message.");
 			break;
 	}
 
 	msg = encode_to_msgb(&gsup_msg);
 	if (!msg)
 	{
-		DBG1(DBG_NET, "GSUP: ULR: Couldn't alloc/encode gsup message.");
+		DBG1(DBG_NET, "epdg: gsupc: ULR: Couldn't alloc/encode gsup message.");
 		return NULL;
 	}
 
-	gsup_request_t *req = gsup_request_create(OSMO_GSUP_MSGT_UPDATE_LOCATION_REQUEST, msg);
+	gsup_request_t *req = gsup_request_create(OSMO_GSUP_MSGT_UPDATE_LOCATION_REQUEST, imsi, msg);
 	osmo_epdg_gsup_response_t *resp = NULL;
 	timedout = enqueue(this, req, 5000);
 	if (timedout)
 	{
-		gsup_request_destroy(this, req);
+		gsup_request_put(req);
 		return NULL;
 	}
 
 	resp = req->resp;
 	req->resp = NULL;
-	gsup_request_destroy(this, req);
+	gsup_request_put(req);
 
 	return resp;
 }
@@ -441,14 +560,14 @@ void tx_insert_data_result(private_osmo_epdg_gsup_client_t *this, const char *im
 			gsup_msg.cn_domain = cn_domain;
 			break;
 		default:
-			DBG1(DBG_NET, "GSUP: ULR: Ignoring invalid cn_domain message.");
+			DBG1(DBG_NET, "epdg: gsupc: ULR: Ignoring invalid cn_domain message.");
 			break;
 	}
 
 	msg = encode_to_msgb(&gsup_msg);
 	if (!msg)
 	{
-		DBG1(DBG_NET, "GSUP: ULR: Couldn't alloc/encode gsup message.");
+		DBG1(DBG_NET, "epdg: gsupc: ULR: Couldn't alloc/encode gsup message.");
 	}
 	this->ipa->send(this->ipa, IPAC_PROTO_EXT_GSUP, msg);
 }
@@ -464,12 +583,14 @@ static void signal_request(gsup_request_t *req, osmo_epdg_gsup_response_t *resp)
 static bool on_recv_pdu(void *data, osmo_epdg_ipa_client_t *client, struct msgb *pdu)
 {
 	private_osmo_epdg_gsup_client_t *this = data;
+	gsup_request_t *req;
 	osmo_epdg_gsup_response_t *resp;
 	int ret;
 
 	resp = calloc(1, sizeof(*resp));
 	if (!resp)
 	{
+		free(pdu);
 		return TRUE;
 	}
 
@@ -479,6 +600,11 @@ static bool on_recv_pdu(void *data, osmo_epdg_ipa_client_t *client, struct msgb
 		goto out;
 	}
 
+	resp->pdu = pdu;
+	DBG1(DBG_NET, "epdg: gsupc: receive gsup message %s/%d",
+	     resp->gsup.imsi, resp->gsup.message_type);
+
+
 	switch (resp->gsup.message_type)
 	{
 		case OSMO_GSUP_MSGT_INSERT_DATA_REQUEST:
@@ -490,30 +616,37 @@ static bool on_recv_pdu(void *data, osmo_epdg_ipa_client_t *client, struct msgb
 		case OSMO_GSUP_MSGT_UPDATE_LOCATION_RESULT:
 		case OSMO_GSUP_MSGT_EPDG_TUNNEL_ERROR:
 		case OSMO_GSUP_MSGT_EPDG_TUNNEL_RESULT:
-			this->mutex->lock(this->mutex);
-			if ((this->current_request->msg_type & 0xfffffffc) != (resp->gsup.message_type & 0xfffffffc))
+			this->pending_mutex->lock(this->pending_mutex);
+			req = get_pending(this->pending, resp->gsup.imsi, resp->gsup.message_type);
+			if (!req)
 			{
-				/* Request, Result, Error, Other are encoded in the last 2 bits */
-				DBG1(DBG_NET, "GSUP: received non matching Result. Requested %s but received %s",
-					osmo_gsup_message_type_name(this->current_request->msg_type),
-					osmo_gsup_message_type_name(resp->gsup.message_type));
+				this->pending_mutex->unlock(this->pending_mutex);
+				DBG1(DBG_NET, "epdg: gsupc: receive gsup message where no matching response could be found. %s/%d",
+				     resp->gsup.imsi, resp->gsup.message_type);
 				goto out;
 			}
-			if (!this->current_request)
+
+			if (!req->lock->try_write_lock(req->lock))
 			{
-				DBG2(DBG_NET, "GSUP: received response when no request waiting %02x. This might came too late.", resp->gsup.message_type);
-				this->mutex->unlock(this->mutex);
+				/* Race Condition, Response came to late! */
+				DBG1(DBG_NET, "epdg: gsupc: %s/%d: Can't aquire try_write_lock. Response too late",
+				     resp->gsup.imsi, resp->gsup.message_type);
+				this->pending_mutex->unlock(this->pending_mutex);
+				gsup_request_put(req);
 				goto out;
 			}
-			signal_request(this->current_request, resp);
-			this->current_request = NULL;
-			this->mutex->unlock(this->mutex);
+			this->pending_mutex->unlock(this->pending_mutex);
+			DBG1(DBG_NET, "epdg: gsupc: %s/%d: Informing requester. %p",
+			     resp->gsup.imsi, resp->gsup.message_type, req);
+
+			signal_request(req, resp);
+			req->lock->unlock(req->lock);
+			gsup_request_put(req);
 			break;
 		default:
-			DBG1(DBG_NET, "GSUP received unknown message type %02x", resp->gsup.message_type);
+			DBG1(DBG_NET, "epdg: gsupc: received unknown message type %02x", resp->gsup.message_type);
 			goto out;
 	}
-	free(pdu);
 	return TRUE;
 
 out:
@@ -528,33 +661,33 @@ static int disconnect_gsup(private_osmo_epdg_gsup_client_t *this)
 	return 0;
 }
 
+static void add_pending(private_osmo_epdg_gsup_client_t *this, gsup_request_t *req)
+{
+	this->pending_mutex->lock(this->pending_mutex);
+	this->pending->insert_last(this->pending, req);
+	this->pending_mutex->unlock(this->pending_mutex);
+}
+
 /* TODO: worker thread which sends out enqueue'd message ! */
 static job_requeue_t queue_worker(private_osmo_epdg_gsup_client_t *this)
 {
 	int ret;
 	gsup_request_t *req;
-	this->mutex->lock(this->mutex);
-	if (this->current_request)
-	{
-		/* TODO: should we join the signal? */
-		this->mutex->unlock(this->mutex);
-		return JOB_REQUEUE_FAIR;
-	}
-	this->mutex->unlock(this->mutex);
 
+	/* should we multiple queue it? */
 	/* TODO: replace pending with a thread safe queue, but non-blocking */
-	req = this->pending->dequeue(this->pending);
-
-	this->mutex->lock(this->mutex);
-	if (this->current_request)
+	req = this->inqueue->dequeue(this->inqueue);
+	if (!req)
 	{
-		/* TODO: how could this happen? */
-		this->mutex->unlock(this->mutex);
-		signal_request(req, NULL);
-		return JOB_REQUEUE_FAIR;
+		return JOB_REQUEUE_NONE;
+	}
+
+	if (!req->lock->try_write_lock(req->lock))
+	{
+		/* request is about to be released */
+		gsup_request_put(req);
+		return JOB_REQUEUE_NONE;
 	}
-	this->current_request = req;
-	this->mutex->unlock(this->mutex);
 
 	ret = this->ipa->send(this->ipa, IPAC_PROTO_EXT_GSUP, req->msg);
 	req->msg = NULL;
@@ -562,15 +695,22 @@ static job_requeue_t queue_worker(private_osmo_epdg_gsup_client_t *this)
 	{
 		/* TODO: disconnect & reconnect, but request is lost for now */
 		/* TODO: wake up request */
+		req->lock->unlock(req->lock);
 		signal_request(req, NULL);
+		gsup_request_put(req);
+	} else {
+		/* add to pending */
+		add_pending(this, req);
+		req->lock->unlock(req->lock);
 	}
+
 	return JOB_REQUEUE_FAIR;
 }
 
 osmo_epdg_gsup_client_t *osmo_epdg_gsup_client_create(char *uri)
 {
 	private_osmo_epdg_gsup_client_t *this;
-	DBG1(DBG_NET, "Starting osmo-epdg");
+	DBG1(DBG_NET, "epdg: gsupc: Starting");
 
 	INIT(this,
 			.public = {
@@ -580,9 +720,9 @@ osmo_epdg_gsup_client_t *osmo_epdg_gsup_client_create(char *uri)
 				.destroy = _destroy,
 			},
 			.uri = strdup(uri),
-			.pending = blocking_queue_create(),
-			.current_request = NULL,
-			.mutex = mutex_create(MUTEX_TYPE_DEFAULT),
+			.inqueue = blocking_queue_create(),
+			.pending = linked_list_create(),
+			.pending_mutex = mutex_create(MUTEX_TYPE_DEFAULT),
 			.ipa = osmo_epdg_ipa_client_create(uri),
 		);
 	this->ipa->on_recv(this->ipa, IPAC_PROTO_EXT_GSUP, on_recv_pdu, this);

src/libcharon/plugins/osmo_epdg/gsup_client.h

@@ -32,9 +32,26 @@
 
 struct osmo_epdg_gsup_response_t {
 	struct osmo_gsup_message gsup;
+	/* keep pdu around because gsup takes ownership of data out of pdu */
+	struct msgb *pdu;
 };
 typedef struct osmo_epdg_gsup_response_t osmo_epdg_gsup_response_t;
 
+static inline void osmo_epdg_gsup_resp_free(osmo_epdg_gsup_response_t *resp)
+{
+	if (!resp)
+	{
+		return;
+	}
+
+	if (resp->pdu)
+	{
+		free(resp->pdu);
+	}
+
+	free(resp);
+}
+
 typedef struct osmo_epdg_gsup_client_t osmo_epdg_gsup_client_t;
 
 /**

src/libcharon/plugins/osmo_epdg/ipa_client.c

@@ -200,7 +200,7 @@ static void ipa_pdu_base_send_id_resp(private_osmo_epdg_ipa_client_t *this, stru
 		return;
 	}
 	
-	resp = chunk_to_msgb(&resp_pdu);
+	resp = epdg_chunk_to_msgb(&resp_pdu);
 
 	/* remove the ipaccess header so we can use msg_pull on the message */
 	msgb_pull(req, sizeof(struct ipaccess_head));
@@ -325,7 +325,7 @@ CALLBACK(on_stream_read, bool,
 		/* TODO: -ENOMEM; */
 		return FALSE;
 	}
-	req = chunk_to_msgb(&req_chunk);
+	req = epdg_chunk_to_msgb(&req_chunk);
 	memcpy(msgb_put(req, sizeof(head)), &head, sizeof(head));
 	/* TODO: either wait here with a timeout or don't care on this stream read */
 	if (!stream->read_all(stream, msgb_put(req, len), len))

src/libcharon/plugins/osmo_epdg/osmo_epdg_db.c

@@ -22,6 +22,8 @@
 #include <threading/rwlock.h>
 #include <unistd.h>
 
+#include <osmocom/gsm/apn.h>
+
 #include "osmo_epdg_plugin.h"
 #include "osmo_epdg_db.h"
 #include "osmo_epdg_utils.h"
@@ -53,23 +55,29 @@ METHOD(osmo_epdg_db_t, create_subscriber, osmo_epdg_ue_t *,
 {
 	osmo_epdg_ue_t *ue;
 	char imsi[16] = {0};
+	char apn[APN_MAXLEN];
 	uint32_t unique = ike_sa->get_unique_id(ike_sa);
 
-	if (get_imsi_ike(ike_sa, imsi, sizeof(imsi) - 1))
+	if (epdg_get_imsi_ike(ike_sa, imsi, sizeof(imsi) - 1))
 	{
 		return NULL;
 	}
 
-	this->lock->write_lock(this->lock);
-	ue = this->subscribers_imsi->get(this->subscribers_imsi, imsi);
-	if (ue)
+	if (epdg_get_apn(ike_sa, apn, APN_MAXLEN))
 	{
-		/* TODO: handle dups! */
-		this->lock->unlock(this->lock);
-		return ue;
+		DBG1(DBG_NET, "epdg: get_quintuplet: Can't get APN.");
+		return FALSE;
 	}
 
-	ue = osmo_epdg_ue_create(unique, imsi);
+	this->lock->write_lock(this->lock);
+	ue = this->subscribers_imsi->remove(this->subscribers_imsi, imsi);
+	if (ue)
+	{
+		/* TODO: handle dups! Will remove it for now */
+		ue->put(ue);
+	}
+
+	ue = osmo_epdg_ue_create(unique, imsi, apn);
 	if (!ue)
 	{
 		DBG1(DBG_NET, "epdg_db: failed to create UE!");
@@ -77,8 +85,8 @@ METHOD(osmo_epdg_db_t, create_subscriber, osmo_epdg_ue_t *,
 		return NULL;
 	}
 
+	/* UE comes with refcount = 1 */
 	this->subscribers_imsi->put(this->subscribers_imsi, ue->get_imsi(ue), ue);
-
 	ue->get(ue);
 	this->lock->unlock(this->lock);
 	return ue;
@@ -103,7 +111,7 @@ METHOD(osmo_epdg_db_t, get_subscriber_ike, osmo_epdg_ue_t *,
 {
 	char imsi[16] = {0};
 
-	if (get_imsi_ike(ike_sa, imsi, sizeof(imsi)))
+	if (epdg_get_imsi_ike(ike_sa, imsi, sizeof(imsi)))
 	{
 		return NULL;
 	}
@@ -111,6 +119,34 @@ METHOD(osmo_epdg_db_t, get_subscriber_ike, osmo_epdg_ue_t *,
 	return this->public.get_subscriber(&this->public, imsi);
 }
 
+METHOD(osmo_epdg_db_t, get_subscriber_id, osmo_epdg_ue_t *,
+       private_osmo_epdg_db_t *this, uint32_t unique_id)
+{
+	/* This could be optimize, but keep it is for now */
+	osmo_epdg_ue_t *ue = NULL;
+	enumerator_t *enumerator;
+
+	this->lock->read_lock(this->lock);
+	enumerator = this->subscribers_imsi->create_enumerator(this->subscribers_imsi);
+	while (enumerator->enumerate(enumerator, NULL, ue))
+	{
+		if (!ue)
+		{
+			continue;
+		}
+		if (ue->get_id(ue) == unique_id)
+		{
+			ue->get(ue);
+			goto out;
+		}
+	}
+	ue = NULL;
+out:
+	this->lock->unlock(this->lock);
+	enumerator->destroy(enumerator);
+	return ue;
+}
+
 METHOD(osmo_epdg_db_t, remove_subscriber, void,
 	private_osmo_epdg_db_t *this, const char *imsi)
 {
@@ -152,6 +188,7 @@ osmo_epdg_db_t *osmo_epdg_db_create()
 			.create_subscriber = _create_subscriber,
 			.get_subscriber = _get_subscriber,
 			.get_subscriber_ike = _get_subscriber_ike,
+			.get_subscriber_id = _get_subscriber_id,
 			.remove_subscriber = _remove_subscriber,
 			.destroy = _destroy,
 		},

src/libcharon/plugins/osmo_epdg/osmo_epdg_db.h

@@ -55,10 +55,16 @@ struct osmo_epdg_db_t {
 	 */
 	osmo_epdg_ue_t *(*get_subscriber_ike)(osmo_epdg_db_t *this, ike_sa_t *ike_sa);
 
+	/**
+	 * Get subscriber by unique id.
+	 * NULL or UE object. The UE object need to called put() when not used.
+	 */
+	osmo_epdg_ue_t *(*get_subscriber_id)(osmo_epdg_db_t *this, uint32_t unique_id);
+
 	/**
 	 * Remove a subscriber from the db.
 	 */
-	void (*remove_subscriber)(osmo_epdg_db_t *this, char *imsi);
+	void (*remove_subscriber)(osmo_epdg_db_t *this, const char *imsi);
 
 	/**
 	 * Destroy a osmo_epdg_db_t.

src/libcharon/plugins/osmo_epdg/osmo_epdg_listener.c

@@ -25,7 +25,6 @@
 
 #include <osmocom/gsm/apn.h>
 
-#include "osmo_epdg_plugin.h"
 #include "osmo_epdg_listener.h"
 #include "osmo_epdg_db.h"
 #include "osmo_epdg_utils.h"
@@ -50,13 +49,14 @@ METHOD(listener_t, eap_authorize, bool,
 {
 	char imsi[16] = {0};
 	osmo_epdg_ue_t *ue = NULL;
+	osmo_epdg_gsup_response_t *resp = NULL;
 
 	if (!id)
 	{
 		DBG1(DBG_NET, "epdg: authorize: no id given. Failing.");
 		goto err;
 	}
-	if (get_imsi(id, imsi, sizeof(imsi) - 1))
+	if (epdg_get_imsi(id, imsi, sizeof(imsi) - 1))
 	{
 		DBG1(DBG_NET, "epdg: authorize: Can't find IMSI in EAP identity.");
 		goto err;
@@ -69,7 +69,7 @@ METHOD(listener_t, eap_authorize, bool,
 		goto err;
 	}
 
-	osmo_epdg_gsup_response_t *resp = this->gsup->update_location(this->gsup, imsi, OSMO_GSUP_CN_DOMAIN_PS);
+	resp = this->gsup->update_location(this->gsup, imsi, OSMO_GSUP_CN_DOMAIN_PS);
 	if (!resp)
 	{
 		DBG1(DBG_NET, "epdg: GSUP: couldn't send Update Location.");
@@ -84,6 +84,7 @@ METHOD(listener_t, eap_authorize, bool,
 	}
 	ue->set_state(ue, UE_LOCATION_UPDATED);
 	ue->put(ue);
+	osmo_epdg_gsup_resp_free(resp);
 	return TRUE;
 
 err:
@@ -94,6 +95,7 @@ err:
 		ue->put(ue);
 	}
 
+	osmo_epdg_gsup_resp_free(resp);
 	/* keep still subscribed */
 	return TRUE;
 }
@@ -128,7 +130,7 @@ METHOD(listener_t, authorize, bool,
 		goto err;
 	}
 
-	if (get_imsi(imsi_id, imsi, sizeof(imsi) - 1))
+	if (epdg_get_imsi(imsi_id, imsi, sizeof(imsi) - 1))
 	{
 		DBG1(DBG_NET, "epdg: authorize: Can't find IMSI in EAP identity.");
 		goto err;
@@ -138,6 +140,7 @@ METHOD(listener_t, authorize, bool,
 	if (!ue)
 	{
 		DBG1(DBG_NET, "epdg: authorize: Can't find match UE for imsi %s via EAP identity.", imsi);
+		goto err;
 	}
 
 	ue->set_state(ue, UE_WAIT_TUNNEL);
@@ -189,15 +192,11 @@ METHOD(listener_t, authorize, bool,
 	ue->put(ue);
 
 	address->destroy(address);
-	free(resp);
+	osmo_epdg_gsup_resp_free(resp);
 	return TRUE;
 
 err:
-
-	if (resp)
-	{
-		free(resp);
-	}
+	osmo_epdg_gsup_resp_free(resp);
 
 	if (ue)
 	{
@@ -211,6 +210,20 @@ err:
 	return TRUE;
 }
 
+METHOD(listener_t, ike_updown, bool,
+       private_osmo_epdg_listener_t *this, ike_sa_t *ike_sa, bool up)
+{
+	char imsi[16] = {0};
+	if (epdg_get_imsi_ike(ike_sa, imsi, sizeof(imsi)))
+	{
+		DBG1(DBG_NET, "epdg_listener: updown: imsi UNKNOWN: IKE_SA went %s", up ? "up" : "down");
+		return TRUE;
+	}
+	DBG1(DBG_NET, "epdg_listener: updown: imsi %s: IKE_SA went %s", imsi, up ? "up" : "down");
+
+	return TRUE;
+}
+
 METHOD(osmo_epdg_listener_t, destroy, void,
 	private_osmo_epdg_listener_t *this)
 {
@@ -229,6 +242,7 @@ osmo_epdg_listener_t *osmo_epdg_listener_create(osmo_epdg_db_t *db, osmo_epdg_gs
 			.listener = {
 				.authorize = _authorize,
 				.eap_authorize = _eap_authorize,
+				.ike_updown = _ike_updown,
 			},
 			.destroy = _destroy,
 		},

src/libcharon/plugins/osmo_epdg/osmo_epdg_provider.c

@@ -23,6 +23,7 @@
 #include "osmo_epdg_ue.h"
 
 #include <daemon.h>
+#include <collections/linked_list.h>
 #include <credentials/keys/shared_key.h>
 #include <osmocom/gsm/apn.h>
 #include <osmocom/gsm/protocol/gsm_04_08_gprs.h>
@@ -81,7 +82,7 @@ METHOD(simaka_provider_t, get_quintuplet, bool,
 	char imsi[17] = {0};
 	ike_sa_t *ike_sa;
 
-	if (get_imsi(id, imsi, sizeof(imsi) - 1))
+	if (epdg_get_imsi(id, imsi, sizeof(imsi) - 1))
 	{
 		DBG1(DBG_NET, "epdg: get_quintuplet: Can't find IMSI in EAP identity.");
 		return FALSE;
@@ -94,7 +95,7 @@ METHOD(simaka_provider_t, get_quintuplet, bool,
 		return FALSE;
 	}
 
-	if (get_apn(ike_sa, apn, APN_MAXLEN))
+	if (epdg_get_apn(ike_sa, apn, APN_MAXLEN))
 	{
 		DBG1(DBG_NET, "epdg: get_quintuplet: Can't get APN.");
 		return FALSE;
@@ -111,14 +112,15 @@ METHOD(simaka_provider_t, get_quintuplet, bool,
 	if (resp->gsup.message_type != OSMO_GSUP_MSGT_SEND_AUTH_INFO_RESULT)
 	{
 		DBG1(DBG_NET, "epdg_provider: SendAuthInfo Error! Cause: %02x", resp->gsup.cause);
-		return FALSE;
+		goto err;
 	}
 
 	struct osmo_auth_vector *auth = &resp->gsup.auth_vectors[0];
 	if (resp->gsup.num_auth_vectors == 0)
 	{
 		/* TODO: invalid auth data received */
-		return FALSE;
+		DBG1(DBG_NET, "epdg_provider: SendAuthInfo Invalid Auth Received!");
+		goto err;
 	}
 
 	memcpy(rand, auth->rand, AKA_RAND_LEN);
@@ -128,8 +130,11 @@ METHOD(simaka_provider_t, get_quintuplet, bool,
 	memcpy(xres, auth->res, auth->res_len);
 	*xres_len = auth->res_len;
 	
-	free(resp);
+	osmo_epdg_gsup_resp_free(resp);
 	return TRUE;
+err:
+	osmo_epdg_gsup_resp_free(resp);
+	return FALSE;
 }
 
 METHOD(simaka_provider_t, resync, bool,
@@ -155,6 +160,11 @@ METHOD(attribute_provider_t, acquire_address, host_t*,
 {
 	/* yes this hurts. We can either move the attribute provider out of this class or do some pointer arithmetic to get the right this object */
 	this = container_of((void *) this, private_osmo_epdg_provider_t, public.attribute);
+	if (requested->get_family(requested) != AF_INET)
+	{
+		return NULL;
+	}
+
 	osmo_epdg_ue_t *ue = this->db->get_subscriber_ike(this->db, ike_sa);
 	host_t *address = NULL;
 	/* TODO: check if we want to limit the pool here as well to "epdg" similar what dhcp does */
@@ -164,6 +174,8 @@ METHOD(attribute_provider_t, acquire_address, host_t*,
 		DBG1(DBG_NET, "epdg_provider: acquire_address: Failed to get the UE by IKE");
 		return NULL;
 	}
+
+	/* TODO: check for IPv4/IPv6 */
 	address = ue->get_address(ue);
 	ue->put(ue);
 
@@ -192,13 +204,55 @@ METHOD(attribute_provider_t, release_address, bool,
 	return found;
 }
 
+/* see attr_provider for similar usage */
+CALLBACK(attribute_enum_filter, bool,
+        void *data, enumerator_t *orig, va_list args)
+{
+	osmo_epdg_attribute_t *entry;
+	configuration_attribute_type_t *type;
+	chunk_t *value;
+
+	VA_ARGS_VGET(args, type, value);
+	while (orig->enumerate(orig, &entry))
+	{
+		if (entry->valid)
+		{
+			*type = entry->type;
+			*value = entry->value;
+			return TRUE;
+		}
+	}
+
+	return FALSE;
+}
+
 METHOD(attribute_provider_t, create_attribute_enumerator, enumerator_t*,
 	private_osmo_epdg_provider_t *this, linked_list_t *pools, ike_sa_t *ike_sa,
 	linked_list_t *vips)
 {
-	/* don't forget fixing the this point if needed */
-	/* no additional attributes for this ike_sa */
-	return enumerator_create_empty();
+	this = container_of((void *) this, private_osmo_epdg_provider_t, public.attribute);
+	enumerator_t *enumerator = NULL;
+	linked_list_t *attributes = NULL;
+	osmo_epdg_ue_t *ue = this->db->get_subscriber_ike(this->db, ike_sa);
+
+	/* create an iterator based on the llist */
+	if (!ue)
+	{
+		return enumerator_create_empty();
+	}
+
+	/* this ref is giving into the enumerator */
+	ue->get(ue);
+	attributes = ue->get_attributes(ue);
+	enumerator = enumerator_create_cleaner(
+			enumerator_create_filter(
+				attributes->create_enumerator(attributes),
+				attribute_enum_filter, NULL, NULL),
+			(void *)ue->put, ue);
+
+	/* this ref was taken by get_subscriber */
+	ue->put(ue);
+	return enumerator;
 }
 
 METHOD(osmo_epdg_provider_t, destroy, void,

src/libcharon/plugins/osmo_epdg/osmo_epdg_ue.c

@@ -20,10 +20,12 @@
 
 #include <sa/ike_sa.h>
 #include <threading/rwlock.h>
+#include <collections/linked_list.h>
 #include <utils/utils.h>
 #include <utils/debug.h>
 
 #include "osmo_epdg_ue.h"
+#include "osmo_epdg_utils.h"
 
 typedef struct private_osmo_epdg_ue_t private_osmo_epdg_ue_t;
 
@@ -48,11 +50,23 @@ struct private_osmo_epdg_ue_t {
 	 */
 	char *imsi;
 
+	/**
+	 * APN encoded as character (foo.example)
+	 */
+	char *apn;
+
 	/**
 	 * IP address of the client. Might become a llist_t in the future
 	 */
 	host_t *address;
 
+	/**
+	 * The requested attributes/PCO options on GTP
+	 * e.g. P-CSCF requests, DNS, ..
+	 * holds attribute_entry_t
+	 */
+	linked_list_t *attributes;
+
 	/**
 	 * Refcount to track this object.
 	 * It will call destroy() when refcount reaches 0
@@ -73,6 +87,24 @@ METHOD(osmo_epdg_ue_t, get_imsi, const char *,
 	return this->imsi;
 }
 
+METHOD(osmo_epdg_ue_t, get_apn, const char *,
+       private_osmo_epdg_ue_t *this)
+{
+	return this->apn;
+}
+
+METHOD(osmo_epdg_ue_t, get_id, uint32_t,
+       private_osmo_epdg_ue_t *this)
+{
+	return this->id;
+}
+
+METHOD(osmo_epdg_ue_t, set_id, void,
+       private_osmo_epdg_ue_t *this, uint32_t unique_id)
+{
+	this->id = unique_id;
+}
+
 METHOD(osmo_epdg_ue_t, set_address, void,
        private_osmo_epdg_ue_t *this, host_t *address)
 {
@@ -121,6 +153,13 @@ METHOD(osmo_epdg_ue_t, get_state, enum osmo_epdg_ue_state,
 	return state;
 }
 
+METHOD(osmo_epdg_ue_t, get_attributes, linked_list_t *,
+       private_osmo_epdg_ue_t *this)
+{
+	/* TODO: check if we need to also take locking .. also refcounting would be great here */
+	return this->attributes;
+}
+
 METHOD(osmo_epdg_ue_t, get, void,
        private_osmo_epdg_ue_t *this)
 {
@@ -136,34 +175,79 @@ METHOD(osmo_epdg_ue_t, put, void,
 	}
 }
 
+
+CALLBACK(destroy_attribute, void,
+	osmo_epdg_attribute_t *attr)
+{
+	if (attr->valid)
+	{
+		chunk_free(&attr->value);
+	}
+	free(attr);
+}
+
 METHOD(osmo_epdg_ue_t, destroy, void,
        private_osmo_epdg_ue_t *this)
 {
 	this->lock->destroy(this->lock);
+	this->attributes->destroy_function(this->attributes, destroy_attribute);
+
+	free(this->apn);
+	free(this->imsi);
 	free(this);
 }
 
-osmo_epdg_ue_t *osmo_epdg_ue_create(uint32_t id, char *imsi)
+osmo_epdg_ue_t *osmo_epdg_ue_create(uint32_t id, const char *imsi, const char *apn)
 {
 	private_osmo_epdg_ue_t *this;
 
+	if (epdg_validate_apn(apn) ||
+	    epdg_validate_imsi(imsi))
+	{
+		return NULL;
+	}
+
 	INIT(this,
 	     .public = {
 		 .get = _get,
 		 .put = _put,
+		 .get_apn = _get_apn,
 		 .get_imsi = _get_imsi,
+		 .get_id = _get_id,
+		 .set_id = _set_id,
 		 .get_address = _get_address,
 		 .set_address = _set_address,
 		 .get_state = _get_state,
 		 .set_state = _set_state,
+		 .get_attributes = _get_attributes,
 		 .destroy = _destroy,
 	     },
+	     .apn = strdup(apn),
 	     .imsi = strdup(imsi),
 	     .id = id,
 	     .lock = rwlock_create(RWLOCK_TYPE_DEFAULT),
 	     .state = UE_WAIT_LOCATION_UPDATE,
+	     .attributes = linked_list_create(),
+	     .refcount = 1,
 	     );
-	ref_get(&this->refcount);
+
+	/* hardcode P-CSCF and DNS entry */
+	osmo_epdg_attribute_t *entry;
+	host_t *host = host_create_from_string_and_family("10.74.0.31", AF_INET, 0);
+	INIT(entry,
+		.type = INTERNAL_IP4_DNS,
+		.value = chunk_clone(host->get_address(host)),
+		.valid = TRUE,
+	);
+	this->attributes->insert_last(this->attributes, entry);
+
+	INIT(entry,
+		.type = P_CSCF_IP4_ADDRESS,
+		.value = chunk_clone(host->get_address(host)),
+		.valid = TRUE,
+	);
+	this->attributes->insert_last(this->attributes, entry);
+	host->destroy(host);
 
 	return &this->public;
 }

src/libcharon/plugins/osmo_epdg/osmo_epdg_ue.h

@@ -55,11 +55,35 @@ enum osmo_epdg_ue_state {
  * UE object
  */
 struct osmo_epdg_ue_t {
+	/**
+	 * Get APN
+	 * Should not change.
+	 */
+	const char *(*get_apn)(osmo_epdg_ue_t *this);
+
 	/**
 	 * Get IMSI
+	 * Should not change.
 	 */
 	const char *(*get_imsi)(osmo_epdg_ue_t *this);
 
+	/**
+	 * Get unique ID
+	 * The unique ID may change either by reconnect or rekey
+	 */
+	uint32_t (*get_id)(osmo_epdg_ue_t *this);
+
+	/**
+	 * Get unique ID
+	 * The unique ID may change either by reconnect or rekey
+	 */
+	void (*set_id)(osmo_epdg_ue_t *this, uint32_t unique_id);
+
+	/**
+	 * Get Linked list of osmo_epdg_attribute_t
+	 */
+	linked_list_t *(*get_attributes)(osmo_epdg_ue_t *this);
+
 	/**
 	 * Get address. Returns NULL or a cloned' host_t object
 	 */
@@ -80,7 +104,6 @@ struct osmo_epdg_ue_t {
 	 */
 	void (*set_state)(osmo_epdg_ue_t *this, enum osmo_epdg_ue_state state);
 
-
 	/**
 	 * Increase the internal refcount. Use put() when done with the object
 	 */
@@ -99,10 +122,17 @@ struct osmo_epdg_ue_t {
 	void (*destroy)(osmo_epdg_ue_t *this);
 };
 
+struct osmo_epdg_attribute_t {
+	configuration_attribute_type_t type;
+	chunk_t value;
+	bool valid;
+};
+typedef struct osmo_epdg_attribute_t osmo_epdg_attribute_t;
+
 /**
  * Create a osmo_epdg_ue instance.
  * A newly created object will come with refcount = 1. Use put() to destroy it.
  */
-osmo_epdg_ue_t *osmo_epdg_ue_create(uint32_t id, char *imsi);
+osmo_epdg_ue_t *osmo_epdg_ue_create(uint32_t id, const char *imsi, const char *apn);
 
 #endif /* OSMO_EPDG_UE_H_ */

src/libcharon/plugins/osmo_epdg/osmo_epdg_utils.c

@@ -17,6 +17,7 @@
  */
 
 #include <errno.h>
+#include <ctype.h>
 
 #include <osmocom/core/msgb.h>
 #include <sa/ike_sa.h>
@@ -25,7 +26,7 @@
 
 #include "osmo_epdg_utils.h"
 
-struct msgb *chunk_to_msgb(chunk_t *chunk)
+struct msgb *epdg_chunk_to_msgb(chunk_t *chunk)
 {
 	struct msgb *msg;
 	if (chunk->len < sizeof(*msg))
@@ -43,7 +44,7 @@ struct msgb *chunk_to_msgb(chunk_t *chunk)
 	return msg;
 }
 
-int get_imsi_ike(ike_sa_t *ike_sa, char *imsi, size_t imsi_len)
+int epdg_get_imsi_ike(ike_sa_t *ike_sa, char *imsi, size_t imsi_len)
 {
 	identification_t *imsi_id = ike_sa->get_other_id(ike_sa);
 	if (!imsi_id)
@@ -51,10 +52,10 @@ int get_imsi_ike(ike_sa_t *ike_sa, char *imsi, size_t imsi_len)
 		return -1;
 	}
 
-	return get_imsi(imsi_id, imsi, imsi_len);
+	return epdg_get_imsi(imsi_id, imsi, imsi_len);
 }
 
-int get_imsi(identification_t *id, char *imsi, size_t imsi_len)
+int epdg_get_imsi(identification_t *id, char *imsi, size_t imsi_len)
 {
 	chunk_t nai = id->get_encoding(id);
 	/* TODO: maybe use regex? */
@@ -76,7 +77,38 @@ int get_imsi(identification_t *id, char *imsi, size_t imsi_len)
 	return 0;
 }
 
-int get_apn(ike_sa_t *sa, char *apn, size_t apn_len)
+int epdg_validate_imsi(const char *imsi)
+{
+	if (!imsi)
+		return 1;
+
+	if (strlen(imsi) != 15)
+		return 1;
+
+	for (int i=0; i<strlen(imsi); i++)
+	{
+		if (!isdigit(imsi[i]))
+		{
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+int epdg_validate_apn(const char *apn)
+{
+	/* don't support empty apn */
+	if (!apn)
+		return 1;
+
+	if (!strlen(apn))
+		return 1;
+
+	return 0;
+}
+
+int epdg_get_apn(ike_sa_t *sa, char *apn, size_t apn_len)
 {
 	identification_t* apn_id;
 	chunk_t apn_chunk;

src/libcharon/plugins/osmo_epdg/osmo_epdg_utils.h

@@ -26,9 +26,12 @@
 
 #define IPA_ALLOC_SIZE 1200
 
-struct msgb *chunk_to_msgb(chunk_t *chunk);
-int get_imsi(identification_t *id, char *imsi, size_t imsi_len);
-int get_imsi_ike(ike_sa_t *ike_sa, char *imsi, size_t imsi_len);
-int get_apn(ike_sa_t *sa, char *apn, size_t apn_len);
+struct msgb *epdg_chunk_to_msgb(chunk_t *chunk);
+int epdg_get_imsi(identification_t *id, char *imsi, size_t imsi_len);
+int epdg_get_imsi_ike(ike_sa_t *ike_sa, char *imsi, size_t imsi_len);
+int epdg_validate_imsi(const char *imsi);
+
+int epdg_get_apn(ike_sa_t *sa, char *apn, size_t apn_len);
+int epdg_validate_apn(const char *apn);
 
 #endif /* OSMO_EPDG_UTILS_H_ */