July 2023 - gerrit-log - lists.osmocom.org

[M] Change in osmo-bts[master]: osmo-bts-trx: unify and enrich 'Received bad data' logging
by laforge 13 Jul '23

13 Jul '23

Attention is currently required from: fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33729 ) Change subject: osmo-bts-trx: unify and enrich 'Received bad data' logging ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33729 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I7902f382e1d83ef9ad2cf6f92e31eeb16f6b797c Gerrit-Change-Number: 33729 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 13 Jul 2023 04:03:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: osmo-bts-trx: rx_tch[fh]_fn(): combine rc-checking ifs
by laforge 13 Jul '23

13 Jul '23

Attention is currently required from: fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33731 ) Change subject: osmo-bts-trx: rx_tch[fh]_fn(): combine rc-checking ifs ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33731 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I7bb341867e362bf2061608ff54c3596ad209af90 Gerrit-Change-Number: 33731 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 13 Jul 2023 04:03:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-msc[master]: ASCI: Add option to switch on or off ASCI support
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33674 ) Change subject: ASCI: Add option to switch on or off ASCI support ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/libmsc/msc_a.c: https://gerrit.osmocom.org/c/osmo-msc/+/33674/comment/4531ea71_152475d9 PS2, Line 2088: struct msc_a *msc_a ( rather add a struct gsm_network * arg. Nothing in this function uses an msc_a->* member. ...or rather even only a 'bool asci_enable' arg? some years ago i opposed it, but now my opinion is that a single global gsmnet pointer is a good idea. we have bsc_gsmnet in osmo-bsc, i think no such thing in osmo-msc ... yet. Maybe we can add that now, and then not need any arg at all here. Up to you, not critical. ) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33674 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Id68deb69f7395f0f8f50b3820e9d51052a34f753 Gerrit-Change-Number: 33674 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 13 Jul 2023 01:03:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-msc[master]: ASCI: Add function to receive VGCS/VBS messages from BSS
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33504 ) Change subject: ASCI: Add function to receive VGCS/VBS messages from BSS ...................................................................... Patch Set 14: (2 comments) File src/libmsc/ran_peer.c: https://gerrit.osmocom.org/c/osmo-msc/+/33504/comment/e38abce0_8f331f33 PS14, Line 396: so below, there is a priority: 1.msc_role 2.vgcs.bss 3.vgcs.cell. When there is an active vgcs.bss connection, is it possible that something also adds an msc_role to the same conn, and then that hides the active VGCS from incoming messages, because msc_role outranks vgcs.bss? I'm asking, because there are mechanisms where a subscriber re-uses an existing connection. For example, if some CM Service is already active (say USSD) and then also a call starts, then it opens another transaction on the same conn. Also a Call Re-Establishment does some more aggressive re-using of transactions and conns. Can this kind of thing happen at all, e.g. the active talker person happens to receive an SMS at the same time, and while the SMS is handled, an important VGCS message is lost? https://gerrit.osmocom.org/c/osmo-msc/+/33504/comment/f5bd4f59_ff26aa55 PS14, Line 413: return (now redundant) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33504 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ie4a2f19ba75140a6f2de02b709597239c01f02a2 Gerrit-Change-Number: 33504 Gerrit-PatchSet: 14 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 13 Jul 2023 00:51:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-msc[master]: ASCI: Add decoder for VGCS/VBS messages to msc_a.c
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33506 ) Change subject: ASCI: Add decoder for VGCS/VBS messages to msc_a.c ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33506 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ief6259ba3914eeaceb063b562a0bcbc48349ce60 Gerrit-Change-Number: 33506 Gerrit-PatchSet: 14 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 13 Jul 2023 00:34:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XL] Change in osmo-msc[master]: ASCI: Add call control for VGCS/VBS
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly, laforge, dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33511 ) Change subject: ASCI: Add call control for VGCS/VBS ...................................................................... Patch Set 14: (11 comments) File src/libmsc/msc_vgcs.c: https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/3eb64441_587e42ec PS14, Line 134: static char string[9]; none of these are critical, but i'd still like to mention them: Since it is only one format string, it might be implemented as a #define GROUP_ID_FMT "%08u" In case of a function, i'd use the following pattern because it is very flexible: int gsm44068_group_id_str_buf(char *buf, size_t buflen, uint32_t callref) { struct osmo_strbuf sb = { .buf = buf, .len = buflen }; OSMO_STRBUF_PRINTF(sb, "%08u", callref); return sb.chars_needed; } - can be used for static alloc - can be used for dynamic alloc with OSMO_NAME_C_IMPL - can be used to append to arbitrary buffer position directly - can be used with OSMO_STRBUF_APPEND() https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/4dbc4771_95119204 PS14, Line 179: OSMO_ASSERT(l3_msg); this would abort the program on an encoding error? rather log and return error. These four lines from ran_a_encode to msgb_free seem to duplicate 1:1 many times in this file, put them in a static function? https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/59a868dd_a420ec16 PS14, Line 231: *callref = ntohl(*(uint32_t *)ie) >> 5; maybe better use osmo_load32be() because it also works reliably when the pointer is not on a word boundary https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/71b5db22_ffc6a28b PS14, Line 801: if (msc_a) { (early exit pattern: 'if (!msc_a) return;' means less indent. some more candidates for early exit below) https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/e6c8b05f_69d73936 PS14, Line 1628: OSMO_ASSERT(l3_msg); encoding error: do not abort the program, log and return error https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/577b08a5_6e8d079c PS14, Line 1666: OSMO_ASSERT . https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/060c9d51_155c550f PS14, Line 1727: osmo_fsm_inst_free(bss->fi); rather use osmo_fsm_inst_term(). It terminates the instance gracefully and then frees it. (I like avoid doing anything at all after dispatching an event or changing a state to avoid possible use-after-free "loops", but I see BSS_ST_NULL has no onenter() action that could trigger those). https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/c8814bfa_c1dd2f9a PS14, Line 1730: talloc_free(bss); (hint, in case you like it too, i like to allocate the struct as a talloc child of the FSM instance, so an osmo_fsm_inst_term() automatically frees the struct, and they are "atomically" removed at the same time without any race conditions possible) https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/4c1901cf_986d9645 PS14, Line 1796: OSMO_ASSERT . https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/1186fa77_1bee1bdb PS14, Line 1938: OSMO_ASSERT . https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/c77a36c0_c57383a6 PS14, Line 1949: OSMO_ASSERT . -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33511 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I9947403fde8212b66758104443c60aaacc8b1e7b Gerrit-Change-Number: 33511 Gerrit-PatchSet: 14 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 13 Jul 2023 00:27:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-msc[master]: ASCI: Add callref to assignment command
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33516 ) Change subject: ASCI: Add callref to assignment command ...................................................................... Patch Set 11: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33516 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I7955c6e0eebc930f85f360dda46be17cbd39e181 Gerrit-Change-Number: 33516 Gerrit-PatchSet: 11 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Wed, 12 Jul 2023 23:27:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in osmo-msc[master]: ASCI: Add functions to transcode VGCS/VBS messages on A-interface
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33505 ) Change subject: ASCI: Add functions to transcode VGCS/VBS messages on A-interface ...................................................................... Patch Set 11: Code-Review+2 (1 comment) Patchset: PS4: > I put it there, because all other A-interface messages are also there. […] no, completely fine as it is -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33505 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I6b1f088201e7ef4a58762937855a1d358973882c Gerrit-Change-Number: 33505 Gerrit-PatchSet: 11 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Wed, 12 Jul 2023 23:26:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: jolly <andreas(a)eversberg.eu> Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-msc[master]: ASCI: Add simple implementation of Group Call Register
by neels 13 Jul '23

13 Jul '23

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33510 ) Change subject: ASCI: Add simple implementation of Group Call Register ...................................................................... Patch Set 11: (1 comment) File src/libmsc/asci_gcr.c: https://gerrit.osmocom.org/c/osmo-msc/+/33510/comment/84ff1bd7_a68bc7b6 PS11, Line 163: OSMO_ASSERT(l < ARRAY_SIZE(pow10)); can this assertion abort the program based on user input? -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33510 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ia74a4a865f943c5fb388cd28f9406005c92e663e Gerrit-Change-Number: 33510 Gerrit-PatchSet: 11 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Wed, 12 Jul 2023 23:25:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: osmo-bts-trx: rx_tchf_fn(): move compute_ber10k() above
by fixeria 13 Jul '23

13 Jul '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/33730 ) Change subject: osmo-bts-trx: rx_tchf_fn(): move compute_ber10k() above ...................................................................... osmo-bts-trx: rx_tchf_fn(): move compute_ber10k() above ... for the sake of consistency with rx_tchh_fn(). Change-Id: Ie331da78eb3831e35d255583466e0d09b093b132 --- M src/osmo-bts-trx/sched_lchan_tchf.c 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/30/33730/1 diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index bdc5140..3973cd0 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -225,6 +225,8 @@ return -EINVAL; } + ber10k = compute_ber10k(n_bits_total, n_errors); + /* average measurements of the last N (depends on mode) bursts */ trx_sched_meas_avg(chan_state, &meas_avg, meas_avg_mode); /* meas_avg.fn now contains TDMA frame number of the first burst */ @@ -240,8 +242,6 @@ rc = 0; /* this is how we signal BFI to l1sap */ } - ber10k = compute_ber10k(n_bits_total, n_errors); - /* FACCH */ if (rc == GSM_MACBLOCK_LEN) { _sched_compose_ph_data_ind(l1ts, fn_begin, bi->chan, -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie331da78eb3831e35d255583466e0d09b093b132 Gerrit-Change-Number: 33730 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0