gtphub: implement restart counter properly.

Force passing a restart counter, by adding such arg to gtphub_start() (test
suite is not affected by this).

In gtphub_main.c, add -r,--restart-file <path> and next_restart_count() to
maintain the counter file. While at it, tweak the cmdline help to unify the
formatting (mostly commas and a missing line break).

Send gtphub's own restart counter. So far, the sender's restart counter was
copied through, which would break as soon as more than one GSN would talk to
the same peer with differing restart counters.

Also fix the in-mem restart counter data type (one octet, not two).

Sponsored-by: On-Waves ehi

4 files changed, 100 insertions, 15 deletions

diff --git a/openbsc/include/openbsc/gtphub.h b/openbsc/include/openbsc/gtphub.h
index 5db6f6430..cc204ef7e 100644
--- a/openbsc/include/openbsc/gtphub.h
+++ b/openbsc/include/openbsc/gtphub.h
@@ -451,7 +451,7 @@ struct gtphub {
 	struct expiry expire_quickly;
 	struct expiry expire_slowly;
 
-	uint16_t restart_counter;
+	uint8_t restart_counter;
 };
 
 struct gtp_packet_desc;
@@ -463,7 +463,8 @@ int gtphub_vty_init(struct gtphub *global_hub, struct gtphub_cfg *global_cfg);
 int gtphub_cfg_read(struct gtphub_cfg *cfg, const char *config_file);
 
 /* Initialize and start gtphub: bind to ports, run expiry timers. */
-int gtphub_start(struct gtphub *hub, struct gtphub_cfg *cfg);
+int gtphub_start(struct gtphub *hub, struct gtphub_cfg *cfg,
+		 uint8_t restart_counter);
 
 /* Close all sockets, expire all maps and peers and free all allocations. The
  * struct is then unusable, unless gtphub_start() is run on it again. */
diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c
index ff1105b8e..db84a00c3 100644
--- a/openbsc/src/gprs/gtphub.c
+++ b/openbsc/src/gprs/gtphub.c
@@ -1302,7 +1302,16 @@ static void gtphub_map_restart_counter(struct gtphub *hub,
 				       struct gtphub_peer_port *from,
 				       struct gtphub_peer_port *to)
 {
-	/* TODO */
+	/* Always send gtphub's own restart counter */
+	if (p->rc != GTP_RC_PDU_C)
+		return;
+
+	int ie_idx;
+	ie_idx = gtpie_getie(p->ie, GTPIE_RECOVERY, 0);
+	if (ie_idx < 0)
+		return;
+
+	p->ie[ie_idx]->tv1.v = hton8(hub->restart_counter);
 }
 
 static int gtphub_unmap_header_tei(struct gtphub_peer_port **to_port_p,
@@ -2164,12 +2173,15 @@ static int gtphub_make_proxy(struct gtphub *hub,
 	return 0;
 }
 
-int gtphub_start(struct gtphub *hub, struct gtphub_cfg *cfg)
+int gtphub_start(struct gtphub *hub, struct gtphub_cfg *cfg,
+		 uint8_t restart_counter)
 {
 	int rc;
 
 	gtphub_init(hub);
 
+	hub->restart_counter = restart_counter;
+
 	/* If a Ctrl plane proxy is configured, ares will never be used. */
 	if (!cfg->ggsn_proxy[GTPH_PLANE_CTRL].addr_str) {
 		if (gtphub_ares_init(hub) != 0) {
diff --git a/openbsc/src/gprs/gtphub_main.c b/openbsc/src/gprs/gtphub_main.c
index 025a2d1a2..6dd316ace 100644
--- a/openbsc/src/gprs/gtphub_main.c
+++ b/openbsc/src/gprs/gtphub_main.c
@@ -23,6 +23,8 @@
 #include <signal.h>
 #include <string.h>
 #include <errno.h>
+#include <inttypes.h>
+#include <sys/stat.h>
 
 #define _GNU_SOURCE
 #include <getopt.h>
@@ -132,19 +134,82 @@ static struct vty_app_info vty_info = {
 
 struct cmdline_cfg {
 	const char *config_file;
+	const char *restart_counter_file;
 	int daemonize;
 };
 
+static uint8_t next_restart_count(const char *path)
+{
+	int umask_was = umask(022);
+
+	uint8_t counter = 0;
+
+	FILE *f = fopen(path, "r");
+	if (f) {
+		int rc = fscanf(f, "%hhu", &counter);
+
+		if (rc != 1)
+			goto failed_to_read;
+
+		char c;
+		while (fread(&c, 1, 1, f) > 0) {
+			switch (c) {
+			case ' ':
+			case '\t':
+			case '\n':
+			case '\r':
+				break;
+			default:
+				goto failed_to_read;
+			}
+		}
+		fclose(f);
+	}
+
+	counter ++;
+
+	f = fopen(path, "w");
+	if (!f)
+		goto failed_to_write;
+	if (fprintf(f, "%" PRIu8 "\n", counter) < 2)
+		goto failed_to_write;
+	if (fclose(f))
+		goto failed_to_write;
+
+	umask(umask_was);
+
+	LOGP(DGTPHUB, LOGL_NOTICE, "Restarted with counter %hhu\n", counter);
+	return counter;
+
+failed_to_read:
+	fclose(f);
+	umask(umask_was);
+	LOGP(DGTPHUB, LOGL_FATAL, "Restart counter file cannot be parsed:"
+	     " %s\n", path);
+	exit(1);
+
+failed_to_write:
+	if (f)
+		fclose(f);
+	umask(umask_was);
+	LOGP(DGTPHUB, LOGL_FATAL, "Restart counter file cannot be written:"
+	     " %s\n", path);
+	exit(1);
+}
+
 static void print_help(struct cmdline_cfg *ccfg)
 {
 	printf("gtphub commandline options\n");
-	printf("  -h --help            This text.\n");
-	printf("  -D --daemonize       Fork the process into a background daemon.\n");
+	printf("  -h,--help            This text.\n");
+	printf("  -D,--daemonize       Fork the process into a background daemon.\n");
 	printf("  -d,--debug <cat>     Enable Debugging for this category.\n");
 	printf("                       Pass '-d list' to get a category listing.\n");
-	printf("  -s --disable-color");
-	printf("  -c --config-file     The config file to use [%s].\n", ccfg->config_file);
-	printf("  -e,--log-level <nr>  Set a global log level.\n");
+	printf("  -s,--disable-color\n");
+	printf("  -c,--config-file <path>  The config file to use [%s].\n",
+	       ccfg->config_file);
+	printf("  -e,--log-level <nr>      Set a global log level.\n");
+	printf("  -r,--restart-file <path> File for counting restarts [%s].\n",
+	       ccfg->restart_counter_file);
 }
 
 static void list_categories(void)
@@ -171,10 +236,11 @@ static void handle_options(struct cmdline_cfg *ccfg, int argc, char **argv)
 			{"disable-color", 0, 0, 's'},
 			{"timestamp", 0, 0, 'T'},
 			{"log-level", 1, 0, 'e'},
+			{"restart-file", 1, 0, 'r'},
 			{NULL, 0, 0, 0}
 		};
 
-		c = getopt_long(argc, argv, "hd:Dc:sTe:",
+		c = getopt_long(argc, argv, "hd:Dc:sTe:r:",
 				long_options, &option_index);
 		if (c == -1) {
 			if (optind < argc) {
@@ -213,6 +279,9 @@ static void handle_options(struct cmdline_cfg *ccfg, int argc, char **argv)
 		case 'e':
 			log_set_log_level(osmo_stderr_target, atoi(optarg));
 			break;
+		case 'r':
+			ccfg->restart_counter_file = optarg;
+			break;
 		default:
 			/* ignore */
 			break;
@@ -228,6 +297,7 @@ int main(int argc, char **argv)
 	struct cmdline_cfg *ccfg = &_ccfg;
 	memset(ccfg, '\0', sizeof(*ccfg));
 	ccfg->config_file = "./gtphub.conf";
+	ccfg->restart_counter_file = "./gtphub_restart_count";
 
 	struct gtphub_cfg _cfg;
 	struct gtphub_cfg *cfg = &_cfg;
@@ -265,7 +335,9 @@ int main(int argc, char **argv)
 		exit(2);
 	}
 
-	if (gtphub_start(hub, cfg) != 0)
+	if (gtphub_start(hub, cfg,
+			 next_restart_count(ccfg->restart_counter_file))
+	    != 0)
 		return -1;
 
 	log_cfg(cfg);
diff --git a/openbsc/tests/gtphub/gtphub_test.c b/openbsc/tests/gtphub/gtphub_test.c
index 418100261..9ba464382 100644
--- a/openbsc/tests/gtphub/gtphub_test.c
+++ b/openbsc/tests/gtphub/gtphub_test.c
@@ -782,7 +782,7 @@ static void test_echo(void)
 		"00"	/* N-PDU 0 */ \
 		"00"	/* No extensions */ \
 		/* IEs */ \
-		"0e" restart /* 14: Recovery = 96 (restart counter: 1 octet) */ \
+		"0e" restart /* 14: Recovery (restart counter: 1 octet) */ \
 		"02"	/* 2 = IMSI */ \
 		  imsi	/* (8 octets) */ \
 		"0f01"	/* 15: Selection mode = MS provided APN, subscription not verified*/ \
@@ -825,7 +825,7 @@ static void test_echo(void)
 		  "80"	/* value = 0b10000000 = response, no rejection. */ \
 		"08"	/* 8: Reordering Required */ \
 		  "00"	/* not required. */ \
-		"0e" restart /* 14: Recovery = 1 */ \
+		"0e" restart /* 14: Recovery */ \
 		"10"	/* 16: TEI Data I */ \
 		  tei_u \
 		"11"	/* 17: TEI Control */ \
@@ -904,7 +904,7 @@ static int create_pdp_ctx()
 	const char *gtp_req_to_ggsn =
 		MSG_PDP_CTX_REQ("0068",
 				"6d31",	/* mapped seq ("abcd") */
-				"60",
+				"23",
 				"42000121436587f9",
 				"00000001", /* mapped TEI Data I ("123") */
 				"00000001", /* mapped TEI Control ("321") */
@@ -938,7 +938,7 @@ static int create_pdp_ctx()
 		MSG_PDP_CTX_RSP("004e",
 				"00000321", /* unmapped TEI ("001") */
 				"abcd", /* unmapped seq ("6d31") */
-				"01",
+				"23",
 				"00000002", /* mapped TEI from GGSN ("567") */
 				"00000002", /* mapped TEI from GGSN ("765") */
 				"0004""7f000101", /* gtphub's address towards SGSNs (Ctrl) */