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

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 29 Jun '23

29 Jun '23

Attention is currently required from: osmith, laforge, fixeria. Hello osmith, Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 to look at the new patch set (#4). Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... modem: properly handle Dedicated mode or TBF IE We need to distinguish between Uplink and Downlink TBF assignment in grr_rx_imm_ass(), because matching the Request Reference IE makes sense only for the Uplink TBF assignment. Uplink TBFs are requested by the UEs by sending RACH, while Downlink TBFs are assigned by the network itself. The Request Reference IE is only valid for Uplink assignments and shall be ignored in messages assigning Downlink TBFs. Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Related: OS#5500 --- M src/host/layer23/src/modem/grr.c 1 file changed, 54 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/14/33414/4 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 4 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmocore[master]: gsmtap_source_free(): Don't crash if NULL is passed
by laforge 29 Jun '23

29 Jun '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33531 ) Change subject: gsmtap_source_free(): Don't crash if NULL is passed ...................................................................... gsmtap_source_free(): Don't crash if NULL is passed A function called *free() should always support passing a NULL input value. Change-Id: If0f6c11ac6ad987368e1d5c63a07fc456f89db2a --- M src/core/gsmtap_util.c 1 file changed, 15 insertions(+), 0 deletions(-) Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/core/gsmtap_util.c b/src/core/gsmtap_util.c index a95c795..20c0ce1 100644 --- a/src/core/gsmtap_util.c +++ b/src/core/gsmtap_util.c @@ -531,6 +531,9 @@ void gsmtap_source_free(struct gsmtap_inst *gti) { + if (!gti) + return; + if (gti->ofd_wq_mode) { osmo_fd_unregister(&gti->wq.bfd); osmo_wqueue_clear(&gti->wq); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33531 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: If0f6c11ac6ad987368e1d5c63a07fc456f89db2a Gerrit-Change-Number: 33531 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: osmith, laforge, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/2a51cdda_7d87f8e0 PS3, Line 278: if ((dm_or_tbf & (1 << 1)) == 0) { > I don't see how this improves readability, sorry. […] "check Request Reference IE if this is an Uplink TBF assignment." That's a better and clearer comment than the one you have in the current patch. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 16:40:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 29 Jun '23

29 Jun '23

Attention is currently required from: osmith, laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/49b2475a_4458d86b PS3, Line 278: if ((dm_or_tbf & (1 << 1)) == 0) { > You can use "bool is_ul_ass = (dm_or_tbf & (1 << 1))", or rather in line 277 simply write "Check UL […] I don't see how this improves readability, sorry. A boolean flag would improve a bit, but it's redundant to add a bool for one-time use in the if statement next line. The comment suggestions... there is already a comment stating that we check Request Reference IE if this is an Uplink TBF assignment. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 16:39:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: osmith, laforge, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/79842a9b_b678aca4 PS3, Line 278: if ((dm_or_tbf & (1 << 1)) == 0) { You can use "bool is_ul_ass = (dm_or_tbf & (1 << 1))", or rather in line 277 simply write "Check UL or DL TBF Assignment" and inside the if: /* DL TBF Assignment */ -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 16:15:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 29 Jun '23

29 Jun '23

Attention is currently required from: osmith, laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/1ce75163_7c817ce1 PS3, Line 278: if ((dm_or_tbf & (1 << 1)) == 0) { > it may be clear to you, I see references to UL TBF, assignment, request reference IE and then some b […] Ok, what exactly am I supposed to clarify? How do I make it cleaner? Copy-paste text/tables from 3GPP TS 44.018, section 10.5.2.25b in a comment? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 15:59:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: osmith, laforge, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/2b495544_e0faa6c1 PS3, Line 278: if ((dm_or_tbf & (1 << 1)) == 0) { > The comment clearly states "Uplink TBF", so the if-condition is obviously checking if it's an Uplink […] it may be clear to you, I see references to UL TBF, assignment, request reference IE and then some bitmask check, so not clear at all for the rest :) -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 15:47:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo'
by fixeria 29 Jun '23

29 Jun '23

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33533 ) Change subject: common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo' ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > maybe ping whoever added that line? git blame/log. It was Harald in `430954630`, adding him as a reviewer. I think it's fine to have this line commented out for now because we don't have any probes for `libbts`. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I829c63988fd51b481cb9f13a81dfaf5e78beb1b8 Gerrit-Change-Number: 33533 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 15:40:18 +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 osmo-bts[master]: common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo'
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33533 ) Change subject: common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo' ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: maybe ping whoever added that line? git blame/log. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I829c63988fd51b481cb9f13a81dfaf5e78beb1b8 Gerrit-Change-Number: 33533 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 15:32:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo'
by fixeria 29 Jun '23

29 Jun '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/33533 ) Change subject: common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo' ...................................................................... common/Makefile.am: comment out 'libbts_la_LDADD = probes.lo' This line makes no sense and triggers the following warnings: src/common/Makefile.am:83: warning: variable 'libbts_la_LDADD' is defined but no program or src/common/Makefile.am:83: library has 'libbts_la' as canonical name (possible typo) The problem is that there is no 'libbts.la' (libtool archive), but 'libbts.a' (normal archive file). There is no way to attach dependencies to *.a archives, it's only possible for *.la archives. Either each of the BTS variants needs to add this 'probes.lo' to their LDADD list manually, or the 'libbts.a' needs to be converted to a libtool archive 'libbts.la'. For comment out this line. Change-Id: I829c63988fd51b481cb9f13a81dfaf5e78beb1b8 --- M src/common/Makefile.am 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/33/33533/1 diff --git a/src/common/Makefile.am b/src/common/Makefile.am index 32f644c..0da62bb 100644 --- a/src/common/Makefile.am +++ b/src/common/Makefile.am @@ -80,5 +80,6 @@ $(LIBTOOL) --mode=compile $(AM_V_lt) --tag=CC env CFLAGS="$(CFLAGS)" $(DTRACE) -C -G -s $< -o $@ BUILT_SOURCES = probes.h probes.lo -libbts_la_LDADD = probes.lo +# FIXME: libbts is not a libtool archive (*.la) +# libbts_la_LIBADD = probes.lo endif -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I829c63988fd51b481cb9f13a81dfaf5e78beb1b8 Gerrit-Change-Number: 33533 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0