Attention is currently required from: neels, laforge, fixeria, pespin.
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 16:

(10 comments)

Patchset:

PS16:
Hi all,

Thanks again for the feedback. I think we're getting much closer. This version 
still needs additional testing on my side to confirm that I can still 
accurately see both per-lchan instance duration and total duration per lchan 
type.

I will keep testing this and await further feedback.

Regards,
-Michael


File src/osmo-bsc/lchan_fsm.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/2c18280f_a249ed27
PS14, Line 465:         struct rate_ctr *active_ms_rate_ctr;
> You could simply set it to NULL here and keep the code unchanged, but not 
> critical.
Done


File src/osmo-bsc/lchan_fsm.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/961f4f23_da339e28
PS15, Line 208:         if (lchan->active_ms.cfg.rate_ctr) {
> I understand now: the rate_ctr is set only for SDCCH and TCH type.
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/ea462e35_9934bd0a
PS15, Line 209:                 osmo_time_cc_cleanup(&lchan->active_ms);
> osmo_time_cc_cleanup() is not necessary, the forget_sum configuration does 
> this implicitly
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/9a20d4f0_4e8b368d
PS15, Line 465: uint
> I like to use "uint" in private projects, but there i typedef it manually as 
> unsigned int. […]
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/eb1c8fac_e76441ee
PS15, Line 478:         counter_id = -1;
> could, yes, but totally fine here IMHO. […]
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/1679d6e6_4c389747
PS15, Line 479:         switch (lchan->type) {
> the lchan->type changes during operation of osmo-bsc, it is not known at the 
> time of lchan_fsm_alloc […]
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/6dcb2dda_6b64141c
PS15, Line 493:                                 .gran_usec = 1*1000,
> Sorry for my earlier comment, I've come to realize what exactly I recommended 
> by mentioning 1 ms gra […]
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/954d0daf_01205823
PS15, Line 498:                                 .T_forget_sum = -18,
> timers X16, X17, X18 are defined to configure a different counter (the 
> all_allocated counters). […]
Done


https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/9bd5ac4e_af62f137
PS15, Line 562:         /* Stop the timekeeper, which triggers a report */
> the comment suggests that the report is triggered only now. […]
Done



--
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: 16
Gerrit-Owner: iedemam <mich...@kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-CC: fixeria <vyanits...@sysmocom.de>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-Attention: neels <nhofm...@sysmocom.de>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Attention: fixeria <vyanits...@sysmocom.de>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Comment-Date: Tue, 15 Mar 2022 16:50:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofm...@sysmocom.de>
Comment-In-Reply-To: fixeria <vyanits...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to