Attention is currently required from: arehbein, fixeria, laforge, neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email )
Change subject: Introduce per-BTS timers, RLC timer commands
......................................................................
Patch Set 17:
(7 comments)
File include/osmocom/bsc/bts.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/410d863c_6d55ea01
PS12, Line 323: GSM_BTS_TDEF_ID_COUNTDOWN_VALUE
> as alluded to above, I was told it'd be good to use positive values for anything coming from the spe […]
Not "anything coming from the specs", but counters/timers explicitly specified with a given T123 or N123 naming.
Hence, since Countdown value is not aexplicitly defined as a T... or N... it must be done as negative (X...).
File src/osmo-bsc/bsc_init.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/7fb0226f_ec45c056
PS16, Line 240: bts_gprs_timer_groups_init(bts);
> (as mentioned above:) I have moved it to `gsm_bts_alloc()`; the function description seemed like a b […]
Done
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/cb270503_135b1952
PS17, Line 466: bts_gprs_timer_groups_init(bts);
did you make sure all counters set up here are not used beforehand in this function when initializing stuff? I bet some counter may be, so this should be far above.
File src/osmo-bsc/bts_init.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/987e30c1_d72af677
PS12, Line 60: 3101
> (this reply is older, hadn't yet published it...) […]
Done
File src/osmo-bsc/bts_init.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/7e893e33_fe82bedb
PS17, Line 57: .desc = "\"In the extended uplink TBF mode, the uplink TBF may be maintained during temporary inactive periods, "
"Keep uplink TBF alive during inactive periods where the mobile station has no RLC information to transmit (3GPP TS 44.060 Version 6.14.0)"
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/d3a42a61_ee58d91c
PS17, Line 61: .desc = "A delayed release of the downlink TBF is when the release of the downlink TBF is delayed following the transmission of a final data block, "
"Delay release of downlink TBF after all available data has been transmitted"
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/3e9bb2c4_44fcf5d0
PS3, Line 1706: /* TODO (BSSGP timer patch): If argument is one of BSSGP Timers strings, then translate to
> what lines are you referring to? This is just a TODO comment for the BSSGP timer patch
aha, I'll rephrase: so if it's a a patch about RLC, why are you adding a TODO about BSSGP timers here?
You can simplify this patch since you are anyway adding them in a follow up patch, it's not something left over "for later".
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675
Gerrit-Change-Number: 31878
Gerrit-PatchSet: 17
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 05 Dec 2023 11:45:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
lynxis lazus has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35218?usp=email )
Change subject: epdg_gtpc_s2b: add TLV Serving Network
......................................................................
epdg_gtpc_s2b: add TLV Serving Network
The open5gs requires Serving Network TLV.
Change-Id: I2a9459859fc660e6433cd8178ab9d1f92ae74fc0
---
M src/epdg_gtpc_s2b.erl
A src/gtp_utils.erl
2 files changed, 63 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/18/35218/1
diff --git a/src/epdg_gtpc_s2b.erl b/src/epdg_gtpc_s2b.erl
index a5658f1..a71e71d 100644
--- a/src/epdg_gtpc_s2b.erl
+++ b/src/epdg_gtpc_s2b.erl
@@ -59,6 +59,9 @@
%% TODO: make APN configurable? get it from HSS?
-define(APN, <<"internet">>).
+-define(MCC, 901).
+-define(MNC, 42).
+-define(MNC_SIZE, 42).
-record(gtp_state, {
socket,
@@ -330,7 +333,10 @@
#v2_access_point_name{instance = 0, apn = [Apn]},
#v2_selection_mode{mode = 0},
#v2_pdn_address_allocation{type = ipv4, address = <<0,0,0,0>>},
- #v2_bearer_context{group = BearersIE}
+ #v2_bearer_context{group = BearersIE},
+ #v2_serving_network{
+ plmn_id = gtp_utils:plmn_to_bin(?MCC, ?MNC, ?MNC_SIZE)
+ }
],
#gtp{version = v2, type = create_session_request, tei = 0, seq_no = SeqNo, ie = IEs}.
@@ -346,5 +352,3 @@
tei = RemoteCtlTEI,
seq_no = Req#gtp.seq_no,
ie = IEs}.
-
-
diff --git a/src/gtp_utils.erl b/src/gtp_utils.erl
new file mode 100644
index 0000000..b899530
--- /dev/null
+++ b/src/gtp_utils.erl
@@ -0,0 +1,45 @@
+% GTP utilities
+%
+% (C) 2023 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de>
+% Author: Alexander Couzens <lynxis(a)fe80.eu>
+%
+% All Rights Reserved
+%
+% This program is free software; you can redistribute it and/or modify
+% it under the terms of the GNU Affero General Public License as
+% published by the Free Software Foundation; either version 3 of the
+% License, or (at your option) any later version.
+%
+% This program is distributed in the hope that it will be useful,
+% but WITHOUT ANY WARRANTY; without even the implied warranty of
+% MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+% GNU General Public License for more details.
+%
+% You should have received a copy of the GNU Affero General Public License
+% along with this program. If not, see <http://www.gnu.org/licenses/>.
+%
+% Additional Permission under GNU AGPL version 3 section 7:
+%
+% If you modify this Program, or any covered work, by linking or
+% combining it with runtime libraries of Erlang/OTP as released by
+% Ericsson on http://www.erlang.org (or a modified version of these
+% libraries), containing parts covered by the terms of the Erlang Public
+% License (http://www.erlang.org/EPLICENSE), the licensors of this
+% Program grant you additional permission to convey the resulting work
+% without the need to license the runtime libraries of Erlang/OTP under
+% the GNU Affero General Public License. Corresponding Source for a
+% non-source form of such a combination shall include the source code
+% for the parts of the runtime libraries of Erlang/OTP used as well as
+% that of the covered work.
+
+-module(gtp_utils).
+-author('Alexander Couzens <lynxis(a)fe80.eu>').
+
+-export([plmn_to_bin/3]).
+
+% ergw/apps/ergw/test/*.erl
+% under GPLv2+
+plmn_to_bin(CC, NC, NCSize) ->
+ MCC = iolist_to_binary(io_lib:format("~3..0b", [CC])),
+ MNC = iolist_to_binary(io_lib:format("~*..0b", [NCSize, NC])),
+ {MCC, MNC}.
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35218?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: erlang/osmo-epdg
Gerrit-Branch: master
Gerrit-Change-Id: I2a9459859fc660e6433cd8178ab9d1f92ae74fc0
Gerrit-Change-Number: 35218
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-MessageType: newchange
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email )
Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port
......................................................................
Patch Set 4:
(4 comments)
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/ada45e97_26192709
PS4, Line 838: osmo_sockaddr_port(&conn->end.addr.u.sa) == 0) {
> the most concise restriction would be: […]
We need to keep it this way I submitted, at least for address, since the configured address at this point may be wrong (the hnb may actually be using another IP address, what osmo-mgw was submitted was just a guess, and it cannot be considered confirmed until IuUP Initialization happens).
So if IP address is set but port is not, we cannot validate IP address of received packet against the configured one, because it is "a guess", we must accept it anyway, at least to carry on with IuUP initialization.
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/01d4c9a3_be3f7c7e
PS4, Line 841: announce
> "not all hNodeB", but some are happily sending RAB Assignment success before IuUP Initialization. […]
that's why I didn't write "not all hNodeb", to leave it undetailed. I can add "some", but since we didn't play with many others I preferred leaving it this way. I think even in the diagrams in specs it shows up this way (IuUP Initialization happening before RAB-Ass-Resp).
Do you know of any HNB not doing it this way?
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/d6994204_0c6123e7
PS4, Line 842: ASs
> (RAB Assignment Response at HNBGW)
Ack
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/238815b4_a883cf2a
PS4, Line 843: MDCX
> Writing "MDCX" is too specific: it is up to the client to do CRCX with SDP all-in-one or adjust with […]
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Idd833997abce46886e9664505b2776fa5dadc8db
Gerrit-Change-Number: 35176
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 05 Dec 2023 10:37:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/158c6533_50887307
PS1, Line 28: Decided for now that it's not worth the extra effort to make this more
: restrictive
> ok, I understand. […]
"we do allow any source address to send MGCP to the MGW and actually". This is easily constraint by selecting a proper IP address like a localhost address when configuring osmo-mgw local MGCP address, or binding to an IP address only available on a given interface.
The problem with "security" from the RTP ports comes from the fact that the remote IP address is selected by a 3rd entity/node connected to us, and osmo-mgw uses ip probing to find out how to connect.
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/d09ff72c_2e8481ca
PS1, Line 838: if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> (There are two places to indicate codecs, one in the MGCP header which we basically ignore, and the […]
With current master (at least from yesterday) yes, you are right. However,
as I mentioned I merged a patch in osmo-hnbgw to announce a "hnb IuUP address" to osmo-mgw right from the first RAN-side MGCP CRCX, by using the Iuh remote IP address at the hnbgw, so that osmo-mgw can guess correctly its binding IuUP address in the assumed general case where Iuh IP address = IuUP IP address in HNB. See
https://gitea.osmocom.org/cellular-infrastructure/osmo-hnbgw/commit/656d1d2…
Besides that, libosmo-mgcp-client needs to be adapted in order to allow submitting an IP address on the wire (CRCX) even if the port is yet not known (port=0). This is what this osmo-mgw.git patch is accomplishing:
https://gerrit.osmocom.org/c/osmo-mgw/+/35152
As you see, with this patch the generated CRCX now contains an SDP, and hence it doesn't go through the add_lco() path, but through the add_sdp() (because it's the only way to provide an IP address to osmo-mgw so it does the ip probing).
Now that you processed all the above, read again my previous comment to understand the modifications (going back more or less to version 1 of the patch to send the codec in the CRCX even if the port=0).
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/6a20a4bb_df864304
PS2, Line 837: != 0)
> (i find this really hard to read. […]
It's not a bool, it's a tristate 1, 0, -1. It's a bool + error. So in here we are handling error as "consider it as not set".
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
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: Tue, 05 Dec 2023 10:30:45 +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>
Gerrit-MessageType: comment
Attention is currently required from: jolly.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35214?usp=email )
Change subject: Correctly assemble measurement result into MEASUREMENT REPORT
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
File src/host/layer23/src/mobile/gsm48_rr.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/35214/comment/3d16b8df_1307504a
PS1, Line 3710: strongest_i
> This is not required. […]
Ok then. I was concerned about potentially uninitialized memory access.
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35214?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Iaeeaf978da31611c47a20af41790bfa6640dcffd
Gerrit-Change-Number: 35214
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Tue, 05 Dec 2023 09:41:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35214?usp=email )
Change subject: Correctly assemble measurement result into MEASUREMENT REPORT
......................................................................
Patch Set 1:
(1 comment)
File src/host/layer23/src/mobile/gsm48_rr.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/35214/comment/5977cae4_4b039f5b
PS1, Line 3710: strongest_i
> Initialize to 0?
This is not required.
The initial "strongest" value is always above any measurement value, so "strongest_i" does not matter. After getting the cell from measurement, "strongest_i" is set and used only when there is another cell with the same level.
"strongest_i" is used to prevent reporting a measurement multiple times.
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35214?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Iaeeaf978da31611c47a20af41790bfa6640dcffd
Gerrit-Change-Number: 35214
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 05 Dec 2023 09:14:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: arehbein, pespin.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31882?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: Make BSSGP timing data configurable
......................................................................
Make BSSGP timing data configurable
Also: Deprecate/hide old respective VTY command,
while preserving backwards compatibility
for BSSGP timing data configuration.
Related: OS#5335
Change-Id: Id4779f033b5eb1742462d4efc28a0398645acfe6
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/vty.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/bts_init.c
M src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c
M src/osmo-bsc/bts_vty.c
M src/osmo-bsc/pcu_sock.c
M tests/bts_features.vty
8 files changed, 174 insertions(+), 25 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/82/31882/8
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31882?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Id4779f033b5eb1742462d4efc28a0398645acfe6
Gerrit-Change-Number: 31882
Gerrit-PatchSet: 8
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-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset