Bug #6094
closedpySim-trace has no test coverage
Added by laforge 10 months ago. Updated 9 months ago.
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
| 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
Updated by dexter 10 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 10 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 10 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 10 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 10 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 10 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 10 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 10 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
