Repository navigation
fix: PCAP traces have wrong timestamps - #430
KonradBreitsprecherBkd wants to merge 2 commits into
Conversation
fixed: the PCAP sink announced nanosecond resolution (magic 0xa1b23c4d) but wrote microseconds, so Wireshark and other tools showed timestamps 1000 times too small; it now writes nanoseconds changed: the PCAP reader honors the resolution of the magic number and also accepts microsecond files (0xa1b2c3d4), e.g., captured by Wireshark or tcpdump note: files written by earlier SIL Kit versions contain microseconds under the nanosecond magic; the reader now reads their timestamps 1000 times too small, like every other PCAP tool already did added: Test_Pcap cases for both resolutions Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Konrad Breitsprecher <Konrad.Breitsprecher@vector.com>
f450803 to
5b80f27
Compare
| uint32_t ts_sec; /* timestamp seconds */ | ||
| uint32_t ts_fraction; /* timestamp nanoseconds or microseconds, depending on the magic number */ | ||
| uint32_t incl_len; /* number of octets of packet saved in file */ | ||
| uint32_t orig_len; /* actual length of packet */ |
There was a problem hiding this comment.
We could add
- a enum struct
TimestampModewith two enumeratorsNanosecondsandMicroseconds - and a method
GetTimestampMode() const -> TimestampModethat compares the magic number
That makes it simpler for code using the PacketHeader structure to get that info without having to know about the magic numbers.
There was a problem hiding this comment.
The magicNumber lives in GlobalHeader, I added GetTimestampMode there.
There was a problem hiding this comment.
i think we should keep this code as simple as possible, and just support microseconds, and clamp the timestamps appropriately
There was a problem hiding this comment.
I'd say the error handling to check for the MicrosecondMagic only in the header and return an error is just as complicated as supporting both cases.
There was a problem hiding this comment.
Is there a downside to using nanoseconds? Since SIL Kit timestamps are nanoseconds that seems like the more natural choice for us.
Also since we can also read PCAP files for replay, shouldn't we support both, since we could get a file that was recorded, e.g., using tcpdump.
Add Pcap::TimestampMode and GlobalHeader::GetTimestampMode(), so code reading PCAP headers does not need to know the magic numbers. An unknown magic number throws a SilKitError. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Konrad Breitsprecher <Konrad.Breitsprecher@vector.com>
dc6f175 to
cbbd033
Compare
The PCAP sink writes the nanosecond magic number (
0xa1b23c4d) into the global header, but it wrote the packet timestamps in microseconds. Wireshark, tcpdump and every other PCAP consumer trust the magic number, so they showed the decimal places of all timestamps 1000 times too small (The seconds are treated seperately, so this was correct and likely the cause why this was overlooked). SIL Kit's ownPcapReaderhid the problem because it made the same mistake the other way round and always read the field as microseconds.I confirmed this with a PCAP export from SIL Kit: A frame sent at T=6782 ms has ts_sec=6, ts_usec=782000, and Wireshark shows it as 6.000782 s instead of 6.782 s. With the fix here, the timestamps in the wireshark trace match the SIL Kit logs.
To reproduce use this config for the EthernetWriterDemo, look at the last timestamp in the log and compare it to the timestamp in wireshark :
This PR makes both sides follow the magic number:
0xa1b2c3d4), so captures made with Wireshark or tcpdump can be replayed.PacketHeader::ts_usecis renamed tots_fraction, because its unit depends on the magic number.Compatibility:
.pcapfiles written by earlier SIL Kit versions contain microseconds under the nanosecond magic. From now on,PcapReaderreads their timestamps 1000 times too small, which is how every other PCAP tool already read them. Replaying such old traces will show compressed timing.Instructions for review / testing
PcapSink::Trace(SilKit/source/tracing/PcapSink.cpp) andPcapReader::ReadGlobalHeader/Seek(SilKit/source/tracing/PcapReader.cpp).0xa1b2c3d4instead. That keeps old files readable, but traces lose sub-microsecond precision.PcapFilesink, open it in Wireshark, and confirm the timestamps match the simulation time.Developer checklist (address before review)