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
---
 openbsc/include/openbsc/gtphub.h   | 11 +++++++--
 openbsc/src/gprs/gtphub.c          | 21 +++++++++-------
 openbsc/tests/gtphub/gtphub_test.c | 49 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/openbsc/include/openbsc/gtphub.h b/openbsc/include/openbsc/gtphub.h
index c43a328..77e43e4 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 7c7f693..155024b 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;
 }
@@ -1806,7 +1809,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);
@@ -1974,7 +1977,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 4e22ac7..b1ebb40 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, ""));

@@ -947,6 +991,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();
-- 
2.1.4

Reply via email to