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.

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

Added extraction of IP_PKTINFO when reading and setting it when sending back #456

Merged
merged 42 commits into from Dec 23, 2022
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
e825aa6
Added pktinfo retrieval and passing when any-bound socket.
Aug 20, 2018
6eea9f7
Fixed bug when setting data for sending. Added support for IPv6
Aug 21, 2018
5a1e508
Fixed usage of source address during handshake
Aug 21, 2018
95af2e3
Extra refactoring for improving call readiness
Aug 21, 2018
828b403
Some cosmetics
Aug 21, 2018
2ac3a99
Added recording the target address from HS packet if rendezvous
Aug 21, 2018
f57ca6f
Fixed build break on Mac. Centralized system headers. Cleaned up warn…
Sep 7, 2018
be0afb8
Merge branch 'upstream' into dev-pktinfo
Sep 7, 2018
76ad98a
Fixed build break on Windows. Added platform_sys as first header to e…
Sep 7, 2018
fec4909
Fixed new system includes for Mac
Sep 7, 2018
c1b4466
Upgraded to latest upstream
May 23, 2019
bcce38f
Fixed max space for CMSG buffer to handle runtime-only CMSG_SPACE on …
May 24, 2019
e2a1426
Fix: workaround for ref-referring to a static constant
May 24, 2019
bdd7ee2
Moved constant out of static const to avoid linker errors
May 24, 2019
ec349dc
Made pktinfo conditional and disabled on Windows
May 24, 2019
f37b21b
Made separate buffers for sending and receiving. They can be used by …
May 24, 2019
b8be09f
Added lacking initialization
Jun 19, 2019
58cd006
Upgraded from upstream
Jun 19, 2019
21e7740
Fixed conditional-syntax problem
Jun 19, 2019
af2870b
Merged with upstream
Sep 16, 2019
5d8dae1
Removed dead code
Sep 16, 2019
7d9eee0
Merge branch 'master' into dev-pktinfo
Sep 19, 2019
7f8a55b
Turning PKTINFO option default off. Fixed description
Sep 23, 2019
39f9565
Fixed unnecessary extra C++ includes in platform_sys
Oct 2, 2019
f1b2db8
Merge branch 'master' into dev-pktinfo
Oct 2, 2019
ba63b76
Added C++11 version of the against-value-to-function-cast error. Fixe…
Oct 22, 2019
b379292
Updated to latest upstream
Oct 22, 2019
c9cd6f6
Merge branch 'dev-fix-incompat-newcompiler' into dev-pktinfo
Oct 22, 2019
4742039
Fixed cmsg fit replacement structure to have last field last. This ca…
Oct 22, 2019
39c430a
Merge branch 'master' into dev-pktinfo
Oct 28, 2019
52f820d
Updated to latest upstream
Nov 5, 2019
f6ed44f
Post-review fixes
Nov 5, 2019
c25f2d7
Merge branch 'master' into dev-pktinfo
Nov 8, 2019
d44d96f
post-review fixes
Nov 12, 2019
cbae059
Merge branch 'master' into dev-pktinfo
Dec 5, 2019
3ac8c18
Merge branch 'master' into dev-pktinfo
Dec 9, 2019
660a126
Updated to latest upstream (untested)
Nov 30, 2022
b2c8e9a
Merge branch 'master' into dev-pktinfo
Dec 6, 2022
d5d84f3
Post-review fixes, phase 1
Dec 12, 2022
41ad2c0
Apply some suggestions from the doc code review, take 1
ethouris Dec 13, 2022
a087907
Applied some doc suggestions
ethouris Dec 19, 2022
a6f01ed
Fix for handling conditional-in-macro pp spec (Windows)
Dec 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions CMakeLists.txt
Expand Up @@ -117,6 +117,10 @@ if (ENABLE_DEBUG)
set(ENABLE_HEAVY_LOGGING_DEFAULT ON)
endif()

# Note: this can be decided to be turned ON by default,
# however this can't be implemented on Windows, so at least
# there it must be default OFF.
set (ENABLE_PKTINFO_DEFAULT OFF)
maxsharabayko marked this conversation as resolved.
Show resolved Hide resolved

set(ENABLE_STDCXX_SYNC_DEFAULT OFF)
set(ENABLE_MONOTONIC_CLOCK_DEFAULT OFF)
Expand Down Expand Up @@ -144,6 +148,7 @@ option(ENABLE_HEAVY_LOGGING "Should heavy debug logging be enabled" ${ENABLE_HEA
option(ENABLE_HAICRYPT_LOGGING "Should logging in haicrypt be enabled" 0)
option(ENABLE_SHARED "Should libsrt be built as a shared library" ON)
option(ENABLE_STATIC "Should libsrt be built as a static library" ON)
option(ENABLE_PKTINFO "Enable target IP address extraction on listner" ${ENABLE_PKTINFO_DEFAULT})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
option(ENABLE_PKTINFO "Enable target IP address extraction on listner" ${ENABLE_PKTINFO_DEFAULT})
option(ENABLE_PKTINFO "Enable using IP_PKTINFO to allow the listener extracting the target IP address from incoming packets" ${ENABLE_PKTINFO_DEFAULT})

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just use OFF here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We may decide in the future to turn it to ON on all supporting platforms, and this way it will be only OFF on platforms that do not support it.

Note also that this IS supported on Windows, just the procedure of sending and receiving UDP packets is using a function that doesn't allow the use of CMSG. To enable that it would have to be reimplemented.

option(ENABLE_RELATIVE_LIBPATH "Should application contain relative library paths, like ../lib" OFF)
option(ENABLE_GETNAMEINFO "In-logs sockaddr-to-string should do rev-dns" OFF)
option(ENABLE_UNITTESTS "Enable unit tests" OFF)
Expand Down Expand Up @@ -711,6 +716,15 @@ if (ENABLE_GETNAMEINFO)
list(APPEND SRT_EXTRA_CFLAGS "-DENABLE_GETNAMEINFO=1")
endif()

if (ENABLE_PKTINFO)
if (WIN32)
message(FATAL_ERROR "PKTINFO is not implemented on Windows.")
endif()

list(APPEND SRT_EXTRA_CFLAGS "-DSRT_ENABLE_PKTINFO=1")
endif()


# ENABLE_EXPERIMENTAL_BONDING is deprecated. Use ENABLE_BONDING. ENABLE_EXPERIMENTAL_BONDING is be removed in v1.6.0.
if (ENABLE_EXPERIMENTAL_BONDING)
message(DEPRECATION "ENABLE_EXPERIMENTAL_BONDING is deprecated. Please use ENABLE_BONDING instead.")
Expand Down
1 change: 1 addition & 0 deletions configure-data.tcl
Expand Up @@ -43,6 +43,7 @@ set cmake_options {
enable-logging "Should logging be enabled (default: ON)"
enable-heavy-logging "Should heavy debug logging be enabled (default: OFF)"
enable-haicrypt-logging "Should logging in haicrypt be enabled (default: OFF)"
enable-pktinfo "Should pktinfo reading and using be enabled (POSIX only) (default: OFF)"
enable-shared "Should libsrt be built as a shared library (default: ON)"
enable-static "Should libsrt be built as a static library (default: ON)"
enable-relative-libpath "Should applications contain relative library paths, like ../lib (default: OFF)"
Expand Down
103 changes: 98 additions & 5 deletions srtcore/channel.cpp
Expand Up @@ -139,7 +139,24 @@ static int set_cloexec(int fd, int set)

srt::CChannel::CChannel()
: m_iSocket(INVALID_SOCKET)
#ifdef SRT_ENABLE_PKTINFO
, m_bBindMasked(true)
#endif
{
#ifdef SRT_ENABLE_PKTINFO
// Do the check for ancillary data buffer size, kinda assertion
static const size_t CMSG_MAX_SPACE = sizeof (CMSGNodeAlike);

if (CMSG_MAX_SPACE < CMSG_SPACE(sizeof(in_pktinfo))
|| CMSG_MAX_SPACE < CMSG_SPACE(sizeof(in6_pktinfo)))
{
LOGC(kmlog.Fatal, log << "Size of CMSG_MAX_SPACE="
<< CMSG_MAX_SPACE << " too short for cmsg "
<< CMSG_SPACE(sizeof(in_pktinfo)) << ", "
<< CMSG_SPACE(sizeof(in6_pktinfo)) << " - PLEASE FIX");
throw CUDTException(MJ_SETUP, MN_NONE, 0);
}
#endif
}

srt::CChannel::~CChannel() {}
Expand Down Expand Up @@ -207,6 +224,9 @@ void srt::CChannel::open(const sockaddr_any& addr)
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);

m_BindAddr = addr;
#ifdef SRT_ENABLE_PKTINFO
m_bBindMasked = m_BindAddr.isany();
maxsharabayko marked this conversation as resolved.
Show resolved Hide resolved
#endif
LOGC(kmlog.Debug, log << "CHANNEL: Bound to local address: " << m_BindAddr.str());

setUDPSockOpt();
Expand Down Expand Up @@ -247,6 +267,12 @@ void srt::CChannel::open(int family)
}
m_BindAddr = sockaddr_any(res->ai_addr, (sockaddr_any::len_t)res->ai_addrlen);

#ifdef SRT_ENABLE_PKTINFO
// We know that this is intentionally bound now to "any",
// so the requester-destination address must be remembered and passed.
m_bBindMasked = true;
#endif
maxsharabayko marked this conversation as resolved.
Show resolved Hide resolved

::freeaddrinfo(res);

HLOGC(kmlog.Debug, log << "CHANNEL: Bound to local address: " << m_BindAddr.str());
Expand Down Expand Up @@ -472,6 +498,19 @@ void srt::CChannel::setUDPSockOpt()
if (0 != ::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVTIMEO, (char*)&tv, sizeof(timeval)))
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
#endif

#ifdef SRT_ENABLE_PKTINFO
if (m_bBindMasked)
{
HLOGP(kmlog.Debug, "Socket bound to ANY - setting PKTINFO for address retrieval");
const int on = 1, off SRT_ATR_UNUSED = 0;
::setsockopt(m_iSocket, IPPROTO_IP, IP_PKTINFO, (char*)&on, sizeof(on));
::setsockopt(m_iSocket, IPPROTO_IPV6, IPV6_RECVPKTINFO, &on, sizeof(on));

// XXX Unknown why this has to be off. RETEST.
//::setsockopt(m_iSocket, IPPROTO_IPV6, IPV6_V6ONLY, &off, sizeof(off));
Comment on lines +510 to +511
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found this line in the old implementation that was there years ago. Unsure what this was about. Left for a case when someone finds a problem on IPv6 using it.

}
#endif
}

void srt::CChannel::close() const
Expand Down Expand Up @@ -610,11 +649,16 @@ void srt::CChannel::getPeerAddr(sockaddr_any& w_addr) const
w_addr.len = namelen;
}

int srt::CChannel::sendto(const sockaddr_any& addr, CPacket& packet) const
int srt::CChannel::sendto(const sockaddr_any& addr, CPacket& packet, const sockaddr_any& source_addr SRT_ATR_UNUSED) const
{
HLOGC(kslog.Debug,
log << "CChannel::sendto: SENDING NOW DST=" << addr.str() << " target=@" << packet.m_iID
<< " size=" << packet.getLength() << " pkt.ts=" << packet.m_iTimeStamp << " " << packet.Info());
<< " size=" << packet.getLength() << " pkt.ts=" << packet.m_iTimeStamp
#ifdef SRT_ENABLE_PKTINFO
<< " sourceIP="
<< (m_bBindMasked && !source_addr.isany() ? source_addr.str() : "default")
#endif
<< " " << packet.Info());

#ifdef SRT_TEST_FAKE_LOSS

Expand Down Expand Up @@ -683,8 +727,28 @@ int srt::CChannel::sendto(const sockaddr_any& addr, CPacket& packet) const
mh.msg_namelen = addr.size();
mh.msg_iov = (iovec*)packet.m_PacketVector;
mh.msg_iovlen = 2;
mh.msg_control = NULL;
mh.msg_controllen = 0;
bool have_set_src = false;

#ifdef SRT_ENABLE_PKTINFO
if (m_bBindMasked && !source_addr.isany())
{
if (!setSourceAddress(mh, source_addr))
{
LOGC(kslog.Error, log << "CChannel::setSourceAddress: source address invalid family #" << source_addr.family() << ", NOT setting.");
}
else
{
HLOGC(kslog.Debug, log << "CChannel::setSourceAddress: setting as " << source_addr.str());
have_set_src = true;
}
}
#endif

if (!have_set_src)
{
mh.msg_control = NULL;
mh.msg_controllen = 0;
}
mh.msg_flags = 0;

const int res = ::sendmsg(m_iSocket, &mh, 0);
Expand Down Expand Up @@ -725,15 +789,33 @@ srt::EReadStatus srt::CChannel::recvfrom(sockaddr_any& w_addr, CPacket& w_packet
}

#ifndef _WIN32
msghdr mh; // will not be used on failure

if (select_ret > 0)
{
msghdr mh;
mh.msg_name = (w_addr.get());
mh.msg_namelen = w_addr.size();
mh.msg_iov = (w_packet.m_PacketVector);
mh.msg_iovlen = 2;

// Default
mh.msg_control = NULL;
mh.msg_controllen = 0;

#ifdef SRT_ENABLE_PKTINFO
// Without m_bBindMasked, we don't need ancillary data - the source
// address will always be the bound address.
if (m_bBindMasked)
{
// Extract the destination IP address from the ancillary
// data. This might be interesting for the connection to
// know to which address the packet should be sent back during
// the handshake and then addressed when sending during connection.
mh.msg_control = (m_acCmsgRecvBuffer);
mh.msg_controllen = sizeof m_acCmsgRecvBuffer;
}
#endif

mh.msg_flags = 0;

recv_size = ::recvmsg(m_iSocket, (&mh), 0);
Expand Down Expand Up @@ -779,6 +861,17 @@ srt::EReadStatus srt::CChannel::recvfrom(sockaddr_any& w_addr, CPacket& w_packet
goto Return_error;
}

#ifdef SRT_ENABLE_PKTINFO
if (m_bBindMasked)
{
// Extract the address. Set it explicitly; if this returns address that isany(),
// it will simply set this on the packet so that it behaves as if nothing was
// extracted (it will "fail the old way").
w_packet.m_DestAddr = getTargetAddress(mh);
HLOGC(krlog.Debug, log << CONID() << "(sys)recvmsg: ANY BOUND, retrieved DEST ADDR: " << w_packet.m_DestAddr.str());
}
#endif

#else
// XXX REFACTORING NEEDED!
// This procedure uses the WSARecvFrom function that just reads
Expand Down
115 changes: 114 additions & 1 deletion srtcore/channel.h
Expand Up @@ -115,9 +115,10 @@ class CChannel
/// Send a packet to the given address.
/// @param [in] addr pointer to the destination address.
/// @param [in] packet reference to a CPacket entity.
/// @param [in] src source address to sent on an outgoing packet (if not ANY)
/// @return Actual size of data sent.

int sendto(const sockaddr_any& addr, srt::CPacket& packet) const;
int sendto(const sockaddr_any& addr, srt::CPacket& packet, const sockaddr_any& src) const;

/// Receive a packet from the channel and record the source address.
/// @param [in] addr pointer to the source address.
Expand Down Expand Up @@ -160,6 +161,118 @@ class CChannel
// although the object itself isn't considered modified.
mutable CSrtMuxerConfig m_mcfg; // Note: ReuseAddr is unused and ineffective.
sockaddr_any m_BindAddr;

// This feature is not enabled on Windows, for now.
// This is also turned off in case of MinGW
#ifdef SRT_ENABLE_PKTINFO
bool m_bBindMasked; // True if m_BindAddr is INADDR_ANY. Need for quick check.

// Calculating the required space is extremely tricky, and whereas on most
// platforms it's possible to define it this way:
//
// size_t s = max( CMSG_SPACE(sizeof(in_pktinfo)), CMSG_SPACE(sizeof(in6_pktinfo)) )
//
// ...on some platforms however CMSG_SPACE macro can't be resolved as constexpr.
//
// This structure is exclusively used to determine the required size for
// CMSG buffer so that it can be allocated in a solid block with CChannel.
// NOT TO BE USED to access any data inside the CMSG message.
struct CMSGNodeAlike
{
union
{
in_pktinfo in4;
in6_pktinfo in6;
};
size_t extrafill;
cmsghdr hdr;
};

// This is 'mutable' because it's a utility buffer defined here
// to avoid unnecessary re-allocations.
mutable char m_acCmsgRecvBuffer [sizeof (CMSGNodeAlike)]; // Reserved space for ancillary data with pktinfo
mutable char m_acCmsgSendBuffer [sizeof (CMSGNodeAlike)]; // Reserved space for ancillary data with pktinfo

// IMPORTANT!!! This function shall be called EXCLUSIVELY just after
// calling ::recvmsg function. It uses a static buffer to supply data
// for the call, and it's stated that only one thread is trying to
// use a CChannel object in receiving mode.
sockaddr_any getTargetAddress(const msghdr& msg) const
{
// Loop through IP header messages
cmsghdr* cmsg = CMSG_FIRSTHDR(&msg);
for (cmsg = CMSG_FIRSTHDR(&msg);
cmsg != NULL;
cmsg = CMSG_NXTHDR(((msghdr*)&msg), cmsg))
{
// This should be safe - this packet contains always either
// IPv4 headers or IPv6 headers.
if (cmsg->cmsg_level == IPPROTO_IP && cmsg->cmsg_type == IP_PKTINFO)
{
in_pktinfo *dest_ip_ptr = (in_pktinfo*)CMSG_DATA(cmsg);
return sockaddr_any(dest_ip_ptr->ipi_addr, 0);
}

if (cmsg->cmsg_level == IPPROTO_IPV6 && cmsg->cmsg_type == IPV6_PKTINFO)
{
in6_pktinfo* dest_ip_ptr = (in6_pktinfo*)CMSG_DATA(cmsg);
return sockaddr_any(dest_ip_ptr->ipi6_addr, 0);
}
}

// Fallback for an error
return sockaddr_any(m_BindAddr.family());
}

// IMPORTANT!!! This function shall be called EXCLUSIVELY just before
// calling ::sendmsg function. It uses a static buffer to supply data
// for the call, and it's stated that only one thread is trying to
// use a CChannel object in sending mode.
bool setSourceAddress(msghdr& mh, const sockaddr_any& adr) const
{
// In contrast to an advice followed on the net, there's no case of putting
// both IPv4 and IPv6 ancillary data, case we could have them. Only one
// IP version is used and it's the version as found in @a adr, which should
// be the version used for binding.

if (adr.family() == AF_INET)
{
mh.msg_control = m_acCmsgSendBuffer;
mh.msg_controllen = CMSG_SPACE(sizeof(in_pktinfo));
cmsghdr* cmsg_send = CMSG_FIRSTHDR(&mh);

// after initializing msghdr & control data to CMSG_SPACE(sizeof(struct in_pktinfo))
cmsg_send->cmsg_level = IPPROTO_IP;
cmsg_send->cmsg_type = IP_PKTINFO;
cmsg_send->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo));
in_pktinfo* pktinfo = (in_pktinfo*) CMSG_DATA(cmsg_send);
pktinfo->ipi_ifindex = 0;
pktinfo->ipi_spec_dst = adr.sin.sin_addr;

return true;
}

if (adr.family() == AF_INET6)
{
mh.msg_control = m_acCmsgSendBuffer;
mh.msg_controllen = CMSG_SPACE(sizeof(in6_pktinfo));
cmsghdr* cmsg_send = CMSG_FIRSTHDR(&mh);

cmsg_send->cmsg_level = IPPROTO_IPV6;
cmsg_send->cmsg_type = IPV6_PKTINFO;
cmsg_send->cmsg_len = CMSG_LEN(sizeof(in6_pktinfo));
in6_pktinfo* pktinfo = (in6_pktinfo*) CMSG_DATA(cmsg_send);
pktinfo->ipi6_ifindex = 0;
pktinfo->ipi6_addr = adr.sin6.sin6_addr;

return true;
}

return false;
}

#endif // SRT_ENABLE_PKTINFO

};

} // namespace srt
Expand Down