gtphub: nr_map: add min,max and wrap.

Implement min/max bounds for nr_pool, adjust nr_pool_init() and current tests,
and create unit tests for nr_map wrapping.

Sequence numbers range from 0 to 65535, while TEIs range from 1 to 0xffffffff.
Both cause problems when the nr_pool surpasses the range: seq exit their valid
range, causing unmappings to fail, and a TEI would be mapped as zero (invalid).

Add a comment about TEI wrapping, and lose the comment about random TEIs (not
really important).

Sponsored-by: On-Waves ehi

3 files changed, 68 insertions, 13 deletions

diff --git a/openbsc/include/openbsc/gtphub.h b/openbsc/include/openbsc/gtphub.h
index c43a32860..77e43e46a 100644
--- a/openbsc/include/openbsc/gtphub.h
+++ b/openbsc/include/openbsc/gtphub.h
@@ -256,7 +256,8 @@ typedef unsigned int nr_t;
  * If this becomes random, the tests need to be fixed. */
 struct nr_pool {
 	nr_t last_nr;
-	/* TODO add min, max, for safe wrapping */
+	nr_t nr_min;
+	nr_t nr_max;
 };
 
 struct nr_mapping {
@@ -275,7 +276,7 @@ struct nr_map {
 };
 
 
-void nr_pool_init(struct nr_pool *pool);
+void nr_pool_init(struct nr_pool *pool, nr_t nr_min, nr_t nr_max);
 
 /* Return the next unused number from the nr_pool. */
 nr_t nr_pool_next(struct nr_pool *pool);
@@ -398,6 +399,12 @@ struct gtphub {
 	/* pointers to an entry of to_ggsns[x].peers */
 	struct gtphub_peer_port *ggsn_proxy[GTPH_PLANE_N];
 
+	/* The TEI numbers will simply wrap and be reused, which will work out
+	 * in practice. Problems would arise if one given peer maintained the
+	 * same TEI for a time long enough for the TEI nr map to wrap an entire
+	 * uint32_t; if a new TEI were mapped every second, this would take
+	 * more than 100 years (in which a single given TEI must not time out)
+	 * to cause a problem. */
 	struct nr_map tei_map[GTPH_PLANE_N];
 	struct nr_pool tei_pool[GTPH_PLANE_N];
 
diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c
index ef622a68f..8af3ff7ec 100644
--- a/openbsc/src/gprs/gtphub.c
+++ b/openbsc/src/gprs/gtphub.c
@@ -577,18 +577,21 @@ void expiring_item_del(struct expiring_item *item)
 
 /* nr_map, nr_pool */
 
-void nr_pool_init(struct nr_pool *pool)
+void nr_pool_init(struct nr_pool *pool, nr_t nr_min, nr_t nr_max)
 {
-	*pool = (struct nr_pool){};
+	*pool = (struct nr_pool){
+		.nr_min = nr_min,
+		.nr_max = nr_max,
+		.last_nr = nr_max
+	};
 }
 
 nr_t nr_pool_next(struct nr_pool *pool)
 {
-	pool->last_nr ++;
-
-	OSMO_ASSERT(pool->last_nr > 0);
-	/* TODO: gracefully handle running out of TEIs. */
-	/* TODO: random TEIs. */
+	if (pool->last_nr >= pool->nr_max)
+		pool->last_nr = pool->nr_min;
+	else
+		pool->last_nr ++;
 
 	return pool->last_nr;
 }
@@ -1739,7 +1742,7 @@ void gtphub_init(struct gtphub *hub)
 
 	int plane_idx;
 	for (plane_idx = 0; plane_idx < GTPH_PLANE_N; plane_idx++) {
-		nr_pool_init(&hub->tei_pool[plane_idx]);
+		nr_pool_init(&hub->tei_pool[plane_idx], 1, 0xffffffff);
 		nr_map_init(&hub->tei_map[plane_idx],
 			    &hub->tei_pool[plane_idx],
 			    &hub->expire_tei_maps);
@@ -1907,7 +1910,7 @@ static struct gtphub_peer *gtphub_peer_new(struct gtphub *hub,
 
 	INIT_LLIST_HEAD(&peer->addresses);
 
-	nr_pool_init(&peer->seq_pool);
+	nr_pool_init(&peer->seq_pool, 0, 0xffff);
 	nr_map_init(&peer->seq_map, &peer->seq_pool, &hub->expire_seq_maps);
 
 	/* TODO use something random to pick the initial sequence nr.
diff --git a/openbsc/tests/gtphub/gtphub_test.c b/openbsc/tests/gtphub/gtphub_test.c
index 2fa28c2ff..f983f0615 100644
--- a/openbsc/tests/gtphub/gtphub_test.c
+++ b/openbsc/tests/gtphub/gtphub_test.c
@@ -157,7 +157,7 @@ static void test_nr_map_basic(void)
 	struct nr_map _map;
 	struct nr_map *map = &_map;
 
-	nr_pool_init(pool);
+	nr_pool_init(pool, 1, 1000);
 	nr_map_init(map, pool, NULL);
 
 	OSMO_ASSERT(llist_empty(&map->mappings));
@@ -253,6 +253,50 @@ static int nr_map_is(struct nr_map *map, const char *str)
 	return 1;
 }
 
+static int test_nr_map_wrap_with(nr_t nr_min, nr_t nr_max, nr_t repl_last,
+				 nr_t orig_start, int orig_n,
+				 const char *expect)
+{
+	struct nr_pool _pool;
+	struct nr_pool *pool = &_pool;
+	struct nr_map _map;
+	struct nr_map *map = &_map;
+
+	nr_pool_init(pool, nr_min, nr_max);
+	nr_map_init(map, pool, NULL);
+
+	pool->last_nr = repl_last;
+
+	void *origin = (void*)0x1234;
+
+	int i;
+	for (i = 0; i < orig_n; i++)
+		LVL2_ASSERT(nr_map_have(map, origin, orig_start + i, 0));
+
+	LVL2_ASSERT(nr_map_is(map, expect));
+
+	nr_map_clear(map);
+	return 1;
+}
+
+static void test_nr_map_wrap(void)
+{
+	OSMO_ASSERT(test_nr_map_wrap_with(
+		0, UINT_MAX, UINT_MAX - 2,
+		1, 5,
+		"(1->4294967294@0), "
+		"(2->4294967295@0), "
+		"(3->0@0), "
+		"(4->1@0), "
+		"(5->2@0), "
+		));
+	OSMO_ASSERT(test_nr_map_wrap_with(
+		5, 10, 8,
+		1, 5,
+		"(1->9@0), (2->10@0), (3->5@0), (4->6@0), (5->7@0), "
+		));
+}
+
 static void test_expiry(void)
 {
 	struct expiry expiry;
@@ -261,7 +305,7 @@ static void test_expiry(void)
 	int i;
 
 	expiry_init(&expiry, 30);
-	nr_pool_init(&pool);
+	nr_pool_init(&pool, 1, 1000);
 	nr_map_init(&map, &pool, &expiry);
 	OSMO_ASSERT(nr_map_is(&map, ""));
 
@@ -993,6 +1037,7 @@ int main(int argc, char **argv)
 	osmo_gtphub_ctx = talloc_named_const(NULL, 0, "osmo_gtphub");
 
 	test_nr_map_basic();
+	test_nr_map_wrap();
 	test_expiry();
 	test_echo();
 	test_create_pdp_ctx();