Attention is currently required from: neels, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32151 )
Change subject: SCCP N-PCSTATE: trigger MSC status on PC availability
......................................................................
Patch Set 3:
(1 comment)
File src/osmo-bsc/osmo_bsc_sigtran.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/32151/comment/88ee2f42_8a1e6b87
PS3, Line 224: connected = false;
I find all this pretty confusing. Cannot you have different connected_* variable and then a final merged "connected" one?
Having "connected" and "disconnected" in different variables feels like Schrödinger cat. Can it happen that (connected && disconnected) == true?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32151
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3a0869598b8395601a16d78dbc46eec400c0ea84
Gerrit-Change-Number: 32151
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 05 Apr 2023 14:39:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32218 )
Change subject: mgcp_codec: fix codec decision
......................................................................
Patch Set 2:
(8 comments)
File include/osmocom/mgcp/mgcp_codec.h:
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/dda23540_0adc1624
PS2, Line 16: int mgcp_codec_decide(struct mgcp_conn_rtp *conn, struct mgcp_conn_rtp *conn_opposite);
my understanding is that this decides which codec is used in one direction between conn A and conn B, hence between conn_src and conn_dst, am I correct?
If that's the case, naming the conns "conn_src" and "conn_dst" may be a lot clearer.
File src/libosmo-mgcp/mgcp_codec.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/86ee1800_07089044
PS2, Line 440: conn->end.codec = mgcp_codec_find_same(conn, found_same_codec_opposite);
I wonder why do you need to call mgcp_codec_find_same() twice here, something looks wrong imho.
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/abbd313c_4c481666
PS2, Line 449: found_same_codec_opposite = codec_find_convertible(conn_opposite, &conn->end.codecs[i]);
I wonder why do you need to call codec_find_convertible() twice here, something looks wrong imho.
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/a17adc26_d74c2c31
PS2, Line 461: }
IIUC you are finding/selecting/setting the same codec A->B and B->A here, but I wonder if that really makes sense?
Couldn't A->B and B->A be different? I think so.
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/9fd74e24_397860ea
PS2, Line 514: /* Lookup a suitable codec in the destination connection. */
I see no lookup being done here anymore.
File src/libosmo-mgcp/mgcp_protocol.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/9f97142b_40e3c482
PS2, Line 809: conn_opposite = mgcp_find_dst_conn(conn->conn);
see, here we have a clear concept of conn_src ("conn" here) and conn_dst (returned by mgcp_find_dst_conn).
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/be543e56_5bc3d460
PS2, Line 816: if (mgcp_codec_decide(conn, conn_rtp_opposite) != 0)
in here you are deciding what codec is used to forward/transmit from conn_src to conn_dst.
File src/libosmo-mgcp/mgcp_vty.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/5eb573ba_7c4d3a95
PS2, Line 703: "allow-transcoding", "Allow transcoding\n")
Add vty_out telling the user that this command is deprecated and is a NO-OP, so that they can drop it from their config.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32218
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c3291f825488e5d8ce136aeb18450156794aeb5
Gerrit-Change-Number: 32218
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 05 Apr 2023 14:27:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin, fixeria.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 )
Change subject: logging: add 'logging timezone (localtime|utc)'
......................................................................
Patch Set 2:
(1 comment)
File src/core/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/f2980c3a_c0f6d239
PS1, Line 874: target->timezone = timezone;
> Going for the bool value local vs. UTC might avoid the entire isue altogether. […]
The bool vs enum aspect doesn't really affect the aspect raised by pespin, does it?
Zooming out a bit, the main purpose of this patch is to add a handful of vty script to logging_vty_test.vty. Maybe the complexity is outgrowing the benefit, and we should simply not test the timestamp format in a .vty test? Then we can skip this API entirely.
All log_set_..() functions so far just set a config option and return void. It would be a first to add a rc here.
Comparing to log_set_category_filter(): if the caller passes an invalid category there, we OSMO_ASSERT() and quit the program.
These ideas:
If the selected function (gmtime_r() or localtime_r()) was not supported at compile time, then at runtime, one of:
- OSMO_ASSERT(false) in logging.c when logging happens
- omit timestamps in log output
- vty_out("err") and return VTY_CMD_WARNING so starting the application fails.
I'd argue for: don't show timestamps. Simple and least adverse effects.
Is this important enough for pivoting CN component operation on that?
I don't like to add a switch() here that mimicks the switch() in logging.c because it is code dup:
switch (target->timezone) {
case LOG_TIMEZONE_LOCALTIME:
#ifdef HAVE_LOCALTIME_R
return 0;
#endif
case LOG_TIMEZONE_UTC:
#ifdef HAVE_GMTIME_R
return 0;
#endif
default:
return -1;
}
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4
Gerrit-Change-Number: 32043
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 05 Apr 2023 13:42:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/docker-playground/+/32223 )
Change subject: debian-bullseye-jenkins: add iproute2
......................................................................
debian-bullseye-jenkins: add iproute2
Add iproute2 to make "ss" available. It is used in the openbsc testsuite
to dump socket statistics. If it isn't available, it does not cause
tests to fail, but prints an error message that looks like it might be
the real cause for the test failures:
> ss -tn
/bin/sh: 1: ss: not found
I've also asked Harald if we should just retire the master-openbsc job,
but we want to keep it around to ensure new libosmocore builds with this
old codebase.
Change-Id: I306caeda9bcd4020734eb3b343420c8115958ded
---
M debian-bullseye-jenkins/Dockerfile
1 file changed, 22 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/docker-playground refs/changes/23/32223/1
diff --git a/debian-bullseye-jenkins/Dockerfile b/debian-bullseye-jenkins/Dockerfile
index 7c02cbe..2edd3f6 100644
--- a/debian-bullseye-jenkins/Dockerfile
+++ b/debian-bullseye-jenkins/Dockerfile
@@ -53,6 +53,7 @@
graphviz \
htop \
inkscape \
+ iproute2 \
latexmk \
lcov \
libaio-dev \
--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/32223
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I306caeda9bcd4020734eb3b343420c8115958ded
Gerrit-Change-Number: 32223
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newchange