[S] Change in osmo-hnbgw[master]: drop legacy hack: do not start MGW endp in loopback mode
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/36634?usp=email )
Change subject: drop legacy hack: do not start MGW endp in loopback mode
..
drop legacy hack: do not start MGW endp in loopback mode
We used to tell osmo-mgw to create an IuUP endpoint in loopback mode, in
order to hack it into responding to an IuUP Initialization. The loopback
mode here in osmo-hnbgw is a leftover from that hack. Drop it.
Change-Id: I0eca75d7abf66f8b9fde9c68ec10d4265f64a189
---
M TODO-RELEASE
M src/osmo-hnbgw/mgw_fsm.c
2 files changed, 18 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/34/36634/1
diff --git a/TODO-RELEASE b/TODO-RELEASE
index b5a838d..a80e188 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,4 +7,7 @@
# If any interfaces have been added since the last public release: c:r:a + 1.
# If any interfaces have been removed or changed since the last public
release: c:r:0.
#library whatdescription / commit summary line
-osmo-iuh >1.5.0 proper decoding of X.213 IPv4 address len=7
\ No newline at end of file
+osmo-iuh >1.5.0 proper decoding of X.213 IPv4 address len=7
+
+MGWMGCP CRCX osmo-hnbgw used to CRCX in loopback mode, to trigger a
legacy IuUP hack. CRCX is no
+ longer in loopback mode now, so older osmo-mgw may fail
to respond to IuUP Initialization.
diff --git a/src/osmo-hnbgw/mgw_fsm.c b/src/osmo-hnbgw/mgw_fsm.c
index d0dc6df..48b4899 100644
--- a/src/osmo-hnbgw/mgw_fsm.c
+++ b/src/osmo-hnbgw/mgw_fsm.c
@@ -175,7 +175,7 @@
mgw_info = (struct mgcp_conn_peer) {
.call_id = (map->rua_ctx_id << 8) | mgw_fsm_priv->rab_id,
.ptime = 20,
- .conn_mode = MGCP_CONN_LOOPBACK,
+ .conn_mode = MGCP_CONN_RECV_SEND,
};
mgw_info.codecs[0] = CODEC_IUFP;
mgw_info.codecs_len = 1;
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36634?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I0eca75d7abf66f8b9fde9c68ec10d4265f64a189
Gerrit-Change-Number: 36634
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[M] Change in osmo-mgw[master]: mgcp-client: always send 'm=audio' line
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/36633?usp=email )
Change subject: mgcp-client: always send 'm=audio' line
..
mgcp-client: always send 'm=audio' line
Re-add the m=audio line to SDP emitted from libosmo-mgcp-client, even if
the audio port is not set yet
Patch a5acaa68db4cc26e342069ad2ef37c1b09e1efc2 introduced a presence
flag for the RTP audio port number. This flag, when unset, also omitted
the 'm=audio...' line completely, dropping the PT number definitions.
Correct:
m=audio 1234 RTP/AVP 96 <--- anounce 96
a=rtpmap:96 VND.3GPP.IUFP/16000 <--- further specify 96
a=fmtp:96 ... <--- further specify 96
When m=audio is missing, we only have orphaned rtpmap and fmtp entries.
They are supposed to further specify the 96 listed in 'RTP/AVP 96',
instead they are without context:
a=rtpmap:96 VND.3GPP.IUFP/16000
a=fmtp:96 ...
When the presence map indicates no port known, we still need to emit the
list of PT numbers; so we're forced to send a port of 0:
m=audio 0 RTP/AVP 96
a=rtpmap:96 VND.3GPP.IUFP/16000
a=fmtp:96 ...
This is an important fix for osmo-hnbgw, which sends the first CRCX with
an IUFP codec. osmo-mgw requires this to accept incoming IuUP
Initializaition requests. When m=audio is missing, osmo-mgw does not
parse the IUFP codec, and 3G voice fails completely. This mgcp-client
patch will emit valid codec config also when port == 0, fixing
osmo-hnbgw voice, because osmo-mgw will know about IUFP from the start.
Related: SYS#6907
Related: osmo-mgw a5acaa68db4cc26e342069ad2ef37c1b09e1efc2
Change-Id: Id95b629453aec999100b5af821c6a0b9562bb597
---
M src/libosmo-mgcp-client/mgcp_client.c
1 file changed, 58 insertions(+), 10 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/33/36633/1
diff --git a/src/libosmo-mgcp-client/mgcp_client.c
b/src/libosmo-mgcp-client/mgcp_client.c
index 8b311d3..a9ad4c0 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -1315,6 +1315,7 @@
int local_ip_family, audio_ip_family;
const char *codec;
unsigned int pt;
+ uint16_t audio_port;
#define MSGB_PRINTF_OR_RET(FMT, ARGS...) do { \
if (msgb_printf(msg, FMT, ##ARGS) != 0) { \
@@ -1366,17 +1367,19 @@
MSGB_PRINTF_OR_RET("t=0 0\r\n");
/* Add RTP address port and codecs */
- if (mgcp_msg->presence & MGCP_MSG_PRESENCE_AUDIO_PORT) {
- if (mgcp_msg->audio_port == 0) {
- LOGPMGW(mgcp, LOGL_ERROR,
- "Invalid port number, can not generate MGCP
message\n");
- return -EINVAL;
- }
- MSGB_PRINTF_OR_RET("m=audio %u RTP/AVP", mgcp_msg->audio_port);
- for (i = 0; i < mgcp_msg->ptmap_len; i++)
- MSGB_PRINTF_OR_RET(" %u", mgcp_msg->ptmap[i].pt);
- MSGB_PRINTF_OR_RET("\r\n");
+ audio_port = 0;
+ if ((mgcp_msg->presence & MGCP_MSG_PRESENCE_AUDIO_PORT)) {
+ audio_port = mgcp_msg->audio_port;
+ if (!audio_port) {
+ LOGPMGW(mgcp, LOGL_ERROR,
+ "Invalid port number, can not generate MGCP message\n");
+ return -EINVAL;
+ }
}
+ MSGB_PRINTF_OR_RET("m=audio %u RTP/AVP", audio_port);
+ for (i = 0; i < mgcp_msg->ptmap_len; i++)
+ MSGB_PRINTF_OR_RET(" %u", mgcp_msg->ptmap[i].pt);
+ MSGB_PRINTF_OR_RET("\r\n");
/* Add optional codec parameters (fmtp) */
if (mgcp_msg->param_present) {
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36633?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: Id95b629453aec999100b5af821c6a0b9562bb597
Gerrit-Change-Number: 36633
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[M] Change in osmo-hnbgw[master]: nft_kpi: retrieve counters in a separate thread
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email ) Change subject: nft_kpi: retrieve counters in a separate thread .. Patch Set 3: (1 comment) Patchset: PS3: > I tend to agree with pespin here. […] Your responses on the blocking behavior still do not have empirical substance. We do also block the main thread for - VTY requests (e.g. to print all counters, to write a config file, ...) - CTRL interface (same) - various syscalls You cannot request to put everything in a different thread just because there is some wait time involved. It is and remains a tradeoff, based on the actual amount of time it takes, and also based on how often it occurs. You are basing your opinions on the impression that asking nft is always very very very slow, which is not a fact. My impression is otherwise. I would appreciate some trust in the experience that I've gathered by actually working with nft myself. I have explained this aspect at least three times now and would like to see this acknowledged in any way, thanks. I still think the entire idea to add a separate thread was premature optimisation, overly complicating and not worth the effort. This is being proliferated by current CR, while the jury is still out on whether there even is a problem in the first place. Be aware that a customer has tested and AFAIK actually is still running the fully non-threaded patch that ALWAYS blocks the main thread for these KPI, and this issue was not being raised. All of this is just a hunch by a person that hasn't tried it out in person and happens to be my boss. Let's rather get this multi-thread patch out to the customer, and improve on it later in case that it still needs to. Instead you guys are blocking progress by being inappropriately pedantic here and missing the point entirely. I explained many times now that it makes sense to add an it_q to this design later, to get rid of the last tiny bit of blocking. This patch is a basis for that -- it is enabling future progress, not excluding it. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Gerrit-Change-Number: 36540 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 17 Apr 2024 21:15:36 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-hnbgw[master]: nft_kpi: retrieve counters in a separate thread
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email ) Change subject: nft_kpi: retrieve counters in a separate thread .. Patch Set 3: (1 comment) Patchset: PS3: > > This is a very generalised statement that seems to be an opinion, and not a > > hard fact. […] Let's stay with this patch here please, and let's stay focused on technical details, and with on-point code review, thanks. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Gerrit-Change-Number: 36540 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 17 Apr 2024 20:20:22 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in docker-playground[master]: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario
Attention is currently required from: daniel, osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email ) Change subject: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario .. Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/docker-playground/+/36590/comment/5359c13b_a5cf80e5 PS1, Line 12: Fixes: 6de89a5fb529928ce9b7b3320e0ca2e3e568d458 > A commit hash should be "Related:", and "Fixes:" is for redmine issues, > right?? […] (disregard the last question, it is answered by the commit log) -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ie02a0754a6ca985e60e08f1f171f532b6cc16264 Gerrit-Change-Number: 36590 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Attention: osmith Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Wed, 17 Apr 2024 19:47:57 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[S] Change in docker-playground[master]: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario
Attention is currently required from: daniel, osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email ) Change subject: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario .. Patch Set 1: (1 comment) File ttcn3-hnbgw-test/with-pfcp/HNBGW_Tests.cfg: https://gerrit.osmocom.org/c/docker-playground/+/36590/comment/2dd6d0cf_bfea053b PS1, Line 120: + > this "+" seems to be a copy paste bug -- let me guess, you haven't actually > tested this patch... (then again not so trivial to test docker-playground patches, at first thought this was osmo-ttcn3-hacks) This seems to duplicate hnbgw.cfg item 'timer X31' -- IMHO we should have one fixed config in osmo-hnbgw.cfg, or only ttcn3 code that adjusts config via VTY. This here says it keeps a value in sync with a .cfg file, so is it being duplicated? I'd favor avoiding that duplication... fine if it's too late / unrelated -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ie02a0754a6ca985e60e08f1f171f532b6cc16264 Gerrit-Change-Number: 36590 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Attention: osmith Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Wed, 17 Apr 2024 19:46:59 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[S] Change in docker-playground[master]: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario
Attention is currently required from: daniel, osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email ) Change subject: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario .. Patch Set 1: Code-Review-1 (2 comments) File ttcn3-hnbgw-test/with-pfcp/HNBGW_Tests.cfg: https://gerrit.osmocom.org/c/docker-playground/+/36590/comment/6190d8fc_814e6365 PS1, Line 120: + this "+" seems to be a copy paste bug -- let me guess, you haven't actually tested this patch... File ttcn3-hnbgw-test/with-pfcp/osmo-hnbgw.cfg: https://gerrit.osmocom.org/c/docker-playground/+/36590/comment/83351049_77367b6b PS1, Line 26: + "+" artifact -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ie02a0754a6ca985e60e08f1f171f532b6cc16264 Gerrit-Change-Number: 36590 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Attention: osmith Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Wed, 17 Apr 2024 19:38:28 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in docker-playground[master]: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario
Attention is currently required from: daniel, osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email ) Change subject: ttcn3-hnbgw: Apply latest changes to 'with-pfcp' scenario .. Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/docker-playground/+/36590/comment/b061dca1_2c5d1408 PS1, Line 12: Fixes: 6de89a5fb529928ce9b7b3320e0ca2e3e568d458 A commit hash should be "Related:", and "Fixes:" is for redmine issues, right?? Are there five distinct bugs in five commits and this patch fixes five distinct issues? -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/36590?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ie02a0754a6ca985e60e08f1f171f532b6cc16264 Gerrit-Change-Number: 36590 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: osmith Gerrit-CC: neels Gerrit-Attention: osmith Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Wed, 17 Apr 2024 19:35:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-hnbgw[master]: nft_kpi: retrieve counters in a separate thread
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email ) Change subject: nft_kpi: retrieve counters in a separate thread .. Patch Set 3: (1 comment) Patchset: PS3: Responding to CR posted on an earlier patch: Your point seems to be that it_q is simpler than the implementation that is using mutexes. I considered this at length and decided against it. Reasons are code complexity as well as performance. Possibly the exact same reasons that osmo-trx has. > Regarding own thread vs multithread: Anything running under the main loop > which blocks the main loop for long periods of time while waiting for some > procedure to finish is broken by design, be it 300ms, 500ms, 1s, 3s, 5s." This is a very generalised statement that seems to be an opinion, and not a hard fact. Let's stay here in this patch. There is a profound difference between blocking 3 ms, 30 ms, 300 ms or 3 s. 300 ms is too much for blocking all traffic on high volume infrastructure, but we do not know whether it is maybe closer to 3 ms yet. And blocking for 300 ms maybe once per day is not a problem that requires immediate action. There is also a difference between real-time phy code and a core network entity that is designed for high latency. IMHO your reasoning lacks these very important qualifications. A productive discussion could be based on "I do not agree with rarely blocking on HNBAP HNB Register events". My response to that is that we need empirical metrics to decide for or against it, and if it turns out bad, I will add the it_q. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Gerrit-Change-Number: 36540 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Mon, 15 Apr 2024 15:40:36 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft .. Patch Set 17: (1 comment) Patchset: PS3: Pau, every so often your CR uses stylistic means of downplaying a submitted patch without a sound base for it. Wording like these come to mind: - "playing around with" belittles a technically sound implementation. - "all this foo" is too general and diffuse, CR needs to be on point. - "everything ends up broken" without anything broken being present. > Fine if you want to keep it in 2 patches, but I wouldn't merge this one until > the other one is ready and can be merged together. Kindly state your reason for this statement, it seems unqualified to me. And kindly state your reason directly at the start of a CR discussion, next time. I would appreciate that very much. I am not playing around, I know what I am doing, and decided things for sound reasons. I need you to acknowledge that. If you disagree, then let's stay with the sound arguments against it. Thanks! Also let's discuss in the proper place. CR is bringing up threading details in CR on patches without any threading. This patch here is not multi threaded. For threading, let's discuss at the patch introducing the nft thread. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Mon, 15 Apr 2024 15:39:55 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: expect TC_attached_imsi_lu_unknown_tmsi to pass
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36559?usp=email ) Change subject: msc: expect TC_attached_imsi_lu_unknown_tmsi to pass .. Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36559/comment/ba5009a4_8cf3f370 PS1, Line 12: https://cgit.osmocom.org/osmo-msc/commit/?id=2fd69e15d36d5a8e87029741ad66632c57d24cd4 (we usually just post only the change-id and/or commit hashes in here, not full links. after all, the infrastructure may move elsewhere some day.) -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36559?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I57a277fa7e6e0d10ff38e23f416ace87472e6602 Gerrit-Change-Number: 36559 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 15:07:21 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: expand TC_lu_tmsi_noauth_notmsi
Attention is currently required from: fixeria, laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email ) Change subject: msc: expand TC_lu_tmsi_noauth_notmsi .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I5e596597add7d585efd27c850067b8d7ba34ecc0 Gerrit-Change-Number: 36460 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 15:05:14 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-ttcn3-hacks[master]: msc: add TC_lu_tmsi_noauth_notmsi
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email ) Change subject: msc: add TC_lu_tmsi_noauth_notmsi .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: If10b9987395670b084ff8ad6d1f033ff46896d75 Gerrit-Change-Number: 36459 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 15:04:28 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: add mi arg to f_perform_lu()
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458?usp=email ) Change subject: msc: add mi arg to f_perform_lu() .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I31aad8eb751528f7237a892702e87ee5855cabbb Gerrit-Change-Number: 36458 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 15:03:47 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-ttcn3-hacks[master]: msc: derive altsteps from f_expect_paging() and use them
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36558?usp=email ) Change subject: msc: derive altsteps from f_expect_paging() and use them .. Patch Set 1: Code-Review+2 (2 comments) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36558/comment/ca13f69b_7dbe0ca3 PS1, Line 1360: RANAP RANAP has no TMSI in paging, the hNodeB figures out which one to use https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36558/comment/12d6725b_2052dcd2 PS1, Line 1367: RANAP same -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36558?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ia0e787fc376acec09e8985a63862872eb89b53a4 Gerrit-Change-Number: 36558 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 14:58:58 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: rework f_expect_paging(): handle mismatch and timeout
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email ) Change subject: msc: rework f_expect_paging(): handle mismatch and timeout .. Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: IMHO same applies as in previous patch https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/ -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I30273e3882e348a2ded88b7b96a5ec1473a56913 Gerrit-Change-Number: 36456 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 14:56:23 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): fix by_tmsi arg
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email ) Change subject: msc: f_expect_paging(): fix by_tmsi arg .. Patch Set 3: Code-Review+2 (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/comment/8a4a445f_89f06c3a PS3, Line 1371: f_expect_paging_tmsi(g_pars.tmsi); These patches are not getting better IMHO, and also there is no practical benefit in getting this perfect. What's still weird here: - say there is a g_pars.tmsi set, but a test case wants to explicitly expect a paging without TMSI (maybe just for failing fast on the wrong response), then to NOT have a TMSI in the template, i have to call f_expect_paging_tmsi(). That's kind of weird, right? - the RANAP part doesn't match the TMSI arguments well, since there is no TMSI involved. I think none of the patch sets (including mine) really nails it, and I *still* think the originally submitted boolean argument is simplest, least confusing way here. f_expect_paging(by_tmsi := false) But let's get over this and just merge it, shall we? -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I9434745b7faeb738caafed8080b9f7b1a6a8079a Gerrit-Change-Number: 36455 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 15 Apr 2024 14:53:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-upf[master]: manual: explain IP forwarding
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email ) Change subject: manual: explain IP forwarding .. Patch Set 4: Code-Review+1 (1 comment) Patchset: PS4: re-adding previous vote because only a trivial tweak was applied in the new patch set -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I7b54f9203c1a77efd43f90b9a1c0105bc5c3efde Gerrit-Change-Number: 35668 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Mon, 15 Apr 2024 14:40:34 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-upf[master]: manual: explain IP forwarding
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email ) Change subject: manual: explain IP forwarding .. Patch Set 4: (2 comments) Patchset: PS1: > the IP Forwarding netfilter rules are not tested still not thoroughly tested, but the example is good enough File doc/manuals/chapters/running.adoc: https://gerrit.osmocom.org/c/osmo-upf/+/35668/comment/31a94155_48c0bd52 PS3, Line 232: the > no "the" here Done -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I7b54f9203c1a77efd43f90b9a1c0105bc5c3efde Gerrit-Change-Number: 35668 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Mon, 15 Apr 2024 14:39:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[M] Change in osmo-upf[master]: manual: explain IP forwarding
Attention is currently required from: laforge. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+1 by laforge, Verified+1 by Jenkins Builder Change subject: manual: explain IP forwarding .. manual: explain IP forwarding Change-Id: I7b54f9203c1a77efd43f90b9a1c0105bc5c3efde --- M doc/manuals/chapters/running.adoc 1 file changed, 50 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/68/35668/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I7b54f9203c1a77efd43f90b9a1c0105bc5c3efde Gerrit-Change-Number: 35668 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: laforge Gerrit-MessageType: newpatchset
[M] Change in osmo-upf[master]: manual: explain GTP Echo workaround for tunmap
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email ) Change subject: manual: explain GTP Echo workaround for tunmap .. manual: explain GTP Echo workaround for tunmap Change-Id: Ic824fc876d1fad181254cb6894e51464c443b53c --- M doc/manuals/chapters/running.adoc 1 file changed, 62 insertions(+), 7 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/doc/manuals/chapters/running.adoc b/doc/manuals/chapters/running.adoc index 8e1d5ee..eda1a6d 100644 --- a/doc/manuals/chapters/running.adoc +++ b/doc/manuals/chapters/running.adoc @@ -99,10 +99,10 @@ GTP kernel module configuration in the `tunend` section can be omitted for sites that serve only as GTP forwarding proxy, without encapsulation/decapsulation of -GTP payloads. +GTP payloads -- except to provide GTP Echo service, see <>. -Likewise, netfilter configuration in the `tunmap` section can be omitted for -sites only serving as GTP tunnel endpoint. +Netfilter configuration in the `tunmap` section can be omitted for sites only +serving as GTP tunnel endpoint. [[gtp_module]] === Configure Linux Kernel GTP Module for `tunend` @@ -167,11 +167,57 @@ The Linux kernel netfilter module is used for GTP tunnel proxying, also known as tunnel forwarding or tunnel mapping. -Using the netfilter module usually requires no configuration in `osmo-upf.cfg`. +When using the netfilter module, you may set up `osmo-upf.cfg` for: +- GTP Echo (required) +- nft table name (optional) -`osmo-upf` creates a new netfilter table, under which it submits rule sets for -GTP tunnel proxying. This table name defaults to `osmo-upf`. A custom table name -can be configured in `osmo-upf.cfg` like this: +[[gtp_echo]] + GTP Echo + +You need to ensure that OsmoUPF responds to GTP Echo requests. +- A GTP device configured for `tunend` implicitly includes a GTP Echo service. +- For `tunmap`, no GTP Echo mechanism is implemented. + +So, when your use case is `tunmap` only, you should still add a GTP device as +for `tunend`, only to provide the GTP Echo service. + +Here are some options to do so: + +If you have no GTP devices configured in `osmo-upf.cfg` yet, you can add a +single GTP device without a specific IP address, in order to respond to GTP-U +Echo requests on all interfaces to anyone that is asking: + + +tunend + dev create gtp-echo + + +Note that `gtp-echo` is just an arbitrary GTP device name, choose any string +that makes a valid network device name and is still available, as in the `dev` +argument in the `ip addr show dev` command on Linux. + +This will bind osmo-upf on 0.0.0.0:2152 to respond to GTP Echo requests. + +If you would like to limit GTP Echo responses to specific network interfaces, +you need to add a separate GTP device per local IP address: + + +tunend + dev create gtp-echo1 192.168.0.23 + dev create gtp-echo2 10.9.8.17 + + +This will bind osmo-upf only on 192.168.0.23:2152 and 10.9.8.17:2152 to respond +to GTP Echo requests. + +For creating and manipulating a GTP device in more versatile ways, see +<>. + + nft Table Name + +For `tunmap`, `osmo-upf` creates a new nft table, under which it submits +rule sets for GTP tunnel proxying. This table name defaults to `osmo-upf`. A +custom table name can be configured in `osmo-upf.cfg` like this: tunmap -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ic824fc876d1fad181254cb6894e51464c443b53c Gerrit-Change-Number: 35667 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: merged
[S] Change in osmo-upf[master]: manual: 'Running': tweak word, fix ws at line end
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/36557?usp=email ) Change subject: manual: 'Running': tweak word, fix ws at line end .. manual: 'Running': tweak word, fix ws at line end Change-Id: Id9a4d2d75f86a252df0da6e7e0ae5ab47e8a7bf9 --- M doc/manuals/chapters/running.adoc 1 file changed, 10 insertions(+), 1 deletion(-) Approvals: pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/doc/manuals/chapters/running.adoc b/doc/manuals/chapters/running.adoc index 009ec16..8e1d5ee 100644 --- a/doc/manuals/chapters/running.adoc +++ b/doc/manuals/chapters/running.adoc @@ -88,7 +88,7 @@ * The GTP module is used for `tunend`: GTP encapsulation/decapsulation from/to "the internet". -* The netfilter framework and nftables is used for `tunmap`: GTP tunnel proxying, +* The netfilter framework and nftables are used for `tunmap`: GTP tunnel proxying, also known as tunnel forwarding or tunnel mapping. .Linux kernel feature usage -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/36557?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Id9a4d2d75f86a252df0da6e7e0ae5ab47e8a7bf9 Gerrit-Change-Number: 36557 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: merged
[M] Change in osmo-upf[master]: manual: explain IP forwarding
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email ) Change subject: manual: explain IP forwarding .. Patch Set 3: (1 comment) This change is ready for review. File doc/manuals/chapters/running.adoc: https://gerrit.osmocom.org/c/osmo-upf/+/35668/comment/014565a2_519b5d6e PS3, Line 232: the no "the" here -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35668?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I7b54f9203c1a77efd43f90b9a1c0105bc5c3efde Gerrit-Change-Number: 35668 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Mon, 15 Apr 2024 14:32:59 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-upf[master]: manual: explain GTP Echo workaround for tunmap
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email ) Change subject: manual: explain GTP Echo workaround for tunmap .. Patch Set 3: (3 comments) File doc/manuals/chapters/running.adoc: https://gerrit.osmocom.org/c/osmo-upf/+/35667/comment/b3d6d7ec_892517df PS1, Line 105: serving as GTP tunnel endpoint. > do you mean this section? Done https://gerrit.osmocom.org/c/osmo-upf/+/35667/comment/7362ffd2_0eefbee2 PS1, Line 177: GTP peer > is that "GTP peer" a term in the specification? The term I have in memory is > "GSN", but that's prob […] Done https://gerrit.osmocom.org/c/osmo-upf/+/35667/comment/ad8d41fd_1cd7f93f PS1, Line 212: netfilter > nft / nftables table Done -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ic824fc876d1fad181254cb6894e51464c443b53c Gerrit-Change-Number: 35667 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: laforge Gerrit-Comment-Date: Sat, 13 Apr 2024 01:42:07 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[M] Change in osmo-upf[master]: manual: explain GTP Echo workaround for tunmap
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email ) Change subject: manual: explain GTP Echo workaround for tunmap .. Patch Set 3: (1 comment) File doc/manuals/chapters/running.adoc: https://gerrit.osmocom.org/c/osmo-upf/+/35667/comment/17b8c48d_78c01203 PS1, Line 172: netfilter > it s a nftables or nft table. […] thanks for the clarification, i wasn't fully aware of that when i wrote the patch -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ic824fc876d1fad181254cb6894e51464c443b53c Gerrit-Change-Number: 35667 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: laforge Gerrit-Comment-Date: Sat, 13 Apr 2024 01:41:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[S] Change in osmo-upf[master]: manual: 'Running': tweak word, fix ws at line end
neels has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/osmo-upf/+/36557?usp=email ) Change subject: manual: 'Running': tweak word, fix ws at line end .. manual: 'Running': tweak word, fix ws at line end Change-Id: Id9a4d2d75f86a252df0da6e7e0ae5ab47e8a7bf9 --- M doc/manuals/chapters/running.adoc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/57/36557/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/36557?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Id9a4d2d75f86a252df0da6e7e0ae5ab47e8a7bf9 Gerrit-Change-Number: 36557 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-CC: Jenkins Builder Gerrit-MessageType: newpatchset
[M] Change in osmo-upf[master]: manual: explain GTP Echo workaround for tunmap
Attention is currently required from: neels. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: manual: explain GTP Echo workaround for tunmap .. manual: explain GTP Echo workaround for tunmap Change-Id: Ic824fc876d1fad181254cb6894e51464c443b53c --- M doc/manuals/chapters/running.adoc 1 file changed, 62 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/67/35667/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/35667?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ic824fc876d1fad181254cb6894e51464c443b53c Gerrit-Change-Number: 35667 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: neels Gerrit-MessageType: newpatchset
[S] Change in osmo-upf[master]: manual: 'Running': tweak, word, fix ws at line end
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-upf/+/36557?usp=email ) Change subject: manual: 'Running': tweak, word, fix ws at line end .. manual: 'Running': tweak, word, fix ws at line end Change-Id: Id9a4d2d75f86a252df0da6e7e0ae5ab47e8a7bf9 --- M doc/manuals/chapters/running.adoc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/57/36557/1 diff --git a/doc/manuals/chapters/running.adoc b/doc/manuals/chapters/running.adoc index 009ec16..8e1d5ee 100644 --- a/doc/manuals/chapters/running.adoc +++ b/doc/manuals/chapters/running.adoc @@ -88,7 +88,7 @@ * The GTP module is used for `tunend`: GTP encapsulation/decapsulation from/to "the internet". -* The netfilter framework and nftables is used for `tunmap`: GTP tunnel proxying, +* The netfilter framework and nftables are used for `tunmap`: GTP tunnel proxying, also known as tunnel forwarding or tunnel mapping. .Linux kernel feature usage -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/36557?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Id9a4d2d75f86a252df0da6e7e0ae5ab47e8a7bf9 Gerrit-Change-Number: 36557 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 3: (1 comment) Patchset: PS3: (BTW, I updated the commit log message to highlight that it's about keeping separate counters in sync with each other) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Sat, 13 Apr 2024 00:09:01 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 3: (1 comment) Patchset: PS2: > > pespin says use intermediate storage with mutexes in a comment at > > https://gerrit.osmocom. […] let's get back to libosmocore and this patch, the particular lock/unlock discussion belongs to whichever patch is using this new API. (here in particular: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540 -- see there for a response on above points.) The important point here is, quoting: A mutex lock around stats reporting is a very basic and very powerful tool. It allows a multi-threaded application to decide which stats updates will always be in sync. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Sat, 13 Apr 2024 00:03:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, laforge, neels. Hello Jenkins Builder, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email to look at the new patch set (#3). Change subject: add osmo_stats_report_lock api .. add osmo_stats_report_lock api Allow multi-threaded access to reported stats: - enable use of a stats mutex with osmo_stats_report_use_lock(true). - lock/unlock externally with osmo_stats_report_lock(true/false). Rationale: A mutex lock around stats reporting is a very basic and very powerful tool. It allows a multi-threaded application to decide which stats updates will always be in sync. For example, what if the main thread reports rate counters at exactly the time a second thread updates rate counter values? - is writing to stats atomic on a data type level? - do applications need stats to be "atomic" as a whole? We probably have (or can make) int64_t atomic, so that individual counters will always be reported correctly. But often, separate stats correspond to each other, which should not be reported out of sync with each other. The simplest way to manage stats sync in all cases is a mutex around the stats reporting. But such a mutex isn't needed in most programs, with stats happening in a single thread only. To completely avoid any overhead the mutex may bring, make using a mutex optional with a global flag. Related: SYS#6773 Related: osmo-hnbgw I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d --- M include/osmocom/core/stats.h M src/core/libosmocore.map M src/core/stats.c M src/vty/stats_vty.c 4 files changed, 126 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/38/36538/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[M] Change in osmo-hnbgw[master]: nft_kpi: retrieve counters in a separate thread
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email ) Change subject: nft_kpi: retrieve counters in a separate thread .. Patch Set 3: (1 comment) Patchset: PS3: Quoting from the "Depends" libosmocore patch discussion: > For this one, my aim was to say that osmo-trx is doing it that way, not that > you > should do it that way. oh, misunderstanding. > If you plan on keep the mutex locked while you gather counters from nft in this patch: the stats lock is held while updating the rate_ctrs only, locking only *after* the nft command has run and returned the complete response in a char buffer. I expect running through that buffer to be very fast, so i lock stats reporting once all around the buffer parsing. If it turns out wrong, we can unlock and re-lock the mutex after every N items (or every N ms?), which would remove "all" blocking of stats. (It is important to keep counters for each HNB in sync, but not important to keep HNBs in sync with each other.) I expect buffer parsing to already be imperceptibly fast; this is O(n) of pure pointer arithmetic, no syscalls. > most probably blocking the main thread also for the same amount of time. (I've written this before in different places, but allow me to re-post, to make sure it is mentioned here:) Blocking of the main thread can happen with this patch, at the times when the main thread wants to run a quick nft command. This happens only at HNBAP HNB Register and Deregister, i.e. when a hNodeB shows up or disappears; to set up / remove nft counter rules. Here the main thread may have to wait for the second thread querying the counters from nftables. There is an ascii art of it in this patch. To circumvent this blocking, an inter-thread queue can be introduced to tell the second thread to carry out the nft commands the main thread wants to run. This is not implemented in this patch on purpose, because: The HNB Register / Deregister are rare events, and reading counters doesn't really take all that long, either. It may well turn out to be overkill to even have a thread for nftables in the first place. It is slow for large numbers of items, but counters for under 1000 HNB from nft should be fast. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Gerrit-Change-Number: 36540 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Fri, 12 Apr 2024 23:53:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: expand TC_lu_tmsi_noauth_notmsi
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email ) Change subject: msc: expand TC_lu_tmsi_noauth_notmsi .. Patch Set 2: Code-Review+2 (2 comments) Patchset: PS2: combine votes File msc/MSC_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460/comment/1f82b37e_a446b592 PS1, Line 7324: f_cl3_or_initial_ue(valueof(ts_PAG_RESP(ts_MI_IMSI_LV(pars.imsi; > This can turn out pretty substantial effort: when working on ttcn3, I do not > want to adjust API ever […] i agree with the point, but would like to keep it separate from this patch, so resolving in this context here -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I5e596597add7d585efd27c850067b8d7ba34ecc0 Gerrit-Change-Number: 36460 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 12 Apr 2024 23:15:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-ttcn3-hacks[master]: msc: add TC_lu_tmsi_noauth_notmsi
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email ) Change subject: msc: add TC_lu_tmsi_noauth_notmsi .. Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: combining votes -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: If10b9987395670b084ff8ad6d1f033ff46896d75 Gerrit-Change-Number: 36459 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 12 Apr 2024 23:11:19 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: rework f_expect_paging(): handle mismatch and timeout
Attention is currently required from: fixeria.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email )
Change subject: msc: rework f_expect_paging(): handle mismatch and timeout
..
Patch Set 2: Code-Review+1
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456/comment/f63f9f89_71e0683a
PS2, Line 13:
you could add
Tweaked-by: fixeria
File msc/BSC_ConnectionHandler.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456/comment/27c67875_d1c3b7a6
PS2, Line 1362: [g_pars.ran_is_geran]
BSSAP.receive(tr_BSSMAP_Paging(g_pars.imsi, tmsi));
(again the weird mix of g_pars.imsi vs not-g_pars tmsi)
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456/comment/c8f8d22d_c3cb0a35
PS2, Line 1366: function f_expect_paging(template OCT4 tmsi := *, float Tval :=
4.0)
it would be very elegant if we could have the mechanism that {expects a
message, but fails on a different one, and has a timeout} in a more general
function, i.e. not specialized for paging in particluar. Then feed a paging PDU
template to that.
Then we can use this generalized function all over MSC_Tests.
We have such in HNBGW_Tests.ttcn, like f_rua_expect() f_bssap_expect()
Would that work here?
AFAICT we need a separate function for each port-type x PDU-type combination,
but not for each message type?
my motivation is for a more generalized approach so that the discussions we had
in CR don't come up so much...
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I30273e3882e348a2ded88b7b96a5ec1473a56913
Gerrit-Change-Number: 36456
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-Attention: fixeria
Gerrit-Comment-Date: Fri, 12 Apr 2024 23:08:44 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): fix by_tmsi arg
Attention is currently required from: fixeria, laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email ) Change subject: msc: f_expect_paging(): fix by_tmsi arg .. Patch Set 2: Code-Review+2 (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/comment/2d5f24d7_0be16b88 PS2, Line 1360: function f_expect_paging(template OCT4 tmsi := *) oh, i expected that it would need to adjust lots of callers... Not blocking; But this patch breaks one paradigm of this function: see below how we use g_pars.imsi. Now this is not using g_pars.tmsi as would be logical, but the caller is passing a TMSI explicitly. This is the reason why i all the time and still and always will think a boolean is semantically the right choice. To fix it, you'd also have to make the imsi an argument that is passed in and not taken from g_pars. But since someone spent time to submit the patch and it's good enough, I'll not make their life harder with my fringe opinion on the far corner of a ttcn3 test's helper function's type of an arg, and will accept the patch despite not matching my opinion. I'd like to see more of that, please, very large wink. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I9434745b7faeb738caafed8080b9f7b1a6a8079a Gerrit-Change-Number: 36455 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 12 Apr 2024 22:29:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: ran-emu: allow receiving Paging without a TMSI
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454?usp=email ) Change subject: ran-emu: allow receiving Paging without a TMSI .. Patch Set 2: (1 comment) File library/RAN_Emulation.ttcnpp: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454/comment/1f09c370_a18e32c6 PS1, Line 507: var template OCT4 tmsi := omit; > That's like asking "this non-const pointer works, why change it?" let's focus on technical reasoning. IOW, there seems to be no difference between with and without "(omit)"? -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I1bdf3488be0f8d4f0905665c4ba642f9468b9777 Gerrit-Change-Number: 36454 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 12 Apr 2024 22:16:19 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in libosmocore[master]: add API logging_vty_subsys_strip_leading_char()
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email ) Change subject: add API logging_vty_subsys_strip_leading_char() .. Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/36546/comment/b7cf1756_e70598cf PS1, Line 17: So this allows an application to remove the odd 'D' from category names, : without any changes in any VTY configuration. > IIUC, the [only?] benefit of using this new API is that you can define a > `struct log_info_cat` array […] (FYI, this is not sysmocom work, but a private itch) The way this character stripping is done in the VTY code introduces an inconsistency. The correct and consistent way would be for the VTY to use exactly the category name that is defined and also output in log output, i.e. including the leading D if the application has one. That way it is up to the application what names it chooses, and the names are the same everywhere. Since I cannot change this VTY behavior now (all osmo applications depend on that), i at least hope to offer a new application a consistent experience with category names on the VTY. An optional switch to change to consistent behavior. My motivation: I always found this leading D annoying, as apparently did the logging_vty.c implementer, unfortunately. I find it even more madly insane that, for example, DLMGCP now becomes "lmgcp" -- why strip the D and not the L? the whole thing about the D is very inelegant -- why print an extra meaningless character on each and every log line? But ok, say we want an extra "D" everywhere; then why not on the VTY??? I'm hacking on "osmo-gsm-shark" in my private time, and wanted to get rid of those Ds there. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5faedf7d6525d744a734ebe54c185fcc904f763e Gerrit-Change-Number: 36546 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-Attention: fixeria Gerrit-Comment-Date: Wed, 10 Apr 2024 02:58:17 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in libosmocore[master]: formalize log subsys stripping for vty
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36545?usp=email ) Change subject: formalize log subsys stripping for vty .. Patch Set 1: (2 comments) File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/36545/comment/abd53216_0a83c47e PS1, Line 432: int cat_idx > probably a good idea to make it `unsigned` TL;DR: I'll move it to a private header. rambling on in case you're interested: it ended up this way because - intended for private API use only, i'd have liked it to be static, as the commit log explains. - at the very heart of the logging subsystem and should rather be lean. - When looking at the callers, it is clear that it's correct. Yet I did think about it quite a bit: we actually do have negative categories, the DL* ones. so i did ask myself, should it convert from one to the other? But: It is called only in places where the idx is already checked, and the local variable at all the callers is an int... So maybe in future it should be able to convert a negative category? Considering these back and forth I ended up at "good enough compromise, not important enough to bother"... But your remark makes me reconsider, I guess it should be less public? https://gerrit.osmocom.org/c/libosmocore/+/36545/comment/eb8fbfce_23f8762d PS1, Line 434: const char *name = log_info->cat[cat_idx].name; > add `OSMO_ASSERT(cat_idx < log_info->num_cat)`? (as above) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36545?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5f81343e8c7b714a4630e64ba654e391435c4244 Gerrit-Change-Number: 36545 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Wed, 10 Apr 2024 02:44:35 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 2: (1 comment) Patchset: PS2: > And for the case at hand, I still belive it makes much more sense to simply > use an osmo_itq to pass […] I'm in a bit of a communication dilemma: - laforge says don't use any mutexes and just directly write to rate_ctr from the other thread. - pespin says use an osmo_it_q here - pespin says use intermediate storage with mutexes in a comment at https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385 - nhofmeyr says directly write to rate_ctr, but still have a mutex around stats reporting, so that counter pairs are always in sync. I interpret it so that laforge is radically "bare-metal" and simple, pespin radically thread-safe and elaborate, and i'm actually in the middle, a little more to laforge's side. Yet it seems you guys are missing the actual point for this patch here: A mutex lock around stats reporting is a very basic and very powerful tool. It allows a multi-threaded application to decide which stats updates will always be in sync. This patch adds this feature, in a completely optional way. It is up to the application how to make use of it, so let's discuss those details at the other patches. I fail to see any problem with this patch and don't know how we can reach a consensus on this. Neither of us seems to accept the others' arguments. At least I still think that I'm right... -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Wed, 10 Apr 2024 02:17:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft .. Patch Set 17: (1 comment) Patchset: PS3: > (I wrote the comment above this morning but actually saw just now I forgot to > press the "submit" but […] I think it makes a lot of sense to keep the patches separate as they are. The patch wrapping a thread around existing code is very nicely orthogonal to the actual nft activity, and it is nice to see exactly where the mutexes etc are added, without the nft implementation along with it to clutter up the attention span. osmo-trx's use of multi-threaded counters: do you have a pointer? I was looking around and apparently hadn't found it... thanks! You say in osmo-trx "we do things a bit adhoc" and further below you say that's "how it should be done" -- IIUC an intermediate storage with mutexes. The underlying point is whether the occasional short blocking for HNB Register and HNB DeRegister (rare events) are noticeable enough to warrant further effort on that. If those blockings of the main thread are not desirable, it seems the best way is a queue fd (osmo_it_q?) because other than mutexes, it avoids all blocking by design. You say "it's broken", yet the patch is actually already being run successfully. I see no inconsistencies nor brokenness, I'd be glad to hear some technical detail to back up your claims. Scalability is an open point -- we are only now able to gather the empirical data on that. My experience is that nft below 1000 items is very fast, and only becomes prohibitively slow with higher numbers, with some exponentiality kicking in. I am not fully convinced that we even need a separate thread. In summary, am interested in how osmo-trx does it; otherwise do not agree / do not yet see the points that were claimed; instead, am engaging in scalability discussion. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 10 Apr 2024 01:58:39 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-hnbgw[master]: nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl
Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl .. nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl So far we have the nftables based counters for total GTP-U bytes (UL, DL), as well as a packet count. Add another counter for the computed UE payload bytes: total_bytes - packets * (20 + 8 + 8) Related: SYS#6773 Change-Id: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca --- M include/osmocom/hnbgw/hnbgw.h M include/osmocom/hnbgw/nft_kpi.h M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/nft_kpi.c 4 files changed, 43 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/39/36539/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca Gerrit-Change-Number: 36539 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[S] Change in osmo-hnbgw[master]: nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email ) Change subject: nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl .. Patch Set 1: (1 comment) File src/osmo-hnbgw/nft_kpi.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/19341813_d8e7edc8 PS1, Line 273: LOGP(DLGLOBAL, LOGL_ERROR, "XXX %s(): packets=%ld btes=%ld ueb=%ld\n", __func__, > This need to go out. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca Gerrit-Change-Number: 36539 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 09 Apr 2024 03:55:36 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft .. Patch Set 17: (3 comments) This change is ready for review. Patchset: PS3: > Thanks for the cool pointers! […] see https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540 Patchset: PS8: > right, it appears we need a compiler switch for the gerrit verification, too. > […] Done PS8: > current problem is getting it to pass the gerrit verification, which has > problems with the nftables […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: laforge Gerrit-Comment-Date: Tue, 09 Apr 2024 03:55:18 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[M] Change in osmo-hnbgw[master]: nft_kpi: retrieve counters in a separate thread
Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: nft_kpi: retrieve counters in a separate thread .. nft_kpi: retrieve counters in a separate thread Introduce an NFT thread which does: - periodically run nftables command to read all counters - parse the response - update rate_ctr values. The main thread still runs the rule addition / removal when a HNB registers or deregisters. See the comment added in nft_kpi.c, starting with "A more scalable solution...". This patch requires the new osmo_stats_report_lock(), see 'Depends'. Related: SYS#6773 Depends: libosmocore Ib335bea7d2a440ca284e6c439066f96456bf2c2d Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f --- M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/nft_kpi.c M src/osmo-hnbgw/osmo_hnbgw_main.c M src/osmo-hnbgw/tdefs.c 4 files changed, 171 insertions(+), 34 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/40/36540/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Gerrit-Change-Number: 36540 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 2: (1 comment) File src/core/stats.c: https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/6acfa08f_c4541089 PS2, Line 791: static pthread_mutex_t *g_report_lock = NULL; just a thought, it would be far simpler to only publish this pointer on public API. But it's good to have these formal three API functions instead, right? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 09 Apr 2024 03:47:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 2: (4 comments) File src/core/stats.c: https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/f1b71d2e_9e393292 PS1, Line 794: * Calling osmo_stats_report_use_lock(true) */ > unfinished comment Done https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/fc865170_26d7a750 PS1, Line 808: pthread_mutex_destroy(g_report_lock); > good point Done https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/4d11a8c8_85346f55 PS1, Line 819: void osmo_stats_report_lock(bool lock) > easier to trace: good point Done https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/4c0f763b_cf3baf1b PS1, Line 831: pthread_mutex_t *lock = g_report_lock; > I remember now, it was a useless precaution against a changing g_report_lock. > […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 09 Apr 2024 03:45:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in libosmocore[master]: formalize log subsys stripping for vty
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/36545?usp=email )
Change subject: formalize log subsys stripping for vty
..
formalize log subsys stripping for vty
In osmocom, we historically have a leading 'D' in all of our logging
subsystem names -- not only in the enum entry name, but also as the
string in struct log_info_cat[].
As a result of this, our logging_vty code strips away the first
character of each logging subsystem name. In the VTY, we don't enter
'dmain', but only 'main' -- the VTY strips the 'D' from "DMAIN".
The intention is to make this stripping behavior optional in a
subsequent patch.
So far the code to do that is a magic "+ 1" thrown in here and there.
Instead, introduce log_subsys_name() and use it where ever logging_vty.c
does removal of the leading 'D'.
I would have liked to keep this within logging_vty.c, but unfortunately
it needs to be public API in logging.h, because of log_parse_category()
which also strips leading D and lives in logging.c.
Change-Id: I5f81343e8c7b714a4630e64ba654e391435c4244
---
M include/osmocom/core/logging.h
M src/core/libosmocore.map
M src/core/logging.c
M src/vty/logging_vty.c
4 files changed, 42 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/45/36545/1
diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h
index 82e686f..da90d58 100644
--- a/include/osmocom/core/logging.h
+++ b/include/osmocom/core/logging.h
@@ -430,6 +430,8 @@
void log_set_category_filter(struct log_target *target, int category,
int enable, int level);
+const char *log_subsys_name(const struct log_info *log_info, int cat_idx);
+
/* management of the targets */
struct log_target *log_target_create(void);
void log_target_destroy(struct log_target *target);
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index c5ab6e3..3bb2abd 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -71,6 +71,7 @@
log_parse_category;
log_parse_category_mask;
log_parse_level;
+log_subsys_name;
logp_stub;
log_reset_context;
log_set_all_filter;
diff --git a/src/core/logging.c b/src/core/logging.c
index e172124..2429a0c 100644
--- a/src/core/logging.c
+++ b/src/core/logging.c
@@ -428,6 +428,13 @@
return get_value_string(loglevel_strs, lvl);
}
+/* skip the leading 'D' in category name */
+const char *log_subsys_name(const struct log_info *log_info, int cat_idx)
+{
+ const char *name = log_info->cat[cat_idx].name;
+ return name + 1;
+}
+
/*! parse a human-readable log category into numeric form
* \param[in] category human-readable log category name
* \returns numeric category value, or -EINVAL otherwise
@@ -441,7 +448,7 @@
for (i = 0; i < osmo_log_info->num_cat; ++i) {
if (osmo_log_info->cat[i].name == NULL)
continue;
- if (!strcasecmp(osmo_log_info->cat[i].name+1, category))
+ if (!strcasecmp(log_subsys_name(osmo_log_info, i), category))
return i;
}
diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c
index 678ae68..9f38c14 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -322,8 +322,7 @@
for (i = 0; i < categories->num_cat; i++) {
if (categories->cat[i].name == NULL)
continue;
- /* skip the leading 'D' in each category name, hence '+ 1' */
- osmo_str_tolower_buf(buf, sizeof(buf), categories->cat[i].name
+ 1);
+ osmo_str_tolower_buf(buf, sizeof(buf),
log_subsys_name(categories, i));
osmo_talloc_asprintf(tall_log_ctx, *cmd_str_p, "%s%s",
i ? "|" : "", buf);
osmo_talloc_asprintf(tall_log_ctx, *doc_str_p, "%s\n",
@@ -530,7 +529,7 @@
if (!info->cat[i].name)
continue;
vty_out(vty, " %-10s %-10s %-8s %s%s",
- info->cat[i].name+1, log_level_str(cat->loglevel),
+ log_subsys_name(info, i), log_level_str(cat->loglevel),
cat->enabled ? "Enabled" : "Disabled",
info->cat[i].description,
VTY_NEWLINE);
@@ -1095,7 +1094,7 @@
if (!osmo_log_info->cat[i].name)
continue;
- osmo_str_tolower_buf(cat_name, sizeof(cat_name),
osmo_log_info->cat[i].name + 1);
+ osmo_str_tolower_buf(cat_name, sizeof(cat_name),
log_subsys_name(osmo_log_info, i));
level_str = get_value_string_or_null(loglevel_strs,
cat->loglevel);
if (!level_str
[S] Change in libosmocore[master]: add API logging_vty_subsys_strip_leading_char()
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email )
Change subject: add API logging_vty_subsys_strip_leading_char()
..
add API logging_vty_subsys_strip_leading_char()
Allow an application to use subsystem names without a leading 'D'. With
this patch, a program can:
- remove the 'D' from struct log_info_cat[] entries -- normally, the VTY
would then strip the 'M' from "MAIN" and we'd get 'ain'.
- call logging_vty_subsys_strip_leading_char(false) during startup --
hence the VTY does not strip anything, and we get 'main' on the VTY
config.
So this allows an application to remove the odd 'D' from category names,
without any changes in any VTY configuration.
Background: I am hacking on some project where I want logging subsys
names without a leading 'D'. So I noticed that the cfg file has mangled
category names.
Change-Id: I5faedf7d6525d744a734ebe54c185fcc904f763e
---
M include/osmocom/core/logging.h
M src/core/libosmocore.map
M src/core/logging.c
3 files changed, 48 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/46/36546/1
diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h
index da90d58..e58c25c 100644
--- a/include/osmocom/core/logging.h
+++ b/include/osmocom/core/logging.h
@@ -430,6 +430,7 @@
void log_set_category_filter(struct log_target *target, int category,
int enable, int level);
+void log_subsys_strip_leading_char(bool do_strip);
const char *log_subsys_name(const struct log_info *log_info, int cat_idx);
/* management of the targets */
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index 3bb2abd..919897a 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -71,6 +71,7 @@
log_parse_category;
log_parse_category_mask;
log_parse_level;
+log_subsys_strip_leading_char;
log_subsys_name;
logp_stub;
log_reset_context;
diff --git a/src/core/logging.c b/src/core/logging.c
index 2429a0c..ba22914 100644
--- a/src/core/logging.c
+++ b/src/core/logging.c
@@ -428,11 +428,32 @@
return get_value_string(loglevel_strs, lvl);
}
+static bool g_subsys_strip_leading_char = true;
+
+/** Set whether on the VTY, the leading 'D' commonly in use for category names
should be stripped.
+ * If true, a category name 'DMAIN' will be identified on VTY as 'main'.
+ * If false, a category name 'FOO' will be identified on VTY as 'foo' (instead
of 'oo').
+ *
+ * This must be called *before* logging_vty_add_cmds() to take effect!
+ *
+ * There is a common coding style in osmocom that all category names start
with a 'D'.
+ * This flag allows programs to name logging categories without a leading 'D'.
+ * \param[in] do_strip true to strip leading D on VTY, false to use names
as-is.
+ */
+void log_subsys_strip_leading_char(bool do_strip)
+{
+ g_subsys_strip_leading_char = do_strip;
+}
+
/* skip the leading 'D' in category name */
const char *log_subsys_name(const struct log_info *log_info, int cat_idx)
{
const char *name = log_info->cat[cat_idx].name;
- return name + 1;
+ /* The category names in internal_cat[] will always have a leading 'D'.
*/
+ if (g_subsys_strip_leading_char
+ || cat_idx >= log_info->num_cat_user)
+ return name + 1;
+ return name;
}
/*! parse a human-readable log category into numeric form
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5faedf7d6525d744a734ebe54c185fcc904f763e
Gerrit-Change-Number: 36546
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: fixeria, pespin. Hello Jenkins Builder, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by fixeria, Verified+1 by Jenkins Builder Change subject: add osmo_stats_report_lock api .. add osmo_stats_report_lock api Allow multi-threaded access to reported stats: - enable use of a stats mutex with osmo_stats_report_use_lock(true). - lock/unlock externally with osmo_stats_report_lock(true/false). Rationale: In osmo-hnbgw, we would like to collect stats from nftables, and do so in a separate thead. The most efficient way is to write the parsing results from nft directly to the rate_ctr destination. But what if the main thread reports rate counters at exactly that time? - is writing to stats atomic on a data type level? - do applications need stats to be "atomic" as a whole? In osmo-hnbgw in particular, there are two counters, 'packets' and 'total_bytes'. These correspond, and it would skew statistics if we reported them out of sync to each other. The simplest way to ensure correctness in all cases is a mutex around the stats reporting. But this mutex isn't needed in most of our programs. To completely avoid any overhead the mutex may bring, make use of it optional with a global flag. This use case is likely to also show up in other programs that would like to collect and report stats from a separate thread. Related: SYS#6773 Related: osmo-hnbgw I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d --- M include/osmocom/core/stats.h M src/core/libosmocore.map M src/core/stats.c M src/vty/stats_vty.c 4 files changed, 128 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/38/36538/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 1: (2 comments) File src/core/stats.c: https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/059126a4_431954e5 PS1, Line 794: * Calling osmo_stats_report_use_lock(true) */ unfinished comment https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/6abcec23_ce7baf8d PS1, Line 831: pthread_mutex_t *lock = g_report_lock; > brevity; […] I remember now, it was a useless precaution against a changing g_report_lock. Not actually important, it hardly helps, and if the caller fails to avoid races as required by the (new) API doc, it's all mayhem anyway. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 09 Apr 2024 03:21:11 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api .. Patch Set 1: (4 comments) Patchset: PS1: > I think with this you are only sorting out one of the concurrency problems > when using rate_ctr/stats […] How the caller chooses to use this remains very specialized for an application, the main aim is to allow a hook to mutex around the stats reporting. The way this is used in my hnbgw patch: The main thread simply also holds a osmo_stats_report_lock() while groups are added/removed. Because the counter-retrieving thread acquires osmo_stats_report_lock() when updating counters, things are guarded. File src/core/stats.c: https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/4578fe90_562cc039 PS1, Line 808: pthread_mutex_destroy(g_report_lock); > What happens if a pthread_mutex is destroyed while used? […] good point https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/37dcc5d3_51ee4c81 PS1, Line 819: void osmo_stats_report_lock(bool lock) > I'd definetly have 2 APIs here, one for lock and one for unlock. […] easier to trace: good point https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/ada84510_3e86 PS1, Line 831: pthread_mutex_t *lock = g_report_lock; > what's the point of this local variable? brevity; but in fact looks like leftovers from evolution of the patch, thx -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 09 Apr 2024 02:28:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-hnbgw[master]: nft_kpi: retrieve counters in a separate thread
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/36540?usp=email )
Change subject: nft_kpi: retrieve counters in a separate thread
..
nft_kpi: retrieve counters in a separate thread
Introduce an NFT thread which does:
- periodically run nftables command to read all counters
- parse the response
- update rate_ctr values.
The main thread still runs the rule addition / removal when a HNB
registers or deregisters. See the comment added in nft_kpi.c, starting
with "A more scalable solution...".
This patch requires the new osmo_stats_report_lock(), see 'Depends'.
Related: SYS#6773
Depends: libosmocore Ib335bea7d2a440ca284e6c439066f96456bf2c2d
Change-Id: I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f
---
M src/osmo-hnbgw/hnbgw.c
M src/osmo-hnbgw/nft_kpi.c
M src/osmo-hnbgw/osmo_hnbgw_main.c
M src/osmo-hnbgw/tdefs.c
4 files changed, 171 insertions(+), 34 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/40/36540/1
diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c
index 6340d10..fa516ea 100644
--- a/src/osmo-hnbgw/hnbgw.c
+++ b/src/osmo-hnbgw/hnbgw.c
@@ -513,6 +513,12 @@
if (!hnbp)
return NULL;
+ /* nft_kpi.c updates hnb stats, and it locks osmo_stats_report_lock()
while it does so. This here should run
+* in the same thread as stats reporting, so there should be no
conflict with stats. But to avoid a hnb in flux
+* while nft_kpi.c looks up hnb, also obtain the
osmo_stats_report_lock() while manipulating hnb records. */
+ osmo_stats_report_lock(true);
+ /* { */
+
hnbp->id = *id;
hnbp->id_str = talloc_strdup(hnbp, umts_cell_id_name(id));
hnbp->ctrs = rate_ctr_group_alloc(hnbp, _ctrg_desc, 0);
@@ -525,14 +531,21 @@
osmo_stat_item_group_set_name(hnbp->statg, hnbp->id_str);
llist_add(>list, _hnbgw->hnb_persistent_list);
+ /* success */
+ goto out_unlock;
- return hnbp;
-
+ /* failure */
out_free_ctrs:
rate_ctr_group_free(hnbp->ctrs);
out_free:
talloc_free(hnbp);
- return NULL;
+ hnbp = NULL;
+
+ /* for both success and failure: */
+out_unlock:
+ /* } */
+ osmo_stats_report_lock(false);
+ return hnbp;
}
struct hnb_persistent *hnb_persistent_find_by_id(const struct umts_cell_id *id)
diff --git a/src/osmo-hnbgw/nft_kpi.c b/src/osmo-hnbgw/nft_kpi.c
index f585415..039c891 100644
--- a/src/osmo-hnbgw/nft_kpi.c
+++ b/src/osmo-hnbgw/nft_kpi.c
@@ -15,7 +15,11 @@
* GNU Affero General Public License for more details.
*/
+#include
+#include
+
#include
+#include
#include
#include "config.h"
@@ -46,13 +50,68 @@
#include
#include
+/* Threading and locks in this implementation:
+ *
+ * - osmo_stats_report_lock() held while updating rate_ctr from nft results.
+ * - g_nft_kpi_state.lock held while running an nftables command buffer.
+ *
+ * contrived example:
+ *
+ *Main Thread
+ *|
+ * osmo_stats_report_use_lock(true)
+ *|
+ * nft_kpi_init()
+ * create nft ctx, create table
+ *|
+ *+---> NFT thread
+ *| |
+ *| sleep(X34)
+ *| |
+ *| LOCK(g_nft_kpi_state.lock)
+ *| |
+ * osmo_stats_report()query all nft counters
+ * LOCK(osmo_stats_report_lock)|
+ * | LOCK(osmo_stats_report_lock)
+ * collect stats: wait because libosmocore
stats reporting is busy
+ * | :
+ * UNLOCK(osmo_stats_report_lock) LOCK--|
+ * send out stats for all hnbp:
rate_ctr_add2()
+ *| |
+ *|
UNLOCK(osmo_stats_report_lock)
+ *| |
+ *| UNLOCK(g_nft_kpi_state.lock)
+ *| |
+ * hnbgw_rx_hnb_register_req() sleep(X34)
+ * hnb_nft_kpi_start() |
+ * LOCK(g_nft_kpi_state.lock) ...
+ * |
+ *nftables: add new rule
+ * |
+ * UNLOCK(g_nft_kpi_state.lock)
+ *|
+ * ...
+ *
+ * So the NFT thread only retrieves counters. The main thread adds and removes
NFT rules for counters. It is possible
+ * that a HNBAP HNB Register or HNB De-Register occurrs while the NFT thread
holds the g_nft_kpi_state.lock, so that the
+ * main thread blocks unti
[S] Change in osmo-hnbgw[master]: nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email )
Change subject: nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl
..
nft_kpi: add rate_ctr gtpu:ue_bytes:ul,dl
So far we have the nftables based counters for total GTP-U bytes (UL,
DL), as well as a packet count.
Add another counter for the computed UE payload bytes:
total_bytes - packets * (20 + 8 + 8)
Related: SYS#6773
Change-Id: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca
---
M include/osmocom/hnbgw/hnbgw.h
M include/osmocom/hnbgw/nft_kpi.h
M src/osmo-hnbgw/hnbgw.c
M src/osmo-hnbgw/nft_kpi.c
4 files changed, 46 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/39/36539/1
diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h
index d323bdf..c9142e9 100644
--- a/include/osmocom/hnbgw/hnbgw.h
+++ b/include/osmocom/hnbgw/hnbgw.h
@@ -139,8 +139,10 @@
HNB_CTR_GTPU_PACKETS_UL,
HNB_CTR_GTPU_TOTAL_BYTES_UL,
+ HNB_CTR_GTPU_UE_BYTES_UL,
HNB_CTR_GTPU_PACKETS_DL,
HNB_CTR_GTPU_TOTAL_BYTES_DL,
+ HNB_CTR_GTPU_UE_BYTES_DL,
};
enum hnb_stat {
diff --git a/include/osmocom/hnbgw/nft_kpi.h b/include/osmocom/hnbgw/nft_kpi.h
index 95304ce..7b3153d 100644
--- a/include/osmocom/hnbgw/nft_kpi.h
+++ b/include/osmocom/hnbgw/nft_kpi.h
@@ -6,7 +6,8 @@
struct nft_kpi_val {
uint64_t packets;
- uint64_t bytes;
+ uint64_t total_bytes;
+ uint64_t ue_bytes;
bool handle_present;
int64_t handle;
diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c
index a3ea265..6340d10 100644
--- a/src/osmo-hnbgw/hnbgw.c
+++ b/src/osmo-hnbgw/hnbgw.c
@@ -468,6 +468,10 @@
"gtpu:total_bytes:ul",
"Count of total GTP-U bytes received from the HNB, including
the GTP-U/UDP/IP headers",
},
+ [HNB_CTR_GTPU_UE_BYTES_UL] = {
+ "gtpu:ue_bytes:ul",
+ "Assuming an IP header length of 20 bytes, GTP-U bytes received
from the HNB, excluding the GTP-U/UDP/IP headers",
+ },
[HNB_CTR_GTPU_PACKETS_DL] = {
"gtpu:packets:dl",
"Count of GTP-U packets sent to the HNB",
@@ -476,6 +480,10 @@
"gtpu:total_bytes:dl",
"Count of total GTP-U bytes sent to the HNB, including the
GTP-U/UDP/IP headers",
},
+ [HNB_CTR_GTPU_UE_BYTES_DL] = {
+ "gtpu:ue_bytes:dl",
+ "Assuming an IP header length of 20 bytes, GTP-U bytes sent to
the HNB, excluding the GTP-U/UDP/IP headers",
+ },
};
diff --git a/src/osmo-hnbgw/nft_kpi.c b/src/osmo-hnbgw/nft_kpi.c
index 2eb5496..f585415 100644
--- a/src/osmo-hnbgw/nft_kpi.c
+++ b/src/osmo-hnbgw/nft_kpi.c
@@ -268,7 +268,24 @@
>packets, packets);
update_ctr(hnbp->ctrs,
ul ? HNB_CTR_GTPU_TOTAL_BYTES_UL :
HNB_CTR_GTPU_TOTAL_BYTES_DL,
- >bytes, bytes);
+ >total_bytes, bytes);
+
+ LOGP(DLGLOBAL, LOGL_ERROR, "XXX %s(): packets=%ld btes=%ld ueb=%ld\n",
__func__,
+packets, bytes, bytes - packets * 36);
+
+ /* Assuming an IP header of 20 bytes, derive the GTP-U payload size:
+*
+* [...] \ \
+* [ UDP ][ TCP ]| UE payload | nft reports these bytes
+* [ IP ]/ |
+* -- payload --|
+* [ GTP-U 8 bytes ]| \
+* [ UDP 8 bytes ] | | need to subtract these, ~20
+ 8 + 8
+* [ IP 20 bytes ] / /
+*/
+ update_ctr(hnbp->ctrs,
+ ul ? HNB_CTR_GTPU_UE_BYTES_UL : HNB_CTR_GTPU_UE_BYTES_DL,
+ >ue_bytes, bytes - OSMO_MIN(bytes, packets * (20 + 8 +
8)));
}
/* In the string section *pos .. end, find the first occurrence of after_str
and return the following token, which ends
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca
Gerrit-Change-Number: 36539
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[M] Change in libosmocore[master]: add osmo_stats_report_lock api
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email )
Change subject: add osmo_stats_report_lock api
..
add osmo_stats_report_lock api
Allow multi-threaded access to reported stats:
- enable use of a stats mutex with osmo_stats_report_use_lock(true).
- lock/unlock externally with osmo_stats_report_lock(true/false).
Rationale:
In osmo-hnbgw, we would like to collect stats from nftables, and do so
in a separate thead. The most efficient way is to write the parsing
results from nft directly to the rate_ctr destination.
But what if the main thread reports rate counters at exactly that time?
- is writing to stats atomic on a data type level?
- do applications need stats to be "atomic" as a whole?
In osmo-hnbgw in particular, there are two counters, 'packets' and
'total_bytes'. These correspond, and it would skew statistics if we
reported them out of sync to each other.
The simplest way to ensure correctness in all cases is a mutex around
the stats reporting.
But this mutex isn't needed in most of our programs. To completely avoid
any overhead the mutex may bring, make use of it optional with a global
flag.
This use case is likely to also show up in other programs that would
like to collect and report stats from a separate thread.
Related: SYS#6773
Related: osmo-hnbgw I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f
Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d
---
M include/osmocom/core/stats.h
M src/core/libosmocore.map
M src/core/stats.c
M src/vty/stats_vty.c
4 files changed, 96 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/38/36538/1
diff --git a/include/osmocom/core/stats.h b/include/osmocom/core/stats.h
index a034a61..4ec448c 100644
--- a/include/osmocom/core/stats.h
+++ b/include/osmocom/core/stats.h
@@ -109,6 +109,8 @@
void osmo_stats_init(void *ctx);
int osmo_stats_report(void);
+void osmo_stats_report_use_lock(bool enable);
+void osmo_stats_report_lock(bool lock);
int osmo_stats_set_interval(int interval);
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index c5ab6e3..a3c1db6 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -488,6 +488,8 @@
osmo_stats_config;
osmo_stats_init;
osmo_stats_report;
+osmo_stats_report_use_lock;
+osmo_stats_report_lock;
osmo_stats_reporter_alloc;
osmo_stats_reporter_create_log;
osmo_stats_reporter_create_statsd;
diff --git a/src/core/stats.c b/src/core/stats.c
index 16e6f62..21c7ddd 100644
--- a/src/core/stats.c
+++ b/src/core/stats.c
@@ -71,6 +71,7 @@
#include
#include
#include
+#include
#ifdef HAVE_SYS_SOCKET_H
#include
@@ -787,13 +788,59 @@
}
}
+static pthread_mutex_t *g_report_lock = NULL;
+
+/*! Enable osmo_stats_report() mutex locking with osmo_stats_report_lock().
+ * Calling osmo_stats_report_use_lock(true) */
+void osmo_stats_report_use_lock(bool enable)
+{
+ static pthread_mutex_t report_lock;
+ if (enable) {
+ /* enable locking */
+ if (!g_report_lock) {
+ pthread_mutex_init(_lock, NULL);
+ g_report_lock = _lock;
+ }
+ return;
+ }
+ /* disable locking */
+ if (g_report_lock) {
+ pthread_mutex_destroy(g_report_lock);
+ g_report_lock = NULL;
+ }
+}
+
+/*! After osmo_stats_report_use_lock(true), lock a mutex to avoid
osmo_stats_report() reading from stats.
+ * The caller must ensure to call osmo_stats_report_lock(false) as soon as
possible, or the application will lock up.
+ * This function has no effect with osmo_stats_report_use_lock(false).
+ * Useful in multi-threaded applications to allow writing to
stats/counters/rate_ctrs directly from various threads.
+ * Particularly useful to ensure that a given set of stats/counters updates
from another thread are reported in sync.
+ */
+void osmo_stats_report_lock(bool lock)
+{
+ if (!g_report_lock)
+ return;
+ if (lock)
+ pthread_mutex_lock(g_report_lock);
+ else
+ pthread_mutex_unlock(g_report_lock);
+}
+
int osmo_stats_report(void)
{
+ pthread_mutex_t *lock = g_report_lock;
+
/* per group actions */
TRACE(LIBOSMOCORE_STATS_START());
+ if (lock)
+ pthread_mutex_lock(lock);
+ /* { */
osmo_counters_for_each(handle_counter, NULL);
rate_ctr_for_each_group(rate_ctr_group_handler, NULL);
osmo_stat_item_for_each_group(osmo_stat_item_group_handler, NULL);
+ /* } */
+ if (lock)
+ pthread_mutex_unlock(lock);
/* global actions */
flush_all_reporters();
diff --git a/src/vty/stats_vty.c b/src/vty/stats_vty.c
index f940018..4a62dc4 100644
--- a/src/vty/stats_vty.c
+++ b/src/vty/stats_vty.c
@@ -426,7 +426,9 @@
if
[S] Change in osmo-hnbgw[master]: jenkins.sh: add NFTABLES axis
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/36537?usp=email )
Change subject: jenkins.sh: add NFTABLES axis
..
jenkins.sh: add NFTABLES axis
Related: osmo-ci I9828b70708dbe466c37df6ffb87b04362f14c71c
Related: OS#6425
Change-Id: I331cce7b187cf427a5cbffb3aedb17054918bcc8
---
M contrib/jenkins.sh
1 file changed, 16 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/37/36537/1
diff --git a/contrib/jenkins.sh b/contrib/jenkins.sh
index dfbbbfe..9493488 100755
--- a/contrib/jenkins.sh
+++ b/contrib/jenkins.sh
@@ -4,9 +4,11 @@
# environment variables:
# * PFCP: configure PFCP support if set to "1" (default)
# * WITH_MANUALS: build manual PDFs if set to "1"
+# * NFTABLES: configure nftables support if set to "1" (default)
# * PUBLISH: upload manuals after building if set to "1" (ignored without
WITH_MANUALS = "1")
#
PFCP=${PFCP:-1}
+NFTABLES=${NFTABLES:-1}
if ! [ -x "$(command -v osmo-build-dep.sh)" ]; then
echo "Error: We need to have scripts/osmo-deps.sh from
http://git.osmocom.org/osmo-ci/ in PATH !"
@@ -45,6 +47,9 @@
osmo-build-dep.sh libosmo-pfcp
CONFIG="$CONFIG --enable-pfcp"
fi
+if [ "$NFTABLES" = "1" ]; then
+ CONFIG="$CONFIG --enable-nftables"
+fi
if [ "$WITH_MANUALS" = "1" ]; then
CONFIG="$CONFIG --enable-manuals"
fi
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36537?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I331cce7b187cf427a5cbffb3aedb17054918bcc8
Gerrit-Change-Number: 36537
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[S] Change in osmo-msc[master]: fixup for re-est: do not succeed on acceptance fail
Attention is currently required from: pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/36520?usp=email )
Change subject: fixup for re-est: do not succeed on acceptance fail
..
Patch Set 1:
(1 comment)
File src/libmsc/msc_a.c:
https://gerrit.osmocom.org/c/osmo-msc/+/36520/comment/80bb0660_a2739f1f
PS1, Line 165: if (conn_accepted) {
> the argument goes both ways. […]
and because it's a bikeshed not even about the current patch, SCNR =)
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36520?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I3679162143e8d7d8c0878de2102faa11eadfccfc
Gerrit-Change-Number: 36520
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin
Gerrit-Attention: pespin
Gerrit-Comment-Date: Wed, 03 Apr 2024 23:31:29 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-msc[master]: fixup for re-est: do not succeed on acceptance fail
Attention is currently required from: pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/36520?usp=email )
Change subject: fixup for re-est: do not succeed on acceptance fail
..
Patch Set 1:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-msc/+/36520/comment/38ec6d3f_8a242e2c
PS1, Line 18: Related: SYS#5130
> Fixes: ae98b97382285420ba81549bc874b9fea5e7daa9
never seen this header pointing at a commit hash before. "Fixes:" is about
issue numbers. AFAIK we hardly ever use this header. I've never used it before
and it has not been indicated that i should, so where is this coming from?
File src/libmsc/msc_a.c:
https://gerrit.osmocom.org/c/osmo-msc/+/36520/comment/1e89b76a_e1b1e176
PS1, Line 165: if (conn_accepted) {
> Can we please try refactoring this function? I see "if (conn_accepted)"
> spread like in 4 different p […]
the argument goes both ways. if you prefer the other way, then i guess you can
suggest a refactoring. i'm not going to spend time on it, because IMHO this is
bad style:
if (conn_accepted) {
update_counters(fi, true);
...
} else {
update_counters(fi, false);
...
}
instead of just
update_counters(fi, conn_accepted);
as we have now.
resolving because it's about code prior to this patch.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36520?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I3679162143e8d7d8c0878de2102faa11eadfccfc
Gerrit-Change-Number: 36520
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin
Gerrit-Attention: pespin
Gerrit-Comment-Date: Wed, 03 Apr 2024 23:27:07 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-msc[master]: fixup for re-est: do not succeed on acceptance fail
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-msc/+/36520?usp=email )
Change subject: fixup for re-est: do not succeed on acceptance fail
..
fixup for re-est: do not succeed on acceptance fail
Fix a bug introduced in commit
implement CM Re-Establish for voice calls
ae98b97382285420ba81549bc874b9fea5e7daa9
Neels Hofmeyr
Thu Jul 29 22:40:59 2021 +0200
I6fa37d6ca9fcb1637742b40e37b68d67664c9b60
We should only succeed when conn_accepted == true!
Related: SYS#5130
Change-Id: I3679162143e8d7d8c0878de2102faa11eadfccfc
---
M src/libmsc/msc_a.c
1 file changed, 20 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/20/36520/1
diff --git a/src/libmsc/msc_a.c b/src/libmsc/msc_a.c
index 70d9bf7..db1d998 100644
--- a/src/libmsc/msc_a.c
+++ b/src/libmsc/msc_a.c
@@ -182,7 +182,7 @@
if (msc_a->complete_layer3_type == COMPLETE_LAYER3_LU)
msc_a_put(msc_a, MSC_A_USE_LOCATION_UPDATING);
- if (msc_a->complete_layer3_type == COMPLETE_LAYER3_CM_RE_ESTABLISH_REQ)
{
+ if (conn_accepted && msc_a->complete_layer3_type ==
COMPLETE_LAYER3_CM_RE_ESTABLISH_REQ) {
/* Trigger new Assignment to recommence the voice call. A
little dance here because normally we verify
* that no CC trans is already active. */
struct gsm_trans *cc_trans = msc_a->cc.active_trans;
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36520?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I3679162143e8d7d8c0878de2102faa11eadfccfc
Gerrit-Change-Number: 36520
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[L] Change in osmo-mgw[master]: Convert RTP/RTCP/OSMUX I/O from osmo_fd to osmo_io
Attention is currently required from: dexter, fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36363?usp=email ) Change subject: Convert RTP/RTCP/OSMUX I/O from osmo_fd to osmo_io .. Patch Set 5: (1 comment) File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/a16ed713_0b0deee4 PS4, Line 1047: * \param[in] msg message buffer that holds the data to be send. > > What works quite well is to put a msgb in OTC_SELECT ... […] gotcha, but i think i did decide to do so once, because it's a powerful pattern, i think it was libosmo-pfcp -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36363?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: I8471960d5d8088a70cf105f2f40dfa5d5458169a Gerrit-Change-Number: 36363 Gerrit-PatchSet: 5 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: jolly Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Tue, 02 Apr 2024 16:17:19 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Comment-In-Reply-To: fixeria Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: Convert RTP/RTCP/OSMUX I/O from osmo_fd to osmo_io
Attention is currently required from: dexter, fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36363?usp=email ) Change subject: Convert RTP/RTCP/OSMUX I/O from osmo_fd to osmo_io .. Patch Set 5: Code-Review+1 (5 comments) Patchset: PS5: this patch is "high density" so probably will be able to know its details only after we merged and use it... File include/osmocom/mgcp/mgcp.h: https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/d88469b5_002c9042 PS5, Line 4: * maybe time to bump the (C) File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/66e82d32_ec1b345d PS4, Line 1047: * \param[in] msg message buffer that holds the data to be send. > Done (What works quite well is to put a msgb in OTC_SELECT, then any code path can talloc_steal() to claim it, and if no-one does it is freed implicitly. and we avoid the numerous stages of "error? then free. else not." Also sometimes nice to avoid msgb copying.) File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/7402418b_c03ae728 PS5, Line 1044: c typo https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/c6d2026b_7e9c1392 PS5, Line 1598: NOTICE ERROR? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36363?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: I8471960d5d8088a70cf105f2f40dfa5d5458169a Gerrit-Change-Number: 36363 Gerrit-PatchSet: 5 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: jolly Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: dexter Gerrit-Comment-Date: Tue, 02 Apr 2024 13:44:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-hnbgw[master]: use json to decode counters from nftables
Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36485?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: use json to decode counters from nftables .. use json to decode counters from nftables Switch nft output to json. Add libjansson dependency, and use it to parse the result. Navigate the json tree to retrieve the counter values. Parsing with json takes longer than the direct string parsing of nft language, but it is the more correct and more stable approach: in case nftables changes its API, json is likely to still be compatible. Related: SYS#6773 Change-Id: Id4e7fa017c31945388a010d8581715d71482116b --- M configure.ac M src/osmo-hnbgw/nft_kpi.c 2 files changed, 115 insertions(+), 72 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/85/36485/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36485?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Id4e7fa017c31945388a010d8581715d71482116b Gerrit-Change-Number: 36485 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: osmith Gerrit-Attention: neels Gerrit-MessageType: newpatchset
[S] Change in libosmocore[master]: sockaddr_str: add conversion to,from osmo_sockaddr
neels has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email )
Change subject: sockaddr_str: add conversion to,from osmo_sockaddr
..
sockaddr_str: add conversion to,from osmo_sockaddr
Add functions to pass struct osmo_sockaddr to the
osmo_sockaddr_str_{to,from}* API directly.
So far the interface to osmo_sockaddr_str_ is:
osmo_sockaddr_str_from_sockaddr(_sa_str, _osa->u.sas);
I'm working a lot with osmo_sockaddr at the moment, and the cumulated
time of forgetting to add 'u.sas' and having another compilation cycle
because of those is justifying this additional API.
Change-Id: I0df84b4bb8cb5d8434b735fa3a38e7f95be43e91
---
M TODO-RELEASE
M include/osmocom/core/sockaddr_str.h
M src/core/libosmocore.map
M src/core/sockaddr_str.c
4 files changed, 48 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
neels: Looks good to me, approved
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 01f7c41..ce04162 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -11,6 +11,7 @@
core ADD osmo_sock_multiaddr_get_ip_and_port(),
osmo_multiaddr_ip_and_port_snprintf(), osmo_sock_multiaddr_get_name_buf()
core ADD osmo_sock_sctp_get_peer_addr_info()
core ADD gsmtap_inst_fd2() core, DEPRECATE gsmtap_inst_fd()
+core ADD osmo_sockaddr_str_from_osa() osmo_sockaddr_str_to_osa()
core behavior change osmo_tdef_fsm_inst_state_chg(): allow
millisecond precision
core ABI change osmo_io_ops now contains a struct of
structs, not union of structs
core ABI change osmo_iofd_set_ioops() now returns a
value (error code)
diff --git a/include/osmocom/core/sockaddr_str.h
b/include/osmocom/core/sockaddr_str.h
index c646f49..8ec514c 100644
--- a/include/osmocom/core/sockaddr_str.h
+++ b/include/osmocom/core/sockaddr_str.h
@@ -34,6 +34,7 @@
struct sockaddr_storage;
struct sockaddr_in;
struct sockaddr_in6;
+struct osmo_sockaddr;
/*! \defgroup sockaddr_str IP address/port utilities.
* @{
@@ -84,6 +85,7 @@
int osmo_sockaddr_str_from_sockaddr_in(struct osmo_sockaddr_str *sockaddr_str,
const struct sockaddr_in *src);
int osmo_sockaddr_str_from_sockaddr_in6(struct osmo_sockaddr_str
*sockaddr_str, const struct sockaddr_in6 *src);
int osmo_sockaddr_str_from_sockaddr(struct osmo_sockaddr_str *sockaddr_str,
const struct sockaddr_storage *src);
+int osmo_sockaddr_str_from_osa(struct osmo_sockaddr_str *sockaddr_str, const
struct osmo_sockaddr *src);
int osmo_sockaddr_str_to_in_addr(const struct osmo_sockaddr_str *sockaddr_str,
struct in_addr *dst);
int osmo_sockaddr_str_to_in6_addr(const struct osmo_sockaddr_str
*sockaddr_str, struct in6_addr *dst);
@@ -92,6 +94,7 @@
int osmo_sockaddr_str_to_sockaddr_in(const struct osmo_sockaddr_str
*sockaddr_str, struct sockaddr_in *dst);
int osmo_sockaddr_str_to_sockaddr_in6(const struct osmo_sockaddr_str
*sockaddr_str, struct sockaddr_in6 *dst);
int osmo_sockaddr_str_to_sockaddr(const struct osmo_sockaddr_str
*sockaddr_str, struct sockaddr_storage *dst);
+int osmo_sockaddr_str_to_osa(const struct osmo_sockaddr_str *sockaddr_str,
struct osmo_sockaddr *dst);
int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str,
uint32_t ip, uint16_t port)
OSMO_DEPRECATED("osmo_sockaddr_str_from_32n() actually uses *host* byte
order. Use osmo_sockaddr_str_from_32h() instead");
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index fb294ba..c5ab6e3 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -400,6 +400,7 @@
osmo_sockaddr_str_from_sockaddr;
osmo_sockaddr_str_from_sockaddr_in;
osmo_sockaddr_str_from_sockaddr_in6;
+osmo_sockaddr_str_from_osa;
osmo_sockaddr_str_from_str;
osmo_sockaddr_str_from_str2;
osmo_sockaddr_str_is_nonzero;
@@ -412,6 +413,7 @@
osmo_sockaddr_str_to_sockaddr;
osmo_sockaddr_str_to_sockaddr_in;
osmo_sockaddr_str_to_sockaddr_in6;
+osmo_sockaddr_str_to_osa;
osmo_sockaddr_to_octets;
osmo_sockaddr_to_str;
osmo_sockaddr_to_str_and_uint;
diff --git a/src/core/sockaddr_str.c b/src/core/sockaddr_str.c
index 9f1e897..bea6184 100644
--- a/src/core/sockaddr_str.c
+++ b/src/core/sockaddr_str.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
/*! \addtogroup sockaddr_str
*
@@ -359,6 +360,17 @@
return -EINVAL;
}
+/*! Convert IPv4 or IPv6 address and port to osmo_sockaddr_str.
+ * \param[out] sockaddr_str The instance to copy to.
+ * \param[in] src IPv4 or IPv6 address and port data.
+ * \return 0 on success, negative if src does not indicate AF_INET nor
AF_INET6 (or if the conversion fails, which
+ * should not be possible in practice).
+ */
+int osmo_sockaddr_str_from_osa(struct osmo_sockaddr_str *sockaddr_str, const
struct osmo_sockaddr *src)
+{
+ return osmo_sockaddr_str_
[S] Change in libosmocore[master]: sockaddr_str: add conversion to,from osmo_sockaddr
Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email ) Change subject: sockaddr_str: add conversion to,from osmo_sockaddr .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0df84b4bb8cb5d8434b735fa3a38e7f95be43e91 Gerrit-Change-Number: 36265 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Comment-Date: Thu, 28 Mar 2024 14:47:29 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-ttcn3-hacks[master]: msc: add TC_lu_tmsi_noauth_notmsi
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email ) Change subject: msc: add TC_lu_tmsi_noauth_notmsi .. Patch Set 1: (1 comment) File msc/MSC_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459/comment/a7055a5f_736c3359 PS1, Line 7312: valueof > 1- a "template (value)" parameter can be passed either a "template (value)" > var or a value val, both […] This code has a lot of complexity to discuss, but that is not in this "valueof()" here. It is, still, really a bikeshed. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: If10b9987395670b084ff8ad6d1f033ff46896d75 Gerrit-Change-Number: 36459 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 28 Mar 2024 03:49:22 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457?usp=email ) Change subject: msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I4d719928f04e5a47d415c38f835451b1f10c713d Gerrit-Change-Number: 36457 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Thu, 28 Mar 2024 03:22:51 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): better detect failure
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email ) Change subject: msc: f_expect_paging(): better detect failure .. Patch Set 1: (1 comment) Patchset: PS1: my opinion on the bikeshed property is unchanged -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I30273e3882e348a2ded88b7b96a5ec1473a56913 Gerrit-Change-Number: 36456 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 28 Mar 2024 03:22:43 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): fix by_tmsi arg
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email ) Change subject: msc: f_expect_paging(): fix by_tmsi arg .. Patch Set 1: (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/comment/2124249d_bc8af427 PS1, Line 1360: boolean by_tmsi := true > I don't even know why I'm doing code review if 99% of what I say seems to be > irrelevant, bikeshed, e […] i appreciate the feedback and myself tend to give strict review, so i know what it's like. i am still of the humble opinion that this is not relevant enough to discuss. When I weigh the possible consequences against the time spent, here is way more time than consequence, so please let's quit this. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I9434745b7faeb738caafed8080b9f7b1a6a8079a Gerrit-Change-Number: 36455 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 28 Mar 2024 03:21:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: ran-emu: allow receiving Paging without a TMSI
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454?usp=email ) Change subject: ran-emu: allow receiving Paging without a TMSI .. Patch Set 1: (1 comment) File library/RAN_Emulation.ttcnpp: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454/comment/92551cee_1c2a6dc4 PS1, Line 507: var template OCT4 tmsi := omit; > that would be a var "template (omit) OCT4" it works, why change it? -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I1bdf3488be0f8d4f0905665c4ba642f9468b9777 Gerrit-Change-Number: 36454 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 28 Mar 2024 03:13:23 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: fix VLR evil twin on LU with unknown TMSI
neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email )
Change subject: fix VLR evil twin on LU with unknown TMSI
..
fix VLR evil twin on LU with unknown TMSI
When a subscriber first attaches by TMSI only, and later tells the IMSI
via ID Response, it may turn out that this IMSI already exists in the
VLR database. If this happens, the TMSI that the subscriber issued was
not known in the existing VLR entry, indicating that the subscriber has
in the meantime camped on a different core. Which means we can assume
that there cannot be any active connections, and the old subscriber can
be discarded, for the benefit of the new one.
(We could also discard the new one, but it is more complex to reparent
the ongoing FSMs for Compl L3 than to copy some dormant VLR state.)
In vlr_subscr_set_imsi(), check for an existing IMSI entry in the VLR.
If such exists, copy any pending Paging and auth tuple state to the new
subscriber, and discard the old one from the VLR.
In order to safely discard a vlr subscriber by force, add a new vlr_ops
function: subscr_inval(), to tell the MSC that a vlr_subscr is no longer
valid.
Upcoming patch I583682d1a35a70b008d7bb2d89ba7c3109a60b21 better clears
TMSI state from the VLR, making it more likely to hit the evil twin
situation this patch fixes; hence this is, sort of, preparation.
Related: SYS#6860 OS#4721
Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd
---
M include/osmocom/msc/paging.h
M include/osmocom/msc/vlr.h
M src/libmsc/gsm_04_08.c
M src/libmsc/paging.c
M src/libvlr/vlr.c
M tests/msc_vlr/msc_vlr_test_rest.err
6 files changed, 139 insertions(+), 2 deletions(-)
Approvals:
Jenkins Builder: Verified
neels: Looks good to me, approved
diff --git a/include/osmocom/msc/paging.h b/include/osmocom/msc/paging.h
index 4de679d..f8ebf9e 100644
--- a/include/osmocom/msc/paging.h
+++ b/include/osmocom/msc/paging.h
@@ -40,6 +40,7 @@
struct paging_request *paging_request_start(struct vlr_subscr *vsub, enum
paging_cause cause,
paging_cb_t paging_cb, struct
gsm_trans *trans,
const char *label);
+void paging_request_join_vsub(struct vlr_subscr *keep_vsub, struct vlr_subscr
*discarding_vsub);
void paging_request_remove(struct paging_request *pr);
void paging_response(struct msc_a *msc_a);
diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h
index 86a72f2..a7707fd 100644
--- a/include/osmocom/msc/vlr.h
+++ b/include/osmocom/msc/vlr.h
@@ -257,6 +257,8 @@
/* notify MSC/SGSN that the given subscriber has been associated
* with this msc_conn_ref */
int (*subscr_assoc)(void *msc_conn_ref, struct vlr_subscr *vsub);
+ /* notify MSC that the given subscriber is no longer valid */
+ void (*subscr_inval)(void *msc_conn_ref, struct vlr_subscr *vsub);
};
/* An instance of the VLR codebase */
diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 70faf95..17350fa 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -1572,6 +1572,24 @@
return 0;
}
+static void msc_vlr_subscr_inval(void *msc_conn_ref, struct vlr_subscr *vsub)
+{
+ /* Search vsub backwards to make sure msc_conn_ref is a valid msc_a
instance. */
+ struct msub *msub;
+ OSMO_ASSERT(vsub);
+ llist_for_each_entry(msub, _list, entry) {
+ struct msc_a *msc_a;
+ if (msub->vsub != vsub)
+ continue;
+
+ msc_a = msub_msc_a(msub);
+ if (msc_a)
+ msc_a_release_cn(msc_a);
+
+ msub->vsub = NULL;
+ }
+}
+
/* operations that we need to implement for libvlr */
const struct vlr_ops msc_vlr_ops = {
.tx_auth_req = msc_vlr_tx_auth_req,
@@ -1586,6 +1604,7 @@
.tx_mm_info = msc_vlr_tx_mm_info,
.subscr_update = msc_vlr_subscr_update,
.subscr_assoc = msc_vlr_subscr_assoc,
+ .subscr_inval = msc_vlr_subscr_inval,
};
struct msgb *gsm48_create_mm_serv_rej(enum gsm48_reject_value value)
diff --git a/src/libmsc/paging.c b/src/libmsc/paging.c
index 9845f99..9b3dad5 100644
--- a/src/libmsc/paging.c
+++ b/src/libmsc/paging.c
@@ -120,6 +120,34 @@
return pr;
}
+/* Two subscribers (e.g. an old TMSI and a new TMSI) turn out to have the same
identity, so in order to discard one of
+ * them, transfer any pending Paging requests to the vsub that will survive. */
+void paging_request_join_vsub(struct vlr_subscr *keep_vsub, struct vlr_subscr
*discarding_vsub)
+{
+ struct paging_request *pr;
+
+ if (!discarding_vsub->cs.is_paging)
+ return;
+
+ /* transfer all Paging Response callbacks */
+ while ((pr = llist_first_entry_or_null(_vsub->cs.requests,
struct paging_request, entry))) {
+ llist_del(>entry);
+
[S] Change in osmo-msc[master]: never page for TMSI with 'no assign-tmsi'
neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/36453?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted
one.
)Change subject: never page for TMSI with 'no assign-tmsi'
..
never page for TMSI with 'no assign-tmsi'
With 'no assign-tmsi', regard any TMSI as invalidated at the end of a
Location Updating procedure. Hence, avoid paging by TMSI.
When 'no assign-tmsi' is set, osmo-msc does not actively assign a new
TMSI at the end of the Location Updating. However, it stores any TMSI
identity that the MS sends in a Location Updating Request. So far, this
caused osmo-msc to use the TMSI that the MS had sent in subsequent
Paging, which goes unanswered by the MS.
(After the long standing evil twin problem regarding TMSI MI has been
fixed in recent Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd, there is no
longer an evil twin risked by clearing out a TMSI.)
Related: SYS#6860 OS#4721
Change-Id: I583682d1a35a70b008d7bb2d89ba7c3109a60b21
---
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_rest.err
2 files changed, 38 insertions(+), 11 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
fixeria: Looks good to me, but someone else must approve
diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index 7500c86..5d8f78b 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -469,6 +469,8 @@
lu_compl_vlr_new_tmsi(fi);
return;
}
+ /* else, any previously used TMSI is now invalid. */
+ vsub->tmsi = GSM_RESERVED_TMSI;
/* Location Updating Accept */
vlr->ops.tx_lu_acc(lcvp->msc_conn_ref, GSM_RESERVED_TMSI);
@@ -514,6 +516,8 @@
/* Wait for TMSI ack */
return;
}
+ /* else, any previously used TMSI is now invalid. */
+ vsub->tmsi = GSM_RESERVED_TMSI;
/* No TMSI needed, accept now. */
vlr->ops.tx_lu_acc(lcvp->msc_conn_ref, GSM_RESERVED_TMSI);
diff --git a/tests/msc_vlr/msc_vlr_test_rest.err
b/tests/msc_vlr/msc_vlr_test_rest.err
index fae302f..3a6442e 100644
--- a/tests/msc_vlr/msc_vlr_test_rest.err
+++ b/tests/msc_vlr/msc_vlr_test_rest.err
@@ -527,7 +527,7 @@
DVLR
lu_compl_vlr_fsm(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){LU_COMPL_VLR_S_INIT}:
state_chg to LU_COMPL_VLR_S_WAIT_SUB_PRES
DVLR
lu_compl_vlr_fsm(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){LU_COMPL_VLR_S_WAIT_SUB_PRES}:
Received Event LU_COMPL_VLR_E_SUB_PRES_COMPL
- sending LU Accept for
IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU
-DREF VLR subscr IMSI-90170004620:MSISDN-46071:TMSI-0x23422342 + attached:
now used by 3 (active-conn,vlr_gsup_rx,attached)
+DREF VLR subscr IMSI-90170004620:MSISDN-46071 + attached: now used by 3
(active-conn,vlr_gsup_rx,attached)
DVLR
lu_compl_vlr_fsm(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){LU_COMPL_VLR_S_WAIT_SUB_PRES}:
state_chg to LU_COMPL_VLR_S_DONE
DVLR
vlr_lu_fsm(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){VLR_ULA_S_WAIT_LU_COMPL}:
Received Event VLR_ULA_E_LU_COMPL_SUCCESS
DVLR
lu_compl_vlr_fsm(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){LU_COMPL_VLR_S_DONE}:
Terminating in cascade, depth 2 (cause = OSMO_FSM_TERM_PARENT, caused by:
upd_hlr_vlr_fsm(TMSI-0x23422342:GERAN-A:LU))
@@ -540,15 +540,15 @@
DMSC
msc_a(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){MSC_A_ST_AUTHENTICATED}:
Received Event MSC_A_EV_UNUSED
DMSC
msc_a(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){MSC_A_ST_AUTHENTICATED}:
state_chg to MSC_A_ST_RELEASING
DBSSAP
msc_a(IMSI-90170004620:MSISDN-46071:TMSI-0x23422342:GERAN-A:LU){MSC_A_ST_RELEASING}:
Releasing: msc_a use is 0 (-)
-DREF VLR subscr IMSI-90170004620:MSISDN-46071:TMSI-0x23422342 +
msc_a_fsm_releasing_onenter: now used by 4
(active-conn,vlr_gsup_rx,attached,msc_a_fsm_releasing_onenter)
-DREF VLR subscr IMSI-90170004620:MSISDN-46071:TMSI-0x23422342 +
vlr_subscr_cancel_attach_fsm: now used by 5
(active-conn,vlr_gsup_rx,attached,msc_a_fsm_releasing_onenter,vlr_subscr_cancel_attach_fsm)
-DREF VLR subscr IMSI-90170004620:MSISDN-46071:TMSI-0x23422342 -
vlr_subscr_cancel_attach_fsm: now used by 4
(active-conn,vlr_gsup_rx,attached,msc_a_fsm_releasing_onenter)
+DREF VLR subscr IMSI-90170004620:MSISDN-46071 +
msc_a_fsm_releasing_onenter: now used by 4
(active-conn,vlr_gsup_rx,attached,msc_a_fsm_releasing_onenter)
+DREF VLR subscr IMSI-90170004620:MSISDN-46071 +
vlr_subscr_cancel_attach_fsm: now used by 5
(active-conn,vlr_gsup_rx,attached,msc_a_fsm_releasing_onenter,vlr_subscr_cancel_attach_fsm)
+DREF VLR subscr IMSI-901700
[S] Change in osmo-msc[master]: invalidate vsub->msc_conn_ref when msc_a is discarded
neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/36451?usp=email )
Change subject: invalidate vsub->msc_conn_ref when msc_a is discarded
..
invalidate vsub->msc_conn_ref when msc_a is discarded
We have an msc_conn_ref pointer from vlr_subscr to an active msc_a
instance. So far, we just keep it pointing at discarded memory. Instead,
make sure it goes back to NULL when the msc_a instance deallocates.
This way the VLR can reliably tell whether a given VLR entry still has
an active connection or is just inactively caching the subscriber.
Related: SYS#6860 OS#4721
Change-Id: Ic63d01d220b63453976fe06a7c6b606f97172c99
---
M src/libmsc/msc_a.c
1 file changed, 22 insertions(+), 0 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
fixeria: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
diff --git a/src/libmsc/msc_a.c b/src/libmsc/msc_a.c
index 4fb30c4..70d9bf7 100644
--- a/src/libmsc/msc_a.c
+++ b/src/libmsc/msc_a.c
@@ -999,6 +999,7 @@
void msc_a_fsm_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause
cause)
{
struct msc_a *msc_a = msc_a_fi_priv(fi);
+ struct vlr_subscr *vsub = msc_a_vsub(msc_a);
trans_conn_closed(msc_a);
@@ -1006,6 +1007,10 @@
LOG_MSC_A(msc_a, LOGL_ERROR, "Deallocating active transactions
failed\n");
LOG_MSC_A_CAT(msc_a, DREF, LOGL_DEBUG, "max total use count was %d\n",
msc_a->max_total_use_count);
+
+ /* Invalidate the active conn in VLR subscriber state, if any. */
+ if (vsub && vsub->msc_conn_ref == msc_a)
+ vsub->msc_conn_ref = NULL;
}
const struct value_string msc_a_fsm_event_names[] = {
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36451?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ic63d01d220b63453976fe06a7c6b606f97172c99
Gerrit-Change-Number: 36451
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-MessageType: merged
[M] Change in osmo-msc[master]: fix VLR evil twin on LU with unknown TMSI
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email ) Change subject: fix VLR evil twin on LU with unknown TMSI .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd Gerrit-Change-Number: 36452 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 28 Mar 2024 03:10:24 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: fix VLR evil twin on LU with unknown TMSI
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email ) Change subject: fix VLR evil twin on LU with unknown TMSI .. Patch Set 2: (1 comment) File src/libvlr/vlr.c: https://gerrit.osmocom.org/c/osmo-msc/+/36452/comment/a1cbfb2c_e038db31 PS1, Line 597: struct vlr_instance *vlr = exists->vlr; > ack […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd Gerrit-Change-Number: 36452 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 28 Mar 2024 03:07:28 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: fix VLR evil twin on LU with unknown TMSI
Attention is currently required from: fixeria, laforge, neels, pespin. Hello Jenkins Builder, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email to look at the new patch set (#3). Change subject: fix VLR evil twin on LU with unknown TMSI .. fix VLR evil twin on LU with unknown TMSI When a subscriber first attaches by TMSI only, and later tells the IMSI via ID Response, it may turn out that this IMSI already exists in the VLR database. If this happens, the TMSI that the subscriber issued was not known in the existing VLR entry, indicating that the subscriber has in the meantime camped on a different core. Which means we can assume that there cannot be any active connections, and the old subscriber can be discarded, for the benefit of the new one. (We could also discard the new one, but it is more complex to reparent the ongoing FSMs for Compl L3 than to copy some dormant VLR state.) In vlr_subscr_set_imsi(), check for an existing IMSI entry in the VLR. If such exists, copy any pending Paging and auth tuple state to the new subscriber, and discard the old one from the VLR. In order to safely discard a vlr subscriber by force, add a new vlr_ops function: subscr_inval(), to tell the MSC that a vlr_subscr is no longer valid. Upcoming patch I583682d1a35a70b008d7bb2d89ba7c3109a60b21 better clears TMSI state from the VLR, making it more likely to hit the evil twin situation this patch fixes; hence this is, sort of, preparation. Related: SYS#6860 OS#4721 Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd --- M include/osmocom/msc/paging.h M include/osmocom/msc/vlr.h M src/libmsc/gsm_04_08.c M src/libmsc/paging.c M src/libvlr/vlr.c M tests/msc_vlr/msc_vlr_test_rest.err 6 files changed, 139 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/52/36452/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd Gerrit-Change-Number: 36452 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[S] Change in osmo-msc[master]: never page for TMSI with 'no assign-tmsi'
Attention is currently required from: neels. Hello Jenkins Builder, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/36453?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder The change is no longer submittable: Verified is unsatisfied now. Change subject: never page for TMSI with 'no assign-tmsi' .. never page for TMSI with 'no assign-tmsi' With 'no assign-tmsi', regard any TMSI as invalidated at the end of a Location Updating procedure. Hence, avoid paging by TMSI. When 'no assign-tmsi' is set, osmo-msc does not actively assign a new TMSI at the end of the Location Updating. However, it stores any TMSI identity that the MS sends in a Location Updating Request. So far, this caused osmo-msc to use the TMSI that the MS had sent in subsequent Paging, which goes unanswered by the MS. (After the long standing evil twin problem regarding TMSI MI has been fixed in recent Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd, there is no longer an evil twin risked by clearing out a TMSI.) Related: SYS#6860 OS#4721 Change-Id: I583682d1a35a70b008d7bb2d89ba7c3109a60b21 --- M src/libvlr/vlr_lu_fsm.c M tests/msc_vlr/msc_vlr_test_rest.err 2 files changed, 38 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/53/36453/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36453?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I583682d1a35a70b008d7bb2d89ba7c3109a60b21 Gerrit-Change-Number: 36453 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-MessageType: newpatchset
[M] Change in osmo-msc[master]: fix VLR evil twin on LU with unknown TMSI
Attention is currently required from: fixeria, laforge, neels, pespin. Hello Jenkins Builder, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by fixeria, Code-Review+1 by laforge, Code-Review+1 by pespin, Verified+1 by Jenkins Builder Change subject: fix VLR evil twin on LU with unknown TMSI .. fix VLR evil twin on LU with unknown TMSI When a subscriber first attaches by TMSI only, and later tells the IMSI via ID Response, it may turn out that this IMSI already exists in the VLR database. If this happens, the TMSI that the subscriber issued was not known in the existing VLR entry, indicating that the subscriber has in the meantime camped on a different core. Which means we can assume that there cannot be any active connections, and the old subscriber can be discarded, for the benefit of the new one. (We could also discard the new one, but it is more complex to reparent the ongoing FSMs for Compl L3 than to copy some dormant VLR state.) In vlr_subscr_set_imsi(), check for an existing IMSI entry in the VLR. If such exists, copy any pending Paging and auth tuple state to the new subscriber, and discard the old one from the VLR. In order to safely discard a vlr subscriber by force, add a new vlr_ops function: subscr_inval(), to tell the MSC that a vlr_subscr is no longer valid. Upcoming patch I583682d1a35a70b008d7bb2d89ba7c3109a60b21 better clears TMSI state from the VLR, making it more likely to hit the evil twin situation this patch fixes; hence this is, sort of, preparation. Related: SYS#6860 OS#4721 Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd --- M include/osmocom/msc/paging.h M include/osmocom/msc/vlr.h M src/libmsc/gsm_04_08.c M src/libmsc/paging.c M src/libvlr/vlr.c M tests/msc_vlr/msc_vlr_test_rest.err 6 files changed, 138 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/52/36452/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd Gerrit-Change-Number: 36452 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[S] Change in libosmocore[master]: sockaddr_str: add conversion to,from osmo_sockaddr
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email ) Change subject: sockaddr_str: add conversion to,from osmo_sockaddr .. Patch Set 2: (2 comments) Patchset: PS1: > I'm not feeling it -- we have the libfoo. […] Done Patchset: PS2: other patches use this, i'd like to merge. I am tempted to count the earlier +1 since there is an entry in TODO-RELEASE now. soon. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0df84b4bb8cb5d8434b735fa3a38e7f95be43e91 Gerrit-Change-Number: 36265 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 28 Mar 2024 02:45:56 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in libosmocore[master]: sockaddr_str: add conversion to,from osmo_sockaddr
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Verified+1 by Jenkins Builder
Change subject: sockaddr_str: add conversion to,from osmo_sockaddr
..
sockaddr_str: add conversion to,from osmo_sockaddr
Add functions to pass struct osmo_sockaddr to the
osmo_sockaddr_str_{to,from}* API directly.
So far the interface to osmo_sockaddr_str_ is:
osmo_sockaddr_str_from_sockaddr(_sa_str, _osa->u.sas);
I'm working a lot with osmo_sockaddr at the moment, and the cumulated
time of forgetting to add 'u.sas' and having another compilation cycle
because of those is justifying this additional API.
Change-Id: I0df84b4bb8cb5d8434b735fa3a38e7f95be43e91
---
M TODO-RELEASE
M include/osmocom/core/sockaddr_str.h
M src/core/libosmocore.map
M src/core/sockaddr_str.c
4 files changed, 48 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/65/36265/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0df84b4bb8cb5d8434b735fa3a38e7f95be43e91
Gerrit-Change-Number: 36265
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: pespin
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-MessageType: newpatchset
[S] Change in libosmocore[master]: sockaddr_str: add conversion to,from osmo_sockaddr
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email ) Change subject: sockaddr_str: add conversion to,from osmo_sockaddr .. Patch Set 1: (2 comments) Patchset: PS1: > Are you the one looking at tons of commit log lines every few months for +10 > projects? I am, and I r […] I'm not feeling it -- we have the libfoo.map files showing similar information that is guaranteed to be correct. Commit log summary should show it. The git log is quick to search for the first appearance of a function. I don't want to sabotage the release process or whine and make your life harder, but I doubt maintaining such lists manually is a good idea. File src/core/libosmocore.map: https://gerrit.osmocom.org/c/libosmocore/+/36265/comment/cc092b05_5969feb2 PS1, Line 401: osmo_sockaddr_str_from_osa; here is a diff of added API -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36265?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0df84b4bb8cb5d8434b735fa3a38e7f95be43e91 Gerrit-Change-Number: 36265 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 28 Mar 2024 02:36:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-hnbgw[master]: use json to decode counters from nftables
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/36485?usp=email )
Change subject: use json to decode counters from nftables
..
use json to decode counters from nftables
Switch nft output to json. Add libjansson dependency, and use it to
parse the result. Navigate the json tree to retrieve the counter values.
Parsing with json takes longer than the direct string parsing of nft
language, but it is the more correct and more stable approach: in case
nftables changes its API, json is likely to still be compatible.
Related: SYS#6773
Change-Id: Id4e7fa017c31945388a010d8581715d71482116b
---
M configure.ac
M src/osmo-hnbgw/nft_kpi.c
2 files changed, 115 insertions(+), 72 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/85/36485/1
diff --git a/configure.ac b/configure.ac
index bea9a76..657ad0d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -60,6 +60,7 @@
PKG_CHECK_MODULES(LIBOSMOHNBAP, libosmo-hnbap >= 1.5.0)
PKG_CHECK_MODULES(LIBOSMOMGCPCLIENT, libosmo-mgcp-client >= 1.12.0)
PKG_CHECK_MODULES(LIBNFTABLES, libnftables >= 1.0.2)
+PKG_CHECK_MODULES(LIBJANSSON, jansson >= 2.14)
# Enable PFCP support for GTP tunnel mapping via UPF
AC_ARG_ENABLE([pfcp], [AS_HELP_STRING([--enable-pfcp], [Build with PFCP
support, for GTP tunnel mapping via UPF])],
diff --git a/src/osmo-hnbgw/nft_kpi.c b/src/osmo-hnbgw/nft_kpi.c
index 0dd702f..0b5ac91 100644
--- a/src/osmo-hnbgw/nft_kpi.c
+++ b/src/osmo-hnbgw/nft_kpi.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include
#include
@@ -51,7 +52,7 @@
OSMO_ASSERT(false);
}
- nft_ctx_output_set_flags(s->nft.nft_ctx, NFT_CTX_OUTPUT_HANDLE);
+ nft_ctx_output_set_flags(s->nft.nft_ctx, NFT_CTX_OUTPUT_JSON);
return s->nft.nft_ctx;
}
@@ -219,99 +220,123 @@
>bytes, bytes);
}
-/* In the string section *pos .. end, find the first occurence of after_str
and return the following token, which ends
- * by a space or at end. If end is NULL, search until the '\0' termination of
*pos.
- * Return true if after_str was found, copy the following token into buf, and
in *pos, return the position just after
- * that token. */
-static bool get_token_after(char *buf, size_t buflen, const char **pos, const
char *end, const char *after_str)
+static int hnb_update_counters_by_id(const char *id_str, bool ul, int64_t
packets, int64_t bytes, int64_t handle,
+struct hnb_persistent **last_hnbp)
{
- const char *found = strstr(*pos, after_str);
- const char *token_end;
- size_t token_len;
- if (!found)
- return false;
- if (end && found >= end) {
- *pos = end;
- return false;
+ struct hnb_persistent *hnbp;
+ LOGP(DNFT, LOGL_DEBUG, "%s(%s, %s, %"PRId64", %"PRId64", %"PRId64")\n",
__func__,
+id_str, ul ? "ul" : "dl", packets, bytes, handle);
+
+ /* Half the time, we already have a pointer to the correct hnb */
+ if (last_hnbp && *last_hnbp && !strcmp((*last_hnbp)->id_str, id_str))
+ hnbp = *last_hnbp;
+ else
+ hnbp = hnb_persistent_find_by_id_str(id_str);
+ if (!hnbp) {
+ LOGP(DNFT, LOGL_DEBUG, "%s(): cannot update counters, hNodeB
not found: %s\n", __func__, id_str);
+ return -ENOENT;
}
- found += strlen(after_str);
- while (*found && *found == ' ' && (!end || found < end))
- found++;
- token_end = found;
- while (*token_end != ' ' && (!end || token_end < end))
- token_end++;
- if (token_end <= found) {
- *pos = found;
- return false;
- }
- if (*found == '"' && token_end > found + 1 && *(token_end - 1) == '"') {
- found++;
- token_end--;
- }
- token_len = token_end - found;
- token_len = OSMO_MIN(token_len, buflen - 1);
- memcpy(buf, found, token_len);
- buf[token_len] = '\0';
- *pos = token_end;
- return true;
+ if (last_hnbp)
+ *last_hnbp = hnbp;
+ hnb_update_counters(hnbp, ul, packets, bytes, handle);
+ return 0;
}
static void decode_nft_response(const char *response)
{
struct nft_kpi_state *s = _nft_kpi_state;
- const char *pos;
- char buf[128];
+ struct hnb_persistent *hnbp = NULL;
+ json_t *json;
+ json_error_t json_err;
+
+ json_t *nftables;
+ json_t *item;
+ int i;
int count = 0;
- /* find and parse all occurences of strings like:
-*[...] counter packets 3 bytes 129 comment "ul:0
[S] Change in osmo-ttcn3-hacks[master]: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email )
Change subject: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
..
fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
osmo-mgw should not respond to unknown peers. The test expected the
wrong thing, because of an old hack for 3G voice. Fix that.
Related: OS#6424
Change-Id: Ibe2ee59d1ed2c25ffef7e8534c172ac190b4983d
---
M mgw/MGCP_Test.ttcn
1 file changed, 29 insertions(+), 5 deletions(-)
Approvals:
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
neels: Looks good to me, approved
diff --git a/mgw/MGCP_Test.ttcn b/mgw/MGCP_Test.ttcn
index 3161079..7090b34 100644
--- a/mgw/MGCP_Test.ttcn
+++ b/mgw/MGCP_Test.ttcn
@@ -1931,11 +1931,22 @@
stats := f_rtpem_stats_get(RTPEM[0]);
- if (stats.num_pkts_tx != stats.num_pkts_rx) {
- setverdict(fail);
- }
- if (stats.bytes_payload_tx != stats.bytes_payload_rx) {
- setverdict(fail);
+ if (one_phase) {
+ /* osmo-mgw knows both local and remote RTP address.
Expect all packets to be reflected. */
+ if (stats.num_pkts_tx != stats.num_pkts_rx) {
+ setverdict(fail);
+ }
+ if (stats.bytes_payload_tx != stats.bytes_payload_rx) {
+ setverdict(fail);
+ }
+ } else {
+ /* osmo-mgw knows only the local RTP address. Expect no
packets to be reflected. */
+ if (stats.num_pkts_rx > 0) {
+ setverdict(fail, "stats.num_pkts_rx=",
stats.num_pkts_rx, ": osmo-mgw should not send RTP packets to an arbitrary
peer");
+ }
+ if (stats.bytes_payload_rx > 0) {
+ setverdict(fail);
+ }
}
f_rtpem_stats_err_check(stats);
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ibe2ee59d1ed2c25ffef7e8534c172ac190b4983d
Gerrit-Change-Number: 36466
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-MessageType: merged
[S] Change in osmo-ttcn3-hacks[master]: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email ) Change subject: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ibe2ee59d1ed2c25ffef7e8534c172ac190b4983d Gerrit-Change-Number: 36466 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Wed, 27 Mar 2024 21:16:50 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: fix missing 'repeat' in as_optional_cc_rel()
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36469?usp=email ) Change subject: msc: fix missing 'repeat' in as_optional_cc_rel() .. Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: appreciating the hours? days? of work just to add a single word. This patch is distilled gold. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36469?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I0143b4d33b1ebe4cce99c09018540524c4626eec Gerrit-Change-Number: 36469 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 27 Mar 2024 20:21:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-ttcn3-hacks[master]: msc: fix f_tc_ho_inter_bsc0(): properly patch n_sd
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36467?usp=email ) Change subject: msc: fix f_tc_ho_inter_bsc0(): properly patch n_sd .. Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: would be more elegant to retrieve the correct n_sd via some port signature, but we can still do that if one day we do need DTAP in such a handover test. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36467?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Iddde391eade716ca5c6c48cb631450ddb543e0d4 Gerrit-Change-Number: 36467 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Attention: fixeria Gerrit-Comment-Date: Wed, 27 Mar 2024 20:19:27 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email ) Change subject: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations .. Patch Set 1: (1 comment) Patchset: PS1: Note that this patch as it is will, accurately, cause the 'latest' run of MGCP_Test.TC_one_crcx_loopback_rtp_implicit to fail. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ibe2ee59d1ed2c25ffef7e8534c172ac190b4983d Gerrit-Change-Number: 36466 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Wed, 27 Mar 2024 01:02:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email )
Change subject: fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
..
fix MGCP_Test.TC_one_crcx_loopback_rtp_implicit expectations
osmo-mgw should not respond to unknown peers. The test expected the
wrong thing, because of an old hack for 3G voice. Fix that.
Related: OS#6424
Change-Id: Ibe2ee59d1ed2c25ffef7e8534c172ac190b4983d
---
M mgw/MGCP_Test.ttcn
1 file changed, 29 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/66/36466/1
diff --git a/mgw/MGCP_Test.ttcn b/mgw/MGCP_Test.ttcn
index 3161079..7090b34 100644
--- a/mgw/MGCP_Test.ttcn
+++ b/mgw/MGCP_Test.ttcn
@@ -1931,11 +1931,22 @@
stats := f_rtpem_stats_get(RTPEM[0]);
- if (stats.num_pkts_tx != stats.num_pkts_rx) {
- setverdict(fail);
- }
- if (stats.bytes_payload_tx != stats.bytes_payload_rx) {
- setverdict(fail);
+ if (one_phase) {
+ /* osmo-mgw knows both local and remote RTP address.
Expect all packets to be reflected. */
+ if (stats.num_pkts_tx != stats.num_pkts_rx) {
+ setverdict(fail);
+ }
+ if (stats.bytes_payload_tx != stats.bytes_payload_rx) {
+ setverdict(fail);
+ }
+ } else {
+ /* osmo-mgw knows only the local RTP address. Expect no
packets to be reflected. */
+ if (stats.num_pkts_rx > 0) {
+ setverdict(fail, "stats.num_pkts_rx=",
stats.num_pkts_rx, ": osmo-mgw should not send RTP packets to an arbitrary
peer");
+ }
+ if (stats.bytes_payload_rx > 0) {
+ setverdict(fail);
+ }
}
f_rtpem_stats_err_check(stats);
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36466?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ibe2ee59d1ed2c25ffef7e8534c172ac190b4983d
Gerrit-Change-Number: 36466
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[M] Change in osmo-msc[master]: fix VLR evil twin on LU with unknown TMSI
Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email ) Change subject: fix VLR evil twin on LU with unknown TMSI .. Patch Set 1: (4 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/36452/comment/ee0aa82e_04e646a3 PS1, Line 15: be discarded, for the benefit of the new one. > we do the same in osmo-pcu with tbfs btw (ms_merge_and_clear_ms()). interesting, which way around is that? do we also drop the old one there? Patchset: PS1: > are we sure this code path only gets triggered after authentication [assuming > authentication is enab […] I thought about that, too. The situation is: This happens before Auth: - the MSC gets a LU request by TMSI, - then requires the IMSI - the ID response for that triggers this de-duplication. - authentication follows. - we can copy auth tuples safely. - i am not copying state like the security status or FSM states that might skip important validations. - we always have to know a subscriber's IMSI, i.e. the code makes sure that an IMSI is known in order to contact the HLR. This triggers only in a situation where the IMSI is not yet known, i.e. only possible in the short phase before the ID Request, before auth. i.e. not possible to sneakily hijack another IMSI's security context. If authentication is switched off, then this doesn't hold of course, but nothing else does either. File src/libmsc/paging.c: https://gerrit.osmocom.org/c/osmo-msc/+/36452/comment/7e963ad9_991297fe PS1, Line 147: vlr_subscr_put(discarding_vsub, VSUB_USE_PAGING); > Ah indeed it missed that one. yes, i wanted to keep that close together, but the use count put should happen in the end, while the if() should happen in the start... i'll drop a comment there File src/libvlr/vlr.c: https://gerrit.osmocom.org/c/osmo-msc/+/36452/comment/e207ac85_0f270790 PS1, Line 597: struct vlr_instance *vlr = exists->vlr; > I'd rpobably move all this code to its own function "vsub_join()" or alike. ack (the paging_request_join() function was supposed to be that own function, but then this code here grew and grew with more aspects to be taken care of...) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/36452?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd Gerrit-Change-Number: 36452 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 26 Mar 2024 14:13:59 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): fix by_tmsi arg
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email ) Change subject: msc: f_expect_paging(): fix by_tmsi arg .. Patch Set 1: (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/comment/17d6ca61_d2639694 PS1, Line 1360: boolean by_tmsi := true > yet this is very very insignificant, and clearly a bikeshed. […] What I meant to say is: this function has a specific signature, and I want to avoid changing that and affecting callers, and then get sucked into more cosmetic issues. I'd appreciate if you agree that this is not grave enough for a -1 CR vote -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I9434745b7faeb738caafed8080b9f7b1a6a8079a Gerrit-Change-Number: 36455 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 26 Mar 2024 13:49:44 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: expand TC_lu_tmsi_noauth_notmsi
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email ) Change subject: msc: expand TC_lu_tmsi_noauth_notmsi .. Patch Set 1: (1 comment) File msc/MSC_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460/comment/24cc34e1_d30bcde8 PS1, Line 7324: f_cl3_or_initial_ue(valueof(ts_PAG_RESP(ts_MI_IMSI_LV(pars.imsi; > in general it's easier if you modify the function to get a "template (value)" > rather than having to […] This can turn out pretty substantial effort: when working on ttcn3, I do not want to adjust API everywhere left and right, because I am usually compounded by real ttcn3 problems permuting real SUT problems, and have very few cycles left for fixing cosmetics. If this is important, then we should sweep the testing API once and fix these arguments everywhere. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I5e596597add7d585efd27c850067b8d7ba34ecc0 Gerrit-Change-Number: 36460 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 26 Mar 2024 13:46:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-ttcn3-hacks[master]: msc: add TC_lu_tmsi_noauth_notmsi
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email ) Change subject: msc: add TC_lu_tmsi_noauth_notmsi .. Patch Set 1: (1 comment) File msc/MSC_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459/comment/86958c0d_fb6a010d PS1, Line 7312: valueof > cosmetic: `use_mi` is a template parameter, so you can pass `ts_MI_TMSI_LV` > directly, without having […] would it not have to be a "template (value)" arg? So far it is "template (omit)", and I do still not understand the fine points of what the difference is. I am guilty as charged: I don't care very much about ttcn3 cosmetics, I'm happy when ttcn3 runs the tests the way it should and that's it, because that consistently already takes 10 times longer than I had patience for it... When there is no actual practical effect, then we are only just sucking even more time into this. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: If10b9987395670b084ff8ad6d1f033ff46896d75 Gerrit-Change-Number: 36459 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 26 Mar 2024 13:42:21 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: add mi arg to f_perform_lu()
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458?usp=email ) Change subject: msc: add mi arg to f_perform_lu() .. Patch Set 1: (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458/comment/8c33fa83_301d815b PS1, Line 836: function f_perform_lu(template (omit) MobileIdentityLV use_mi := omit) > I also wanted to suggest this, but no, this will not work. […] thanks for the +2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I31aad8eb751528f7237a892702e87ee5855cabbb Gerrit-Change-Number: 36458 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 26 Mar 2024 13:35:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457?usp=email ) Change subject: msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI .. Patch Set 1: (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457/comment/a12cb1a7_8490f0fa PS1, Line 832: setverdict(pass); > meme "Yo dawg I heard you like putting setverdict(pass) everywhere" ;) it does actually make sense to setverdict(pass) on everything that the test verifies. Otherwise a test may succeed 100 checks and still have a "none" verdict. It's like living with teenagers, i only complain when they *don't* do their chores, i should setverdict(pass) on them more often. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I4d719928f04e5a47d415c38f835451b1f10c713d Gerrit-Change-Number: 36457 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 26 Mar 2024 13:34:15 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): better detect failure
Attention is currently required from: fixeria, pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email )
Change subject: msc: f_expect_paging(): better detect failure
..
Patch Set 1:
(1 comment)
File msc/BSC_ConnectionHandler.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456/comment/dbe2fb96_4b7f021f
PS1, Line 1366: if (g_pars.ran_is_geran) {
> Ack, and f_expect_paging can actually be an altstep.
Instead of going into this code again i'd rather drop this mild convenience
patch that only helps when osmo-msc is buggy, i.e. quite soon never.
I see how your feedback can make sense but it is simply not worth the effort,
let's not idealize our test suite.
On a larger scale:
I don't like that ttcn3 forces us to build up these alt-steps-with-timers and
catching non-matching messages, all over the place. Instead we should implement
and apply a generalized soultion for this, sort of like f_rua_expect()
functions.
So instead of spreading these constructs over and over again, we should find a
way to *generally* fix our test suite usability.
So, no, I disagree very much with going into these bike sheds.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36456?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I30273e3882e348a2ded88b7b96a5ec1473a56913
Gerrit-Change-Number: 36456
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-CC: pespin
Gerrit-Attention: pespin
Gerrit-Attention: fixeria
Gerrit-Comment-Date: Tue, 26 Mar 2024 13:30:12 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Comment-In-Reply-To: fixeria
Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: f_expect_paging(): fix by_tmsi arg
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email ) Change subject: msc: f_expect_paging(): fix by_tmsi arg .. Patch Set 1: (1 comment) File msc/BSC_ConnectionHandler.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/comment/aeb54a8f_d26c760c PS1, Line 1360: boolean by_tmsi := true > agree with fixeria yet this is very very insignificant, and clearly a bikeshed. do you insist on another dev cycle on this and possibly having to adjust callers or can we just let this pass please -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I9434745b7faeb738caafed8080b9f7b1a6a8079a Gerrit-Change-Number: 36455 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 26 Mar 2024 13:20:07 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[S] Change in osmo-ttcn3-hacks[master]: msc: add mi arg to f_perform_lu()
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458?usp=email )
Change subject: msc: add mi arg to f_perform_lu()
..
msc: add mi arg to f_perform_lu()
Allow testing Location Updating by TMSI MI.
Prepares for TC_lu_tmsi_noauth_notmsi in
If10b9987395670b084ff8ad6d1f033ff46896d75
Change-Id: I31aad8eb751528f7237a892702e87ee5855cabbb
---
M msc/BSC_ConnectionHandler.ttcn
1 file changed, 22 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/58/36458/1
diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn
index df3a1c8..422390a 100644
--- a/msc/BSC_ConnectionHandler.ttcn
+++ b/msc/BSC_ConnectionHandler.ttcn
@@ -833,9 +833,15 @@
}
}
-function f_perform_lu()
+function f_perform_lu(template (omit) MobileIdentityLV use_mi := omit)
runs on BSC_ConnHdlr {
- var PDU_ML3_MS_NW l3_lu := f_build_lu_imsi(g_pars.imsi)
+ var MobileIdentityLV mi;
+ if (istemplatekind(use_mi, "omit")) {
+ mi := valueof(ts_MI_IMSI_LV(g_pars.imsi));
+ } else {
+ mi := valueof(use_mi);
+ }
+ var PDU_ML3_MS_NW l3_lu := f_build_lu(mi);
var PDU_DTAP_MT dtap_mt;
/* tell GSUP dispatcher to send this IMSI to us */
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36458?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I31aad8eb751528f7237a892702e87ee5855cabbb
Gerrit-Change-Number: 36458
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[S] Change in osmo-ttcn3-hacks[master]: ran-emu: allow receiving Paging without a TMSI
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454?usp=email )
Change subject: ran-emu: allow receiving Paging without a TMSI
..
ran-emu: allow receiving Paging without a TMSI
Fix access of non-present tMSI.* if tMSI == omit, allowing to match
incoming Paging with a UnitData receiver that has no TMSI.
Change-Id: I1bdf3488be0f8d4f0905665c4ba642f9468b9777
---
M library/RAN_Emulation.ttcnpp
1 file changed, 17 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/54/36454/1
diff --git a/library/RAN_Emulation.ttcnpp b/library/RAN_Emulation.ttcnpp
index 25727f3..4d9b51b 100644
--- a/library/RAN_Emulation.ttcnpp
+++ b/library/RAN_Emulation.ttcnpp
@@ -504,8 +504,11 @@
runs on RAN_Emulation_CT return template PDU_BSSAP {
if (match(bssap, tr_BSSMAP_Paging)) {
var RAN_ConnHdlr client := null;
- client := f_imsi_table_find(bssap.pdu.bssmap.paging.iMSI.digits,
-
bssap.pdu.bssmap.paging.tMSI.tmsiOctets);
+ var template OCT4 tmsi := omit;
+ if (ispresent(bssap.pdu.bssmap.paging.tMSI)) {
+ tmsi := bssap.pdu.bssmap.paging.tMSI.tmsiOctets;
+ }
+ client :=
f_imsi_table_find(bssap.pdu.bssmap.paging.iMSI.digits, tmsi);
if (client != null) {
log("CommonBssmapUnitdataCallback: IMSI/TMSI found in
table, dispatching to ",
client);
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36454?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I1bdf3488be0f8d4f0905665c4ba642f9468b9777
Gerrit-Change-Number: 36454
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[M] Change in osmo-ttcn3-hacks[master]: msc: add TC_lu_tmsi_noauth_notmsi
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email )
Change subject: msc: add TC_lu_tmsi_noauth_notmsi
..
msc: add TC_lu_tmsi_noauth_notmsi
Add test case for handling a LU by TMSI MI when 'no assign-tmsi' is
configured.
This test will pass from osmo-msc patch
I583682d1a35a70b008d7bb2d89ba7c3109a60b21 on
Depends: osmo-msc I583682d1a35a70b008d7bb2d89ba7c3109a60b21
Related: SYS#6860 OS#4721
Change-Id: If10b9987395670b084ff8ad6d1f033ff46896d75
---
M msc/MSC_Tests.ttcn
1 file changed, 59 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/59/36459/1
diff --git a/msc/MSC_Tests.ttcn b/msc/MSC_Tests.ttcn
index 9db18d3..1275656 100644
--- a/msc/MSC_Tests.ttcn
+++ b/msc/MSC_Tests.ttcn
@@ -7291,10 +7291,52 @@
vc_conn.done;
}
+/* MSC <-> BSC: ID req/rsp for IMSI */
+private altstep as_id_req_imsi()
+runs on BSC_ConnHdlr {
+ [] BSSAP.receive(tr_PDU_DTAP_MT(tr_ML3_MT_MM_ID_Req(CM_ID_TYPE_IMSI))) {
+ var MobileIdentityLV mi := valueof(ts_MI_IMSI_LV(g_pars.imsi));
+ BSSAP.send(ts_PDU_DTAP_MO(ts_ML3_MO_MM_ID_Rsp(mi)));
+ repeat;
+ }
+}
+
+/* MSC is configured to not assign a TMSI; MS sends LU Request with a TMSI MI
(from another cell), and MSC shall not use
+ * that TMSI. */
+private function f_tc_lu_tmsi_noauth_notmsi(charstring id, BSC_ConnHdlrPars
pars) runs on BSC_ConnHdlr {
+ f_init_handler(pars, t_guard := 20.0);
+
+ /* Perform Location Updating using an unknown TMSI MI. Expect an ID
Request to come from the MSC and answer
+* that with as_id_req_imsi(). */
+ activate(as_id_req_imsi());
+ f_perform_lu(use_mi := valueof(ts_MI_TMSI_LV(pars.tmsi)));
+
+ f_sleep(1.0);
+
+ /* Attached by invalid TMSI, and the MSC has asked for the IMSI.
Initiate Paging and make sure the MSC doesn't
+* use the invalid TMSI for it. */
+ f_ran_register_imsi(g_pars.imsi, omit);
+ f_vty_transceive(MSCVTY, "subscriber imsi " & hex2str(g_pars.imsi) & "
paging");
+ f_expect_paging(by_tmsi := false);
+}
+testcase TC_lu_tmsi_noauth_notmsi() runs on MTC_CT {
+ var BSC_ConnHdlrPars pars;
+ var BSC_ConnHdlr vc_conn;
+ f_init();
+ f_vty_config(MSCVTY, "msc", "no assign-tmsi");
+ pars := f_init_pars(101);
+ pars.net.expect_tmsi := false;
+ pars.tmsi := '0badbad0'O;
+ pars.mm_info := false;
+ vc_conn :=
f_start_handler_with_pars(refers(f_tc_lu_tmsi_noauth_notmsi), pars);
+ vc_conn.done;
+}
+
control {
execute( TC_cr_before_reset() );
execute( TC_lu_imsi_noauth_tmsi() );
execute( TC_lu_imsi_noauth_notmsi() );
+ execute( TC_lu_tmsi_noauth_notmsi() );
execute( TC_lu_imsi_reject() );
execute( TC_lu_imsi_timeout_gsup() );
execute( TC_lu_imsi_auth_tmsi() );
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36459?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If10b9987395670b084ff8ad6d1f033ff46896d75
Gerrit-Change-Number: 36459
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[S] Change in osmo-ttcn3-hacks[master]: msc: expand TC_lu_tmsi_noauth_notmsi
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email )
Change subject: msc: expand TC_lu_tmsi_noauth_notmsi
..
msc: expand TC_lu_tmsi_noauth_notmsi
>From running this test repeatedly, I noticed that osmo-msc's new patch
to avoid storing a TMSI may also trigger more evil twin situations in
the VLR as described in OS#4721.
Always run this test twice, to probe for the evil twin problem.
This test will pass from osmo-msc patch
Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd on.
Depends: osmo-msc Ifdabe0b65bffafbf7b8e5cc10e2d225d1ed1cecd
Related: SYS#6860 OS#4721
Change-Id: I5e596597add7d585efd27c850067b8d7ba34ecc0
---
M msc/MSC_Tests.ttcn
1 file changed, 44 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/60/36460/1
diff --git a/msc/MSC_Tests.ttcn b/msc/MSC_Tests.ttcn
index 1275656..0d89a26 100644
--- a/msc/MSC_Tests.ttcn
+++ b/msc/MSC_Tests.ttcn
@@ -7318,6 +7318,17 @@
f_ran_register_imsi(g_pars.imsi, omit);
f_vty_transceive(MSCVTY, "subscriber imsi " & hex2str(g_pars.imsi) & "
paging");
f_expect_paging(by_tmsi := false);
+
+ /* Respond to paging, to clean up internal paging state for this
subscriber, so we can get a clean second run
+* out of this test code. Don't use the TMSI in the paging response. */
+ f_cl3_or_initial_ue(valueof(ts_PAG_RESP(ts_MI_IMSI_LV(pars.imsi;
+ f_mm_common();
+ /* The paging was by VTY, so nothing happens, just a release. */
+ f_expect_clear();
+
+ /* Clean up ttcn state for the second test run to work out. */
+ f_unregister_gsup_imsi(hex2str(pars.imsi));
+ f_ran_unregister_imsi(pars.imsi);
}
testcase TC_lu_tmsi_noauth_notmsi() runs on MTC_CT {
var BSC_ConnHdlrPars pars;
@@ -7330,6 +7341,19 @@
pars.mm_info := false;
vc_conn :=
f_start_handler_with_pars(refers(f_tc_lu_tmsi_noauth_notmsi), pars);
vc_conn.done;
+
+ /* Now run the same test *again*, to test against an evil twin VLR
entry:
+* A vlr_subscr with the correct IMSI is now present in the VLR.
+* We again ask for a LU with the 0x0bad TMSI. The VLR will initially
create another vlr_subsrc(TMSI=0x0bad).
+* When it learns the IMSI via ID Request, it needs to realize that
this IMSI is already present on the first
+* vsub, and sort out the VLR record so that only one entry for this
IMSI exists.
+*/
+ pars := f_init_pars(101);
+ pars.net.expect_tmsi := false;
+ pars.tmsi := '0badbad0'O;
+ pars.mm_info := false;
+ vc_conn :=
f_start_handler_with_pars(refers(f_tc_lu_tmsi_noauth_notmsi), pars);
+ vc_conn.done;
}
control {
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36460?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I5e596597add7d585efd27c850067b8d7ba34ecc0
Gerrit-Change-Number: 36460
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
[S] Change in osmo-ttcn3-hacks[master]: msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457?usp=email )
Change subject: msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI
..
msc: allow f_verify_vty_lac_ci() to pass for CompL3 by TMSI MI
Obviously, when only a TMSI has been used, searching for an IMSI will
return no subscriber. Don't fail in that case when testing for
verify_vlr := false.
Prepares TC_lu_tmsi_noauth_notmsi in
If10b9987395670b084ff8ad6d1f033ff46896d75
Related: SYS#6860 OS#4721
Change-Id: I4d719928f04e5a47d415c38f835451b1f10c713d
---
M msc/BSC_ConnectionHandler.ttcn
1 file changed, 20 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/57/36457/1
diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn
index ac973d7..df3a1c8 100644
--- a/msc/BSC_ConnectionHandler.ttcn
+++ b/msc/BSC_ConnectionHandler.ttcn
@@ -827,7 +827,9 @@
}
if (not verify_vlr and not connection_present) {
- setverdict(fail, "f_verify_vty_lac_ci(verify_vlr := false)
called, which requires an active connection, but there is no 'Connection:' part
to verify in ", result);
+ /* If the Compl L3 was done by TMSI, the VLR has no IMSI until
the ID Request / Response for IMSI has
+* been answered. So if verify_vlr == false, a "missing"
connection is one of the expected scenarios. */
+ setverdict(pass);
}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36457?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I4d719928f04e5a47d415c38f835451b1f10c713d
Gerrit-Change-Number: 36457
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
