This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Stefan Sperling gerrit-no-reply at lists.osmocom.orgPatch Set 2: (9 comments) https://gerrit.osmocom.org/#/c/6324/2/include/osmocom/bsc/acc_ramp.h File include/osmocom/bsc/acc_ramp.h: Line 1: /* (C) 2018 Stefan Sperling <ssperling at sysmocom.de> > (C) sysmocom / Author: Stefan Sperling, please look at other examples, than Fine, fixed in next patch set. Line 41: struct gsm_bts *bts; /* backpointer to BTS using this ACC ramp */ > one could avoid the back-pointer if we used offsetof() to go back from acc_ I agree. I will investigate this idea later and provide a follow-up patch if I find a nicer way of doing this. https://gerrit.osmocom.org/#/c/6324/2/src/libbsc/acc_ramp.c File src/libbsc/acc_ramp.c: Line 68: static void allow_all_allowed_accs(struct acc_ramp *acc_ramp) > the naming is a bit ... odd. :) I will change the naming in the next patch set to make a distinction between "allowed ACC" (this ACC is not currently barred) and "enabled ACC" (the user has not permanently barred this ACC via VTY). I hope that will be clearer. PS2, Line 93: DRLL > this is not related to the Radio Link Layer (which is a sub-layer of RSL). I will switch these to DRSL in the next patch set. I could also introduce a separate log category in a follow-up patch if desired. Line 98: static void send_bts_system_info(struct gsm_bts *bts) > this should become a public function that's part of the general bts/trx/sys There already is one in bsc_init.c, gsm_bts_set_system_infos(). I will use that instead in the next patch set. Line 107: static void do_ramping_step(void *data) > 'acc' in the name might be useful in backtraces/debugging/... to indicate w Agreed. Will rename in next patch set. https://gerrit.osmocom.org/#/c/6324/2/src/libbsc/bsc_init.c File src/libbsc/bsc_init.c: Line 335: acc_ramp_start(&trx->bts->acc_ramp); > your first ramp step is executed before the call to gsm_bts_trx_set_system_ Indeed, thanks for catching this! I think we can solve this by again calling acc_ramp_init() when we enter this function, to ensure ACC barring overrides take effect in the initial system infos bootstrap_rsl() sends via gsm_bts_trx_set_system_infos(). And then start the actual ramping process at the end of bootstrap_rsl(). This call might update system infos again if needed, but that's what we do as part of each ramping step anyway. Calling acc_ramp_init() in bootstrap_rsl() will also help in case ACC ramp VTY configuration has changed when a BTS reconnects, which happens without going through bootstrap_bts() again as far as can I tell. https://gerrit.osmocom.org/#/c/6324/2/src/libbsc/bsc_vty.c File src/libbsc/bsc_vty.c: PS2, Line 3135: enabled (0|1) > while we have some commands like this dating back to the early days, it is Changed to work as suggested (Switch/Router style) in the next patch set. The commands which modify step size and interval were also changed slightly as a side effect. https://gerrit.osmocom.org/#/c/6324/2/src/libbsc/system_information.c File src/libbsc/system_information.c: Line 662: static void apply_acc_ramp(struct gsm48_rach_control *rach_control, struct acc_ramp *acc_ramp) > I think that should be part of the acc related code, not here? At that poi In the next patch set I am moving this to acc_ramp.h as an inline function, named as suggested. -- To view, visit https://gerrit.osmocom.org/6324 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0a5ac3a08f992f326435944f17e0a9171911afb0 Gerrit-PatchSet: 2 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com> Gerrit-HasComments: Yes