Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[15482] UDPv6 support for fast-discovery-server tool and ROS_DISCOVERY_SERVER #2948

Merged
merged 15 commits into from
Sep 22, 2022
Merged

[15482] UDPv6 support for fast-discovery-server tool and ROS_DISCOVERY_SERVER #2948

merged 15 commits into from
Sep 22, 2022

Conversation

MiguelBarro
Copy link
Contributor

@MiguelBarro MiguelBarro commented Sep 10, 2022 •
edited
Loading

Description

ROS_DISCOVERY_SERVER

  • Now supports IPv6 addresses for UDP.
  • The DNS names can be decorated to specify which address family to use if IPv4 and IPv6 addresses are available.

fast-discovery-server

  • Now IPv6 addresses can be specified at the cli.
  • DNS resolution supports IPv6 but available IPv4 addresses will take precedence for backward compatibility.

Testing has been updated to take into account IPv6 addresses.

Documentation eProsima/Fast-DDS-docs#401

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added.
  • Any new/modified methods have been properly documented using Doxygen.
  • Fast DDS test suite has been run locally.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • Documentation builds and tests pass locally.
  • New feature has been added to the versions.md file (if applicable).
  • New feature has been documented/Current behavior is correctly described in the documentation.

Reviewer Checklist

  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@MiguelBarro MiguelBarro self-assigned this Sep 10, 2022
Miguel Barro added 10 commits September 10, 2022 16:43
Refs 15482: disabling dns testing if ci setup not available
f5f9056 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482: load_environment_server_info() update
98ddcb4 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482: fixes for the ipv4 testing
31b9d83 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Adding IPv4 and DNS testing.
69acd6b 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Upgrading DNS syntax to choose a specific kind.
ef6a372 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Include UDPv6 among builitin transports if required by th…
66bc902 
…e environment variable

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Updated participant creation.
d5317e4 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Customary piggyback: fixing inclusion issues on optionpar…
b8a4d2d 
…ser.h

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. fast-discovery-server update to admit IPv6
4db044f 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Introduce fast-discovery-server test for IPv6
13897a6 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro MiguelBarro force-pushed the feature/ros_env_var/ipv6 branch from dfa8035 to 23aba1e Compare September 10, 2022 14:45
Miguel Barro added 2 commits September 10, 2022 16:57
Refs 15482. Linter pass.
3e79a48 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 15482. Increase fast-discovery-server version.
ffddee6 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro MiguelBarro force-pushed the feature/ros_env_var/ipv6 branch from 23aba1e to ffddee6 Compare September 10, 2022 14:59
@MiguelBarro
Copy link
Contributor Author

@richiprosima Please test this for me 🤯

Refs 15482. Solving gcc warnings (which for once were useful)
a1a996c 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro
Copy link
Contributor Author

@richiprosima Please test this for me 🤯

@MiguelBarro MiguelBarro mentioned this pull request Sep 12, 2022
Merged
@MiguelBarro MiguelBarro marked this pull request as ready for review September 12, 2022 20:27
Refs 15482. Fixing possible nslooukp absence on ci
e5bc2dc 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro
Copy link
Contributor Author

@richiprosima Please, I'm iterating, test linux 🙄

jsan-rt
jsan-rt suggested changes Sep 19, 2022
View reviewed changes
src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp Show resolved Hide resolved
test/unittest/dds/participant/ParticipantTests.cpp Outdated
@@ -691,7 +699,7 @@ void set_server_qos(

void set_environment_variable()
{
std::string environment_servers = "84.22.253.128:8888;;localhost:1234";
std::string environment_servers = "84.22.253.128:8888;;UDPv4:[localhost]:1234;[2a02:ec80:600:ed1a::3]:8783";
Copy link
Contributor

@jsan-rt jsan-rt Sep 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I would add a check that showed that using a single DNS entry both an IPv4 locator and an IPv6 locator are added, along with the required UDPv6 transport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And here it is: ParticipantTests.SimpleParticipantRemoteServerListConfigurationDNS

Refs 15482. Add extra DNS test hinted by reviewer
31b49e2 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro
Copy link
Contributor Author

@richiprosima Please test this for me 🤯

@MiguelBarro MiguelBarro requested a review from jsan-rt September 21, 2022 13:29
jsan-rt
jsan-rt approved these changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@jsan-rt jsan-rt left a comment

Choose a reason for hiding this comment

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

LGTM

@MiguelCompany MiguelCompany merged commit 02c410c into master Sep 22, 2022
@MiguelCompany MiguelCompany deleted the feature/ros_env_var/ipv6 branch September 22, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsan-rt jsan-rt jsan-rt approved these changes

Assignees

@MiguelBarro MiguelBarro

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MiguelBarro @jsan-rt @MiguelCompany