[PATCH 3/3] 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 --- 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