Commit graph

1054 commits

Author SHA1 Message Date
Erik Boasson
5a4193ca37 Do send HEARTBEATs if there are new readers
The emphasis is on "do": with this commit it does it even when there is
no data available in the writer.  These were suppressed previously
because of a quirk in the DDSI specification in versions prior to 2.3,
where it impossible for a writer to send a valid heartbeat if its
history cache was empty.

Not sending them has negative consequences, as establishing a reliable
connection then becomes dependent on the reader sending a pre-emptive
ACKNACK message.  Uusally, this makes no observable difference, but if
the writer temporarily disconnects from the reader (but not vice-versa)
it may require the publishing of a sample to resynchronize the two.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
dff08536c4 Quick-n-dirty statistics framework
This adds a set of functions:

* dds_create_statistics
* dds_refresh_statistics
* dds_delete_statistics
* dds_lookup_statistic

to poll entities for information on their state, returned as a set of
name-value pairs.  The interface and selection of statistics (and
naming) is all provisional, and for this reason the
dds/ddsc/dds_statistisc.h file is not included by dds.h.

Currently, the only statistics available relate to retansmits and are
optionally output by ddsperf.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
b116e6e41e ddsperf and throughput-test script improvements
* Bandwidth usage is now printed in Mb/s if no reference rate is given

* Trailing average rate over the last 10s (approximated as the last 10
  lines of output) is printed

* An option to wait until the expected number of peers is present

* The test script now pushes data to the remotes, instead of using the
  first remote as the publisher

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
63b1a7179b Change ACKNACK policy to be less aggressive
Overly aggressive sending of ACKNACKs eats bandwidth and causes
unnecessary retransmits and lowers performance; but overly timid sending
of them also reduces performance.  This commit reduces the
aggressiveness.

* It keeps more careful track of what ACKNACK (or NACKFRAG) was last
  sent and when, suppressing ACKs that don't provide new information for
  a few milliseconds and suppressing NACKs for the NackDelay
  setting.  (The setting was there all long, but it didn't honor it when
  the writer asked for a response.)

* It ignores the NackDelay when all that was requested has arrived, or
  when it receives a directed heartbeat from a Cyclone peer.  The latter
  is taken as an indication that no more is following, and allows the
  recipient to ask far arbitrary amounts of data and rely on the sender
  to limit the retransmit to what seems reasonable.  (For NACKFRAG one
  can do it in the recipient, but for ACKNACK one cannot, and so one
  might as well do it at the sender always.)

* Sufficient state is maintained in the match object for the ACKNACK
  generator to decide whether or not to send an ACKNACK following the
  rules, and it may decide to send just an ACK even though there is data
  missing, or nothing at all.

* If HEARTBEAT processing requires an immediate response, the response
  message is generated by the receive thread, but still queued for
  transmission.  If a delayed response is required, it schedules the
  ACKNACK event.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
0a4c863f11 Combine heartbeats in response to ACKNACK/NACKFRAG
This adds tracking of whether a heartbeat should be generated until
processing of the message is complete or an ACKNACK or NACKFRAG from
another reader requires a response.  This way, an ACKNACK + NACKFRAG
pair does not trigger multiple heartbeat messages.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
b5c0eab2fb Fix malformed GAPs by sending an empty bitset
The DDSI spec version 2.3 allows empty bit sets, so malformed GAPs
caused by a bug in the code for avoiding those is most easily fixed by
generating a GAP with an empty bit set.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
312df32605 Include payload in retransmit queue length
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
66daba9f2f Adapt message and burst sizes to receive buffers
This changes a few intertwined things at the same time:

* It allows configuring sending a partial message for large messages,
with a maximum derived from the discovered receive buffer sizes;

* It uses a different message size limit for datagrams that include
  retransmits than for those that don't.  The argument here is that,
  having seen flaky networks where large datagrams cause trouble, it
  makes sense to default to sending retransmits as datagrams that fit in
  individual packets.

* The best performance is generally obtained using the maximum data gram
  size, but the benefits do fall off quite quickly once they are
  largish.  For flaky networks, it doesn't make sense to go for 64kB
  datagrams.  This tries to find a reasonable compromise.

* It now packs mutiple fragments into a single DATAFRAG message to
  eliminate the cost of using small fragment sizes.

The changes in buffer sizes cause the ddsperf sanity check to fail:

* The larger amounts of unacknowledged data cause the used memory to be
  higher, failing the RSS check.  Raising the limit seems
  reasonable (the alternative would be to configure it back to the old
  values, but it is all empirically determined anyway).

* The same also causes the publisher thread to get to run more and the
  ping/pong bit gets less of a chance.  Using fixed-frequency bursts
  helps with this.

This therefore also adjust the test configuration and the thresholds a
bit.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
82e6a7972c Represent fragment size as uint16_t
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
7fbbc13e65 Include receive buffer sizes in discovery
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
eb2a273c1e Send heartbeat on NACKFRAG of latest sample
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
18d4bc9699 Allow old NACKFRAG count after a delay
An asymmetrical disconnect where the reader undiscovers and rediscovers
the writer, but the reader remains alive all the time for the writer
results in the "count" field of NACKFRAGs restarting.  According to the
spec these must be ignored to protect against multi-pathing, but in this
scenario, ignoring them results in ignoring valid retransmit requests
until the "count" value catches up, which can take a very long time.

For ACKNACKs and HEARTBEATs the same problem exists, there it was
already handled by accepting backward jumps after some time has passed.
This reuses the same logic for NACKFRAGs.

This also changes the "count" fields to uint32_t throughout: the spec
defines them as int32_t, requires them to be strictly monotonically
increasing and omits any mention of a valid range or at what value the
counter should start.  Thus, everything in [-2^31,2^31-1] is allowed,
switching to an uint32_t merely shifts the range.  It also appears that
all implementations start at 0 or 1.  The "strictly monotonically" part
was impossible to do without disconnecting anyway.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-06 11:40:56 +02:00
Erik Boasson
fb7034e28f Do not set "present" flag in deser_locator
It is done by "do_locator" after it has decided that the locator is
well-formed and, crucially, not to be ignored.  Setting it when there
are only ignored locators (of the unicast/multicast, data/metadata
variety) causes further processing to rely on uninitialized memory.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
22965a0fd7 Constness in transport functions
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
bffe386f2d No extra transmit socket in MSM_NO_UNICAST mode
Reuse unicast data socket in MSM_NO_UNICAST, just like it did in all
modes before the extra socket was introduced in
d1ed8df9f3.  This restores support for the
"raw ethernet" transport on Linux by no longer requiring the transport
to create a socket with an arbitrary "port".

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
f33867e884 Accept "raweth" locators and check vendorid
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
e1ac4d7095 IPv6 needs scope_id for link-local addresses
This was accidentally deleted in
4df38f5bf9

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
5c94a07771 Fix bogus deinit of security state on init error
The deinitialize would happen on most errors, but in all those cases it
would not have been initialized yet.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
fe89d216a5 Remove IPv4/IPv6-specific code in adding peers
This removes the special handling of IP addresses in adding peer
locators from the configuration, instead relying on the general
string-to-locator conversion routines.

* This extends the common IP handling to code to handle the optional
  presence of a port and the use of brackets, allowing them always for
  IPv6 addresses, but requiring them only when needed for disambiguating
  numerical IPv6 addresses when a port is present.

* The "multicast generator" format is now handled in UDPv4 code.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
e005c89240 Move openssl/ssl.h dependency out of the way
The src/core/ddsi/tests/locators.c test directly includes the header
files related to DDSI support for TCP and this pulled in openssl/ssl.h,
which in turn results in a build error in some environments because the
file can't be found.

There was no good reason why this dependency existed, the definitions
that relied on it were used only in the implementation of the TCP and
TLS support.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Erik Boasson
94c7965077 Correctly print AllowMulticast=false in config log
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-07-03 13:01:47 +02:00
Jeroen Koekkoek
f22d7974f7 Rename print to printspc to fix compile issue on Xilinx SDK
Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
2020-06-26 13:10:20 +02:00
Jeroen Koekkoek
f4e99f41f6 Fix xsd generation in ddsconf
Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
2020-06-26 13:10:20 +02:00
Jeroen Koekkoek
93c75186f0 Fix CID 304858 reported by Coverity Scan
Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
2020-06-26 13:10:20 +02:00
Erik Boasson
5d7f2fd438 Compile applink.c with -D_CRT_SECURE_NO_WARNINGS
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-26 11:47:56 +02:00
Erik Boasson
812fa3e42f Deal with OpenSSL in a DLL on Windows
OpenSSL doesn't support using BIOs of the "fd" or "file" type when it is
built as a DLL and the executable didn't provide it with access to the
executable's CRT.  Requiring all applications that wish to use security
to worry about this "applink.c" thing is too onerous a requirement.

* Check for the existence of "applink.c" in the OpenSSL include
  directory, adding it to the security tests if it exists.  This way,
  all of OpenSSL can be used by the tests.

* Include it in the security core and built-in plugin tests.  This way,
  the test code can use the entirety of OpenSSL.

* In the authentication and access-control plugins, load X509 and
  private keys from files by first reading them into a "mem" type BIO,
  then reading them from that BIO.

* Take care not to call ddsrt_free on OpenSSL-allocated memory, either
  by calling OPENSSL_free, or by allocating the memory using
  ddsrt_malloc and letting OpenSSL fill that buffer.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-26 11:47:56 +02:00
Jeroen Koekkoek
ad2fa331bd Remove perl and trang dependencies from package.xml
Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
2020-06-25 14:10:11 +02:00
Jeroen Koekkoek
b25f10ff33 Add ddsconf to generate md, rnc and xsd configuration documentation
Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
2020-06-25 14:10:11 +02:00
Jeroen Koekkoek
9c04099937 Enable CXX for CMake versions less than 3.12
This fixes #546. Thanks @robinlinden.

Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
2020-06-19 13:21:10 +02:00
Mikael Arguedas
768b92774f move options above where they're used to generate features header
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
2020-06-18 12:55:36 +02:00
Robin Lindén
8029274fec Allow building without a C++ compiler
Signed-off-by: Robin Lindén <_@robinlinden.eu>
2020-06-12 16:00:19 +02:00
Marcel Jordense
c67f8bc73c Remove from ddsrt dynlib and filesystem when building FreeRTOS
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-06-11 18:04:30 +02:00
Erik Boasson
f08b4e0c5d Remove some dead stores signalled by clang analyzer
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-08 15:31:32 +02:00
Erik Boasson
ece3a78211 Help clang static analyzer in security tests
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-08 15:31:32 +02:00
Erik Boasson
8a748e1868 Fix incorrect cast of size_t* to uint32_t*
CID 304509 - it does not affect behaviour because the called function
uses it as an out parameter and the result is never inspected.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-08 15:31:32 +02:00
Erik Boasson
3705744681 Fix va_end arg in formatting security exception
CID 304494

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-08 15:31:32 +02:00
Erik Boasson
ca79eeb7b9 Fix error handling in generating test signatures
Failure to generate a signature for in handshake tests attempted to free
the address of the pointer, instead of the pointed-to memory (CID
304462).

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-08 15:31:32 +02:00
Erik Boasson
5588edb33e Move auth plug-in invalid parameter returns forward
Triggered by CID 304462, 304471, 304517: dereference before null check.
Note that it is a second-order problem because it would require the
plug-in functions to be called with a null pointer for the plug-in
instance.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-08 15:31:32 +02:00
Erik Boasson
84abf48d4a Perform type conversion for transient-local data
The changes in d92d491b83 to deal with
local readers and writers with the same topic and type name but
different underlying `struct ddsi_sertopic`s did not include the
provisioning of historical data from a (local) transient-local writer to
a (local) transient-local reader.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-06 13:52:01 +02:00
Marcel Jordense
efefb5009f Remove ssm argument when ssm not available
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-06-06 13:34:12 +02:00
Erik Boasson
9c31f48bea Build OpenSSL dependent parts only if present
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 09:18:46 +02:00
Erik Boasson
6161f5c44d Add some documentation to plist/xqos functions
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 08:51:58 +02:00
Erik Boasson
2579256535 Drop unused parameter from ddsi_plist_quickscan
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 08:51:58 +02:00
Erik Boasson
99d1364145 Improve chance of catching uninitialized plist/qos
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 08:51:58 +02:00
Erik Boasson
4cc018cfd7 Set durability_service on SPDP writer QoS
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 08:51:58 +02:00
Erik Boasson
8ae005a05a plist/xqos fini early out if nothing to be done
All incoming samples end up in ddsi_plist_fini, usually one with nothing
present, sometimes one containing status info or a keyhash.  The
"present" flags allow this to be a very quick operation in these simple
cases, and this should be made use of.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 08:51:58 +02:00
Erik Boasson
ca8742bfba Drop macOS 10.12 build from Travis CI cron job
The macOS 10.12 build was put in because of ROS2 "Dashing" specified
10.12 as the supported version, but Eloquent and later specify
10.14. The relevance of this is no longer there because of Foxy. The
build itself took an inordinate amount of time with lots of warnings
about the platform being deprecated.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-06-05 08:47:50 +02:00
eboasson
2287d7cd06
Merge pull request #491 from prasannabhat/feature/docker
Support docker build for cyclonedds
2020-06-05 08:47:24 +02:00
Prasanna Bhat (RBEI/EBB)
54c15c2b7e Add copyright headers
Signed-off-by: Prasanna Bhat (RBEI/EBB) <prasanna.bhat@in.bosch.com>
2020-06-05 07:52:03 +05:30
eboasson
3030a81528
Merge pull request #527 from eclipse-cyclonedds/security
Merge security into master
2020-06-04 17:47:28 +02:00