libmsc: move subscriber expiration timer T3212 to libvlr

Since the split of OsmoNiTB, OsmoMSC does not deal with the radio
access network directly. Therefore the only purpose of T3212 is to
control subscriber expiration in the local VLR. The timeout value
indicated in System Information Type 3 needs to be configured
separately in the BSC/RNC.

This means that we don't need to store it in deci-hours anymore.
Let's move T3212 to the group of VLR specific timers, so it can
be configured and introspected using the generic 'timer' command,
and deprecate the old '[no] periodic location update' command.

It should be also noted that in the old code subscriber expiration
timeout was actually set to twice the T3212 value plus one minute.
After this change, we apply the configured value 'as-is', but
keep the old behaviour for 'periodic location update' command.

Change-Id: I9b12066599a7c834a53a93acf5902d91273bc74f

8 files changed, 54 insertions, 69 deletions

diff --git a/doc/manuals/vty/msc_vty_reference.xml b/doc/manuals/vty/msc_vty_reference.xml
index 6418425f6..3939c089b 100644
--- a/doc/manuals/vty/msc_vty_reference.xml
+++ b/doc/manuals/vty/msc_vty_reference.xml
@@ -2744,22 +2744,6 @@
         <param name='timezone' doc='Disable network timezone override, use system tz' />
       </params>
     </command>
-    <command id='periodic location update &lt;6-1530&gt;'>
-      <params>
-        <param name='periodic' doc='Periodic Location Updating Interval' />
-        <param name='location' doc='Periodic Location Updating Interval' />
-        <param name='update' doc='Periodic Location Updating Interval' />
-        <param name='&lt;6-1530&gt;' doc='Periodic Location Updating Interval in Minutes' />
-      </params>
-    </command>
-    <command id='no periodic location update'>
-      <params>
-        <param name='no' doc='Negate a command or set its defaults' />
-        <param name='periodic' doc='Periodic Location Updating Interval' />
-        <param name='location' doc='Periodic Location Updating Interval' />
-        <param name='update' doc='Periodic Location Updating Interval' />
-      </params>
-    </command>
     <command id='call-waiting'>
       <params>
         <param name='call-waiting' doc='Enable Call Waiting on the Network' />
diff --git a/include/osmocom/msc/gsm_data.h b/include/osmocom/msc/gsm_data.h
index 3459d154d..2122d4b21 100644
--- a/include/osmocom/msc/gsm_data.h
+++ b/include/osmocom/msc/gsm_data.h
@@ -205,9 +205,6 @@ struct gsm_network {
 
 	struct vlr_instance *vlr;
 
-	/* Periodic location update default value */
-	uint8_t t3212;
-
 	/* Global MNCC guard timer value */
 	int mncc_guard_timeout;
 	/* Global guard timer value for NCSS sessions */
diff --git a/src/libmsc/msc_net_init.c b/src/libmsc/msc_net_init.c
index 8c8fb861a..adb9ca74f 100644
--- a/src/libmsc/msc_net_init.c
+++ b/src/libmsc/msc_net_init.c
@@ -67,9 +67,6 @@ struct gsm_network *gsm_network_init(void *ctx, mncc_recv_cb_t mncc_recv)
 	net->a5_encryption_mask = (1 << 3) | (1 << 1);
 	net->uea_encryption = true;
 
-	/* Use 30 min periodic update interval as sane default */
-	net->t3212 = 5;
-
 	net->mncc_guard_timeout = 180;
 	net->ncss_guard_timeout = 30;
 
diff --git a/src/libmsc/msc_vty.c b/src/libmsc/msc_vty.c
index 53d44dc7d..e2e892af4 100644
--- a/src/libmsc/msc_vty.c
+++ b/src/libmsc/msc_vty.c
@@ -303,32 +303,44 @@ DEFUN(cfg_net_no_timezone,
 	return CMD_SUCCESS;
 }
 
-DEFUN(cfg_net_per_loc_upd, cfg_net_per_loc_upd_cmd,
-      "periodic location update <6-1530>",
-      "Periodic Location Updating Interval\n"
-      "Periodic Location Updating Interval\n"
-      "Periodic Location Updating Interval\n"
-      "Periodic Location Updating Interval in Minutes\n")
+/* NOTE: actually this is subscriber expiration timeout */
+#define PER_LOC_UPD_STR "Periodic Location Updating Interval\n"
+
+DEFUN_DEPRECATED(cfg_net_per_loc_upd, cfg_net_per_loc_upd_cmd,
+		 "periodic location update <6-1530>",
+		 PER_LOC_UPD_STR PER_LOC_UPD_STR PER_LOC_UPD_STR
+		 "Periodic Location Updating Interval in Minutes\n")
 {
-	struct gsm_network *net = vty->index;
+	int minutes = atoi(argv[0]);
+	int rc;
 
-	net->t3212 = atoi(argv[0]) / 6;
+	vty_out(vty, "%% 'periodic location update' is now deprecated: "
+		     "use 'timer T3212' to change subscriber expiration "
+		     "timeout.%s", VTY_NEWLINE);
 
-	return CMD_SUCCESS;
+	/* We used to double this value and add a minute when scheduling the
+	 * expiration timer. Let's emulate the old behaviour here. */
+	minutes = minutes * 2 + 1;
+	vty_out(vty, "%% Setting T3212 to %d minutes "
+		     "(emulating the old behaviour).%s",
+		     minutes, VTY_NEWLINE);
+
+	rc = osmo_tdef_set(msc_tdefs_vlr, 3212, minutes, OSMO_TDEF_M);
+	return rc ? CMD_WARNING : CMD_SUCCESS;
 }
 
-DEFUN(cfg_net_no_per_loc_upd, cfg_net_no_per_loc_upd_cmd,
-      "no periodic location update",
-      NO_STR
-      "Periodic Location Updating Interval\n"
-      "Periodic Location Updating Interval\n"
-      "Periodic Location Updating Interval\n")
+DEFUN_DEPRECATED(cfg_net_no_per_loc_upd, cfg_net_no_per_loc_upd_cmd,
+		 "no periodic location update",
+		 NO_STR PER_LOC_UPD_STR PER_LOC_UPD_STR PER_LOC_UPD_STR)
 {
-	struct gsm_network *net = vty->index;
+	int rc;
 
-	net->t3212 = 0;
+	vty_out(vty, "%% 'periodic location update' is now deprecated: "
+		     "use 'timer T3212' to change subscriber expiration "
+		     "timeout.%s", VTY_NEWLINE);
 
-	return CMD_SUCCESS;
+	rc = osmo_tdef_set(msc_tdefs_vlr, 3212, 0, OSMO_TDEF_M);
+	return rc ? CMD_WARNING : CMD_SUCCESS;
 }
 
 DEFUN(cfg_net_call_wait, cfg_net_call_wait_cmd,
@@ -389,11 +401,6 @@ static int config_write_net(struct vty *vty)
 			vty_out(vty, " timezone %d %d%s",
 				gsmnet->tz.hr, gsmnet->tz.mn, VTY_NEWLINE);
 	}
-	if (gsmnet->t3212 == 0)
-		vty_out(vty, " no periodic location update%s", VTY_NEWLINE);
-	else
-		vty_out(vty, " periodic location update %u%s",
-			gsmnet->t3212 * 6, VTY_NEWLINE);
 
 	if (gsmnet->emergency.route_to_msisdn) {
 		vty_out(vty, " emergency-call route-to-msisdn %s%s",
@@ -869,7 +876,6 @@ static void vty_dump_one_conn(struct vty *vty, const struct msub *msub,
 static void vty_dump_one_subscr(struct vty *vty, struct vlr_subscr *vsub,
 				int offset, uint8_t dump_flags)
 {
-	struct gsm_network *net;
 	struct timespec now;
 	char buf[128];
 
@@ -943,9 +949,7 @@ static void vty_dump_one_subscr(struct vty *vty, struct vlr_subscr *vsub,
 			     VTY_NEWLINE);
 	}
 
-	/* XXX move t3212 into struct vlr_instance? */
-	net = vsub->vlr->user_ctx;
-	if (!net->t3212) {
+	if (!vlr_timer(vsub->vlr, 3212)) {
 		MSC_VTY_DUMP(vty, offset, "Expires: never (T3212 is disabled)%s",
 			     VTY_NEWLINE);
 	} else if (vsub->expire_lu == VLR_SUBSCRIBER_NO_EXPIRATION) {
diff --git a/src/libvlr/vlr.c b/src/libvlr/vlr.c
index a1489f219..887ceb810 100644
--- a/src/libvlr/vlr.c
+++ b/src/libvlr/vlr.c
@@ -64,7 +64,7 @@ const struct value_string vlr_ciph_names[] = {
 
 /* 3GPP TS 24.008, table 11.2 Mobility management timers (network-side) */
 struct osmo_tdef msc_tdefs_vlr[] = {
-	/* TODO: also define T3212 here */
+	{ .T = 3212, .default_val = 60, .unit = OSMO_TDEF_M, .desc = "Subscriber expiration timeout" },
 	{ .T = 3250, .default_val = 12, .desc = "TMSI Reallocation procedure" },
 	{ .T = 3260, .default_val = 12, .desc = "Authentication procedure" },
 	{ .T = 3270, .default_val = 12, .desc = "Identification procedure" },
@@ -75,6 +75,9 @@ struct osmo_tdef msc_tdefs_vlr[] = {
  * TODO: we should start using osmo_tdef_fsm_inst_state_chg() */
 uint32_t vlr_timer(struct vlr_instance *vlr, uint32_t timer)
 {
+	/* NOTE: since we usually do not need more than one instance of the VLR,
+	 * and since libosmocore's osmo_tdef API does not (yet) support dynamic
+	 * configuration, we always use the global instance of msc_tdefs_vlr. */
 	return osmo_tdef_get(msc_tdefs_vlr, timer, OSMO_TDEF_S, 0);
 }
 
@@ -496,14 +499,11 @@ int vlr_subscr_changed(struct vlr_subscr *vsub)
 
 void vlr_subscr_enable_expire_lu(struct vlr_subscr *vsub)
 {
-	struct gsm_network *net = vsub->vlr->user_ctx; /* XXX move t3212 into struct vlr_instance? */
 	struct timespec now;
 
-	/* The T3212 timeout value field is coded as the binary representation of the timeout
-	 * value for periodic updating in decihours. Mark the subscriber as inactive if it missed
-	 * two consecutive location updates. Timeout is twice the t3212 value plus one minute. */
+	/* Mark the subscriber as inactive if it stopped to do periodical location updates. */
 	if (osmo_clock_gettime(CLOCK_MONOTONIC, &now) == 0) {
-		vsub->expire_lu = now.tv_sec + (net->t3212 * 60 * 6 * 2) + 60;
+		vsub->expire_lu = now.tv_sec + vlr_timer(vsub->vlr, 3212);
 	} else {
 		LOGP(DVLR, LOGL_ERROR,
 		     "%s: Could not enable Location Update expiry: unable to read current time\n", vlr_subscr_name(vsub));
@@ -516,13 +516,11 @@ void vlr_subscr_expire_lu(void *data)
 {
 	struct vlr_instance *vlr = data;
 	struct vlr_subscr *vsub, *vsub_tmp;
-	struct gsm_network *net;
 	struct timespec now;
 
 	/* Periodic location update might be disabled from the VTY,
 	 * so we shall not expire subscribers until explicit IMSI Detach. */
-	net = vlr->user_ctx; /* XXX move t3212 into struct vlr_instance? */
-	if (!net->t3212)
+	if (!vlr_timer(vlr, 3212))
 		goto done;
 
 	if (llist_empty(&vlr->subscribers))
@@ -1263,6 +1261,9 @@ struct vlr_instance *vlr_alloc(void *ctx, const struct vlr_ops *ops)
 	/* defaults */
 	vlr->cfg.assign_tmsi = true;
 
+	/* reset shared timer definitions */
+	osmo_tdefs_reset(msc_tdefs_vlr);
+
 	/* osmo_auth_fsm.c */
 	OSMO_ASSERT(osmo_fsm_register(&vlr_auth_fsm) == 0);
 	/* osmo_lu_fsm.c */
diff --git a/tests/msc_vlr/msc_vlr_test_no_authen.c b/tests/msc_vlr/msc_vlr_test_no_authen.c
index 7b684fe1b..5d3db69ae 100644
--- a/tests/msc_vlr/msc_vlr_test_no_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_no_authen.c
@@ -941,7 +941,7 @@ static void test_no_authen_subscr_expire()
 	vlr_subscr_put(vsub, __func__);
 
 	/* Let T3212 (periodic Location update timer) expire */
-	fake_time_passes((net->t3212 * 60 * 6 * 2) + 60*4, 0);
+	fake_time_passes(vlr_timer(net->vlr, 3212) + 60 * 4, 0);
 
 	/* The subscriber should now be gone. */
 	vsub = vlr_subscr_find_by_imsi(net->vlr, imsi, __func__);
diff --git a/tests/test_nodes.vty b/tests/test_nodes.vty
index b87a3719d..5a81c611d 100644
--- a/tests/test_nodes.vty
+++ b/tests/test_nodes.vty
@@ -24,8 +24,6 @@ OsmoMSC(config-net)# list
   timezone <-19-19> (0|15|30|45)
   timezone <-19-19> (0|15|30|45) <0-2>
   no timezone
-  periodic location update <6-1530>
-  no periodic location update
   call-waiting
   no call-waiting
 
@@ -153,7 +151,6 @@ network
  authentication optional
  rrlp mode none
  mm info 1
- periodic location update 30
 msc
  mncc guard-timeout 180
  ncss guard-timeout 30
diff --git a/tests/vty_test_runner.py b/tests/vty_test_runner.py
index f954b5d30..242114467 100755
--- a/tests/vty_test_runner.py
+++ b/tests/vty_test_runner.py
@@ -198,17 +198,22 @@ class TestVTYMSC(TestVTYBase):
         self.vty.verify("periodic location update 5", ['% Unknown command.'])
         self.vty.verify("periodic location update 1531", ['% Unknown command.'])
 
-        # Enable periodic lu..
-        self.vty.verify("periodic location update 60", [''])
+        depr_str = "% 'periodic location update' is now deprecated: " \
+                   "use 'timer T3212' to change subscriber expiration timeout."
+        set_str  = "% Setting T3212 to 121 minutes (emulating the old behaviour)."
+
+        # Enable periodic LU (deprecated command)
+        self.vty.verify("periodic location update 60", [depr_str, set_str])
         res = self.vty.command("write terminal")
-        self.assertTrue(res.find('periodic location update 60') > 0)
+        self.assertTrue(res.find('timer vlr T3212 121') > 0)
+        self.assertEqual(res.find('periodic location update 60'), -1)
         self.assertEqual(res.find('no periodic location update'), -1)
 
-        # Now disable it..
-        self.vty.verify("no periodic location update", [''])
+        # Now disable it (deprecated command)
+        self.vty.verify("no periodic location update", [depr_str])
         res = self.vty.command("write terminal")
-        self.assertEqual(res.find('periodic location update 60'), -1)
-        self.assertTrue(res.find('no periodic location update') > 0)
+        self.assertEqual(res.find('no periodic location update'), -1)
+        self.assertEqual(res.find('timer vlr T3212 121'), -1)
 
     def testShowNetwork(self):
         res = self.vty.command("show network")