Skip to content
This repository was archived by the owner on Dec 20, 2023. It is now read-only.

Weave over UDP changes #393

Merged
merged 8 commits into from
Nov 10, 2019
Merged

Weave over UDP changes #393

merged 8 commits into from
Nov 10, 2019

Conversation

jaylogue
Copy link
Contributor

This is a series of commits related to the handling of UDP send and receive in the Weave Message and Exchange Layers. The goals of these changes are:

  • to simplify the code, in particular the code in SendMessage() and RefreshEndpoints()
  • add support for new UDP send features
  • correct bugs related to source address selection, multicast handling and others
  • improve testing

The individual commits each have detailed commit messages, however the highlights are reproduced here:


-- Rewrote the Message Layer’s SendMessage() and RefreshEndpoints() methods to simplify the code and support new features. As part of this, a number of global tables were eliminated, leading to an overall reduction in RAM use.

-- Added support for ephemeral UDP source ports. Client applications can use this feature to support multiple simultaneous Weave initiators running in different processes.

-- Enhanced the SendMessage() method such that messages sent to the IPv4 broadcast address (255.255.255.255) are automatically sent to all interfaces. This follows similar behavior for IPv6 multicast messages.

-- Added support in the Weave Message Layer for an additional listening UDP endpoint bound to the IPv4 broadcast address (255.255.255.255). This endpoint is initialized whenever the Message Layer is bound to a specific IPv4 address, and allows the node to receive broadcast messages when bound in this way.

-- Altered the logic in RefreshEndpoints() that decides when re-initialize UDP endpoints. Specifically, already listening UDP endpoints are only re-initialized if the message layer receives specific errors when sending UDP messages indicating the underlying socket/PCB has failed.

-- Implemented a set of basic functionality tests for sending and receiving Weave messages over UDP. Test cases cover:

  • IPv4 and IPv6
  • Unicast, IPv6 multicast and IPv4 broadcast
  • Sending to/from IPv6 ULA and link-local addresses
  • Sending over specific network interfaces
  • Sending and receiving while bound to specific local addresses (IPv4 and IPv6)

@jaylogue
Copy link
Contributor Author

This PR addresses the issues described in 367 and 368.

Copy link
Contributor

@gerickson gerickson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments; however, overall, looks thorough and exhaustive.

@codecov-io
Copy link

codecov-io commented Oct 23, 2019

Codecov Report

Merging #393 into master will decrease coverage by 0.01%.
The diff coverage is 2.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
- Coverage   54.47%   54.46%   -0.02%     
==========================================
  Files         340      340              
  Lines       58425    58448      +23     
==========================================
+ Hits        31826    31831       +5     
- Misses      26599    26617      +18
Impacted Files Coverage Δ
src/lib/core/WeaveMessageLayer.h 28.57% <ø> (-11.43%) ⬇️
src/test-apps/ToolCommonOptions.h 100% <ø> (ø) ⬆️
src/inet/IPAddress.cpp 80.4% <0%> (-3.37%) ⬇️
src/lib/core/WeaveFabricState.h 27.27% <0%> (ø) ⬆️
src/lib/core/WeaveMessageLayer.cpp 17.4% <0%> (+0.38%) ⬆️
src/lib/core/WeaveExchangeMgr.cpp 6.63% <0%> (-0.06%) ⬇️
src/lib/core/ExchangeContext.cpp 0% <0%> (ø) ⬆️
src/lib/core/WeaveFabricState.cpp 48.4% <0%> (-0.42%) ⬇️
src/test-apps/ToolCommon.cpp 14.73% <0%> (-0.03%) ⬇️
src/inet/UDPEndPoint.cpp 41.5% <10%> (-1.66%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8ad459...54baa9b. Read the comment docs.

@jaylogue jaylogue force-pushed the feature/udp-rework branch 2 times, most recently from dfa8915 to 3de6d3a Compare November 8, 2019 15:04
Jay Logue added 8 commits November 9, 2019 06:27
-- Added the IsIPv6() and MakeIPv4Broadcast() methods to the IPAddress class.
-- Fixed logic bug in IPEndPointBasis::GetSocket() that caused spurious error messages
whenever an IPv4 UDP endpoint was initialized.

-- Added UDPEndPoint::GetBoundPort() method.  This method returns the port number to which
the UDPEndPoint has been bound.  For endpoints that are not bound to a specific port, this
returns the system-assigned ephemeral port.
-- Fixed a subtle bug in Weave Exchange Manager where UDP response messages would be sent
from the wrong source address when the Message Layer was bound to a particular local IP
address.  This was caused by the exchange code always specifying an interface id when
sending a UDP message. Because of the way sockets work, this causes the Linux stack to
ignore the bound address of the socket for the purposes of source address selection,
resulting in the response message coming from the wrong source address in some contexts.

The fix is to only capture the source interface when the sender's address is an IPv6 link
local.
-- Rewrote the Message Layer’s SendMessage() and RefreshEndpoints() methods to simplify the
code, add support for new UDP send features, and correct bugs related to source address
selection and multicast handling.

-- Added support for initiating Weave exchanges from an ephemeral UDP source port.  Client
applications can use this feature to support multiple simultaneous Weave initiators running
in different processes.  As part of this change, a new ExchangeContext flag was added,
UseEphemeralUDPPort, to track whether messages sent on the exchange should be sent from the
local ephemeral source port.  This flag is set true by default when the ephemeral UDP source
port feature is enabled.

-- Altered the logic in RefreshEndpoints() that decides when to re-initialize UDP endpoints.
In the new code, already listening UDP endpoints are only re-initialized if the message
layer receives specific send errors indicating that the underlying socket/PCB has failed.
Previously the code always re-initialized UDP endpoints when RefreshEndpoints() was called,
however this behavior was incompatible with support for ephemeral source ports.

-- Enhanced the SendMessage() method such that a message sent to the IPv4 broadcast address
(255.255.255.255) is automatically sent to all interfaces.  This follows similar behavior
for IPv6 multicast messages.

-- Added support in the Weave Message Layer for an additional listening UDP endpoint bound
to the IPv4 broadcast address (255.255.255.255).  This endpoint is initialized whenever the
Message Layer is bound to a specific IPv4 address, and allows nodes in this state to receive
broadcast messages.

-- Eliminated the Message Layer’s interfaces table.  The code now enumerates the system’s
interfaces as needed when sending UDP messages.

-- Eliminated the table of UDPEndPoints bound to specific local addresses
(mIPv6UDPLocalAddr).  When sending a message from a specific source address, the Message
Layer now controls the source address via the IPPacketInfo.SrcAddress argument to
UDPEndPoint::SendMsg().

-- Renamed ‘MulticastFromLinkLocal’ flags to ‘DefaultMulticastSourceAddress’ to better
reflect the true behavior.  The old name is retained as an alias.

-- Simplified logging emitted while the Message Layer is initializing its listening
endpoints.
-- Revised logic in WeaveDeviceManager for handling the ‘rendezvous link-local’ feature
based on changes to UDP message handling in the Weave Message Layer.  The code now uses the
new DefaultMulticastSourceAddress send flag to instruct the Message Layer to send multicast
Identify messages using the system default source address selection algorithm.  For messages
sent to the IPv6 all-nodes, link-local address (FF02::1) this causes the source address to
be the node's link-local address.

As part of this change, the ‘rendezvous link-local’ logic no longer alters its behavior
based on the whether the local node is bound to a specific IPv6 address, as the Message
Layer now handles this case automatically.
-- Fixed a bug in the old WeaveEchoClient object that preventing sending Echo messages to a
specific remote port.
-- Added a command line option to all ToolCommon-based test applications to enable the use
of ephemeral UDP source ports when initiating outbound Weave exchanges.  This option is
only available when the WEAVE_CONFIG_ENABLE_EPHEMERAL_UDP_PORT build config is enabled.
-- Implemented a set of basic functionality tests for sending and receiving Weave messages
over UDP.  Test cases cover:

. IPv4 and IPv6
. Unicast, IPv6 multicast and IPv4 broadcast
. Sending to/from IPv6 ULA and link-local addresses
. Sending over specific network interfaces
. Sending and receiving while bound to specific local addresses (IPv4 and IPv6)
@jaylogue jaylogue merged commit 9238c86 into master Nov 10, 2019
@jaylogue jaylogue deleted the feature/udp-rework branch November 10, 2019 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants