Project

General

Profile

Actions

Bug #4368

closed

VLR timers T3250, T3260, and 3270 are not configurable and always set to 0

Added by fixeria about 4 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
VLR
Target version:
-
Start date:
01/18/2020
Due date:
% Done:

100%

Resolution:
Spec Reference:

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...

Actions #1

Updated by fixeria about 4 years ago

  • Status changed from New to In Progress
  • Assignee set to fixeria
  • % Done changed from 0 to 40
Actions #2

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!

Actions #3

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.

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)