Synchronization of state between connecting peers now skips over
identifiers that aren't initialized with a value yet. If they're
assigned a value later, that will be synchronized like usual.
When using --enable-debug, values keep track of the last identifier
to which they were bound by storing a ref'd ID pointer. This could
lead to some circular dependencies in which an ID is never reclaimed
because the Val is bound to the ID and the ID is bound to the Val, with
both holding references to each other.
There might be more cases where this feature of --enable-debug caused
a leak, but it showed up in particular when running the
core.leaks.remote unit test due to the internal
SendID("peer_description") call during the handshake between remote
processes. Other tests showed the send_id() BIF leaked more generally.
Tracking the ID last bound to a Val through just the identifier string
instead of a ref'd ID pointer fixes the leak.
If a log filter attempts to write to a path for which a writer is
already instantiated due to remote logging, it will re-use the writer
as long as the fields of the filter and writer are compatible, else
the filter path will be auto-adjusted to not conflict with existing
writer's. Conflicts between two local filters are still always
auto-adjusted even if field types agree (since they could still
be semantically different).
Addresses #842.
frameworks.
There were a number of cases that weren't thread-safe. In particular,
we don't use std::string anymore for anything that's passed between
threads (but instead plain old const char*, with manual memmory
managmenet).
This is still a check-point commit, I'll do more testing.
* robin/topic/writer-info:
Extending the log writer DoInit() API.
Reworking log writer API to make it easier to pass additional information to a writer's initialization method.
Conflicts:
src/logging/WriterBackend.cc
src/logging/WriterBackend.h
src/logging/WriterFrontend.cc
* origin/fastpath:
Fix inconsistencies in random number generation.
Updating input framework unit tests.
Add front-end name to InitMessage from WriterFrontend to Backend.
Small tweak to make test complete quicker.
Drain events before terminating log/thread managers.
Fix strict-aliasing warning in RemoteSerializer.cc (fixes#834).
Fix typos in event documentation
Fix typos in NEWS for Bro 2.1 beta
* origin/topic/jsiwek/ipv6-comm:
Enable Bro to communicate with peers over non-global IPv6 addresses.
Add unit tests for Broccoli SSL and Broccoli IPv6 connectivity.
Remove AI_ADDRCONFIG getaddrinfo hints flag for listening sockets.
Undo communication protocol version bump.
Add support to Bro for connecting with peers over IPv6.
Closes#820.
Conflicts:
src/bro.bif
This usually requires specifying an additional zone identifier
(see RFC 4007). The connect() and listen() BIFs have been
changed to accept this zone identifier as an argument.
Because, according to RFC 3493, that will cause getaddrinfo to
overlook the ::1 loopback if there's not some other interface with
a global IPv6 address. The rationale being that the flag helps
prevent unnecessary AAAA lookups, but since I set AI_NUMERICHOST,
lookups aren't going to happen anyway.
Also update the IPv6 Bro communication test to get it to work more
reliably.
Looks like it wasn't necessary because no message between remote peers
needed to be changed to support IPv6, just messages between Bro
parent and child processes were changed.
- Communication::listen_ipv6 needs to be redef'd to true in order
for IPv6 listening sockets to be opened.
- Added Communication::listen_retry option as an interval at which
to retry binding to socket addresses that were already in use.
- Added some explicit baselines to check in the istate.events
and istate.events-ssl tests -- the SSL test was incorrectly
passing because it compared two empty files. (The files being
empty because "http/base" was given as an argument to Bro which
it couldn't handle because that script doesn't exist anymore).
Also renaming --enable-perftools to --enable-perftool-debug to
indicate that the switch is only relevant for debugging the heap. It's
not needed to pick up tcmalloc for better performance.
--with-perftools can still (and always) be used to give a hint where
to find the libraries.
With the threading, using tcmalloc improves memory usage on FreeBSD
significantly when running on a trace. If it fixes the live problems,
remains to be seen ...
As we can't use the IPAddr class (because it's not thread-safe), this
involved a bit manual address manipulation and also shuffling some
things around a bit.
Not fully working yet, the tests for remote logging still fail.
pass yet.
Changes:
- Gave IPAddress/IPPrefix methods AsString() so that one doesn't need
to cast to get a string represenation.
- Val::AsAddr()/AsSubnet() return references rather than pointers. I
find that more intuitive.
- ODesc/Serializer/SerializationFormat get methods to support
IPAddress/IPPrefix directly.
- Reformatted the comments in IPAddr.h from /// to /** style.
- Given IPPrefix a Contains() method.
- A bit of cleanup.
Internally, all BROv6 preprocessor switches were removed and
addr/subnet representations wrapped in the new IPAddr/IPPrefix classes.
Some script-layer changes of note:
- dns_AAAA_reply event signature changed: the string representation
of an IPv6 addr is easily derived from the addr value, it doesn't
need to be another parameter. This event also now generated directly
by the DNS analyzer instead of being "faked" into a dns_A_reply event.
- removed addr_to_count BIF. It used to return the host-order
count representation of IPv4 addresses only. To make it more
generic, we might later add a BIF to return a vector of counts
in order to support IPv6.
- changed the result of enclosing addr variables in vertical pipes
(e.g. |my_addr|) to return the bit-width of the address type which
is 128 for IPv6 and 32 for IPv4. It used to function the same
way as addr_to_count mentioned above.
- remove bro_has_ipv6 BIF
This is based on Gilbert's code but I ended up refactoring it quite a
bit. That's why I didn't do a direct merge but started with a new
branch and copied things over to adapt. It looks quite a bit different
now as I tried to generalize things a bit more to also support the
Input Framework.
The larger changes code are:
- Moved all logging code into subdirectory src/logging/. Code
here is in namespace "logging".
- Moved all threading code into subdirectory src/threading/. Code
here is in namespace "threading".
- Introduced a central thread manager that tracks threads and is
in charge of termination and (eventually) statistics.
- Refactored logging independent threading code into base classes
BasicThread and MsgThread. The former encapsulates all the
pthread code with simple start/stop methods and provides a
single Run() method to override.
The latter is derived from BasicThread and adds bi-directional
message passing between main and child threads. The hope is that
the Input Framework can reuse this part quite directly.
- A log writer is now split into a general WriterFrontend
(LogEmissary in Gilbert's code) and a type-specific
WriterBackend. Specific writers are implemented by deriving from
the latter. (The plugin interface is almost unchanged compared
to the 2.0 version.).
Frontend and backend communicate via MsgThread's message
passing.
- MsgThread (and thus WriterBackend) has a Heartbeat() method that
a thread can override to execute code on a regular basis. It's
triggered roughly once a second by the main thread.
- Integration into "the rest of Bro". Threads can send messages to
the reporter and do debugging output; they are hooked into the
I/O loop for sending messages back; and there's a new debugging
stream "threading" that logs, well, threading activity.
This all seems to work for the most part, but it's not done yet.
TODO list:
- Not all tests pass yet. In particular, diffs for the external
tests seem to indicate some memory problem (no crashes, just an
occasional weird character).
- Only tested in --enable-debug mode.
- Only tested on Linux.
- Needs leak check.
- Each log write is currently a single inter-thread message. Bring
Gilbert's bulk writes back.
- Code needs further cleanup.
- Document the class API.
- Document the internal structure of the logging framework.
- Check for robustness: live traffic, aborting, signals, etc.
- Add thread statistics to profile.log (most of the code is there).
- Customize the OS-visible thread names on platforms that support it.
Broccoli doesn't support expressions, and we now no longer send them
when serializing attributes. This is the Bro change mentioned in #606.
It's needs a correspondinly modified Broccoli identifying itself as
such, and it isn't tested yet ...
Addresses #606.
* origin/topic/robin/interpreter-exceptions:
Adding test for new error handling.
Experimental code to better handle interpreter errors.
This seems to work fine and it catches some potentially nasty crashes
so I'm merging it in even though it's not the final word on error
handling yet. #646 tracks the work scheduled for later.
Currently, a lot of interpreter runtime errors, such as an access to
an unset optional record field, cause Bro to abort with an internal
error. This is an experimental branch that turns such errors into
non-fatal runtime errors by internally raising exceptions. These are
caught upstream and processing continues afterwards.
For now, not many errors actually raise exceptions (the example above
does though). We'll need to go through them eventually and adapt the
current Internal() calls (and potentially others). More generally, at
some point we should cleanup the interpreter error handling (unifying
errors reported at parse- and runtime; and switching to exceptions for
all Expr/Stmt/Vals). But that's a larger change and left for later.
The main question for now is if this code is already helpful enough to
go into 2.0. It will quite likely prevent a number of crashes due to
script errors.
- Removing unnecessary log flushing. Closes#498.
- Adding new BiF disconnect() that shuts a connection to a peer down.
- terminate_connection() now first flushes any still buffered log
messages.
The communication subsystem is now disabled until a new BiF,
enable_communication(), is called. The base scripts do this
automatically when either a Communication::Node is defined, or Bro is
asked to listen for incoming connections.
When using a `print` statement to write to a file that has raw output
enabled, NUL characters in string are no longer interpreted into "\0",
no newline is appended afterwards, and each argument to `print` is
written to the file without any additional separation.
(Re)Assigning to identifiers with the &raw_output attribute should also
now correctly apply the attribute to the file value being assigned.
Note that the write_file BiF should already be capable of raw string
data to a file, expect it bypasses the print_hook event.
Addresses #474
