Skip to content

Proposal: Refactor ICE

Jump to bottom
Justin Wilson edited this page Nov 17, 2023 · 1 revision

Goal

The goal of this proposal is to refactor the current ICE implementation using Internal DDS Topics.

Value Proposition

The RtpsRelay, ICE, and security make OpenDDS a candidate for DDS applications that operate on the Internet. Existing experiences with ICE show that it causes performance problems for the RtpsRelay, has bugs, and is very difficult to test. Refactoring ICE using Internal DDS Topics will

  • Make OpenDDS better suited for Internet-wide applications.
  • Reduce the long-term cost of maintenance due to better concurrency semantics.
  • Improve the ability to test the ICE implementation due to better modularity.
  • Provide another data point for Internal DDS Topics before incorporating it into more critical parts of the code.

Objectives

Exchanging Agent and Endpoint Info

Per the ICE specification, each OpenDDS process contains a single ICE agent. This agent has general information that must be sent to other agents to execute the ICE protocol. An endpoint corresponds to a UDP socket, e.g., SPDP, SEDP, user RTPS. ICE agents must also exchange information about the endpoints including the local address and the server reflexive address.

  • Define internal DDS Topics
    • The ICE Agent owns these topics
    • Define a new internal topic for local ICE Agent info (This corresponds to IceGeneral_t).
    • Define a new internal topic for remote ICE Agent info
    • Define a new internal topic for local endpoint info (This is a collection of IceCandidate_t)
    • Define a new internal topic for remote endpoint info
  • Write local agent and local endpoint info
    • Make the ICE Agent write the internal local topics
    • Define a new topic type in IDL for ICE
      • This will be a union
      • One branch will be for agent info
        • Agent_type
        • Username
        • Password
      • The other branch will be for endpoint info
        • key (uniquely identified endpoint)
        • candidates (sequence)
          • Locator
          • Foundation
          • Priority
          • Type
        • entities (sequence of EntityId)
          • interpretation is: these are the entitites that are available at this endpoint.
    • Define a new builtin writer for ICE
      • This is a secure writer
    • Transduce the internal DDS topic for Agent Info and endpoint info into the builtin writer
  • Read remote agent and endpoint info
    • Define a new builtin reader for ICE
      • This is a secure reader
    • Transduce the remote agent info and endpoint info into the internal topics
      • When transducing, the keys need to be unique so that remotes cannot stomp on each other, e.g., prefix with the guid prefix of the remote
    • Update the agent to read the internal topics and cache it
  • Add flag to indicate ICE SEDP endpoints are available (see PID_OPENDDS_RTPS_RELAY_APPLICATION_PARTICIPANT).
  • Cleanup, Integration, and Testing
    • Remove the ICE related PIDs and processing from RtpsCore.idl
    • Remove the AgentInfoListener interface from Ice.h
      • Implementations of AgentInfoListener must switch to the internal topics.
    • Remove the get_local_agent_info method.
    • Remove the AgentInfo parameter from the start_ice method.
      • The ICE agent will use the internal topics to retrieve the necessary information.
    • Update the ParticipantLocationTopic test to have 4 Internal DataReaders for the 4 ICE topics with listeners.
      • The listeners should log the samples.
      • The test should be updated to check that at least one sample is received for each topic. If a tighter check is possible, then use a tighter check.

Starting and stopping ICE

ICE can be started when a local and remote entity match assuming that ICE information is available for the remote entity. Similarly, ICE is stopped when one of the entities goes away or the entities don’t match anymore. The entities can be builtins or for the user. Sedp.cpp and Spdp.cpp have a fair bit of code dedicated to starting and stopping ICE. To simplify this code:

  • Define a new Internal DDS Topic owned by the service participant that describes all of the matched endpoints.
  • Each discovery instance or participant discovery instance will create an Internal DataWriter and write matches to the topic.
    • This only needs to be implemented for RTPS. InfoRepo and StaticDiscovery can be implemented later if necessary.
  • Update the ICE Agent to read the new topic and start ICE if possible.
    • This is a merge problem where both the Agend and Endpoint info need to be considered with the matches. If all of the data is available, start ice. If some of the data is missing, stop ice.
  • Cleanup, Integration, and Testing
    • Remove Agent::start_ice and Agent::stop_ice and all supporting code in the callers.
    • In the ParticipantLocation test, add an internal DDS DataReader for the match topic and check that appropriate samples are received.

Local endpoint info

ICE endpoints provide a list of host addresses and a STUN server address through the Endpoint interface. The ICE Agent uses the STUN server address to determine the server reflexive address (SRA) for the endpoint. This is technically correct because the ICE spec says that all ICE transactions should be governed by the Agent to avoid sending to many messages per second. However, the implementation of the RtpsRelay has evolved and STUN connectivity checks are used to maintain connectivity. As a byproduct, these connectivity checks determine the SRA. In cases where the RtpsRelay is used, the SRA is effectively determined twice.

  • Update Endpoint implementations to write their endpoint info to the internal DDS Topic for local endpoint info.
    • This includes local addresses and the SRA.
    • Changes in local address via the NetworkConfigMonitor should result in a new sample.
    • Changes in the SRA should result in a new sample.
      • At this juncture, we assume that the RtpsRelay will be used to determine the SRA. Support for an external STUN server is not required but could be added in the future.
  • Remove SRA related code in the ICE Agent.
  • Make the ICE Agent read the local endpoint info topic instead of reading it.
  • Cleanup and Testing
    • Remove
      • Endpoint::host_addresses
      • Endpoint::stun_server_address
      • Agent::add_endpoint
      • Agent::remove_endpoint
    • The ParticipantLocation test should be testing the required functionality.

Notifications of connect and disconnect

When ICE succeeds or fails, the ICE Agent calls back into the Endpoint to signal the change in state. These callbacks create concurrency problems.

  • Define a new internal DDS Topic for communicating the state of ICE
    • This topic owned and written by the ICE Agent.
    • Two key structures come to mind
      • Option 1: The key consists of an identifier for the local endpoint and an identifier for the remote endpoint.
        • The value consists of
          • A status: success or failure.
            • Other states will be considered if they seem useful.
          • The address determined by ICE.
          • Information like the local and remote entities of the endpoint may be necessary.
      • Option 2: The key consists of a local-remote guid pair.
        • The value consists of
          • A status: success or failure.
            • Other states will be considered if they seem useful.
          • The address determined by ICE.
          • The local endpoint
          • An identifier for the remote endpoint
      • Studying the usage will determine which design is correct or closer to correct.
    • Endpoints that override ice_connect and ice_disconnect should read the status from this topic.
    • Readers of the topic will only be interested in a subset of the samples, i.e., those with a specific endpoint. In this phase, implement content filtering in the Internal DDS DataReader.
  • Cleanup and Testing
    • Remove Endpoint::ice_connect and Endpoint::ice_disconnect.
    • Remove Agent::get_address.
      • Callers of this method should read the topic and cache the addresses determined by ICE.
    • Update the ParticipantLocation test to log and check the new topic.

Connectivity Checking: Receive

At this point, the remaining methods to deal with are Agent::receive and Endpoint::send. This objective is concerned with removing Agent::receive.

  • Create an Internal DDS Topic for receiving STUN messages.
    • This topic is owned and read by the ICE Agent.
    • This topic is written by callers of Agent::receive.
  • Cleanup and Testing
    • Remove Agent::receive
    • Update the ParticipantLocation test to log and check for received STUN messages.

Connectivity Checking: Send

The only method left in Endpoint is send. This method is used by the ICE Agent when it is necessary to send a STUN message for connectivity checking.

  • Replace references to Endpoints (or RcHandles of Endpoints) with an opaque unique identifier, e.g., this cast to const void*, in Internal DDS Topic types.
  • Create a new internal DDS topic for sending STUN messages.
    • The topic is owned and written by the ICE Agent.
    • The topic is read by all Endpoint implementers.
    • The topic must include the Endpoint identifier.
    • Endpoints should use content filtering to only read STUN messages that they are responsible for sending.
    • This suggests another use of content filter.
      • This time, push back the content filtering into the Internal DataWriter so that the filtering is more efficient.
      • Rewrite existing usages of the content filtering to take advantage of this.
  • Cleanup and Testing
    • Remove the Endpoint interface.
    • Update the ParticipantLocation test to log and check for sent STUN messages.

Configuration

  • Update the ICE agent to use the ConfigStore of the ServiceParticipant.
  • Cache configuration values that are used frequently or can be updated dynamically.
  • Move ICE configuration from RtpsDiscovery to its own section in the config file.
  • Update the documentation.

Unit Testing

The internal DDS Topics Developed for ICE create an isolated service that is. We can test the ICE Agent by reading and writing the internal topics that it uses. The purpose of this object is to ensure that unit testing ICE is feasible. Complete coverage is not expected, rather, the goal is to verify the assumption that internal DDS Topics may unit testing reactive code in OpenDDS easier.

  • ICE uses an internal timer to make progress through checklists. Externalize this to use a TimeSource so that the test can gain control over the passage of time.
  • Write a unit test for ICE that simulates a happy path scenario.
    • Assume the existing of a local and remote entity.
    • The simulated local endpoint will have a local address and SRA.
    • The simulated remote endpoint will have a local address and SRA.
    • Messages sent to local addresses should be dropped to simulate the case where the endpoints are behind different firewalls.
    • Messages sent to remote endpoints should illicit responses that cause ICE to succeed.
    • Fabricating the necessary responses “by hand” may be tedious. Another idea is for the test to create two ICE agents and connect them so messages sent from one are received by the other.
    • Check that ICE succeeds for the two simulated entities and endpoints.
    • Except for randomly generated numbers, the test should be deterministic.

AgentImpl Cleanup

  • Can dds/DCPS/Ice.h be removed or moved?
  • AgentImpl implements DCPS::ShutdownListener, DCPS::InternalDataReaderListenerDCPS::NetworkInterfaceAddress, and DCPS::ReactorInterceptor. Are all of these needed?
Clone this wiki locally