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

switch off udp receiving of other hosts if network_mode=local #1231

Closed
wants to merge 7 commits into from
Closed

switch off udp receiving of other hosts if network_mode=local #1231

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
90c5566
switch off udp receiving of other hosts if network_mode=local
rex-schilasky Oct 26, 2023
3bd45ad
join multicast group "localhost" in case of network_mode disabled
rex-schilasky Oct 27, 2023
2039a74
introduce localhost mode in udp snd/rec classes
rex-schilasky Oct 27, 2023
4b3036c
force ttl parameter on udp sender side to 0 in local network mode
rex-schilasky Oct 31, 2023
be9439f
better naming and commenting
rex-schilasky Oct 31, 2023
287dfa0
clang and code style
rex-schilasky Oct 31, 2023
79cce5d
redesign to better configure local and none local udp communication
rex-schilasky Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ecal/core/src/ecal_log_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ namespace eCAL
attr.ttl = Config::GetUdpMulticastTtl();
attr.sndbuf = Config::GetUdpMulticastSndBufSizeBytes();

// for local only communication we force the ttl (udp package 'hop limit') to 0
if (!Config::IsNetworkEnabled())
{
attr.ttl = 0;
}

m_udp_sender = std::make_unique<CUDPSender>(attr);
}

Expand Down
6 changes: 6 additions & 0 deletions ecal/core/src/ecal_registration_provider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ namespace eCAL
attr.loopback = true;
attr.sndbuf = Config::GetUdpMulticastSndBufSizeBytes();

// for local only communication we force the ttl (udp package 'hop limit') to 0
if (!Config::IsNetworkEnabled())
{
attr.ttl = 0;
}

// create udp sample sender
m_reg_sample_snd = std::make_shared<CSampleSender>(attr);
}
Expand Down
21 changes: 6 additions & 15 deletions ecal/core/src/io/ecal_receiver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,13 @@ namespace eCAL
////////////////////////////////////////////////////////
struct SReceiverAttr
{
SReceiverAttr() :
port(0),
broadcast(false),
unicast(false),
localhost(false),
loopback(true),
rcvbuf(1024 * 1024)
{};

std::string ipaddr;
int port;
bool broadcast;
bool unicast;
bool localhost;
bool loopback;
int rcvbuf;
bool localhost = false;
int port = 0;
bool broadcast = false;
bool unicast = false;
bool loopback = true;
int rcvbuf = 1024 * 1024;
};

class CReceiver
Expand Down
75 changes: 61 additions & 14 deletions ecal/core/src/io/udp_receiver_asio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,40 @@
#include "linux/ecal_socket_option_linux.h"
#endif

namespace
{
bool isLocalAddress(const asio::ip::address& address)
{
// Check if the address is a loopback address.
if (address.is_loopback())
{
return true;
}

// Check if the address is in the private IP address ranges.
// For IPv4, the private address ranges are 10.0.0.0/8, 172.16.0.0/12, and 192.168.0.0/16.
if (address.is_v4())
{
uint32_t ip = address.to_v4().to_ulong();
return ((ip & 0xFF000000) == 0x0A000000) || // 10.0.0.0/8
Copy link
Member

Choose a reason for hiding this comment

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

I am puzzled why you would care about those. Why not just use a loopback address (127.0.0.0 - 127.255.255.255 https://de.wikipedia.org/wiki/Loopback)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my first approach. On my local system, however, I see packets that have my own IPv4 address as the sender 10.x.x.x instead of "127.x.x.x". I need to investigate this further - IT's A DRAFT :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good news is that with this draft fix all gtest (including local UDP Pub/Sub) and all samples are running correctly with and without local mode setting and with and without activated npcap mode.

Copy link
Member

Choose a reason for hiding this comment

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

ok, but then you are probably using DNS to resolve your hostname. Just skip DNS and just send to 127.0.0.1 directly. Why use DNS, if you already know the IP.

Btw, there is no guarantee that DNS will resolve your hostname to a local IP. It could also be a public IP, if you have one. I guess you rather want to check whether the resolved IP address is a multicast or a unicast address. But still I think you don't need DNS at all to send to a loopback adapter.

((ip & 0xFFF00000) == 0xAC100000) || // 172.16.0.0/12
((ip & 0xFFFF0000) == 0xC0A80000); // 192.168.0.0/16
}

#if 0 // not used currently
// Check if the address is in the link-local range for IPv6.
// Link-local IPv6 addresses typically start with fe80::/10.
if (address.is_v6())
{
const auto& bytes = address.to_v6().to_bytes();
return bytes[0] == 0xFE && (bytes[1] & 0xC0) == 0x80;
}
#endif

return false;
}
}

namespace eCAL
{
////////////////////////////////////////////////////////
Expand All @@ -32,9 +66,9 @@ namespace eCAL
CUDPReceiverAsio::CUDPReceiverAsio(const SReceiverAttr& attr_) :
CUDPReceiverBase(attr_),
m_created(false),
m_localhost(attr_.localhost),
m_broadcast(attr_.broadcast),
m_unicast(attr_.unicast),
m_localhost(attr_.localhost),
m_socket(m_iocontext)
{
if (m_broadcast && m_unicast)
Expand Down Expand Up @@ -102,14 +136,7 @@ namespace eCAL
}

// join multicast group
if (m_localhost)
{
AddMultiCastGroup("127.0.0.1");
}
else
{
AddMultiCastGroup(attr_.ipaddr.c_str());
}
AddMultiCastGroup(attr_.ipaddr.c_str());

// state successful creation
m_created = true;
Expand Down Expand Up @@ -196,23 +223,43 @@ namespace eCAL
// run for timeout ms
RunIOContext(asio::chrono::milliseconds(timeout_));

// retrieve underlaying raw socket informations
// did we receive anything ?
if (reclen == 0)
{
return (0);
}

// is the caller interested in the source address ?
if (address_)
{
// retrieve underlaying raw socket informations
if (m_sender_endpoint.address().is_v4())
{
asio::detail::sockaddr_in4_type* in4 = reinterpret_cast<asio::detail::sockaddr_in4_type*>(m_sender_endpoint.data());
address_->sin_addr = in4->sin_addr;
address_->sin_family = in4->sin_family;
address_->sin_port = in4->sin_port;
memset(&(address_->sin_zero), 0, 8);
if (address_)
{
address_->sin_addr = in4->sin_addr;
address_->sin_family = in4->sin_family;
address_->sin_port = in4->sin_port;
memset(&(address_->sin_zero), 0, 8);
}
}
else
{
std::cout << "CUDPReceiverAsio: ipv4 address conversion failed." << std::endl;
}
}

// are we running in 'local host only' receiving mode ?
if (m_localhost)
{
// if this is not a local address, return 0
if (!isLocalAddress(m_sender_endpoint.address()))
Copy link
Member

Choose a reason for hiding this comment

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

Your definition of a "Local address" has nothing to do with a loopback address (-> localhost)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I didn't name that unambiguously. What is meant is that the network packets come from your own machine.

Copy link
Member

Choose a reason for hiding this comment

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

But your isLocalAddress doesn't check if that IP belongs to your own machine. It checks whether the IP belongs to any machine in any local area network.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I got it .. yes that is true. So this approach will not work at all I think.

{
return (0);
}
}

return (reclen);
}

Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/io/udp_receiver_asio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ namespace eCAL
void RunIOContext(const asio::chrono::steady_clock::duration& timeout);

bool m_created;
bool m_localhost;
bool m_broadcast;
bool m_unicast;
bool m_localhost;
asio::io_context m_iocontext;
asio::ip::udp::socket m_socket;
asio::ip::udp::endpoint m_sender_endpoint;
Expand Down
71 changes: 53 additions & 18 deletions ecal/core/src/io/udp_receiver_npcap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@
*/
#include <io/udp_receiver_npcap.h>

namespace
{
bool isLocalAddress(Udpcap::HostAddress& address)
{
// Check if the address is a loopback address.
if (address.isLoopback())
{
return true;
}

// Check if the address is in the private IP address ranges.
// For IPv4, the private address ranges are 10.0.0.0/8, 172.16.0.0/12, and 192.168.0.0/16.
if (address.isValid())
{
uint32_t ip = address.toInt();
return ((ip & 0xFF000000) == 0x0A000000) || // 10.0.0.0/8
((ip & 0xFFF00000) == 0xAC100000) || // 172.16.0.0/12
((ip & 0xFFFF0000) == 0xC0A80000); // 192.168.0.0/16
}

return false;
}
}

namespace eCAL
{
////////////////////////////////////////////////////////
Expand All @@ -26,8 +50,8 @@ namespace eCAL
CUDPReceiverPcap::CUDPReceiverPcap(const SReceiverAttr& attr_)
: CUDPReceiverBase(attr_)
, m_created(false)
, m_unicast(attr_.unicast)
, m_localhost(attr_.localhost)
, m_unicast(attr_.unicast)
{
// set receive buffer size (default = 1 MB)
int rcvbuf = 1024 * 1024;
Expand All @@ -54,14 +78,7 @@ namespace eCAL
}

// join multicast group
if (m_localhost)
{
AddMultiCastGroup("127.0.0.1");
}
else
{
AddMultiCastGroup(attr_.ipaddr.c_str());
}
AddMultiCastGroup(attr_.ipaddr.c_str());

// state successful creation
m_created = true;
Expand Down Expand Up @@ -99,25 +116,43 @@ namespace eCAL
{
if (!m_created) return 0;

size_t bytes_received;
if (address_)
// receive datagram
Udpcap::HostAddress source_address;
uint16_t source_port;
size_t bytes_received = m_socket.receiveDatagram(buf_, len_, static_cast<unsigned long>(timeout_), &source_address, &source_port);

// did we receive anything ?
if (bytes_received == 0)
{
Udpcap::HostAddress source_address;
uint16_t source_port;
bytes_received = m_socket.receiveDatagram(buf_, len_, static_cast<unsigned long>(timeout_), &source_address, &source_port);
return 0;
}

if (bytes_received && source_address.isValid())
// is the caller interested in the source address ?
if (address_)
{
if (source_address.isValid())
{
address_->sin_addr.s_addr = source_address.toInt();
address_->sin_family = AF_INET;
address_->sin_port = source_port;
address_->sin_port = source_port;
memset(&(address_->sin_zero), 0, 8);
}
else
{
std::cout << "CUDPReceiverPcap: source address conversion failed." << std::endl;
}
}
else

// are we running in 'local host only' receiving mode ?
if (m_localhost)
{
bytes_received = m_socket.receiveDatagram(buf_, len_, static_cast<unsigned long>(timeout_));
// if this is not a local address, return 0
if (!isLocalAddress(source_address))
{
return 0;
}
}

return bytes_received;
}
}
2 changes: 1 addition & 1 deletion ecal/core/src/io/udp_receiver_npcap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ namespace eCAL

protected:
bool m_created;
bool m_unicast;
bool m_localhost;
bool m_unicast;
Udpcap::UdpcapSocket m_socket;
};

Expand Down
Loading