Attention is currently required from: arehbein, laforge.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/31877 )
Change subject: gm_12_21.h: Add cross-component parameter defaults (NSE timing)
......................................................................
Patch Set 2:
(1 comment)
File include/osmocom/gsm/protocol/gsm_12_21.h:
https://gerrit.osmocom.org/c/libosmocore/+/31877/comment/c09f18ec_c9981c29
PS2, Line 294: enum abis_nm_par_defaults {
What's the drawback of using an enum here?
I already explained in the comment previous to the one where you ask it. I'll extend:
This header contains information about Abis protocol and implementation, from TS 12.21.
All these offsets that you are defining here, are not specified in TS 12.21 nor have much
to do with it. If you look at where are these offsets are used, (eg, take rlc_cfg_default)
you will see that those are only used to *internally* store values inside each app
process, and that this "array" is never transmitted over the wire. So it's
only a given implementation decision of several apps to store the different values that
way.
Actually, imho, those apps would need to be changed to drop those RLC_T3142* enums they
have and simply have an struct with the different fields. It would be clearer for
everyone, and would simplify the code.
What you could add in a related library include, is a packed struct send over the wire
containing the value of NM_ATT_IPACC_RLC_CFG. You can find its contents in
nanobts_gen_set_cell_attr() in osmo-bsc.git:
"""
/* all timers in seconds, unless otherwise stated */
buf[0] = 20; /* T3142 */
buf[1] = 5; /* T3169 */
buf[2] = 5; /* T3191 */
buf[3] = 160; /* T3193 (units of 10ms) */
buf[4] = 5; /* T3195 */
buf[5] = 10; /* N3101 */
buf[6] = 4; /* N3103 */
buf[7] = 8; /* N3105 */
buf[8] = 15; /* RLC CV countdown */
msgb_tl16v_put(msgb, NM_ATT_IPACC_RLC_CFG, 9, buf);
"""
But again, this is sorta out to the scope in OS#5335, which is about to add "Add VTY
commands to configure T3.. and N3.. timers counters for gprs"
So in summary, don't attempt to move timer/counter specific context out of each app
into a lib, it won't be the good way to work in this case. If at all, we can look if
it may make sense later, but first let's clean up everything on each app.
Should we add a centralised gsm_data.h to libosmocore
that keeps shared values?
As a conculision from the text I wrote above: NO.
--
To view, visit
https://gerrit.osmocom.org/c/libosmocore/+/31877
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I51d1fd8b596523ae2ac8fb6a186ce7a702334c27
Gerrit-Change-Number: 31877
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 01 Sep 2023 16:49:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment