Attention is currently required from: dexter.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/upf-benchmark/+/39420?usp=email )
Change subject: osmo-upf-load-gen: Improve VTY command helper strings
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-upf-load-gen/pfcp_tool_vty.c:
https://gerrit.osmocom.org/c/upf-benchmark/+/39420/comment/62c846d2_cac9cc3… :
PS1, Line 222: "Add a core GTP port, to emit GTP1U traffic to\n"
> Maybe write "remote" instead of "core"? Even though it should be clear that the core network is remo […]
Not sure if remote would help here, since anyway that would create doubts regarding remote to the UPF or to the app.
--
To view, visit https://gerrit.osmocom.org/c/upf-benchmark/+/39420?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: upf-benchmark
Gerrit-Branch: master
Gerrit-Change-Id: I69701e875f5c38f434265d93a047c8157daffd29
Gerrit-Change-Number: 39420
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Jan 2025 10:19:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/39199?usp=email )
Change subject: global_platform: add new command "install_cap"
......................................................................
global_platform: add new command "install_cap"
Installing JAVA-card applets from a CAP file is a multi step process, which is
difficult when done manually. Fortunately it is easy to automate the process,
so let's add a dedicated command for that.
Change-Id: I6cbd37f0fad5579b20e83c27349bd5acc129e6d0
Related: OS#6679
---
A docs/cap-tutorial.rst
M docs/shell.rst
M pySim/global_platform/__init__.py
A pySim/global_platform/install_param.py
M tests/unittests/test_globalplatform.py
5 files changed, 244 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/docs/cap-tutorial.rst b/docs/cap-tutorial.rst
new file mode 100644
index 0000000..64e5681
--- /dev/null
+++ b/docs/cap-tutorial.rst
@@ -0,0 +1,103 @@
+
+Guide: Installing JAVA-card applets
+===================================
+
+Almost all modern-day UICC cards have some form of JAVA-card / Sim-Toolkit support, which allows the installation
+of customer specific JAVA-card applets. The installation of JAVA-card applets is usually done via the standardized
+GlobalPlatform (GPC_SPE_034) ISD (Issuer Security Domain) application interface during the card provisioning process.
+(it is also possible to load JAVA-card applets in field via OTA-SMS, but that is beyond the scope of this guide). In
+this guide we will go through the individual steps that are required to load JAVA-card applet onto an UICC card.
+
+
+Preparation
+~~~~~~~~~~~
+
+In this example we will install the CAP file HelloSTK_09122024.cap [1] on an sysmoISIM-SJA2 card. Since the interface
+is standardized, the exact card model does not matter.
+
+The example applet makes use of the STK (Sim-Toolkit), so we must supply STK installation parameters. Those
+parameters are supplied in the form of a hexstring and should be provided by the applet manufacturer. The available
+parameters and their exact encoding is specified in ETSI TS 102 226, section 8.2.1.3.2.1. The installation of
+HelloSTK_09122024.cap [1], will require the following STK installation parameters: "010001001505000000000000000000000000"
+
+During the installation, we also have to set a memory quota for the volatile and for the non volatile card memory.
+Those values also should be provided by the applet manufacturer. In this example, we will allow 255 bytes of volatile
+memory and 255 bytes of non volatile memory to be consumed by the applet.
+
+To install JAVA-card applets, one must be in the possession of the key material belonging to the card. The keys are
+usually provided by the card manufacturer. The following example will use the following keyset:
+
++---------+----------------------------------+
+| Keyname | Keyvalue |
++=========+==================================+
+| DEK/KIK | 5524F4BECFE96FB63FC29D6BAAC6058B |
++---------+----------------------------------+
+| ENC/KIC | 542C37A6043679F2F9F71116418B1CD5 |
++---------+----------------------------------+
+| MAC/KID | 34F11BAC8E5390B57F4E601372339E3C |
++---------+----------------------------------+
+
+[1] https://osmocom.org/projects/cellular-infrastructure/wiki/HelloSTK
+
+
+Applet Installation
+~~~~~~~~~~~~~~~~~~~
+
+To prepare the installation, a secure channel to the ISD must be established first:
+
+::
+
+ pySIM-shell (00:MF)> select ADF.ISD
+ {
+ "application_id": "a000000003000000",
+ "proprietary_data": {
+ "maximum_length_of_data_field_in_command_message": 255
+ }
+ }
+ pySIM-shell (00:MF/ADF.ISD)> establish_scp02 --key-dek 5524F4BECFE96FB63FC29D6BAAC6058B --key-enc 542C37A6043679F2F9F71116418B1CD5 --key-mac 34F11BAC8E5390B57F4E601372339E3C --security-level 1
+ Successfully established a SCP02[01] secure channel
+
+.. warning:: In case you get an "EXCEPTION of type 'ValueError' occurred with message: card cryptogram doesn't match" error message, it is very likely that there is a problem with the key material. The card may lock the ISD access after a certain amount of failed tries. Carefully check the key material any try again.
+
+
+When the secure channel is established, we are ready to install the applet. The installation normally is a multi step
+procedure, where the loading of an executable load file is announced first, then loaded and then installed in a final
+step. The pySim-shell command ``install_cap`` automatically takes care of those three steps.
+
+::
+
+ pySIM-shell (SCP02[01]:00:MF/ADF.ISD)> install_cap /home/user/HelloSTK_09122024.cap --install-parameters-non-volatile-memory-quota 255 --install-parameters-volatile-memory-quota 255 --install-parameters-stk 010001001505000000000000000000000000
+ loading cap file: /home/user/HelloSTK_09122024.cap ...
+ parameters:
+ security-domain-aid: a000000003000000
+ load-file: 569 bytes
+ load-file-aid: d07002ca44
+ module-aid: d07002ca44900101
+ application-aid: d07002ca44900101
+ install-parameters: c900ef1cc80200ffc70200ffca12010001001505000000000000000000000000
+ step #1: install for load...
+ step #2: load...
+ Loaded a total of 573 bytes in 3 blocks. Don't forget install_for_install (and make selectable) now!
+ step #3: install_for_install (and make selectable)...
+ done.
+
+The applet is now installed on the card. We can now quit pySim-shell and remove the card from the reader and test the
+applet in a mobile phone. There should be a new STK application with one menu entry shown, that will greet the user
+when pressed.
+
+
+Applet Removal
+~~~~~~~~~~~~~~
+
+To remove the applet, we must establish a secure channel to the ISD (see above). Then we can delete the applet using the
+``delete_card_content`` command.
+
+::
+
+ pySIM-shell (SCP02[01]:00:MF/ADF.ISD)> delete_card_content D07002CA44 --delete-related-objects
+
+The parameter "D07002CA44" is the load-file-AID of the applet. The load-file-AID is encoded in the .cap file and also
+displayed during the installation process. It is also important to note that when the applet is installed, it cannot
+be installed (under the same AID) again until it is removed.
+
+
diff --git a/docs/shell.rst b/docs/shell.rst
index 8bb0a17..026f5c8 100644
--- a/docs/shell.rst
+++ b/docs/shell.rst
@@ -67,6 +67,7 @@
:caption: Tutorials for pySIM-shell:
suci-tutorial
+ cap-tutorial
Advanced Topics
@@ -1053,6 +1054,12 @@
:module: pySim.global_platform
:func: ADF_SD.AddlShellCommands.load_parser
+install_cap
+~~~~~~~~~~~
+.. argparse::
+ :module: pySim.global_platform
+ :func: ADF_SD.AddlShellCommands.install_cap_parser
+
install_for_personalization
~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. argparse::
diff --git a/pySim/global_platform/__init__.py b/pySim/global_platform/__init__.py
index ed4ee68..d82bd08 100644
--- a/pySim/global_platform/__init__.py
+++ b/pySim/global_platform/__init__.py
@@ -30,6 +30,7 @@
from pySim.utils import ResTuple
from pySim.card_key_provider import card_key_provider_get_field
from pySim.global_platform.scp import SCP02, SCP03
+from pySim.global_platform.install_param import gen_install_parameters
from pySim.filesystem import *
from pySim.profile import CardProfile
from pySim.ota import SimFileAccessAndToolkitAppSpecParams
@@ -858,6 +859,58 @@
_rsp_hex, _sw = self._cmd.lchan.scc.send_apdu_checksw(cmd_hex)
self._cmd.poutput("Loaded a total of %u bytes in %u blocks. Don't forget install_for_install (and make selectable) now!" % (total_size, block_nr))
+ install_cap_parser = argparse.ArgumentParser()
+ install_cap_parser.add_argument('cap_file', type=str, metavar='FILE',
+ help='JAVA-CARD CAP file to install')
+ install_cap_parser_inst_prm_g = install_cap_parser.add_mutually_exclusive_group()
+ install_cap_parser_inst_prm_g.add_argument('--install-parameters', type=is_hexstr, default=None,
+ help='install Parameters (GPC_SPE_034, section 11.5.2.3.7, table 11-49)')
+ install_cap_parser_inst_prm_g_grp = install_cap_parser_inst_prm_g.add_argument_group()
+ install_cap_parser_inst_prm_g_grp.add_argument('--install-parameters-volatile-memory-quota',
+ type=int, default=None,
+ help='volatile memory quota (GPC_SPE_034, section 11.5.2.3.7, table 11-49)')
+ install_cap_parser_inst_prm_g_grp.add_argument('--install-parameters-non-volatile-memory-quota',
+ type=int, default=None,
+ help='non volatile memory quota (GPC_SPE_034, section 11.5.2.3.7, table 11-49)')
+ install_cap_parser_inst_prm_g_grp.add_argument('--install-parameters-stk',
+ type=is_hexstr, default=None,
+ help='Load Parameters (ETSI TS 102 226, section 8.2.1.3.2.1)')
+
+ @cmd2.with_argparser(install_cap_parser)
+ def do_install_cap(self, opts):
+ """Perform a .cap file installation using GlobalPlatform LOAD and INSTALL commands."""
+
+ self._cmd.poutput("loading cap file: %s ..." % opts.cap_file)
+ cap = CapFile(opts.cap_file)
+
+ security_domain_aid = self._cmd.lchan.selected_file.aid
+ load_file = cap.get_loadfile()
+ load_file_aid = cap.get_loadfile_aid()
+ module_aid = cap.get_applet_aid()
+ application_aid = module_aid
+ if opts.install_parameters:
+ install_parameters = opts.install_parameters;
+ else:
+ install_parameters = gen_install_parameters(opts.install_parameters_non_volatile_memory_quota,
+ opts.install_parameters_volatile_memory_quota,
+ opts.install_parameters_stk)
+ self._cmd.poutput("parameters:")
+ self._cmd.poutput(" security-domain-aid: %s" % security_domain_aid)
+ self._cmd.poutput(" load-file: %u bytes" % len(load_file))
+ self._cmd.poutput(" load-file-aid: %s" % load_file_aid)
+ self._cmd.poutput(" module-aid: %s" % module_aid)
+ self._cmd.poutput(" application-aid: %s" % application_aid)
+ self._cmd.poutput(" install-parameters: %s" % install_parameters)
+
+ self._cmd.poutput("step #1: install for load...")
+ self.do_install_for_load("--load-file-aid %s --security-domain-aid %s" % (load_file_aid, security_domain_aid))
+ self._cmd.poutput("step #2: load...")
+ self.load(load_file)
+ self._cmd.poutput("step #3: install_for_install (and make selectable)...")
+ self.do_install_for_install("--load-file-aid %s --module-aid %s --application-aid %s --install-parameters %s --make-selectable" %
+ (load_file_aid, module_aid, application_aid, install_parameters))
+ self._cmd.poutput("done.")
+
est_scp02_parser = argparse.ArgumentParser()
est_scp02_parser.add_argument('--key-ver', type=auto_uint8, default=0, help='Key Version Number (KVN)')
est_scp02_parser.add_argument('--host-challenge', type=is_hexstr,
diff --git a/pySim/global_platform/install_param.py b/pySim/global_platform/install_param.py
new file mode 100644
index 0000000..45d1d1a
--- /dev/null
+++ b/pySim/global_platform/install_param.py
@@ -0,0 +1,72 @@
+# GlobalPlatform install parameter generator
+#
+# (C) 2024 by Sysmocom s.f.m.c. GmbH
+# All Rights Reserved
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+from osmocom.construct import *
+from osmocom.utils import *
+from osmocom.tlv import *
+
+class AppSpecificParams(BER_TLV_IE, tag=0xC9):
+ # GPD_SPE_013, table 11-49
+ _construct = HexAdapter(GreedyBytes)
+
+class VolatileMemoryQuota(BER_TLV_IE, tag=0xC7):
+ # GPD_SPE_013, table 11-49
+ _construct = StripHeaderAdapter(GreedyBytes, 4, steps = [2,4])
+
+class NonVolatileMemoryQuota(BER_TLV_IE, tag=0xC8):
+ # GPD_SPE_013, table 11-49
+ _construct = StripHeaderAdapter(GreedyBytes, 4, steps = [2,4])
+
+class StkParameter(BER_TLV_IE, tag=0xCA):
+ # GPD_SPE_013, table 11-49
+ # ETSI TS 102 226, section 8.2.1.3.2.1
+ _construct = HexAdapter(GreedyBytes)
+
+class SystemSpecificParams(BER_TLV_IE, tag=0xEF, nested=[VolatileMemoryQuota, NonVolatileMemoryQuota, StkParameter]):
+ # GPD_SPE_013 v1.1 Table 6-5
+ pass
+
+class InstallParams(TLV_IE_Collection, nested=[AppSpecificParams, SystemSpecificParams]):
+ # GPD_SPE_013, table 11-49
+ pass
+
+def gen_install_parameters(non_volatile_memory_quota:int, volatile_memory_quota:int, stk_parameter:str):
+
+ # GPD_SPE_013, table 11-49
+
+ #Mandatory
+ install_params = InstallParams()
+ install_params_dict = [{'app_specific_params': None}]
+
+ #Conditional
+ if non_volatile_memory_quota and volatile_memory_quota and stk_parameter:
+ system_specific_params = []
+ #Optional
+ if non_volatile_memory_quota:
+ system_specific_params += [{'non_volatile_memory_quota': non_volatile_memory_quota}]
+ #Optional
+ if volatile_memory_quota:
+ system_specific_params += [{'volatile_memory_quota': volatile_memory_quota}]
+ #Optional
+ if stk_parameter:
+ system_specific_params += [{'stk_parameter': stk_parameter}]
+ install_params_dict += [{'system_specific_params': system_specific_params}]
+
+ install_params.from_dict(install_params_dict)
+ return b2h(install_params.to_bytes())
diff --git a/tests/unittests/test_globalplatform.py b/tests/unittests/test_globalplatform.py
index 597d51d..069886c 100644
--- a/tests/unittests/test_globalplatform.py
+++ b/tests/unittests/test_globalplatform.py
@@ -21,6 +21,7 @@
from pySim.global_platform import *
from pySim.global_platform.scp import *
+from pySim.global_platform.install_param import gen_install_parameters
KIC = h2b('100102030405060708090a0b0c0d0e0f') # enc
KID = h2b('101102030405060708090a0b0c0d0e0f') # MAC
@@ -289,5 +290,13 @@
self.assertEqual(compute_kcv('aes', KEYSET_AES128.dek), h2b('840DE5'))
+class Install_param_Test(unittest.TestCase):
+ def test_gen_install_parameters(self):
+ load_parameters = gen_install_parameters(256, 256, '010001001505000000000000000000000000')
+ self.assertEqual(load_parameters, 'c900ef1cc8020100c7020100ca12010001001505000000000000000000000000')
+
+ load_parameters = gen_install_parameters(None, None, '')
+ self.assertEqual(load_parameters, 'c900')
+
if __name__ == "__main__":
unittest.main()
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39199?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I6cbd37f0fad5579b20e83c27349bd5acc129e6d0
Gerrit-Change-Number: 39199
Gerrit-PatchSet: 9
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Attention is currently required from: pespin.
dexter has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/upf-benchmark/+/39421?usp=email )
Change subject: osmo-upf-load-gen: Add vty cmd to skip binding local GTPU ports
......................................................................
Patch Set 1: Code-Review+1
(3 comments)
Patchset:
PS1:
Didn't see any real problems here. I think this patch should be fine.
File src/osmo-upf-load-gen/pfcp_tool.c:
https://gerrit.osmocom.org/c/upf-benchmark/+/39421/comment/a0e23805_9ce1e7a… :
PS1, Line 260: static
Maybe make this function static in a patch before this one?
https://gerrit.osmocom.org/c/upf-benchmark/+/39421/comment/31263be9_3211696… :
PS1, Line 306: {
Maybe make this function static in a different patch?
--
To view, visit https://gerrit.osmocom.org/c/upf-benchmark/+/39421?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: upf-benchmark
Gerrit-Branch: master
Gerrit-Change-Id: Idc6e013d6c7de24889bc500625707c5b5cc1766b
Gerrit-Change-Number: 39421
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Jan 2025 08:57:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: pespin.
dexter has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/upf-benchmark/+/39420?usp=email )
Change subject: osmo-upf-load-gen: Improve VTY command helper strings
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
File src/osmo-upf-load-gen/pfcp_tool_vty.c:
https://gerrit.osmocom.org/c/upf-benchmark/+/39420/comment/283e6689_6a4b49a… :
PS1, Line 222: "Add a core GTP port, to emit GTP1U traffic to\n"
Maybe write "remote" instead of "core"? Even though it should be clear that the core network is remote "core GTP port" sounds a bit strange to me.
--
To view, visit https://gerrit.osmocom.org/c/upf-benchmark/+/39420?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: upf-benchmark
Gerrit-Branch: master
Gerrit-Change-Id: I69701e875f5c38f434265d93a047c8157daffd29
Gerrit-Change-Number: 39420
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Jan 2025 08:47:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: daniel, laforge, neels, pespin.
Hello Jenkins Builder, daniel, laforge, neels,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Code-Review+1 by daniel, Code-Review+1 by neels, Verified+1 by Jenkins Builder
Change subject: vlr: add PS support
......................................................................
vlr: add PS support
Add vlr_ra_update() similar to vlr_lu_update().
Implement resending of GMM PDUs. While CS has a reliable connection
between the MS and MSC, the MS and SGSN doesn't have such connection.
The PDU is resend N times before the LU fsm is failing.
Change-Id: Ie9ffeb140c9d354b3a0f4822e2619f623235add0
---
M src/libvlr/vlr.c
M src/libvlr/vlr_lu_fsm.c
2 files changed, 214 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/90/38490/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie9ffeb140c9d354b3a0f4822e2619f623235add0
Gerrit-Change-Number: 38490
Gerrit-PatchSet: 6
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Attention is currently required from: neels, pespin.
Hello Jenkins Builder, neels, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-msc/+/38488?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Code-Review+1 by neels, Code-Review+1 by pespin, Verified+1 by Jenkins Builder
Change subject: vlr: extend the subscriber invalidate callback with reasons
......................................................................
vlr: extend the subscriber invalidate callback with reasons
The VLR must be allowed to notify the MSC if a subscriber becomes invalid.
There are multiple cases when this happens:
a) if the subscriber didn't do a Location Update Procedure within the
given periodic timer.
b) if the HLR does a Cancel Location Procedure with reason withdraw
c) if the HLR does a Cancel Location Procedure with reason update location.
Only in case c) must the user be informed with the cause code sent by the HLR.
Change-Id: Ie5b687318b106a230fcee52deba86649641004b3
---
M include/osmocom/vlr/vlr.h
M src/libmsc/gsm_04_08.c
M src/libvlr/vlr.c
3 files changed, 8 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/88/38488/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38488?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie5b687318b106a230fcee52deba86649641004b3
Gerrit-Change-Number: 38488
Gerrit-PatchSet: 6
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Attention is currently required from: laforge, neels, pespin.
lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email )
Change subject: vlr: add PS support
......................................................................
Patch Set 5:
(7 comments)
File src/libvlr/vlr.c:
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/af3cff48_9c583ff3?usp… :
PS5, Line 1490: return osmo_fsm_inst_dispatch(vsub->lu_fsm,
> can go in same line.
Done
File src/libvlr/vlr_lu_fsm.c:
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/3759abb5_720d31c8?usp… :
PS5, Line 357: *!
> about the doxygen format comment: […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/0fab90b5_12deb888?usp… :
PS5, Line 589: LOGPFSML(fi, LOGL_ERROR, "LU Compl timeout %d / %d\n", fi->T, lcvp->N);
> let's have "T%d / N%d"
Done
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/c79d807a_bbd82098?usp… :
PS5, Line 598: break;
> (we often have a default: OSMO_ASSERT(false) in these to ensure we notice enum bugs)
in timeouts?
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/c3348b61_547ac77a?usp… :
PS5, Line 771: /*! count times timer T timed out */
> .
I would like to replace the words "count" and "out" with a word starting with t.
Any ideas?
Not sure if this comment can be more cleared, because the spec depends on this knowledge anyways.
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/e6dfe964_6b4c48b9?usp… :
PS5, Line 1597: mian
> typo
Done
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/85d503e9_20c3c779?usp… :
PS5, Line 1611: fi->T
> technically, middle arg should be 0 or -1 or so, because vlr_is_cs() == false from above. […]
In this case, you could also pass -1 to it.
We could use `osmo_tdef_get(vlr_tdefs, ps_timer, OSMO_TDEF_S, 0);`, not sure if it makes it clearer the code instead of using vlr_timer_secs() as we did in all other places.
vlr_timer_secs() is usually used by fsm_state_chg() and you pass the CS and PS timer values of the state into it, because CS has different Ts than PS.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie9ffeb140c9d354b3a0f4822e2619f623235add0
Gerrit-Change-Number: 38490
Gerrit-PatchSet: 5
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Jan 2025 00:01:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>