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

Change in osmo-bsc[master]: bsc_ctrl_commands: Add GET for bts neighbor-list (local bts numbers)

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/30982 ) Change subject: bsc_ctrl_commands: Add GET for bts neighbor-list (local bts numbers) ...................................................................... Patch Set 1: Code-Review-1 (8 comments) File src/osmo-bsc/bts_ctrl.c: https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/b9aa24e8_15da6ebb PS1, Line 587: OSMO_STRBUF_PRINTF(*csv, "%" PRIu8 ",", n->bts_nr); You actually need to print bts->nr, not bts->bts_nr. The later is the bts id in the specific BTS<->BSC link, which is usually 0. "nr" is the ID you see in the VTY config and which identifies the BTS within the BSC. https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/354204ec_b593d6c2 PS1, Line 607: if (csv.chars_needed >= csv.len) { /* Just to be save, although this is very unlikely */ It's impossible that this is triggered. We can only have up to 255 BTS (limited by uint8_t bts->nr). Hence, even if all of the ids would be 3 digits, the string would be 255*3 + 255 (for commas), so we can never reach 4096. You can even lower the buffer up to 1024 actually. https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/20a71361_dbfa986f PS1, Line 614: write_neighbors(&bts->neighbors, &csv); since you no longer need this function twice, just drop te function and inline it's just a loop. https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/381461b7_e6929272 PS1, Line 617: cmd->reply = "No BTS neighbors configured through BTS number"; return empty string here, it's way easier to parse by clients. https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/c481a1de_d28cdc62 PS1, Line 630: CTRL_CMD_DEFINE_RO(bts_neighbor_list, "neighbor-list"); My understanding is that we already have "bts.N.neighbor-bts.add" and "bts.N.neighbor-bts.del", so adding here a "bts.N.neighbor-bts.bts-list" cmd would make more sense. You could also name it "bts.N.neighbor-bts.by-bts-nr" File tests/ctrl/osmo-bsc-neighbor-list.cfg: https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/f27dc1e3_74eaee4d PS1, Line 1: network Please if possible drop all config lines which are not really needed/used by the test. File tests/ctrl_test_runner.py: https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/b9514dfd_3eb3172a PS1, Line 529: Unrelated change, submit a new patch if needed. https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/17f884e1_66a13067 PS1, Line 580: self.assertEqual(r['value'], 'No BTS neighbors configured through BTS number') this makes it difficult for the client. Simply return an empty string in osmo-bsc in this case. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/30982 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I79aeffd93089086f57c66787fe20b439a4d8b6b4 Gerrit-Change-Number: 30982 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Jan 2023 08:53:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in mncc-python[master]: MNCC data size check: allow trailing data

by osmith

osmith has submitted this change. ( https://gerrit.osmocom.org/c/mncc-python/+/30857 ) Change subject: MNCC data size check: allow trailing data ...................................................................... MNCC data size check: allow trailing data Verify is the parsed data is at least the size of the struct, not exactly the size. Make it accept messages with additional data, like the SDP information the TTCN-3 testsuite is sending since Ic9568c8927507e161aadfad1a4d20aa896d8ae30. This change makes the size checks consistent with the other size checks in MNCC implementations such as osmo-sip-connector Related: OS#4282 Related: osmo-sip-connector I522ce7f206932a816a64f03d916799c3215bb8c7 Change-Id: Ic8c24e6988ae2d3c4278c4adccae46e248c893b8 --- M mncc_sock.py 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: pespin: Looks good to me, but someone else must approve daniel: Looks good to me, but someone else must approve osmith: Looks good to me, approved; Verified diff --git a/mncc_sock.py b/mncc_sock.py index b2d0705..cb10449 100644 --- a/mncc_sock.py +++ b/mncc_sock.py @@ -142,7 +142,7 @@ '(0x%04x vs 0x%04x)\n' % (msg.version, mncc.MNCC_SOCK_VERSION)) # Match expected message sizes / offsets - if (msg.mncc_size != ctypes.sizeof(mncc.gsm_mncc) or + if (msg.mncc_size < ctypes.sizeof(mncc.gsm_mncc) or msg.data_frame_size != ctypes.sizeof(mncc.gsm_data_frame) or msg.called_offset != mncc.gsm_mncc.called.offset or msg.signal_offset != mncc.gsm_mncc.signal.offset or -- To view, visit https://gerrit.osmocom.org/c/mncc-python/+/30857 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: mncc-python Gerrit-Branch: master Gerrit-Change-Id: Ic8c24e6988ae2d3c4278c4adccae46e248c893b8 Gerrit-Change-Number: 30857 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 5 months

1
0
0 0

Change in mncc-python[master]: MNCC data size check: allow trailing data

by osmith

Attention is currently required from: laforge. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/mncc-python/+/30857 ) Change subject: MNCC data size check: allow trailing data ...................................................................... Patch Set 1: Verified+1 -- To view, visit https://gerrit.osmocom.org/c/mncc-python/+/30857 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: mncc-python Gerrit-Branch: master Gerrit-Change-Id: Ic8c24e6988ae2d3c4278c4adccae46e248c893b8 Gerrit-Change-Number: 30857 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 16 Jan 2023 08:46:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in mncc-python[master]: MNCC data size check: allow trailing data

by osmith

Attention is currently required from: laforge. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/mncc-python/+/30857 ) Change subject: MNCC data size check: allow trailing data ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/mncc-python/+/30857 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: mncc-python Gerrit-Branch: master Gerrit-Change-Id: Ic8c24e6988ae2d3c4278c4adccae46e248c893b8 Gerrit-Change-Number: 30857 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 16 Jan 2023 08:45:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-gsm-tester[master]: jenkins-build-common: use 'git submodule update'

by osmith

osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-gsm-tester/+/30781 ) Change subject: jenkins-build-common: use 'git submodule update' ...................................................................... jenkins-build-common: use 'git submodule update' Now that we use git submodules in osmo-trx, make sure to initialize and update them before attempting to build. Fix for: Makefile.am:32: error: required directory ./osmocom-bb/src/host/trxcon does not exist Change-Id: Ic2b9207b942a8a9edff82737117b2ed9d6d3cfe3 --- M contrib/jenkins-build-common.sh 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, approved; Verified diff --git a/contrib/jenkins-build-common.sh b/contrib/jenkins-build-common.sh index 4a17d4d..b41fc05 100644 --- a/contrib/jenkins-build-common.sh +++ b/contrib/jenkins-build-common.sh @@ -102,6 +102,8 @@ echo "$(git rev-parse HEAD) $repo" >> "$prefix_real/${name}_git_hashes.txt" + git submodule update --init + cd "$base" } -- To view, visit https://gerrit.osmocom.org/c/osmo-gsm-tester/+/30781 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Change-Id: Ic2b9207b942a8a9edff82737117b2ed9d6d3cfe3 Gerrit-Change-Number: 30781 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 5 months

1
0
0 0

Change in osmo-gsm-tester[master]: jenkins-build-common: use 'git submodule update'

by osmith

osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-gsm-tester/+/30781 ) Change subject: jenkins-build-common: use 'git submodule update' ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-gsm-tester/+/30781 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Change-Id: Ic2b9207b942a8a9edff82737117b2ed9d6d3cfe3 Gerrit-Change-Number: 30781 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Jan 2023 08:41:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmo-gsm-manuals[master]: rate counter: add StatsD note

by osmith

Attention is currently required from: msuraev. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/30715 ) Change subject: rate counter: add StatsD note ...................................................................... Patch Set 3: (2 comments) File common/chapters/counters-overview.adoc: https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/30715/comment/8c052da2_9781… PS2, Line 127: does > The agreed upon limit is 120 which is way more convenient than 80. I can see that it's more convenient for code, but IMHO text is more readable with lines that aren't as long. The coding standards says _up_to_ 120 characters, as I understand it's fine to use less characters in a line where it makes sense. I'd keep it at 80 characters for readability and so it's consistent with the rest of the file. But this is cosmetics, if you insist then just ignore this review point. https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/30715/comment/dac1989b_8dbc… PS2, Line 129: StatsD > The rest of the file can be fixed in a separate commit (unless we're talking about the name of the b […] I mean, it would be good to have consistent spelling of it. in your patch its "statsd", "StatsD" and "statsD" ;) So I suggest using the spelling that was already used in this document for consistency, which is "statsd". If you want to change it in the existing document, IMHO it should be done as separate patch before this one to keep consistency. -- To view, visit https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/30715 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-gsm-manuals Gerrit-Branch: master Gerrit-Change-Id: I124503c3707bbe005abbfb4245abe2829c6ff57c Gerrit-Change-Number: 30715 Gerrit-PatchSet: 3 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Jan 2023 08:26:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: Add missing header dependencies to several files

by fixeria

Attention is currently required from: neels, pespin, msuraev. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30959 ) Change subject: layer23: Add missing header dependencies to several files ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > the aim for .h files is to minimize the amount of other headers included. Instead, . […] Agreeing with Neels. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30959 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9819b12d1c24f6ee197daa887452b09418d689e8 Gerrit-Change-Number: 30959 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Jan 2023 07:47:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: mobile: fix -Wlogical-not-parentheses in gsm48_cc_init()

by fixeria

Attention is currently required from: neels, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30963 ) Change subject: mobile: fix -Wlogical-not-parentheses in gsm48_cc_init() ...................................................................... Patch Set 1: (1 comment) File src/host/layer23/src/mobile/gsm48_cc.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30963/comment/bbaa0407_15294d77 PS1, Line 54: if (cc->mncc_upqueue.next != NULL) > Ack Using llist_empty() means doing this: if (!(cc->mncc_upqueue.next == &cc->mncc_upqueue)) so when gsm48_cc_init() is called for the first time: if (!(NULL == &cc->mncc_upqueue)) // some random address from heap this condition will be true, and we will return early. This is wrong. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30963 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ic7ffd3aa25339e24a31bae1b7428f1f93e261858 Gerrit-Change-Number: 30963 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Jan 2023 07:42:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: fix integer overflow in l1ctl_tx_data_req()

by fixeria

Attention is currently required from: neels. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30962 ) Change subject: layer23: fix integer overflow in l1ctl_tx_data_req() ...................................................................... Patch Set 1: (1 comment) File src/host/layer23/src/common/l1ctl.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30962/comment/9bc47f88_0562de3f PS1, Line 397: chan_ts, gsmtap_chan_type, chan_ss, 0, 127, 0, > Because it's Tx data (Uplink), so there cannot be Signal-Noise-Ratio. I mean, it does not make sense to find the correct maximum value for a parameter that makes no sense for Uplink... -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30962 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I23e9ea5ad59099c24db60057c8e7da1e6a0d2293 Gerrit-Change-Number: 30962 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Jan 2023 07:33:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

2025

2024

2023

2022

gerrit-log January 2023