July 2023 - gerrit-log - lists.osmocom.org

[S] Change in pysim[master]: tests: add test script for pySim-trace
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: dexter. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/33963 to look at the new patch set (#4). Change subject: tests: add test script for pySim-trace ...................................................................... tests: add test script for pySim-trace pySim-trace has no test coverage yet. Let's use a script to run a GSAMTAP pcacp through it and check that no exceptions are raised. Related: OS#6094 Change-Id: Icfabfa7c59968021eef0399991bd05b92467d8d2 --- M contrib/jenkins.sh A pysim-testdata/pySim-trace_test_gsmtap.pcapng A tests/pySim-trace_test.sh 3 files changed, 35 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/63/33963/4 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33963 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Icfabfa7c59968021eef0399991bd05b92467d8d2 Gerrit-Change-Number: 33963 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in pysim[master]: pySim-trace: mark card reset in the trace
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: laforge. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/33960 to look at the new patch set (#2). Change subject: pySim-trace: mark card reset in the trace ...................................................................... pySim-trace: mark card reset in the trace The trace log currently does not contain any information about card resets. This makes the trace difficult to follow. Let's use the CardReset object to display the ATR in the trace. Related: OS#6094 Change-Id: Ia550a8bd2f45d2ad622cb2ac2a2905397db76bce --- M pySim-trace.py M pySim/apdu/__init__.py M pySim/apdu_source/gsmtap.py M pySim/apdu_source/pyshark_gsmtap.py M pySim/apdu_source/pyshark_rspro.py 5 files changed, 32 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/60/33960/2 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33960 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ia550a8bd2f45d2ad622cb2ac2a2905397db76bce Gerrit-Change-Number: 33960 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in pysim[master]: utils: tolerate uninitialized fields in dec_addr_tlv
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: fixeria. Hello Jenkins Builder, laforge, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/33954 to look at the new patch set (#2). Change subject: utils: tolerate uninitialized fields in dec_addr_tlv ...................................................................... utils: tolerate uninitialized fields in dec_addr_tlv TLV fields holding an address may still be uninitialized and hence filled with 0xff bytes. Lets interpret those fields in the same way as we interpret empty fields. Related: OS#6094 Change-Id: Idc0a92ea88756266381c8da2ad62de061a8ea7a1 --- M pySim/utils.py 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/54/33954/2 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33954 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Idc0a92ea88756266381c8da2ad62de061a8ea7a1 Gerrit-Change-Number: 33954 Gerrit-PatchSet: 2 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> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in pysim[master]: utils: tolerate uninitialized fields in dec_addr_tlv
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33954 ) Change subject: utils: tolerate uninitialized fields in dec_addr_tlv ...................................................................... Patch Set 2: (1 comment) File pySim/utils.py: https://gerrit.osmocom.org/c/pysim/+/33954/comment/c6a3fb52_0e55bc8c PS1, Line 775: if tlv[2] == [0xff] * len(tlv[2]): > Not critical: I would recommend doing `if all([v == 0xff for v in tlv[2]]):`, as it's more self-expl […] Self explanatory is in the eye of the beholder. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33954 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Idc0a92ea88756266381c8da2ad62de061a8ea7a1 Gerrit-Change-Number: 33954 Gerrit-PatchSet: 2 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> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 28 Jul 2023 08:37:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: pySim-trace: mark card reset in the trace
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: laforge. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33960 ) Change subject: pySim-trace: mark card reset in the trace ...................................................................... Patch Set 2: (1 comment) File pySim-trace.py: https://gerrit.osmocom.org/c/pysim/+/33960/comment/4d0a2ca4_ed0e4836 PS1, Line 97: """Out > copy+paste mistake: doesn't apply here. Ack -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33960 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ia550a8bd2f45d2ad622cb2ac2a2905397db76bce Gerrit-Change-Number: 33960 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Fri, 28 Jul 2023 08:37:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: pySim-trace: catch StopIteration exception on trace file end
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33962 ) Change subject: pySim-trace: catch StopIteration exception on trace file end ...................................................................... Patch Set 2: (1 comment) File pySim-trace.py: https://gerrit.osmocom.org/c/pysim/+/33962/comment/9984b1be_4a84e9d8 PS1, Line 108: no more data, stop iteration > Might be useful to print number of APDUs parsed here. Done -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33962 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6ab5689b909333531d08bf46e5dfea59b161a79e Gerrit-Change-Number: 33962 Gerrit-PatchSet: 2 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> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 28 Jul 2023 08:37:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: pySim-trace: catch StopIteration exception on trace file end
by dexter 28 Jul '23

28 Jul '23

Attention is currently required from: fixeria. Hello Jenkins Builder, laforge, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/33962 to look at the new patch set (#2). Change subject: pySim-trace: catch StopIteration exception on trace file end ...................................................................... pySim-trace: catch StopIteration exception on trace file end When the trace file end is reaced, pyShark raises a StopIteration exception. Let's catch this exception and exit gracefully. Related: OS#6094 Change-Id: I6ab5689b909333531d08bf46e5dfea59b161a79e --- M pySim-trace.py 1 file changed, 20 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/62/33962/2 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33962 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6ab5689b909333531d08bf46e5dfea59b161a79e Gerrit-Change-Number: 33962 Gerrit-PatchSet: 2 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> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in pysim[master]: pySim-trace: remove stray debug print
by dexter 28 Jul '23

28 Jul '23

dexter has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/33961 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: pySim-trace: remove stray debug print ...................................................................... pySim-trace: remove stray debug print Related: OS#6094 Change-Id: I5f030a8552a84f721bd12ab4751933fc6eeae256 --- M pySim-trace.py 1 file changed, 11 insertions(+), 1 deletion(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/pySim-trace.py b/pySim-trace.py index 453ee43..8a9fad7 100755 --- a/pySim-trace.py +++ b/pySim-trace.py @@ -114,7 +114,7 @@ continue if self.suppress_status and isinstance(inst, UiccStatus): continue - #print(inst) + self.format_capdu(apdu, inst) option_parser = argparse.ArgumentParser(description='Osmocom pySim high-level SIM card trace decoder', -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I5f030a8552a84f721bd12ab4751933fc6eeae256 Gerrit-Change-Number: 33961 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-MessageType: merged

1 0

[S] Change in pysim[master]: pySim-trace: add commandline option --show-raw-apdu
by dexter 28 Jul '23

28 Jul '23

dexter has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/33959 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: pySim-trace: add commandline option --show-raw-apdu ...................................................................... pySim-trace: add commandline option --show-raw-apdu The trace log currently only shows the parsed APDU. However, depending on the problem to investigate it may be required to see the raw APDU string as well. Let's add an option for this. Related: OS#6094 Change-Id: I1a3bc54c459e45ed3154479759ceecdc26db9d37 --- M pySim-trace.py 1 file changed, 24 insertions(+), 4 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/pySim-trace.py b/pySim-trace.py index d457bf4..453ee43 100755 --- a/pySim-trace.py +++ b/pySim-trace.py @@ -83,10 +83,13 @@ # parameters self.suppress_status = kwargs.get('suppress_status', True) self.suppress_select = kwargs.get('suppress_select', True) + self.show_raw_apdu = kwargs.get('show_raw_apdu', False) self.source = kwargs.get('source', None) - def format_capdu(self, inst: ApduCommand): + def format_capdu(self, apdu: Apdu, inst: ApduCommand): """Output a single decoded + processed ApduCommand.""" + if self.show_raw_apdu: + print(apdu) print("%02u %-16s %-35s %-8s %s %s" % (inst.lchan_nr, inst._name, inst.path_str, inst.col_id, inst.col_sw, inst.processed)) print("===============================") @@ -95,7 +98,6 @@ while True: # obtain the next APDU from the source (blocking read) apdu = self.source.read() - #print(apdu) if isinstance(apdu, CardReset): self.rs.reset() @@ -113,7 +115,7 @@ if self.suppress_status and isinstance(inst, UiccStatus): continue #print(inst) - self.format_capdu(inst) + self.format_capdu(apdu, inst) option_parser = argparse.ArgumentParser(description='Osmocom pySim high-level SIM card trace decoder', formatter_class=argparse.ArgumentDefaultsHelpFormatter) @@ -128,6 +130,9 @@ help=""" Don't suppress displaying STATUS APDUs. We normally suppress them as they don't provide any information that was not already received in resposne to the most recent SEELCT.""") +global_group.add_argument('--show-raw-apdu', action='store_true', dest='show_raw_apdu', + help="""Show the raw APDU in addition to its parsed form.""") + subparsers = option_parser.add_subparsers(help='APDU Source', dest='source', required=True) @@ -172,7 +177,8 @@ elif opts.source == 'gsmtap-pyshark-pcap': s = PysharkGsmtapPcap(opts.pcap_file) - tracer = Tracer(source=s, suppress_status=opts.suppress_status, suppress_select=opts.suppress_select) + tracer = Tracer(source=s, suppress_status=opts.suppress_status, suppress_select=opts.suppress_select, + show_raw_apdu=opts.show_raw_apdu) logger.info('Entering main loop...') tracer.main() -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33959 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I1a3bc54c459e45ed3154479759ceecdc26db9d37 Gerrit-Change-Number: 33959 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-MessageType: merged

1 0

[M] Change in libosmo-gprs[master]: rlcmac: Use enum gprs_rlcmac_radio_priority internally everywhere
by osmith 28 Jul '23

28 Jul '23

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/33981 ) Change subject: rlcmac: Use enum gprs_rlcmac_radio_priority internally everywhere ...................................................................... Patch Set 2: (2 comments) File src/rlcmac/gre.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/33981/comment/81395015_3aadbc26 PS2, Line 226: ll_pdu_len, get_value_string(osmo_gprs_rlcmac_llc_sapi_names, sapi), radio_prio + 1); I wonder if it made sense to adjust the enum in types_private.h: ``` /* TS 44.060 Table Table 11.2.5. "Radio Priority" */ enum gprs_rlcmac_radio_priority { GPRS_RLCMAC_RADIO_PRIORITY_1 = 0, /* Radio Priority 1 (Highest priority) */ GPRS_RLCMAC_RADIO_PRIORITY_2 = 1, /* Radio Priority 2 */ GPRS_RLCMAC_RADIO_PRIORITY_3 = 2, /* Radio Priority 3 */ GPRS_RLCMAC_RADIO_PRIORITY_4 = 3, /* Radio Priority 4 (Lower priority) */ }; ``` to add GPRS_RLCMAC_RADIO_PRIORITY_INVALID = 0 and shift the rest. Then it wouldn't be necessary to add the +1 here. What do you think about that? File tests/rlcmac/rlcmac_prim_test.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/33981/comment/b951568f_6e6e67da PS2, Line 991: 2 2 instead of 1 on purpose? -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/33981 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: If2d1946522bc4a1c19d65acb23605f1a3f05ab45 Gerrit-Change-Number: 33981 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 28 Jul 2023 07:45:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0