Project

General

Profile

Actions

Bug #6094

closed

pySim-trace has no test coverage

Added by laforge 8 months ago. Updated 7 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
pySim-trace
Target version:
-
Start date:
07/11/2023
Due date:
% Done:

100%

Spec Reference:

Description

Right now we wouldn't notice if we introduce any patches that break pySim-trace, even in the most obvious way.

We should think about how we can test pySim-trace, e.g. by running a few pcap files through it and doing at least some kind of basic validation, such as no exceptions are raised.


Files

cardem_trace_filtered.pcapng cardem_trace_filtered.pcapng 96.1 KB dexter, 07/19/2023 02:57 PM
cardem.log cardem.log 242 KB dexter, 07/19/2023 03:11 PM

Checklist

  • get pySim-trace to run without throwing expections
  • add test that runs pySim-trace.py and checks if there were any exceptions
  • extend test so that the trace log is also verified
Actions #1

Updated by dexter 7 months ago

I have tried out pySim-trace now. As it seems there is still some stuff to fix.

Here are some patches:
remote: https://gerrit.osmocom.org/c/pysim/+/33831 requirements: add pyshark to requirements lists
remote: https://gerrit.osmocom.org/c/pysim/+/33834 README.md: add tshark to required debian packages
remote: https://gerrit.osmocom.org/c/pysim/+/33837 apdu/ts_102_221: extract channel number from dict before calling del_lchan

There is also a problem with opening channels. It is not possible to open a channel twice. The code in runtime.py:add_lchan also reflects that. It raises an exception when it finds the lchan in the self.lchan dict. However in the trace I use for testing (see attached file) already existing channels are opened and the status word is 9000. Then I noticed that in the cardem log I see a lot of warm-resets. This explains those hick ups. I guess there is no way to log those warm reset events to GSMTAP.

We probably cannot fix this the lazy way by just ignoring consecutive channel openings or by closing+re-opening the channel. I think for now I will cut my test-trace a bit so that it does only contain the data until the first warm-reset. Also I might try RSPRO traces, those should contain reset events.

Actions #2

Updated by laforge 7 months ago

On Wed, Jul 19, 2023 at 03:18:51PM +0000, dexter wrote:

There is also a problem with opening channels. It is not possible to open a channel twice. The code in runtime.py:add_lchan also reflects that. It raises an exception when it finds the lchan in the self.lchan dict. However in the trace I use for testing (see attached file) already existing channels are opened and the status word is 9000. Then I noticed that in the cardem log I see a lot of warm-resets. This explains those hick ups. I guess there is no way to log those warm reset events to GSMTAP.

A warm reset should always contain a GSMTAP_SIM_ATR sub-type after every reset before sending the first GSMTAP_SIM_APDU. This should be used to reset all state about logical channels, etc.

Actions #3

Updated by dexter 7 months ago

  • Status changed from New to In Progress

Thanks. I have tracked that down. Apparantly simtrace2_cardem does not log the resets to GSMTAP. (I should better use simtrace2_sniff)

Actions #4

Updated by dexter 7 months ago

  • % Done changed from 0 to 10

There is a problem when parsing UTF-8 strings: Uninitialized files commonly contain a string of 0xff. When we parse such strings as utf-8 we will get an exception from the parser. We should interpret those uninitialized contents as empty strings. The following patch fixes the problem:

https://gerrit.osmocom.org/c/pysim/+/33941 construct: add adapter Utf8Adapter to safely interpret utf8 text

Actions #5

Updated by dexter 7 months ago

  • % Done changed from 10 to 60

Along with a couple of other fixes I have now added a script that tests pySim-trace.py with the pcap file that I was using for testing:

https://gerrit.osmocom.org/c/pysim/+/33963 tests: add test script for pySim-trace

This also means that pySim-trace.py now runs through the trace without raising an execption. Unfortunately there seems to be a compatibility problem with pyShark/tshark on debian 10. For my tests I have used debian 11 and there it works fine. Since debian 10 is a bit old now I will check back if there is a possibility to upgrade the simtester to debian 11.

Actions #6

Updated by laforge 7 months ago

On Thu, Jul 27, 2023 at 02:15:49PM +0000, dexter wrote:

This also means that pySim-trace.py now runs through the trace without raising an execption. Unfortunately there seems to be a compatibility problem with pyShark/tshark on debian 10. For my tests I have used debian 11 and there it works fine. Since debian 10 is a bit old now I will check back if there is a possibility to upgrade the simtester to debian 11.

I think it might make sense to migrate straight to Debian 12, something which osmith is doing for a lot of our other build / CI infrastructure these days. There will likely be some fall-out during the upgrade anyway, and if we d o Debian 12 right away we avoid having to do another upgrade with fall-out whenever 11 becomes unsupported.

If there is a version dependency on a specific wireshark version or Debian version, it would also be good to
mention this in the pySim-trace documentation. Even a vague note like "it's known it doesn't work with
wireshark pacakged in Debian before 11" can be useful to some users.

Actions #7

Updated by dexter 7 months ago

  • Checklist item get pySim-trace to run without throwing expections added
  • Checklist item add test that runs pySim-trace.py and checks if there were any exceptions added
  • Checklist item extend test so that the trace log is also verified added

I have discussed the problem with roh, he has upgraded the VM to Debian 12 now. The pySim-trace testscript runs fine now and the other tests are fine as well. I have also put a note in the README.md that mentions the problem with older Debian versions.

Actions #8

Updated by dexter 7 months ago

  • % Done changed from 60 to 70
Actions #9

Updated by roh 7 months ago

dexter wrote in #note-7:

I have discussed the problem with roh, he has upgraded the VM to Debian 12 now. The pySim-trace testscript runs fine now and the other tests are fine as well. I have also put a note in the README.md that mentions the problem with older Debian versions.

just for keeping confusion down and details correct: its debian 11 now. we went from buster(10) to bullseye(11).
i have not seen this ticket yesterday. anyhow.. there is no direct path from 10 to 12, so it would have been via 11 either way.

Actions #10

Updated by dexter 7 months ago

  • Checklist item extend test so that the trace log is also verified set to Done
  • % Done changed from 70 to 90

roh thanks for the heads up. I have checked the README.md, there we mention version 11, so everything is correct for now.

As suggested I have extended the script so that the output is also verified:
https://gerrit.osmocom.org/c/pysim/+/34039 pySim-trace_test: verify output of pySim-trace.py

Actions #11

Updated by dexter 7 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 90 to 100

The last patch (verify the output of pySim-trace.py) is now merged. We have good test coverage now, so I think we can close this ticket.

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)