Bug #6094
closedpySim-trace has no test coverage
100%
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
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
Updated by dexter 9 months ago
- File cardem_trace_filtered.pcapng cardem_trace_filtered.pcapng added
- File cardem.log cardem.log added
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.
Updated by laforge 9 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.
Updated by dexter 9 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
Updated by dexter 9 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.
Updated by laforge 9 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.
Updated by dexter 9 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.
Updated by roh 9 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.
Updated by dexter 9 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
