Now that dry sources are properly reaped and freed, an offline packet
source would be deleted once dry, resulting in GetPktSrc() returning
a wild pointer. Don't manage the packet source lifetime and instead
free it during Manager destruction.
If an IO source is registered and becomes dry at runtime, the IO
manager would not honor its manage_lifetime or dont_count attribute
during collection, resulting in memory leaks.
This probably hasn't mattered so far as there's no IO sources registered
in-tree at runtime using manage_lifetime=true.
This largely copies over Spicy's `.clang-format` configuration file. The
one place where we deviate is header include order since Zeek depends on
headers being included in a certain order.
Testing io_poll_interval_live tweaks with @dopheide-esnet on a Myricom based
system to reduce CPU usage showed no visible effect.
It turns out, the pkt_src->IsLive() call used to update poll_interval is only
valid *after* calling ->Register() with the source. The conditional updating
of the poll_interval introduced in 4fa3e4b9b4
never worked out how it was intended to.
The fix ensures that
* we actually use a poll_interval of 10 in the live case
* changing io_poll_interval_live does have an effect
This is a bit of a major change due to lowering the default poll_interval
by a magnitude, but that seemed to have been the intention always. It's also
tunable via redef, so worst case it can be adapted via configuration.
As reference, with the default a Pcap::non_fd_timeout of 20usec *and* a
poll_interval of 100, theoretically we'd be trying to ask a non-selectable
packet source 500000 per second for a new packet. This is not a likely packet
rate that a single worker would currently observe or manage to process.
This probably should not be changed by users, but it's useful for
testing and experimentation rather than needing to recompile.
Processing 100 packets without checking an FD based IO source can
actually mean that FD based sources are never checked during a read
of a very small pcap...
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.
- 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.
- Did a few whitespace re-adjustments during merge
* origin/topic/timw/266-namespaces-part5:
Update plugin btests for namespace changes
Plugins: Clean up explicit uses of namespaces in places where they're not necessary.
Base: Clean up explicit uses of namespaces in places where they're not necessary.
For a non-live PktSrc, it had a special-case to be considered "ready"
every iteration, but additionally every 1 in 100 iterations (the polling
frequency), if there were no other "ready" IOSources, it would get added
to the "ready" set a 2nd time.
This commit completely excludes PktSrc from being processed during the
1/100 runloop iteration where a Poll() happens. That exclusion is
desirable for a second reason: if reading a pcap happens to do its final
Process() during that 1/100 polling-iteration and there's other
IOSources ready to process like EventMgr/TimerMgr, those sources have
logic to advance network-time to current-time if a PktSrc is no longer
open. So in such a case, PktSrc::Process() closes, then
EventMgr::Process() sees there's no longer an active PktSrc and advances
to current-time, then EventMgr::Drain() happens and may dispatch
various events that were previous scheduled, with those events now
unexpectedly seeing a network_time() returning current-time.
It's generally expected for a PktSrc to not be Open yet right after
instantiation, but rather from InitSource() called during the
registration process. Besides that, the logic in question would
potentially replace an error message that is useful/detailed with one
that is not.
* origin/topic/timw/776-using-statements:
Remove 'using namespace std' from SerialTypes.h
Remove other using statements from headers
GH-776: Remove using statements added by PR 770
Includes small fixes in files that changed since the merge request was
made.
Also includes a few small indentation fixes.
This unfortunately cuases a ton of flow-down changes because a lot of other
code was depending on that definition existing. This has a fairly large chance
to break builds of external plugins, considering how many internal ones it broke.
The Zeek code base has very inconsistent #includes. Many sources
included a few headers, and those headers included other headers, and
in the end, nearly everything is included everywhere, so missing
#includes were never noticed. Another side effect was a lot of header
bloat which slows down the build.
First step to fix it: in each source file, its own header should be
included first to verify that each header's includes are correct, and
none is missing.
After adding the missing #includes, I replaced lots of #includes
inside headers with class forward declarations. In most headers,
object pointers are never referenced, so declaring the function
prototypes with forward-declared classes is just fine.
This patch speeds up the build by 19%, because each compilation unit
gets smaller. Here are the "time" numbers for a fresh build (with a
warm page cache but without ccache):
Before this patch:
3144.94user 161.63system 3:02.87elapsed 1808%CPU (0avgtext+0avgdata 2168608maxresident)k
760inputs+12008400outputs (1511major+57747204minor)pagefaults 0swaps
After this patch:
2565.17user 141.83system 2:25.46elapsed 1860%CPU (0avgtext+0avgdata 1489076maxresident)k
72576inputs+9130920outputs (1667major+49400430minor)pagefaults 0swaps
- Removes entire FindSoonest method that includes all of the calls to select() for checking for ready sources
- Removes FD_Set checking against IOSources
- Adds system for registering and unregistering file descriptors from IOSources. This allows individual sources to mark themselves as ready to be checked by the loop as they become available.
- Adds entirely new loop architecture based on checking the IOSources for when their next timeout is, and then waiting for either that timeout or when the next source is ready. This also implements the polling based on what the OS supports, instead of just calling select() on all platforms. Currently it supports kqueue, epoll, and plain poll.
- Adds system for pinging the loop to force it to wake up
For fuzzed/damaged/corrupted pcaps, a timestamp of 0 could lead to an
infinite loop in Bro as it interprets that as meaning the packet source
is not ready yet.
There were two problems actually: the iomanager wasn't properly
deleting sourcesl; and in some situations, the remote serialize wasn't
registered with it to begin with.
Addresses BIT-1306 and probably also BIT-1356.
