Tests fail with "Found something in the pcap file that either is non-IP, or cannot be parsed (RuntimeError)"
See e.g. https://jenkins.tails.boum.org/view/RM/job/test_Tails_ISO_stable/1829/cucumberTestReport/. I've not looked very closely but the first few test suite jobs I had to look at today exposed this problem.
@anonym, I had never seen it before, could this be related to the recent merge of feature/16148-unfiltered-pcaps?
Test suite: don't fail DHCP check for non-IP packets.
Since we started recording the router's traffic in our PCAP
files (refs: #16148) we have started to see this check fail. Indeed,
in some of the captures I've seen the router send an ARP request,
which we don't have a case for. It is sufficient to check only IP
packets, since DHCP only uses IP packets, so we don't have to fail if
we find something else, just ignore them.
Test suite: only accept IP/ARP during DHCP check.
While it is true that DHCP only uses IP we might as well be more
thorough. The only non-IP packets that are expected here are ARP
packets, so let's just fail on anything else, like we did prior to
- % Done changed from 0 to 40
- Feature Branch set to test/16788-fix-pcap-analysis
anonym, I had never seen it before, could this be related to the recent merge of feature/16148-unfiltered-pcaps?
Yup. Since we started including all traffic, including the router's, in #16148 I only adapted
pcap_connections_helper() but didn't think about this check. It's an easy fix!
I've grepped for
pcap and couldn't find any other code that needs adaptation, so we should be good now. Jenkins?
- Status changed from Needs Validation to In Progress
- Assignee changed from intrigeri to anonym
The reasoning behind this fix implicitly trusts that the system under test behaves as expected ("DHCP only uses IP packets") and then we ignore unexpected behavior. Assuming that a cleaner fix (e.g. ignoring only ARP from the router) is non-trivial, OK, fine, let's do this, but then please explain the reasoning and trade-off in a comment, not just in the commit message: otherwise, next time I'll read this code, I'll be like "uh, seriously?!" :)
- Status changed from In Progress to Needs Validation
- Assignee changed from anonym to intrigeri
- % Done changed from 50 to 60
The reasoning behind this fix implicitly trusts that the system under test behaves as expected ("DHCP only uses IP packets") and then we ignore unexpected behavior.
I see your point. In practice we are not really testing DHCP only, since there' a full system running (hence the unrelated ARP requests), so I think what you propose is likely to cause robustness issues in general, but since this step will only be used in this precise situation (where ARP is the only unrelated thing that can reasonably be expected, AFAICT) I think it is fine, so I pushed something I think you'll like more.