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

[S] Change in docker-playground[master]: ttcn3-ggsn: Speed up tests by lowering retransmission T3,N3 timers
by daniel 17 Apr '23

17 Apr '23

Attention is currently required from: pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/32284 ) Change subject: ttcn3-ggsn: Speed up tests by lowering retransmission T3,N3 timers ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/32284 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ica346bd791a67b72fad597702d750383e8ac5895 Gerrit-Change-Number: 32284 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 17 Apr 2023 18:01:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-ttcn3-hacks[master]: ggsn: Speed up tests by lowering retransmission T3,N3 timers
by daniel 17 Apr '23

17 Apr '23

Attention is currently required from: pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32283 ) Change subject: ggsn: Speed up tests by lowering retransmission T3,N3 timers ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32283/comment/36f78293_c1f2… PS3, Line 9: GGSN_Tests.default GGSN_Tests.cfg ? -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32283 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I3798fba89c2c357afeaa83a73759442c6c433cea Gerrit-Change-Number: 32283 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 17 Apr 2023 18:00:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: codec: add SID preening functions for FR & EFR
by falconia 17 Apr '23

17 Apr '23

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/32184 to look at the new patch set (#5). Change subject: codec: add SID preening functions for FR & EFR ...................................................................... codec: add SID preening functions for FR & EFR Those network elements which receive a stream of codec frames that may come from the uplink of GSM call A and which are responsible for preparing the frame stream for the downlink of GSM call B (OsmoMGW feeding TRAU-DL, or OsmoBTS receiving RTP and feeding DL to its PHY) must be prepared for the possibility that their incoming frame stream may contain corrupted SID frames, presumably from bit errors on radio link A. Per the rules of section 6.1.1 of GSM 06.31 for FR and GSM 06.81 for EFR, SID frames with just one errored bit are still to be accepted as valid, whereas frames with more corrupted bits which are still recognizable as SID are classified as invalid SID. In the case of a TrFO call, the entity switching from leg A UL to leg B DL is responsible for *not* transmitting invalid SID frames on the destination leg (they should be treated like BFIs), and any deemed-valid SID frames that are forwarded should be preened, correcting that one bit error they may exhibit. The functions added here provide that functionality. Change-Id: Iec5c1f2619a82499f61cb3e5a7cd03ff0f020ad8 --- M include/osmocom/codec/codec.h M src/codec/gsm610.c M src/codec/gsm660.c 3 files changed, 114 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/84/32184/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32184 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iec5c1f2619a82499f61cb3e5a7cd03ff0f020ad8 Gerrit-Change-Number: 32184 Gerrit-PatchSet: 5 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in libosmocore[master]: codec: add SID preening functions for FR & EFR
by falconia 17 Apr '23

17 Apr '23

Attention is currently required from: laforge, pespin. falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32184 ) Change subject: codec: add SID preening functions for FR & EFR ...................................................................... Patch Set 4: (1 comment) File src/codec/gsm610.c: https://gerrit.osmocom.org/c/libosmocore/+/32184/comment/02277292_9e454ab0 PS4, Line 432: return false; > I think the compiler knows this can never be reached and hence there's no need for explicit return h […] Looking closer, I see that you are right - the macro definition of OSMO_ASSERT() and the declaration of osmo_panic() combined together do give the compiler enough info that no return is possible from that OSMO_ASSERT(0) call. I am submitting another patch version with those superfluous lines removed. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32184 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iec5c1f2619a82499f61cb3e5a7cd03ff0f020ad8 Gerrit-Change-Number: 32184 Gerrit-PatchSet: 4 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 17 Apr 2023 17:28:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-abis[master]: osmo_rtp2trau() for FR & EFR: set SP=0 in DL if the frame is a SID
by falconia 17 Apr '23

17 Apr '23

falconia has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32079 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: osmo_rtp2trau() for FR & EFR: set SP=0 in DL if the frame is a SID ...................................................................... osmo_rtp2trau() for FR & EFR: set SP=0 in DL if the frame is a SID The SP bit in TRAU DL frames is supposed to be 1 for regular speech frames and 0 for SID frames. The previous code always set SP=1; the present change sets this flag per the spec (TS 48.060) using osmo_{fr,efr}_check_sid() functions from libosmocodec. The present change does not affect TRAU UL frame generation - correct generation of SID bits in the TRAU UL frame is still missing and would require further work if someone wishes to use Osmocom software to implement a new T1/E1 BTS or some other entity that needs to generate TRAU UL frames. Change-Id: I371f8072c0afedbd1b065efecfed3734fb6d31ab --- M src/Makefile.am M src/trau/trau_rtp_conv.c 2 files changed, 33 insertions(+), 7 deletions(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/Makefile.am b/src/Makefile.am index 2c99150..e1a3495 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -40,7 +40,7 @@ -version-info $(TRAU_LIBVERSION) \ -no-undefined \ $(NULL) -libosmotrau_la_LIBADD = $(COMMONLIBS) $(ORTP_LIBS) +libosmotrau_la_LIBADD = $(COMMONLIBS) $(LIBOSMOCODEC_LIBS) $(ORTP_LIBS) libosmotrau_la_SOURCES = trau/osmo_ortp.c \ trau/trau_frame.c \ trau/trau_pcu_ericsson.c \ diff --git a/src/trau/trau_rtp_conv.c b/src/trau/trau_rtp_conv.c index a434c2a..e08b0b9 100644 --- a/src/trau/trau_rtp_conv.c +++ b/src/trau/trau_rtp_conv.c @@ -269,8 +269,6 @@ /* FR Data Bits according to TS 48.060 Section 5.5.1.1.2 */ - /* FIXME: Generate SID frames? */ - /* set c-bits and t-bits */ if (tf->dir == OSMO_TRAU_DIR_UL) { /* C1 .. C5: FR UL */ @@ -302,14 +300,19 @@ tf->c_bits[11] = 1; /* C12: BFI */ else tf->c_bits[11] = 0; /* C12: BFI */ + /* FIXME: set C13 & C14 per GSM 06.31 section 6.1.1 */ tf->c_bits[12] = 0; /* C13: SID=0 */ tf->c_bits[13] = 0; /* C14: SID=0 */ tf->c_bits[14] = 0; /* C15: TAF (SACCH or not) */ tf->c_bits[15] = 1; /* C16: spare */ tf->c_bits[16] = 0; /* C17: DTXd not applied */ } else { - memset(&tf->c_bits[11], 1, 10); /* C12 .. C15: spare */ - tf->c_bits[15] = 1; /* C16: SP=1 */ + memset(&tf->c_bits[11], 1, 4); /* C12 .. C15: spare */ + if (data_len && osmo_fr_check_sid(data, data_len)) + tf->c_bits[15] = 0; /* C16: SP=0 */ + else + tf->c_bits[15] = 1; /* C16: SP=1 */ + tf->c_bits[16] = 1; /* C17: spare */ } memset(&tf->c_bits[17], 1, 4); /* C18 .. C21: spare */ memset(&tf->t_bits[0], 1, 4); @@ -462,7 +465,7 @@ tf->c_bits[11] = 1; /* C12: BFI=1 */ else tf->c_bits[11] = 0; /* C12: BFI=1 */ - /* FIXME: Generate SID frames? */ + /* FIXME: set C13 & C14 per GSM 06.81 section 6.1.1 */ tf->c_bits[12] = 0; /* C13: SID=0 */ tf->c_bits[13] = 0; /* C14: SID=0 */ tf->c_bits[14] = 0; /* C15: TAF (SACCH) */ @@ -471,7 +474,10 @@ } else { tf->c_bits[11] = 1; /* C12: UFE (good uplink) */ memset(&tf->c_bits[12], 1, 3); /* C13 .. C15: spare */ - tf->c_bits[15] = 1; /* C16: SP=1 */ + if (data_len && osmo_efr_check_sid(data, data_len)) + tf->c_bits[15] = 0; /* C16: SP=0 */ + else + tf->c_bits[15] = 1; /* C16: SP=1 */ tf->c_bits[16] = 1; /* C17: spare */ } memset(&tf->c_bits[17], 1, 4); /* C18 .. C21: spare */ -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32079 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I371f8072c0afedbd1b065efecfed3734fb6d31ab Gerrit-Change-Number: 32079 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in libosmo-abis[master]: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case
by falconia 17 Apr '23

17 Apr '23

falconia has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32078 ) Change subject: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case ...................................................................... osmo_rtp2trau() for FR & EFR: correctly handle the no-data case 3GPP TS 48.060 sections 6.5.2 and 6.5.3 prescribe the behavior of TRAU frame emitters under conditions of not having any speech or SID frame to send. In the case of osmo_rtp2trau() this no-data condition is indicated with a zero rtp_len argument to the function, but this case was not handled correctly: a garbage speech frame (previous buffer content for the D bits in struct osmo_trau_frame) was being generated. The present change implements the rules of TS 48.060 sections 6.5.2 and 6.5.3: standard FR/EFR speech frame with BFI=1 for UL, special idle speech frame pattern for DL. Change-Id: I7f21e2210bba9ef87f1af515a001a0cfc1c0a1d8 --- M src/trau/trau_rtp_conv.c 1 file changed, 60 insertions(+), 16 deletions(-) Approvals: Jenkins Builder: Verified falconia: Looks good to me, approved diff --git a/src/trau/trau_rtp_conv.c b/src/trau/trau_rtp_conv.c index ebbdbef..a434c2a 100644 --- a/src/trau/trau_rtp_conv.c +++ b/src/trau/trau_rtp_conv.c @@ -273,19 +273,28 @@ /* set c-bits and t-bits */ if (tf->dir == OSMO_TRAU_DIR_UL) { - /* C1 .. C5 */ + /* C1 .. C5: FR UL */ tf->c_bits[0] = 0; tf->c_bits[1] = 0; tf->c_bits[2] = 0; tf->c_bits[3] = 1; tf->c_bits[4] = 0; - } else { - /* C1 .. C5 */ - tf->c_bits[0] = 1; - tf->c_bits[1] = 1; - tf->c_bits[2] = 1; - tf->c_bits[3] = 0; - tf->c_bits[4] = 0; + } else { /* DL */ + if (data_len == 0) { + /* C1 .. C5: idle speech */ + tf->c_bits[0] = 0; + tf->c_bits[1] = 1; + tf->c_bits[2] = 1; + tf->c_bits[3] = 1; + tf->c_bits[4] = 0; + } else { + /* C1 .. C5: FR DL */ + tf->c_bits[0] = 1; + tf->c_bits[1] = 1; + tf->c_bits[2] = 1; + tf->c_bits[3] = 0; + tf->c_bits[4] = 0; + } } memset(&tf->c_bits[5], 0, 6); /* C6 .. C11: Time Alignment */ if (tf->dir == OSMO_TRAU_DIR_UL) { @@ -302,11 +311,14 @@ memset(&tf->c_bits[11], 1, 10); /* C12 .. C15: spare */ tf->c_bits[15] = 1; /* C16: SP=1 */ } - memset(&tf->c_bits[17], 1, 4); /* C18 .. C12: spare */ + memset(&tf->c_bits[17], 1, 4); /* C18 .. C21: spare */ memset(&tf->t_bits[0], 1, 4); - if (!data_len) + if (!data_len) { + /* idle speech frame if DL, BFI speech frame if UL */ + memset(&tf->d_bits[0], 1, 260); return 0; + } /* reassemble d-bits */ i = 0; /* counts bits */ @@ -428,11 +440,21 @@ /* FR Data Bits according to TS 48.060 Section 5.5.1.1.2 */ /* set c-bits and t-bits */ - tf->c_bits[0] = 1; - tf->c_bits[1] = 1; - tf->c_bits[2] = 0; - tf->c_bits[3] = 1; - tf->c_bits[4] = 0; + if (data_len == 0 && tf->dir == OSMO_TRAU_DIR_DL) { + /* C1 .. C5: idle speech */ + tf->c_bits[0] = 0; + tf->c_bits[1] = 1; + tf->c_bits[2] = 1; + tf->c_bits[3] = 1; + tf->c_bits[4] = 0; + } else { + /* C1 .. C5: EFR */ + tf->c_bits[0] = 1; + tf->c_bits[1] = 1; + tf->c_bits[2] = 0; + tf->c_bits[3] = 1; + tf->c_bits[4] = 0; + } memset(&tf->c_bits[5], 0, 6); /* C6 .. C11: Time Alignment */ if (tf->dir == OSMO_TRAU_DIR_UL) { @@ -455,8 +477,11 @@ memset(&tf->c_bits[17], 1, 4); /* C18 .. C21: spare */ memset(&tf->t_bits[0], 1, 4); - if (data_len == 0) + if (data_len == 0) { + /* idle speech frame if DL, BFI speech frame if UL */ + memset(&tf->d_bits[0], 1, 260); return 0; + } /* reassemble d-bits */ tf->d_bits[0] = 1; -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32078 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7f21e2210bba9ef87f1af515a001a0cfc1c0a1d8 Gerrit-Change-Number: 32078 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in libosmo-abis[master]: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case
by falconia 17 Apr '23

17 Apr '23

Attention is currently required from: neels, laforge. falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32078 ) Change subject: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32078 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7f21e2210bba9ef87f1af515a001a0cfc1c0a1d8 Gerrit-Change-Number: 32078 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 17 Apr 2023 17:10:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmo-abis[master]: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case
by falconia 17 Apr '23

17 Apr '23

Attention is currently required from: falconia, neels, laforge. Hello Jenkins Builder, laforge, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-abis/+/32078 to look at the new patch set (#3). Change subject: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case ...................................................................... osmo_rtp2trau() for FR & EFR: correctly handle the no-data case 3GPP TS 48.060 sections 6.5.2 and 6.5.3 prescribe the behavior of TRAU frame emitters under conditions of not having any speech or SID frame to send. In the case of osmo_rtp2trau() this no-data condition is indicated with a zero rtp_len argument to the function, but this case was not handled correctly: a garbage speech frame (previous buffer content for the D bits in struct osmo_trau_frame) was being generated. The present change implements the rules of TS 48.060 sections 6.5.2 and 6.5.3: standard FR/EFR speech frame with BFI=1 for UL, special idle speech frame pattern for DL. Change-Id: I7f21e2210bba9ef87f1af515a001a0cfc1c0a1d8 --- M src/trau/trau_rtp_conv.c 1 file changed, 60 insertions(+), 16 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/78/32078/3 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32078 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7f21e2210bba9ef87f1af515a001a0cfc1c0a1d8 Gerrit-Change-Number: 32078 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-pcu[master]: ms: store in bts->ms_list during alloc/destroy of ms object
by pespin 17 Apr '23

17 Apr '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/32346 ) Change subject: ms: store in bts->ms_list during alloc/destroy of ms object ...................................................................... ms: store in bts->ms_list during alloc/destroy of ms object With this change the MS no longer is removed from the llist without potentially skipping free() if not idle in bts_ms_idle_cb(). As a result, some unit tests now can free it during bts tear down instead of having them leaked. The tests int MsTest need changes because the tbfs created are fake and cannot be freed using tbf_free(), and hence cannot be detached from MS using regular code paths. Instead first call explicit talloc_free(ms) like other unit tests in the file already do. Change-Id: Id53f8dfb9963366dd4b19a324615bbc83c4f23e7 --- M src/bts.cpp M src/gprs_ms.c M tests/ms/MsTest.cpp M tests/ms/MsTest.err M tests/ulc/PdchUlcTest.err 5 files changed, 33 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/46/32346/1 diff --git a/src/bts.cpp b/src/bts.cpp index 84eea08..3b27676 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -244,7 +244,6 @@ while ((ms = llist_first_entry_or_null(&bts->ms_list, struct GprsMs, list))) { ms_set_callback(ms, NULL); ms_set_timeout(ms, 0); - llist_del(&ms->list); bts_stat_item_dec(bts, STAT_MS_PRESENT); talloc_free(ms); } @@ -1201,7 +1200,6 @@ static void bts_ms_idle_cb(struct GprsMs *ms) { - llist_del(&ms->list); bts_stat_item_dec(ms->bts, STAT_MS_PRESENT); if (ms_is_idle(ms)) talloc_free(ms); @@ -1225,7 +1223,6 @@ ms_set_callback(ms, &bts_ms_cb); ms_set_timeout(ms, osmo_tdef_get(bts->pcu->T_defs, -2030, OSMO_TDEF_S, -1)); - llist_add(&ms->list, &bts->ms_list); bts_stat_item_inc(bts, STAT_MS_PRESENT); return ms; diff --git a/src/gprs_ms.c b/src/gprs_ms.c index b48961b..48867fd 100644 --- a/src/gprs_ms.c +++ b/src/gprs_ms.c @@ -110,6 +110,8 @@ talloc_set_destructor(ms, ms_talloc_destructor); + llist_add(&ms->list, &bts->ms_list); + ms->bts = bts; ms->cb = gprs_default_cb; ms->tlli = GSM_RESERVED_TMSI; @@ -119,7 +121,6 @@ ms->current_cs_ul = UNKNOWN; ms->current_cs_dl = UNKNOWN; ms->is_idle = true; - INIT_LLIST_HEAD(&ms->list); INIT_LLIST_HEAD(&ms->old_tbfs); int codel_interval = LLC_CODEL_USE_DEFAULT; @@ -158,6 +159,8 @@ LOGPMS(ms, DRLCMAC, LOGL_INFO, "Destroying MS object\n"); + llist_del(&ms->list); + ms_set_reserved_slots(ms, NULL, 0, 0); osmo_timer_del(&ms->timer); diff --git a/tests/ms/MsTest.cpp b/tests/ms/MsTest.cpp index 45051d4..84cb52e 100644 --- a/tests/ms/MsTest.cpp +++ b/tests/ms/MsTest.cpp @@ -457,6 +457,7 @@ ms = bts_get_ms_by_tlli(bts, tlli + 1, GSM_RESERVED_TMSI); OSMO_ASSERT(ms == NULL); + talloc_free(ms); talloc_free(ul_tbf); talloc_free(bts); printf("=== end %s ===\n", __func__); @@ -548,6 +549,7 @@ OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms, ms_mode(ms))) == 2); + talloc_free(ms); talloc_free(dl_tbf); talloc_free(bts); printf("=== end %s ===\n", __func__); @@ -617,6 +619,8 @@ ms_set_mode(ms2, GPRS); dump_ms(ms2, "2: after mode set "); + talloc_free(ms1); + talloc_free(ms2); talloc_free(dl_tbf); talloc_free(bts); printf("=== end %s ===\n", __func__); diff --git a/tests/ms/MsTest.err b/tests/ms/MsTest.err index 218ae59..d667740 100644 --- a/tests/ms/MsTest.err +++ b/tests/ms/MsTest.err @@ -61,8 +61,13 @@ Creating MS object The MS object cannot fully confirm an unexpected TLLI: 0xffeeddbb, partly confirmed MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0) Attaching DL TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xffeeddbb) +MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0:DL) Destroying MS object +MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0) Detaching TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xffeeddbb) Creating MS object The MS object cannot fully confirm an unexpected TLLI: 0xdeadbeef, partly confirmed Creating MS object The MS object cannot fully confirm an unexpected TLLI: 0xdeadbef0, partly confirmed MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0) Attaching DL TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xdeadbef0) +MS(TLLI-0xdeadbeef:TA-220:MSCLS-0-0) Destroying MS object +MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0:DL) Destroying MS object +MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0) Detaching TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xdeadbef0) diff --git a/tests/ulc/PdchUlcTest.err b/tests/ulc/PdchUlcTest.err index a2ec373..41aa9b4 100644 --- a/tests/ulc/PdchUlcTest.err +++ b/tests/ulc/PdchUlcTest.err @@ -72,6 +72,8 @@ PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=34, reason=UL_ASS): TBF(DL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0x12345678) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=39 is still reserved! PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=39, reason=UL_ASS): TBF(DL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0x12345678) +MS(TLLI-0x12345678:TA-220:MSCLS-0-0:DL) Destroying MS object +MS(TLLI-0x12345678:TA-220:MSCLS-0-0) Detaching TBF: TBF(DL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0x12345678) PDCH(bts=0,trx=0,ts=0) PDCH state: disabled => enabled PDCH(bts=0,trx=0,ts=0) Reserving FN 104 for type SBA PDCH(bts=0,trx=0,ts=0) Reserving FN 117 for type SBA -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/32346 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Id53f8dfb9963366dd4b19a324615bbc83c4f23e7 Gerrit-Change-Number: 32346 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[L] Change in osmo-pcu[master]: ms: Merge ms_storage into bts.cpp
by pespin 17 Apr '23

17 Apr '23

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/32344 to look at the new patch set (#3). Change subject: ms: Merge ms_storage into bts.cpp ...................................................................... ms: Merge ms_storage into bts.cpp That class is mostly a C++ class holding a llist plus some callbacks. Having that in a separate class makes code more complex for no good reason. This patch moves the llist into bts and allocates stuff directly from within bts. This will allow further cleanup of MS lieficyle in future patches. Change-Id: I627f5db5073189c23ddf2b7f09c90abb24846f62 --- M debian/copyright M src/Makefile.am M src/bts.cpp M src/bts.h M src/gprs_bssgp_rim.c M src/gprs_ms.c D src/gprs_ms_storage.cpp D src/gprs_ms_storage.h M src/pcu_l1_if.cpp M src/pcu_vty_functions.cpp M src/pdch.cpp M src/tbf.cpp M src/tbf_dl.cpp M tests/ms/MsTest.cpp M tests/tbf/TbfTest.cpp 15 files changed, 128 insertions(+), 259 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/44/32344/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/32344 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I627f5db5073189c23ddf2b7f09c90abb24846f62 Gerrit-Change-Number: 32344 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0