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
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
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
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
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
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
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