Bug #4368
closedVLR timers T3250, T3260, and 3270 are not configurable and always set to 0
100%
Description
Being tired of seeing multiple FIXME / TODO comments stating that we should move T3212 from struct gsm_network to struct vlr_instance, I found a nice potential place:
/* An instance of the VLR codebase */
struct vlr_instance {
struct llist_head subscribers;
struct llist_head operations;
struct gsup_client_mux *gcm;
struct vlr_ops ops;
struct osmo_timer_list lu_expire_timer;
struct {
bool retrieve_imeisv_early;
bool retrieve_imeisv_ciphered;
bool assign_tmsi;
bool check_imei_rqd;
int auth_tuple_max_reuse_count;
bool auth_reuse_old_sets_on_error;
bool parq_retrieve_imsi;
bool is_ps;
uint32_t timer[_NUM_VLR_TIMERS]; // <----- here?
} cfg;
/* A free-form pointer for use by the caller */
void *user_ctx;
};
where we already have the following timers:
enum vlr_timer {
VLR_T_3250,
VLR_T_3260,
VLR_T_3270,
_NUM_VLR_TIMERS
};
However, as far as I can see, neither they are properly initialized (talloc_zero() sets them all to 0), nor there is a way to configure them. The only related API I could find is vlr_timer(), which takes a timer number and returns its value. This function looks quite odd and meaningless, why not just use the enum and access timers directly, like vlr->cfg.timer[VLR_T_3250]?
I started osmo-msc in gdb, and checked the memory contents at run-time:
(gdb) p ((struct vlr_instance *) 0x613000000ea0)->cfg.timer $1 = {0, 0, 0}
But wait, we're actively using these timers in various FSM definitions of libvlr:
/* This macros is here just for the reference */
#define osmo_fsm_inst_state_chg(fi, new_state, timeout_secs, T) ...
osmo_fsm_inst_state_chg(fi, VLR_ST_TUWAT, vlr_timer(vsub->vlr, 3260), 3260);
therefore the state timeouts are always inactive. I believe it makes sense to start using osmo_tdef...
Updated by fixeria over 4 years ago
- Status changed from New to In Progress
- Assignee set to fixeria
- % Done changed from 0 to 40
Updated by fixeria about 4 years ago
- Status changed from In Progress to Feedback
- % Done changed from 40 to 90
Please see:
https://gerrit.osmocom.org/c/osmo-msc/+/16932/ VTY: add osmo_tdef introspection and configuration commands
https://gerrit.osmocom.org/c/osmo-msc/+/16933/ libvlr: use generic osmo_tdef API for T3250, T3260, and T3270
I also discovered a similar problem related to the MGW timers:
https://gerrit.osmocom.org/c/osmo-msc/+/16931/ osmo-msc: fix: properly initialize default values for MGW timers
Looking forward to your CR!
Updated by fixeria about 4 years ago
- Status changed from Feedback to Resolved
- % Done changed from 90 to 100
All related changes have been merged.