Project

General

Profile

Feature #4539

review timer numbers: use (negative) X-timers instead of non-existing T-timers (like T23001), and avoid collisions of X-timer numbers across osmocom programs

Added by neels 5 months ago. Updated about 1 month ago.

Status:
In Progress
Priority:
Normal
Assignee:
Target version:
-
Start date:
05/09/2020
Due date:
% Done:

90%

Spec Reference:

Description

the 3GPP specs define various timers like T3001, which we made configurable via vty using the osmo_tdef API.
Later on, we introduced the use of negative timer numbers to indicate timers that are not specified by 3GPP, and called them X-timers, like X1, X2,...
At that point, various invented timer numbers have already been in use, see for example timeslot_fsm.c and lchan_fsm.c in osmo-bsc: T23001 etc.

Timer numbers can be re-used in different contexts, so per se we could use X1 any number of times in different contexts.
However, since we have a huge number space available for X-timers, there is no need to create "collisions" between osmocom programs.
It is much clearer to just pick unused numbers for each program, sort of like out Port_Numbers.

Create one common overview of all X-timers (and while at it, why not also all T-timers) available in osmocom programs.
Replace osmocom-invented T-timers with X-timers,
avoid X-timer collisions between osmocom programs.

Not sure whether we should provide a backwards compat shim to still allow configuring old timer numbers, so that setting the old timer numbers actually adjusts the new ones?
Or maybe just make sure that configuring the new timer number causes an error message on program start indicating the matching new X-timer to use?


Related issues

Related to OsmoBSC - Bug #4538: timeslot_fsm.c: make the PDCH act/deact timeout configurableNew05/09/2020

History

#1 Updated by neels 5 months ago

  • Related to Bug #4538: timeslot_fsm.c: make the PDCH act/deact timeout configurable added

#2 Updated by neels 5 months ago

IMO we should not use numbers like X23001 but start with lower numbers.
We could think of either creating a number space for each program, like osmo-bts has X1000-X1999, osmo-bsc X2000-X2999 etc.,
or just start with X1 and assign the next X2, X3 etc, whichever comes along next, like we did in Port_Numbers.

#3 Updated by osmith 5 months ago

Thanks for creating the issue!

neels wrote:

IMO we should not use numbers like X23001 but start with lower numbers.
We could think of either creating a number space for each program, like osmo-bts has X1000-X1999, osmo-bsc X2000-X2999 etc.,
or just start with X1 and assign the next X2, X3 etc, whichever comes along next, like we did in Port_Numbers.

I'd prefer the number space, so it's clear to which program a timer belongs.

Do we need more opinions on this (laforge, fixeria, pespin?), or can I create such a wiki page and start using these numbers in my open related osmo-bsc patch?

#4 Updated by fixeria 5 months ago

Do we need more opinions on this (laforge, fixeria, pespin?)

I definitely like the idea of having unique 'X' timer numbers. But still, it's unclean to me how do we ensure backwards compatibility, i.e. what should we do with the existing timer numbers that are used in more than one project (e.g. X1)?

can I create such a wiki page and start using these numbers in my open related osmo-bsc patch?

It probably makes sense to have a header file in libosmocore listing all used timers, so then we could automatically generate the wiki page from it. This way it's less likely that somebody forgets to reserve a port number, and somebody else starts to use it in another osmo-* program.

#5 Updated by pespin 5 months ago

I'm fine with having a unique list in a wiki page listing all X timers.

Regarding starting timers from X1 and so: I don't think it's generally a good idea (though it can be for some cases). What I mean is: Sometimes afair some of our X timers are highly related to a standard timer T, in which cases it makes sense to keep either Xn for Tn, or some value around n, specially since sometimes in standard some related timers have similar numbering.

#6 Updated by laforge 5 months ago

I don't have any comments on the detailed allocation

Having some systematic approach with per program number ranges and wanted associated wiki page looks like a good idea.

#7 Updated by neels 5 months ago

I've been pondering the thought of fixing X timer numbers.
It is easiest to just change the number, hence the config changes and probably fails, saying "timer X23 does not exist" all of a sudden.
But ideally, when we change a timer number, we would keep some backwards compatibility in the configuration.
We are currently missing any approach to provide backwards compat for old timer numbers.
Most (new) timers are managed by tdef_vty.h which makes no provision for backwards compat.
It might be possible to implement separate vty functions, without the need for a generic approach, like:

DEFUN(deprecated_timers, deprecated_timers_cmd,
"timer (T23|T42|X99) .IGNORED") {
vty_out("Timer numbers have changed, please adjust your config: T23->X17 T42->X18 X99->X19");
return CMD_WARNING;
}

could possibly be advanced to actually redirecting the legacy number to a new proper number, maybe like:

DEFUN(deprecated_timer_T23, deprecated_timer_T23_cmd,
"timer T23 .ARGS") {
vty_out("Timer T23 is now X17, please adjust your config");
argv[0] = "X17"; // somehow something like this
return cfg_timer_cmd(vty, argc, argv); // meaning the tdef_vty.c cfg_timer_cmd
}

#8 Updated by neels about 1 month ago

  • Status changed from New to Resolved
  • Assignee set to neels
  • % Done changed from 0 to 100

After https://gerrit.osmocom.org/c/osmo-bsc/+/17973 was assigned to me, I figured now is as good a time as ever to clean up the timers.

I collected all osmo_tdef timers I could find in:
https://osmocom.org/projects/cellular-infrastructure/wiki/List_of_Timer_numbers
(the wiki page reflects the status after merging pending patches https://gerrit.osmocom.org/q/topic:%22timers%22+(status:open%20OR%20status:merged) )

From that it became apparent that only OsmoBSC exposes T timers that are not specified by 3GPP.
Adding timer groups to OsmoBSC like in OsmoMSC (backwards compatibly) was a nice fit to move those to X timers.
https://gerrit.osmocom.org/c/osmo-bsc/+/20155

Also fixed an mgw timer in osmo-msc https://gerrit.osmocom.org/c/osmo-msc/+/20156

#9 Updated by neels about 1 month ago

  • Status changed from Resolved to In Progress
  • % Done changed from 100 to 90

wait for merge of course

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)