Change in ...osmo-ggsn[master]: gtp: Manage queue timers internally

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Wed Aug 28 18:09:41 UTC 2019


pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/15309


Change subject: gtp: Manage queue timers internally
......................................................................

gtp: Manage queue timers internally

Currently each user (application) of libgtp needs to manage its own
timers in order to call gtp_retrans_timeout() and gtp_retrans() and
maintain retransmit and duplicate queues working correctly. This adds
unnecesary complexity to applications since nowadays, as a libosmocore
user, libgtp can handle this internally in an easy way.
Furthermore, keeping the timers internal to the library allows for
easier extension of features as well as re-implementation of related
code in the future.
Last but not least, it was detected that existing known applications
(osmo-sgsn, osmo-ggsn, sgsnemu) are not using correctly the API, since
they should be updating their timers through gtp_retrans_timeout()
everytime a message is enqueued/transmitted, otherwise they may fire
gtp_retrans() for retransmition too late in some cases.

Related: OS#4178
Change-Id: Ife7cfd66d6356f413263fe5bda9e43091f5c9e98
---
M gtp/gtp.c
M gtp/gtp.h
2 files changed, 128 insertions(+), 72 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ggsn refs/changes/09/15309/1

diff --git a/gtp/gtp.c b/gtp/gtp.c
index 2ea949d..66aedb8 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -386,6 +386,109 @@
 	}
 }
 
+static int queue_timer_retrans(struct gsn_t *gsn)
+{
+	/* Retransmit any outstanding packets */
+	/* Remove from queue if maxretrans exceeded */
+	time_t now;
+	struct qmsg_t *qmsg;
+	now = time(NULL);
+	/*printf("Retrans: New beginning %d\n", (int) now); */
+
+	/* get first element in queue, as long as the timeout of that
+	 * element has expired */
+	while ((!queue_getfirst(gsn->queue_req, &qmsg)) &&
+	       (qmsg->timeout <= now)) {
+		/*printf("Retrans timeout found: %d\n", (int) time(NULL)); */
+		if (qmsg->retrans > N3_REQUESTS) {	/* To many retrans */
+			LOGP(DLGTP, LOGL_NOTICE, "Timeout of seq %" PRIu16 "\n",
+			     qmsg->seq);
+			if (gsn->cb_conf)
+				gsn->cb_conf(qmsg->type, EOF, NULL, qmsg->cbp);
+			queue_freemsg(gsn->queue_req, qmsg);
+		} else {
+			LOGP(DLGTP, LOGL_INFO, "Retransmit (%d) of seq %" PRIu16 "\n",
+			     qmsg->retrans, qmsg->seq);
+			if (sendto(qmsg->fd, &qmsg->p, qmsg->l, 0,
+				   (struct sockaddr *)&qmsg->peer,
+				   sizeof(struct sockaddr_in)) < 0) {
+				gsn->err_sendto++;
+				LOGP(DLGTP, LOGL_ERROR,
+					"Sendto(fd0=%d, msg=%lx, len=%d) failed: Error = %s\n",
+					gsn->fd0, (unsigned long)&qmsg->p,
+					qmsg->l, strerror(errno));
+			}
+			queue_back(gsn->queue_req, qmsg);
+			qmsg->timeout = now + T3_REQUEST;
+			qmsg->retrans++;
+		}
+	}
+
+	/* Also clean up reply timeouts */
+	while ((!queue_getfirst(gsn->queue_resp, &qmsg)) &&
+	       (qmsg->timeout < now)) {
+		/*printf("Retrans (reply) timeout found: %d\n", (int) time(NULL)); */
+		queue_freemsg(gsn->queue_resp, qmsg);
+	}
+
+	return 0;
+}
+
+static int queue_timer_retranstimeout(struct gsn_t *gsn, struct timeval *timeout)
+{
+	time_t now, later, diff;
+	struct qmsg_t *qmsg;
+	timeout->tv_usec = 0;
+
+	if (queue_getfirst(gsn->queue_req, &qmsg)) {
+		timeout->tv_sec = 10;
+	} else {
+		now = time(NULL);
+		later = qmsg->timeout;
+		timeout->tv_sec = later - now;
+		if (timeout->tv_sec < 0)
+			timeout->tv_sec = 0;	/* No negative allowed */
+		if (timeout->tv_sec > 10)
+			timeout->tv_sec = 10;	/* Max sleep for 10 sec */
+	}
+
+	if (queue_getfirst(gsn->queue_resp, &qmsg)) {
+		/* already set by queue_req, do nothing */
+	} else { /* trigger faster if earlier timeout exists in queue_resp */
+		now = time(NULL);
+		later = qmsg->timeout;
+		diff = later - now;
+		if (diff < 0)
+			diff = 0;
+		if (diff < timeout->tv_sec)
+			timeout->tv_sec = diff;
+	}
+
+	return 0;
+}
+
+static void queue_timer_start(struct gsn_t *gsn)
+{
+	struct timeval next;
+
+	/* Retrieve next retransmission as timeval */
+	queue_timer_retranstimeout(gsn, &next);
+
+	/* re-schedule the timer */
+	osmo_timer_schedule(&gsn->queue_timer, next.tv_sec, next.tv_usec/1000);
+}
+
+/* timer callback for libgtp retransmission and ping */
+static void queue_timer_cb(void *data)
+{
+	struct gsn_t *gsn = data;
+
+	/* do all the retransmissions as needed */
+	queue_timer_retrans(gsn);
+
+	queue_timer_start(gsn);
+}
+
 /* ***********************************************************
  * Reliable delivery of signalling messages
  *
@@ -532,6 +635,10 @@
 		qmsg->fd = fd;
 		if (pdp) /* echo requests are not pdp-bound */
 			llist_add(&qmsg->entry, &pdp->qmsg_list_req);
+
+		/* Rearm timer: Retrans time for qmsg just queued may be required
+		   before an existing one (for instance a gtp echo req) */
+		queue_timer_start(gsn);
 	}
 	gsn->seq_next++;	/* Count up this time */
 	return 0;
@@ -587,82 +694,15 @@
 
 int gtp_retrans(struct gsn_t *gsn)
 {
-	/* Retransmit any outstanding packets */
-	/* Remove from queue if maxretrans exceeded */
-	time_t now;
-	struct qmsg_t *qmsg;
-	now = time(NULL);
-	/*printf("Retrans: New beginning %d\n", (int) now); */
-
-	/* get first element in queue, as long as the timeout of that
-	 * element has expired */
-	while ((!queue_getfirst(gsn->queue_req, &qmsg)) &&
-	       (qmsg->timeout <= now)) {
-		/*printf("Retrans timeout found: %d\n", (int) time(NULL)); */
-		if (qmsg->retrans > N3_REQUESTS) {	/* To many retrans */
-			LOGP(DLGTP, LOGL_NOTICE, "Timeout of seq %" PRIu16 "\n",
-			     qmsg->seq);
-			if (gsn->cb_conf)
-				gsn->cb_conf(qmsg->type, EOF, NULL, qmsg->cbp);
-			queue_freemsg(gsn->queue_req, qmsg);
-		} else {
-			LOGP(DLGTP, LOGL_INFO, "Retransmit (%d) of seq %" PRIu16 "\n",
-			     qmsg->retrans, qmsg->seq);
-			if (sendto(qmsg->fd, &qmsg->p, qmsg->l, 0,
-				   (struct sockaddr *)&qmsg->peer,
-				   sizeof(struct sockaddr_in)) < 0) {
-				gsn->err_sendto++;
-				LOGP(DLGTP, LOGL_ERROR,
-					"Sendto(fd0=%d, msg=%lx, len=%d) failed: Error = %s\n",
-					gsn->fd0, (unsigned long)&qmsg->p,
-					qmsg->l, strerror(errno));
-			}
-			queue_back(gsn->queue_req, qmsg);
-			qmsg->timeout = now + T3_REQUEST;
-			qmsg->retrans++;
-		}
-	}
-
-	/* Also clean up reply timeouts */
-	while ((!queue_getfirst(gsn->queue_resp, &qmsg)) &&
-	       (qmsg->timeout < now)) {
-		/*printf("Retrans (reply) timeout found: %d\n", (int) time(NULL)); */
-		queue_freemsg(gsn->queue_resp, qmsg);
-	}
-
+	/* dummy API, deprecated. */
 	return 0;
 }
 
 int gtp_retranstimeout(struct gsn_t *gsn, struct timeval *timeout)
 {
-	time_t now, later, diff;
-	struct qmsg_t *qmsg;
+	timeout->tv_sec = 24*60*60;
 	timeout->tv_usec = 0;
-
-	if (queue_getfirst(gsn->queue_req, &qmsg)) {
-		timeout->tv_sec = 10;
-	} else {
-		now = time(NULL);
-		later = qmsg->timeout;
-		timeout->tv_sec = later - now;
-		if (timeout->tv_sec < 0)
-			timeout->tv_sec = 0;	/* No negative allowed */
-		if (timeout->tv_sec > 10)
-			timeout->tv_sec = 10;	/* Max sleep for 10 sec */
-	}
-
-	if (queue_getfirst(gsn->queue_resp, &qmsg)) {
-		/* already set by queue_req, do nothing */
-	} else { /* trigger faster if earlier timeout exists in queue_resp */
-		now = time(NULL);
-		later = qmsg->timeout;
-		diff = later - now;
-		if (diff < 0)
-			diff = 0;
-		if (diff < timeout->tv_sec)
-			timeout->tv_sec = diff;
-	}
-
+	/* dummy API, deprecated. Return a huge timer to do nothing */
 	return 0;
 }
 
@@ -723,6 +763,10 @@
 		/* No need to add to pdp list here, because even on pdp ctx free
 		   we want to leave messages in queue_resp until timeout to
 		   detect duplicates */
+
+		/* Rearm timer: Retrans time for qmsg just queued may be required
+		   before an existing one (for instance a gtp echo req) */
+		queue_timer_start(gsn);
 	}
 	return 0;
 }
@@ -872,6 +916,9 @@
 	/* Initialise pdp table */
 	pdp_init(*gsn);
 
+	/* Initialize internal queue timer */
+	osmo_timer_setup(&(*gsn)->queue_timer, queue_timer_cb, *gsn);
+
 	/* Initialise call back functions */
 	(*gsn)->cb_create_context_ind = 0;
 	(*gsn)->cb_delete_context = 0;
@@ -959,12 +1006,18 @@
 		return -errno;
 	}
 
+	/* Start internal queue timer */
+	queue_timer_start(*gsn);
+
 	return 0;
 }
 
 int gtp_free(struct gsn_t *gsn)
 {
 
+	/* Cleanup internal queue timer */
+	osmo_timer_del(&gsn->queue_timer);
+
 	/* Clean up retransmit queues */
 	queue_free(gsn->queue_req);
 	queue_free(gsn->queue_resp);
diff --git a/gtp/gtp.h b/gtp/gtp.h
index f2a4e1d..e03d77d 100644
--- a/gtp/gtp.h
+++ b/gtp/gtp.h
@@ -14,6 +14,7 @@
 
 #include <osmocom/core/utils.h>
 #include <osmocom/core/defs.h>
+#include <osmocom/core/timer.h>
 
 #include "pdp.h"
 
@@ -268,6 +269,8 @@
 	struct pdp_t pdpa[PDP_MAX];	/* PDP storage */
 	struct pdp_t *hashtid[PDP_MAX];	/* Hash table for IMSI + NSAPI */
 
+	struct osmo_timer_list queue_timer; /* internal queue_{req,resp} timer */
+
 	/* Call back functions */
 	int (*cb_delete_context) (struct pdp_t *);
 	int (*cb_create_context_ind) (struct pdp_t *);
@@ -348,8 +351,8 @@
 extern int gtp_decaps0(struct gsn_t *gsn);
 extern int gtp_decaps1c(struct gsn_t *gsn);
 extern int gtp_decaps1u(struct gsn_t *gsn);
-extern int gtp_retrans(struct gsn_t *gsn);
-extern int gtp_retranstimeout(struct gsn_t *gsn, struct timeval *timeout);
+extern int gtp_retrans(struct gsn_t *gsn) OSMO_DEPRECATED("This API is a no-op, libgtp already does the job internally");
+extern int gtp_retranstimeout(struct gsn_t *gsn, struct timeval *timeout) OSMO_DEPRECATED("This API is a no-op and will return a 1 day timeout");
 
 extern int gtp_set_cb_delete_context(struct gsn_t *gsn,
 				     int (*cb_delete_context) (struct pdp_t *

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/15309
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Change-Id: Ife7cfd66d6356f413263fe5bda9e43091f5c9e98
Gerrit-Change-Number: 15309
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190828/ae2f4b33/attachment.htm>


More information about the gerrit-log mailing list