Attention is currently required from: neels, laforge. iedemam 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 14: (8 comments) Patchset: PS12: > Thanks for the confidence boost. […] Done Patchset: PS14: Hi Neels, The feedback was really helpful. I think the patch is looking more like what you have in mind now. I am fighting a new handover test failure. Apologies for the silly question but how can I even see what is going on there? I see the expout and testsuite.log files but they don't offer any detail other than pointing out which test failed. Currently, I only know that test_max_ta.ho_vty is the failing sub-test. Obviously I'm missing something basic in the Osmo dev env / work cycle. Many thanks again, -Michael File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/5d9f6d10_ab744b10 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. […] Thanks! I'm understanding the lifecycle better now. File src/osmo-bsc/gsm_data.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/259bec1c_e6b62b68 PS13, Line 354: !&lchan->activate.timekeeper > a) the address of a struct member is *always* non-NULL. […] Done File src/osmo-bsc/lchan_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/d202e612_9f1d93c4 PS13, Line 226: lchan->activate.timekeeper = (struct osmo_time_cc){ > also you probably want to do this only once during lchan allocation, i.e. […] Done https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/65022e02_97ddbd57 PS13, Line 228: .gran_usec = 1*1000000, > if you want milliseconds, 1000 is your factor. […] Changed to milliseconds and changed forget_sum_usec to 1 to discard remainders. https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/2dc3c41c_a7ab9d1b PS13, Line 237: /* TMP HACK: this fixes "make check" handover tests... */ > yeah because you re-init the lost state after lchan_reset(). Done https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/4d0227f9_b8723593 PS13, Line 558: if (&lchan->activate.timekeeper) { > this condition is never false, as above. simply set the the flag without > condition. The check has been changed. I think it's still necessary as there isn't any sanity checking in osmo_time_cc_set_flag itself and not all lchans will have a timer attached. -- 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: 14 Gerrit-Owner: iedemam <mich...@kapsulate.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Attention: neels <nhofm...@sysmocom.de> Gerrit-Attention: laforge <lafo...@osmocom.org> Gerrit-Comment-Date: Fri, 04 Mar 2022 18:31:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: iedemam <mich...@kapsulate.com> Comment-In-Reply-To: neels <nhofm...@sysmocom.de> Comment-In-Reply-To: laforge <lafo...@osmocom.org> Gerrit-MessageType: comment
