search

google / packetdrill

The official Google release of packetdrill
GNU General Public License v2.0
887 stars 220 forks source link

net-test: packetdrill: tcpdump: reduce sleep #81

Open matttbe opened 9 months ago

matttbe commented 9 months ago

As suggested by Davide, TCPDump's '--immediate-mode' will force the kernel to flush packets to userspace immediately after the reception. Thanks to that, the 'sleep 1' at the end of the test is no longer needed: the userspace will have all the packets when the kill signal will be received. Just to be on the safe side, we can keep a sleep 0.1, as recommended by Willem.

It looks like this TCPDump option is usually not recommended but because there should not be too many packets generated here with Packetdrill, it seems fine to have it and saves 1 second for each test. Note that it is not an exotic option as it is used by default when packets are not written in a file. Also, we now use '--packet-buffered' to reduce IO.

Now that the "immediate mode" is being used, it seems enough to just wait for the '.pcap' file to be created to know when TCPDump is ready.

While at it, the directory where the capture will be store is created if it didn't exist.

nealcardwell commented 9 months ago

Thanks. Should we also add the --packet-buffered flag for tcpdump?

nealcardwell commented 9 months ago

Adding author @wdebruij to the thread...

wdebruij commented 9 months ago

Should it keep a 0.1s sleep to avoid a tight race between last packet and close?

The proof is in the pudding. If there are no regressions in any of the (by now many) .pkt tests, sounds like a very nice speed-up indeed.

matttbe commented 9 months ago

Thank you both for the quick feedback!

Should we also add the --packet-buffered flag for tcpdump?

@nealcardwell: Good idea!

Should it keep a 0.1s sleep to avoid a tight race between last packet and close?

@wdebruij: Good idea, just to be on the safe side.

The proof is in the pudding. If there are no regressions in any of the (by now many) .pkt tests, sounds like a very nice speed-up indeed.

That's quite hard to track down regressions. I ran 'tcp' tests and check the differences of size of the produced .pkt files: most of them were the same but some not. I didn't check all of them but from what I saw, the differences were due to instabilities with some tests (e.g. tolerance issues) and packets like ICMP6, router solicitation present in some but not all.

@nealcardwell @wdebruij: I just pushed a new version containing your two suggestions but also a way to reduce the sleep just after having started tcpdump. So we can save up to 2 seconds per test now :)

On my side, it seems to do the job well but I was able to do the tests only on one setup. I would be glad if someone else can try on their side, just to be on the safe side :)

matttbe commented 4 months ago

@nealcardwell : is there anything else I need to do for this PR? :)