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

[S] Change in libosmocore[master]: bts_features: Add features for HR formats (TS 101813 vs. RFC5993)

by dexter

Attention is currently required from: neels, pespin, fixeria, msuraev. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31415 ) Change subject: bts_features: Add features for HR formats (TS 101813 vs. RFC5993) ...................................................................... Patch Set 8: (1 comment) File include/osmocom/gsm/bts_features.h: https://gerrit.osmocom.org/c/libosmocore/+/31415/comment/379b4fa6_dda6ba66 PS7, Line 39: _TX > A couple of extra points here: […] @neels: You are correct. Thats how it works. (In the DSP case the RTP packets are going through l1sap.c as well, but the DSP is generating the payloads.) @pespin: In a correctly configured setup we should always see the same format in both directions. I also can understand the objections against the "_TX_", so I am fine with dropping that again. I thought the OML feature flags were introduced for things like this. I also see no other way to find out which BTS model supports which format. The BSC knows that it talks to an osmo-bts, but it does not know if this BTS model is a PHY based model with TS 101318 support or an osmo-trx based model with RFC 5993 support. Extending IPACC can be also a way, here we can at least be sure that osmo-bts type BTSs will support the extension but we will have to make sure that older osmo-bts versions will tolerate/ignore the extensions. To me this sounds difficult and since the implementation in osmo-mgw is already done I would opt for the OML flag based method. Please let me know what you think. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31415 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I05e4ed7a85f3a0451de7cd07380503a7ac76d043 Gerrit-Change-Number: 31415 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> 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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:16:57 +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> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 3: (1 comment) File include/osmo-bts/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/80644552_54bd56c5 PS2, Line 8: #define PCU_SOCK_QLENGTH_MAX_DEFAULT 10 > I would say this is rather consistent with https://gerrit.osmocom. […] Yes because that path is used by both ends to connect. It would be like defining a default TCP port here. But this queue length is something really not related to what both ends use to communicate, so it doesn't belong here. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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: Tue, 28 Feb 2023 13:16:24 +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

2 years, 3 months

1
0
0 0

[S] Change in osmo-bsc[master]: bssmap_handle_ass_req_tp_codec_list: tweak log msg

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31550 ) Change subject: bssmap_handle_ass_req_tp_codec_list: tweak log msg ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31550 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Idfab0b7f6e97a6b67d140f967ddfe9b29818586e Gerrit-Change-Number: 31550 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:06:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bsc[master]: rsl_tx_ipacc_crcx/mdcx: omit speech mode for CSD

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31549 ) Change subject: rsl_tx_ipacc_crcx/mdcx: omit speech mode for CSD ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I6641b713177276bcf798f08123e1dd2e88ffdce6 Gerrit-Change-Number: 31549 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:06:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bsc[master]: bssmap_handle_ass_req_ct_data: implement

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31547 ) Change subject: bssmap_handle_ass_req_ct_data: implement ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31547 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I350bea15fd2158eb6edc9bc92f2dca48930736e9 Gerrit-Change-Number: 31547 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:05:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bsc[master]: codec_pref: split test_codec_support_bts_rate

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31544 ) Change subject: codec_pref: split test_codec_support_bts_rate ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31544 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Iea8a23ef3c66ed556110038fe9f3bc7f6eef3e96 Gerrit-Change-Number: 31544 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:05:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bsc[master]: bssmap_handle_ass_req_ct_speech: refactor

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31543 ) Change subject: bssmap_handle_ass_req_ct_speech: refactor ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31543 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I6bb16b26d89056dffa50bd8296fefe9315c587ca Gerrit-Change-Number: 31543 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:04:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by arehbein

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 2: (7 comments) File include/osmo-bts/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/e798b3f5_d70a3eaa PS2, Line 8: #define PCU_SOCK_QLENGTH_MAX_DEFAULT 10 > that's application/peer implementation specific, not really part of the shared protocol and hence sh […] I would say this is rather consistent with https://gerrit.osmocom.org/c/osmo-bts/+/6981 , which put PCU_SOCK_DEFAULT there in the first place. File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/d906cb23_9a0cfe37 PS2, Line 988: if (osmo_wqueue_enqueue(&state->upqueue, msg) == -ENOSPC) { > Simply check for any error "< 0" Done https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/c90c5a50_b825bf92 PS2, Line 989: LOGP(DLGLOBAL, LOGL_NOTICE, "PCU not reacting (more thatn %d messages waiting). Closing connection\n", > max_length is an int? or an unsigned? Yeah it's a bit confusing because `osmo_wqueue_init()` takes a regular `int` for I suppose historical reasons (I pushed a patch for that), so that's what I remembered last. Format specifier should be fixed now https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/10ff445f_2b2a210e PS2, Line 992: } else { > Usually we do early return on error, then drop this "else" block. Done https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/79fa212c_7c519e51 PS2, Line 1092: return -1; > In heere probably -EBADF needs to be returned so that the wqueue potentially calls the write_cb() wi […] It doesn't change the control flow afaik (I looked at `osmo_wqueue_bfd_cb()`). But I could check on the errno stuff in these callbacks and improve on previously existing code https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/d1139b96_f34845bf PS2, Line 1102: osmo_fd_write_disable(&state->upqueue.bfd); > This is wrong. You should disable the bfd after successful write() only if the queue is empty. From my understanding/by looking at this code, it looks like `osmo_fd_write_disable()` is meant to prevent concurrent fd write-access via osmo-wqueue code; with that in mind it would make sense to disable before write. What has changed? I didn't change the logic of this function. https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/242b7e4a_e343f70f PS2, Line 1181: osmo_wqueue_init(&state->upqueue, PCU_SOCK_QLENGTH_MAX_DEFAULT); > I would split this into 2 commits if possible: […] No it's not possible, with the current code the current length of the wqueue is always incremented/there's always a length check. I find `PCU_SOCK_WQUEUE_LEN` misleading, because I would say the queue length is a property subject to change, so maybe `PCU_SOCK_WQUEUE_MAX_LEN` (I would also prefer not omitting the `_DEFAULT` suffix). Or maybe `PCU_SOCK_WQ_LEN_MAX_DEFAULT` ? I don't find `WQ` that absurd considering it's used right around `osmo_wqueue`-identifiers. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 12:54:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by arehbein

Attention is currently required from: arehbein. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 to look at the new patch set (#3). Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... common: Have PCU socket connection use osmo_wqueue Fixes memleak in case of connected PCU process being suspended without proper close on socket Related: OS#5774 Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 --- M include/osmo-bts/pcuif_proto.h M src/common/pcu_sock.c 2 files changed, 59 insertions(+), 67 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/33/31533/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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-MessageType: newpatchset

2 years, 3 months

1
0
0 0

[S] Change in libosmocore[master]: osmo_wqueue_init(): Fix parameter type

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31595 ) Change subject: osmo_wqueue_init(): Fix parameter type ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31595 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I6c9295750e1755bf1d80f3a3b32efc2aefb11a57 Gerrit-Change-Number: 31595 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: Tue, 28 Feb 2023 12:36:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 3 months

1
0
0 0

2025

2024

2023

2022

gerrit-log February 2023