This would generally happen the next loop iteration around anyway, but
seems nice to ensure a zero timeout source will be processed at the same
time as sources with ready FDs.
Previously, if two iosources returned 0.0 as their timeout, only
one of them would be considered ready. An always ready source
therefore may starve other ready ones due to this and minimally
this behavior seems surprising.
Offline pcap sources are always ready and return 0.0 for
GetNextTimeout() (unless in pseudo-realtime), so we can
also remove the offline source special case.
One subtle side-effect of this change is that if an IO source
returns a 0.0 timeout *and* it's file descriptor is ready in
the same loop iteration, it may be processed twice.
This reworks 2aec7640dd (zeek/zeek#2039) to
avoid calling ExtractNextPacketInternal() within GetNextTimeout() for
the non-pseudo-realtime case. Also relates to zeek/zeek#2842.
The intention of the referenced change was to avoid a 0.00002 timeout when
a non-selectable packet source has more packets queued. This was implemented
by checking for a new packet within GetNextTimeout().
The proposed change switches to an predictive approach: Use the result of
the previous ExtractNextPacket() call (stored as had_packet) as an indication
whether more packets are to be expected.
Calling ExtractNextPacketInternal() within GetNextTimeout() may cause
surprising behavior as some packet source may block [1] or spent a significant
amount of time (e.g. applying BPF filters [2]) within ExtractNextPacket().
The result of GetNextTimeout() should be available immediately as guidance
for the main-loop and the actual work should happen within the ->Process()
method.
This change also attempts to separate the pseudo-realtime logic from the
non-pseudo-realtime in an attempt show pseudo-realtime as special.
[1] 00c4d657e0/src/Napatech.cc (L116)
[2] 58b25c8eba/src/Myricom.cc (L250)
This reverts commit 957825441a, reversing
changes made to c8cdc75f2b.
Caused spurious CI failures in the external testing baselines. See zeek/zeek#2842.
* microsoft/master: (71 commits)
Clang formatting
Mask ports before inserting them into the map
Fix compiler warning from applied patch
Remove statistics plugin in favor of stats bif
Add EventHandler version of stats plugin
Mark a few EventHandler methods const
Changed implementation from std::map to std::unordered_map of Val.cc
Removed const, Windows build is now working
Added fixes suggested in PR
Update src/packet_analysis/protocol/ip/IP.cc
Apply suggestions from code review
Clang format again but now with v13.0.1
Rewrote usages of define(_MSC_VER) to ifdef _MSC_VER
Clang format it all
Fixed initial CR comments
Add NEWS entry about Windows port
Add a couple of extra unistd.h includes to fix a build failure
Use std::chrono instead of gettimeofday
Update libkqueue submodule [nomail]
Don't call tokenize_string if the input string is empty
...
- iosource_mgr can now track write events to file descriptors as well
as read events. This adds an argument to both RegisterFd() and
UnregisterFd() for setting the mode, defaulting to read.
- IOSources can now implement a ProcessFd() method that allows them to
handle events to single file descriptors instead of of having to
loop through/track sets of them at processing time.
PktSrc::GetNextTimeout always returned a fixed timeout of 20 microseconds for non-selectable packet sources regardless of whether they have packets available. This adds unnecessary delay every FindReadySources poll_interval when packets are available to be read.
Instead, for non-selectable packet sources, check whether packets are available and return a timeout of 0 to indicate data is available. This is closer to the behaviour of the old capture loop.
This was mitigated somewhat by the fact FindReadySources poll interval defaults to 100 packets, and live sources are added to the ready list regardless of whether they have packets available (unless it is time to force a poll).
This commit also changes the PcapDumper to automatically flush after
every called to Dump(). This is because pcap_dump has an internal buffer
of some sort that only writes to the file after a set amount of bytes.
When using the new option on a low-traffic network, it might be a while
before you see any packets written since it has to overcome that buffer
limit first.
This is apparently a problem with the Myricom version of libpcap, where
instead of returning a null or a zero if no packets are available, it
returns the previous packet. This causes Zeek to improperly parse the
packet and crash. We thought we had fixed this previously with a check
for a null packet but that fix was not enough.
This also includes:
- Deprecating the NetSessions name.
- Renaming the zeek::sessions global to zeek::session_mgr and deprecating the old name.
- Renaming Sessions.{h,cc} to SessionManager.{h,cc}.
Some libpcaps (observed in Myricom's) may claim to have read a packet,
but either did not really read a packet or at least provide no way
to access its contents, so this adds a check for null-data to
handle those cases.
* origin/topic/vern/zval: (42 commits)
whitespace tweaks
resolved some TODO comments
remove unnecessary casts, and change necessary ones to use static_cast<>
explain cmp_func default
change functions for ZVal type management to static members
fix some unsigned/signed integer warnings
address lint concern about uninitialized variable
Remove use of obsolete forward-declaration macros
fix #include's that lack zeek/ prefixes
explicitly populate holes created in vectors
fixes for now-incorrect assumption that GetField always returns an existing ValPtr
memory management for assignment to vector elements
memory management for assignment to record fields
destructor cleanup from ZAM_vector/ZAM_record
fix #include's that lack zeek/ prefixes
overlooked another way in which vector holes can be created
initialize vector holes to the correct corresponding type
explicitly populate holes created in vectors
fix other instances of GetField().get() assuming long-lived ValPtr's
fix for now-incorrect assumption that GetField always returns an existing ValPtr
...
