pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40386?usp=email )
Change subject: 5gc: Initial support encoding NG NAS UL messages
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
With this I can already encode a correct NAS message and I get an answer from the 5GC.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40386?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Id59564114cf18ae745e3e385e2c91779a453e545
Gerrit-Change-Number: 40386
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Wed, 28 May 2025 13:56:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40385?usp=email )
Change subject: deps: Depend on osmocom fork of forge.etsi.org nas.git
......................................................................
deps: Depend on osmocom fork of forge.etsi.org nas.git
This module provides types, templates and functions to operate NG NAS
(5G).
A fork is needed because:
* Upstream code doesn't compile against eclipse titan (A PR has been
submitted upstream and we should eventually be able to compile ttcn3
code just fine in that regard).
* Some system/external functions need to be implemented, such as
fx_GetSystemTime(). We currently do that in our fork.
Related: SYS#7073
Change-Id: I004fc26e5d691ee7e3be984e28f5516e2b55258c
---
M deps/Makefile
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/85/40385/1
diff --git a/deps/Makefile b/deps/Makefile
index 0c5a033..b4a411a 100644
--- a/deps/Makefile
+++ b/deps/Makefile
@@ -69,7 +69,8 @@
OSMOGERRIT_REPOS= osmo-uecups
-OSMOGITEA_REPOS= titan.ProtocolModules.BSSMAP \
+OSMOGITEA_REPOS= nas \
+ titan.ProtocolModules.BSSMAP \
titan.ProtocolModules.MAP \
titan.TestPorts.AF_PACKET \
titan.TestPorts.USB
@@ -84,6 +85,7 @@
# Do not put references to branches here, except for local testing: this breaks the caching
# logic of docker containers, which only invalidate their cached ttcn3 dependencies if this
# file changed.
+nas_commit= 205e24167517500ba2d49322dfa1754ab8a0fc43
titan.Libraries.TCCUsefulFunctions_commit= R.35.B-6-gb3687da
titan.ProtocolEmulations.M3UA_commit= b58f92046e48a7b1ed531e243a2319ebca53bf4c
titan.ProtocolEmulations.SCCP_commit= 750a3e836831e58eae59d4757ef5d0c759f9ca5d
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40385?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I004fc26e5d691ee7e3be984e28f5516e2b55258c
Gerrit-Change-Number: 40385
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, laforge.
pespin has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email )
Change subject: enft_kpi: retrieve per-eNB traffic counters
......................................................................
Patch Set 8:
(2 comments)
File config/sys.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/a81661e6_4544… :
PS8, Line 24: %% {enft_kpi_enable, true}, %% whether to enable the NFT KPI module (default: false)
> > IMHO the fact that you are using enftables is totally irrelevant ... […]
"nft" is a well known and established system-wide component, which users can easily understand from general knowledge. Since the code is operating on a system-wide component shared with other stuff running in the system, I consider this to be important enough to be something to tell the user, specially since one of the params is totally directly related to the fact that "nft" is used ("nft_kpi_table_name").
gtpu_kpi would also be definetly better than "enft", because again GTPU is some well known component which means user doesn't need to figure out what "enft" means in this particular context.
If for you proepr naming of configuration parameters which is to be read and set by users is a "trivial detail" and equals the color of a bikeshed, then so be it an I have nothing more to say.
If you think joining the all-is-bikeshed trend is really helping here, also I have nothing more to say.
In any case, I already provided a +1 beforehand, do as you please, my argument have been given.
File rebar.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/7739e93a_7d7c… :
PS8, Line 13: {git, "https://gitea.osmocom.org/vyanitskiy/enftables.git", {branch, "fixeria/json"}}},
> Again, I'll move the library to https://gitea.osmocom.org/erlang/ and push an updated patch revision soon (before merging it)
That's exactly what I meant, sorry if it was not clear, fine with it.
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I498d2854447a2d53d2abddd38652f3e2bbb1fbdd
Gerrit-Change-Number: 40281
Gerrit-PatchSet: 8
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 28 May 2025 13:46:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: laforge, pespin.
fixeria has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email )
Change subject: enft_kpi: retrieve per-eNB traffic counters
......................................................................
Patch Set 8:
(2 comments)
File config/sys.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/73038fb4_c584… :
PS8, Line 24: %% {enft_kpi_enable, true}, %% whether to enable the NFT KPI module (default: false)
> IMHO the fact that you are using enftables is totally irrelevant ...
If I were Neels, I would have said this is bikeshedding:
> Bikeshedding refers to the phenomenon of spending excessive time and energy on trivial details (like the color of a bike shed) while neglecting more important or critical issues ...
Maybe this is not the best naming choice, but this is certainly not the most important detail at the moment. We could go further and omit the `nft` completely together with the `e`, because the user should not care if the counters are obtained via NFT or any other means. Should it be `gtpu_kpi_` then?
> The PFCP stuff is also coming from a "pfcplib", and yet they are not named "pfcplib_*".
The stuff coming from `enftables` library does not have the prefix either.
It's just the module name and env parameters reflecting the module they're related to.
File rebar.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/c13e0f34_47f5… :
PS8, Line 13: {git, "https://gitea.osmocom.org/vyanitskiy/enftables.git", {branch, "fixeria/json"}}},
> IMHO if this is a repo which is not meant to end up in another namespace
Not sure what you mean here. Again, I'll move the library to https://gitea.osmocom.org/erlang/ and push an updated patch revision soon (before merging it). What's wrong here?
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I498d2854447a2d53d2abddd38652f3e2bbb1fbdd
Gerrit-Change-Number: 40281
Gerrit-PatchSet: 8
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 28 May 2025 12:41:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40383?usp=email )
Change subject: log_merge.sh: Merge all logs if no test cases are found
......................................................................
log_merge.sh: Merge all logs if no test cases are found
This can happen for instance if titan exits with an error during startup
even before starting any test.
In that scenario, titan still writes stuff to a file (eg.
C5G_Tests--efc28eab6a6d-hc-1305.log).
Let's try to make everything logged available in that case, so user can
debug what went wrong.
Change-Id: Ifc68e42022e8b2990d3c5221b3901f255e4e759b
---
M log_merge.sh
1 file changed, 15 insertions(+), 10 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
diff --git a/log_merge.sh b/log_merge.sh
index 65a3003..85ec0c3 100755
--- a/log_merge.sh
+++ b/log_merge.sh
@@ -51,16 +51,21 @@
TEST_CASES=$(ls -1 $LOG_FILES | awk 'BEGIN { FS = "-" } { print $2 }' | sort | uniq)
-for t in $TEST_CASES; do
- PREFIX="$BASE_NAME-$t"
- OUTPUT="$(get_new_prefix "$PREFIX").merged"
- if [ -e "$OUTPUT" ]; then
- >&2 echo "log_merge: ERROR: file already exists: $OUTPUT"
- exit 1
- fi
- ttcn3_logmerge $PREFIX-*.log > "$OUTPUT"
- echo "Generated $OUTPUT"
-done
+if [ -n "$TEST_CASES" ]; then
+ for t in $TEST_CASES; do
+ PREFIX="$BASE_NAME-$t"
+ OUTPUT="$(get_new_prefix "$PREFIX").merged"
+ if [ -e "$OUTPUT" ]; then
+ >&2 echo "log_merge: ERROR: file already exists: $OUTPUT"
+ exit 1
+ fi
+ ttcn3_logmerge $PREFIX-*.log > "$OUTPUT"
+ echo "Generated $OUTPUT"
+ done
+else
+ >&2 echo "log_merge: WARNING: Couldn't find logs for test cases! Merging everything"
+ ttcn3_logmerge $LOG_FILES > "$BASE_NAME.merged"
+fi
if [ "$2" = "--rm" ]; then
echo "Removing Input log files !!!"
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40383?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ifc68e42022e8b2990d3c5221b3901f255e4e759b
Gerrit-Change-Number: 40383
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, laforge.
pespin has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email )
Change subject: enft_kpi: retrieve per-eNB traffic counters
......................................................................
Patch Set 8: Code-Review+1
(5 comments)
File config/sys.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/e0ead3c8_7f74… :
PS8, Line 24: %% {enft_kpi_enable, true}, %% whether to enable the NFT KPI module (default: false)
> It's because I named the library `enftables` (Erlang NIFs for libnftables), and this `e` prefix made […]
IMHO the fact that you are using enftables is totally irrelevant from user point of view. The user really only knows the counters are obtained over nft rules, hence why I consider the "e" in there to just be adding confusion.
The PFCP stuff is also coming from a "pfcplib", and yet they are not named "pfcplib_*".
Just my 5 cents, do as you like.
File include/s1gw_metrics.hrl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/f3b92493_bda7… :
PS8, Line 40: %% NOTE: these counters shall not be listed in ?S1GW_COUNTERS,
> Right. Other counters will be made per-eNB too at some point. […]
It's fine having a comment, just though the comment there kind of created an alamr in my head (as in "BE CAREFUL!"), but in the end it's just that this is a per-enb counter..
File rebar.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/1e31e8ca_e2fc… :
PS8, Line 13: {git, "https://gitea.osmocom.org/vyanitskiy/enftables.git", {branch, "fixeria/json"}}},
> The whole repository currently lives in my personal Gitea profile and will be migrated to https://gi […]
IMHO if this is a repo which is not meant to end up in another namespace, it should be already moved wherever needed before being used in this project.
File src/enft_kpi.erl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/6a47556a_03f9… :
PS8, Line 142: R1 = [nft_expr_match_ip_proto("udp", ?OP_NEQ), nft_expr_accept()],
> Actually, a lot of this stuff can be moved to `enftables`. But for now it can live here.
Acknowledged
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/accc2b7d_cb47… :
PS8, Line 341: case enb_add_nft_counters(ES0#{addr => Addr}, Cfg) of
> Good point. I'll migrate this to use `cast` instead of `call`.
Be careful though because that may need some work/complexity as it becomes async. I'm fine with merging this as is now, just wanted you to be aware of it and make sure it gets fixed at some point.
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I498d2854447a2d53d2abddd38652f3e2bbb1fbdd
Gerrit-Change-Number: 40281
Gerrit-PatchSet: 8
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 28 May 2025 12:19:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: laforge, pespin.
fixeria has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email )
Change subject: enft_kpi: retrieve per-eNB traffic counters
......................................................................
Patch Set 8:
(6 comments)
File config/sys.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/63319ec0_4b19… :
PS8, Line 24: %% {enft_kpi_enable, true}, %% whether to enable the NFT KPI module (default: false)
> where does the "e" from "enft" come from? At first sight looks like "nft" would be much more compreh […]
It's because I named the library `enftables` (Erlang NIFs for libnftables), and this `e` prefix made it here too (the code is using `enftables`, so the module name is `enft_kpi` and thus param names).
File include/s1gw_metrics.hrl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/94c79ab8_139f… :
PS8, Line 40: %% NOTE: these counters shall not be listed in ?S1GW_COUNTERS,
> So simply this can be announced as "Per-eNB counters"?
Right. Other counters will be made per-eNB too at some point.
I am adding a NOTE here because a NOTE below states that new entries need to be added to `s1gw_metric:init()` (actually `?S1GW_COUNTERS` or `?S1GW_GAUGES`). I don't think it hurts?
File rebar.config:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/5e948f09_fd04… :
PS8, Line 13: {git, "https://gitea.osmocom.org/vyanitskiy/enftables.git", {branch, "fixeria/json"}}},
> can we have this in a "osmocom/master" or "master" branch instead?
The whole repository currently lives in my personal Gitea profile and will be migrated to https://gitea.osmocom.org/erlang/. I'll also merge this branch into `master`. I'll do this once the customer completes initial testing.
File src/enft_kpi.erl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/c9c94a34_c914… :
PS8, Line 142: R1 = [nft_expr_match_ip_proto("udp", ?OP_NEQ), nft_expr_accept()],
> sounds like you may want to introduce atoms in the nft lib to express OP_NEQ and OP_EQ instead.
Actually, a lot of this stuff can be moved to `enftables`. But for now it can live here.
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/25dbb0bc_d691… :
PS8, Line 341: case enb_add_nft_counters(ES0#{addr => Addr}, Cfg) of
> fyi this can take some time, and afaict you are doing it synchronously (handle_call), which means th […]
Good point. I'll migrate this to use `cast` instead of `call`.
File src/osmo_s1gw_sup.erl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281/comment/67f245fc_decb… :
PS8, Line 81: EnftKpi = {enft_kpi, {enft_kpi, start_link, [enft_kpi_cfg()]},
> I?m still wondering why this is called "enft" but above it's called "Pfcp" and not "Epfcp" or "Sctp" […]
See the other comment.
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40281?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I498d2854447a2d53d2abddd38652f3e2bbb1fbdd
Gerrit-Change-Number: 40281
Gerrit-PatchSet: 8
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 28 May 2025 12:07:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, osmith, pespin.
Hello Jenkins Builder, fixeria, osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40383?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 osmith, Verified+1 by Jenkins Builder
Change subject: log_merge.sh: Merge all logs if no test cases are found
......................................................................
log_merge.sh: Merge all logs if no test cases are found
This can happen for instance if titan exits with an error during startup
even before starting any test.
In that scenario, titan still writes stuff to a file (eg.
C5G_Tests--efc28eab6a6d-hc-1305.log).
Let's try to make everything logged available in that case, so user can
debug what went wrong.
Change-Id: Ifc68e42022e8b2990d3c5221b3901f255e4e759b
---
M log_merge.sh
1 file changed, 15 insertions(+), 10 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/83/40383/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40383?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ifc68e42022e8b2990d3c5221b3901f255e4e759b
Gerrit-Change-Number: 40383
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>