Change in osmo-msc[master]: libmsc: move subscriber expiration timer T3212 to libvlr

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Mon Jan 27 12:15:40 UTC 2020


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/16934 )

Change subject: libmsc: move subscriber expiration timer T3212 to libvlr
......................................................................

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
---
M doc/manuals/vty/msc_vty_reference.xml
M include/osmocom/msc/gsm_data.h
M src/libmsc/msc_net_init.c
M src/libmsc/msc_vty.c
M src/libvlr/vlr.c
M tests/msc_vlr/msc_vlr_test_no_authen.c
M tests/test_nodes.vty
M tests/vty_test_runner.py
8 files changed, 54 insertions(+), 69 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved



diff --git a/doc/manuals/vty/msc_vty_reference.xml b/doc/manuals/vty/msc_vty_reference.xml
index 6418425..3939c08 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 <6-1530>'>
-      <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='<6-1530>' 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 3459d15..2122d4b 100644
--- a/include/osmocom/msc/gsm_data.h
+++ b/include/osmocom/msc/gsm_data.h
@@ -205,9 +205,6 @@
 
 	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 8c8fb86..adb9ca7 100644
--- a/src/libmsc/msc_net_init.c
+++ b/src/libmsc/msc_net_init.c
@@ -67,9 +67,6 @@
 	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 53d44dc..e2e892a 100644
--- a/src/libmsc/msc_vty.c
+++ b/src/libmsc/msc_vty.c
@@ -303,32 +303,44 @@
 	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 @@
 			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_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 @@
 			     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 a1489f2..887ceb8 100644
--- a/src/libvlr/vlr.c
+++ b/src/libvlr/vlr.c
@@ -64,7 +64,7 @@
 
 /* 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 @@
  * 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 @@
 
 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 @@
 {
 	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 @@
 	/* 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 7b684fe..5d3db69 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 @@
 	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 b87a371..5a81c61 100644
--- a/tests/test_nodes.vty
+++ b/tests/test_nodes.vty
@@ -24,8 +24,6 @@
   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 @@
  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 f954b5d..2421144 100755
--- a/tests/vty_test_runner.py
+++ b/tests/vty_test_runner.py
@@ -198,17 +198,22 @@
         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")

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/16934
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I9b12066599a7c834a53a93acf5902d91273bc74f
Gerrit-Change-Number: 16934
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200127/34f3be44/attachment.htm>


More information about the gerrit-log mailing list