Attention is currently required from: neels, fixeria, dexter.
Hello Jenkins Builder, neels,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31497
to look at the new patch set (#5).
Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect
......................................................................
pcu_sock: activate/deactivate PDCH on pcu reconnect
When the PCU is disconnected while the BSC keeps running the PDCH should
be closed. Also the PDCH should be reopened when the PCU is
reconnected.
Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678
Related: OS#5198
---
M doc/timeslot.msc
M include/osmocom/bsc/timeslot_fsm.h
M src/osmo-bsc/pcu_sock.c
M src/osmo-bsc/timeslot_fsm.c
4 files changed, 112 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/97/31497/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678
Gerrit-Change-Number: 31497
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, fixeria, msuraev, dexter.
pespin 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/dfc81cd8_142911f5
PS7, Line 39: _TX
> In a correctly configured setup we should always see the same format in both directions.
If you say so... I don't think that's really stated anywhere in the spec, and sounds like a personal expectancy of what a "correctly configured setup" is imho 😊
> I thought the OML feature flags were introduced for things like this
Yes, to know that a BTS supports a given feature. Not to indicate which of the several supported is to be used.
> BTS model is a PHY based model with TS 101318 support or an osmo-trx based model with RFC 5993 support.
So our osmo-bts-sysmo is transmitting different RTP type than our osmo-bts-trx? what a mess! 😮
> The BSC knows that it talks to an osmo-bts, but it does not know if this BTS model
My point is only that you still need to store that in osmo-bsc hardcoded somehow for other types of BTS, like nanobts, etc. so you could simply do the same for osmobts (you will need this for older versions of osmo-bts anyway). Maybe that's actually done using this set of features?
> BTS model is a PHY based model with TS 101318 support or an osmo-trx based model with RFC 5993 support.
So our osmo-bts-sysmo is transmitting different RTP type than our osmo-bts-trx? what a mess! :O
> 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.
Yes, have a look if adding the new IE is really a problem for old versions (I think it won't be an issue). If it was, then you could add a new BTS feature stating whether the BTS supports this new IE and then only send it based on it.
--
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: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 13:33:25 +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
Attention is currently required from: pespin, dexter.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/31214
to look at the new patch set (#10).
Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format
......................................................................
mgcp_sdp: add fmtp string to define HR GSM RTP format
There are two different RTP HR GSM formats defined (TS 101.318 and
RFC5993). Lets add an fmtp parameter In order to select between the
two formats (and convert if necessary) via MGCP/SDP. Unfortunately there
is no official fmtp string defined, so we have to define an osmocom
specific string.
Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4
Related: OS#5688
---
M include/osmocom/mgcp/mgcp_common.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_sdp.c
4 files changed, 102 insertions(+), 17 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/14/31214/10
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4
Gerrit-Change-Number: 31214
Gerrit-PatchSet: 10
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31591 )
Change subject: osmo_wqueue_enqueue(): Avoid redundant if-check
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
I'm not sure it's really worth it and it's clearer the way it is now. So I vote for not merging this. If enough think differently then I won't block.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31591
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I3ecc1698e836ca40e178a5b84c2946ae0e6bbfc9
Gerrit-Change-Number: 31591
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 13:23:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin, dexter.
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 )
Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format
......................................................................
Patch Set 9:
(4 comments)
File src/libosmo-mgcp/mgcp_network.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169):
https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/00e89359_c2fc46e4
PS9, Line 704: switch(msgb_length(msg))
that open brace { should be on the previous line
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169):
https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/dc1b001a_398a1359
PS9, Line 704: switch(msgb_length(msg))
space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169):
https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/cb2a49a9_6640a804
PS9, Line 706: case GSM_HR_BYTES + sizeof(struct rtp_hdr):
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169):
https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/89d6e140_9512b368
PS9, Line 706: case GSM_HR_BYTES + sizeof(struct rtp_hdr):
please, no spaces at the start of a line
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4
Gerrit-Change-Number: 31214
Gerrit-PatchSet: 9
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 13:21:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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: Code-Review-1
(3 comments)
File src/common/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/71841978_da3ed4aa
PS2, Line 1092: return -1;
> It doesn't change the control flow afaik (I looked at `osmo_wqueue_bfd_cb()`). […]
osmo_wqueue_bfd_cb:
if (rc == -EBADF)
goto err_badfd;
You have to return -EBADF while handling the socket fd if you close/free it, otherwise osmo_wqueue_bfd_cb() continues using it and potentially calling the write_cb for a fd/mem which is no longer existing which can cause problems.
https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/1c4ccd75_92870a87
PS2, Line 1102: osmo_fd_write_disable(&state->upqueue.bfd);
> From my understanding/by looking at this code, it looks like `osmo_fd_write_disable()` is meant to p […]
that's not to prevent concurrent write-access. It's to manage the main loop poll() to tell the kernel to trigger us when the socket is available for writing again.
https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/be99ebb8_52a56719
PS2, Line 1181: osmo_wqueue_init(&state->upqueue, PCU_SOCK_QLENGTH_MAX_DEFAULT);
> No it's not possible, with the current code the current length of the wqueue is always incremented/t […]
I'm fine with whatever but use WQUEUE instead of the QLENGTH stuff :)
--
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:21:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment