Attention is currently required from: iedemam. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27081 ) Change subject: stats: new trackers for lchan life duration ...................................................................... Patch Set 13: (5 comments) File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/82510adb_73ed7fcf PS13, Line 771: struct osmo_time_cc timekeeper; here is your problem: the lchan is completely cleared out every time an lchan is activated / unused. See lchan_reset(), there it loses its pointer to the ctr. In lchan_reset(), you need to make sure the timekeeper stays unchanged by adding .timekeeper = lchan->timekeeper, lchan->activate is intended as a volatile place describing the state of a single activation. Rather put your timekeeper in the "lchan root" please. Nitpick on the name: let's call it 'active_ms' or 'active_time' or something more concise. File src/osmo-bsc/gsm_data.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/88951373_c33c89fe PS13, Line 354: !&lchan->activate.timekeeper a) the address of a struct member is *always* non-NULL. (ok if a struct pointer is NULL and the member is the first in the struct, it could be NULL, but that's beside the point) b) even if you drop the '&', timekeeper is not a pointer. This condition will always evaluate false. the compiler should have told you that, too, wondering why jenkins passes the build. Completely drop this 'if' File src/osmo-bsc/lchan_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/538b2512_a7f5c4b8 PS13, Line 228: .gran_usec = 1*1000000, if you want milliseconds, 1000 is your factor. This says after how much active time the rate counter increments; using one second for that does make sense, just saying. The total_sum is always in microseconds regardless, so probably this is correct after all. https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/d75ec044_d7534a91 PS13, Line 237: /* TMP HACK: this fixes "make check" handover tests... */ yeah because you re-init the lost state after lchan_reset(). https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/587ae6f2_260775a4 PS13, Line 558: if (&lchan->activate.timekeeper) { this condition is never false, as above. simply set the the flag without condition. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27081 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347 Gerrit-Change-Number: 27081 Gerrit-PatchSet: 13 Gerrit-Owner: iedemam <michael(a)kapsulate.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: iedemam <michael(a)kapsulate.com> Gerrit-Comment-Date: Thu, 03 Mar 2022 18:01:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment