Skip to content

Commit 8a89a3a

Browse files
ethourisethouris
authored
[core][tests] Clarified conditions for setting IPTOS/TTL options. Added UT (#3096).
* Added requirement for IPv6 for UDP Options test. * Updated documentation.
1 parent 1ec61fc commit 8a89a3a

File tree

5 files changed

+137
-44
lines changed

5 files changed

+137
-44
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,6 +1526,7 @@ if (ENABLE_UNITTESTS AND ENABLE_CXX11)
15261526
srt_add_program_dont_install(test-srt ${SOURCES_unittests})
15271527
srt_make_application(test-srt)
15281528
target_include_directories(test-srt PRIVATE ${SSL_INCLUDE_DIRS} ${GTEST_INCLUDE_DIRS})
1529+
target_compile_definitions(test-srt PRIVATE "-DSRT_TEST_SYSTEM_NAME=\"${CMAKE_SYSTEM_NAME}\"")
15291530

15301531
target_link_libraries(
15311532
test-srt

‎docs/API/API-socket-options.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,8 @@ See [`SRTO_INPUTBW`](#SRTO_INPUTBW).
614614
IPv4 Type of Service (see `IP_TOS` option for IP) or IPv6 Traffic Class (see `IPV6_TCLASS`
615615
of IPv6) depending on socket address family. Applies to sender only.
616616

617+
NOTE: This option has been tested to work correctly on Linux only.
618+
617619
When *getting*, the returned value is the user preset for non-connected sockets
618620
and the actual value for connected sockets.
619621

@@ -632,6 +634,8 @@ and the actual value for connected sockets.
632634
IPv4 Time To Live (see `IP_TTL` option for IP) or IPv6 unicast hops (see
633635
`IPV6_UNICAST_HOPS` for IPv6) depending on socket address family. Applies to sender only.
634636

637+
NOTE: This option has been tested to work correctly on Linux only.
638+
635639
When *getting*, the returned value is the user preset for non-connected sockets
636640
and the actual value for connected sockets.
637641

‎srtcore/channel.cpp‎

Lines changed: 99 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,20 @@ void srt::CChannel::attach(UDPSOCKET udpsock, const sockaddr_any& udpsocks_addr)
289289
setUDPSockOpt();
290290
}
291291

292+
static inline string fmt_opt(bool value, const string& label)
293+
{
294+
string out;
295+
out.reserve(label.size() + 2);
296+
out = value ? "+" : "-";
297+
out += label;
298+
return out;
299+
}
300+
301+
static inline string fmt_alt(bool value, const string& label, const string& unlabel)
302+
{
303+
return value ? label : unlabel;
304+
}
305+
292306
void srt::CChannel::setUDPSockOpt()
293307
{
294308
#if defined(SUNOS)
@@ -297,13 +311,13 @@ void srt::CChannel::setUDPSockOpt()
297311
// Retrieve starting SND/RCV Buffer sizes.
298312
int startRCVBUF = 0;
299313
optSize = sizeof(startRCVBUF);
300-
if (0 != ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&startRCVBUF, &optSize))
314+
if (-1 == ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&startRCVBUF, &optSize))
301315
{
302316
startRCVBUF = -1;
303317
}
304318
int startSNDBUF = 0;
305319
optSize = sizeof(startSNDBUF);
306-
if (0 != ::getsockopt(m_iSocket, SOL_SOCKET, SO_SNDBUF, (void*)&startSNDBUF, &optSize))
320+
if (-1 == ::getsockopt(m_iSocket, SOL_SOCKET, SO_SNDBUF, (void*)&startSNDBUF, &optSize))
307321
{
308322
startSNDBUF = -1;
309323
}
@@ -312,13 +326,12 @@ void srt::CChannel::setUDPSockOpt()
312326
// maximum value.
313327
// However, do not reduce the buffer size.
314328
const int maxsize = 64000;
315-
if (0 !=
316-
::setsockopt(
317-
m_iSocket, SOL_SOCKET, SO_RCVBUF, (const char*)&m_mcfg.iUDPRcvBufSize, sizeof m_mcfg.iUDPRcvBufSize))
329+
if (-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF,
330+
(const char*)&m_mcfg.iUDPRcvBufSize, sizeof m_mcfg.iUDPRcvBufSize))
318331
{
319332
int currentRCVBUF = 0;
320333
optSize = sizeof(currentRCVBUF);
321-
if (0 != ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&currentRCVBUF, &optSize))
334+
if (-1 == ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&currentRCVBUF, &optSize))
322335
{
323336
currentRCVBUF = -1;
324337
}
@@ -327,13 +340,12 @@ void srt::CChannel::setUDPSockOpt()
327340
::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (const char*)&maxsize, sizeof maxsize);
328341
}
329342
}
330-
if (0 !=
331-
::setsockopt(
332-
m_iSocket, SOL_SOCKET, SO_SNDBUF, (const char*)&m_mcfg.iUDPSndBufSize, sizeof m_mcfg.iUDPSndBufSize))
343+
if (-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_SNDBUF,
344+
(const char*)&m_mcfg.iUDPSndBufSize, sizeof m_mcfg.iUDPSndBufSize))
333345
{
334346
int currentSNDBUF = 0;
335347
optSize = sizeof(currentSNDBUF);
336-
if (0 != ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&currentSNDBUF, &optSize))
348+
if (-1 == ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&currentSNDBUF, &optSize))
337349
{
338350
currentSNDBUF = -1;
339351
}
@@ -346,13 +358,13 @@ void srt::CChannel::setUDPSockOpt()
346358
// Retrieve ending SND/RCV Buffer sizes.
347359
int endRCVBUF = 0;
348360
optSize = sizeof(endRCVBUF);
349-
if (0 != ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&endRCVBUF, &optSize))
361+
if (-1 == ::getsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)&endRCVBUF, &optSize))
350362
{
351363
endRCVBUF = -1;
352364
}
353365
int endSNDBUF = 0;
354366
optSize = sizeof(endSNDBUF);
355-
if (0 != ::getsockopt(m_iSocket, SOL_SOCKET, SO_SNDBUF, (void*)&endSNDBUF, &optSize))
367+
if (-1 == ::getsockopt(m_iSocket, SOL_SOCKET, SO_SNDBUF, (void*)&endSNDBUF, &optSize))
356368
{
357369
endSNDBUF = -1;
358370
}
@@ -368,90 +380,137 @@ void srt::CChannel::setUDPSockOpt()
368380
#elif defined(BSD) || TARGET_OS_MAC
369381
// BSD system will fail setsockopt if the requested buffer size exceeds system maximum value
370382
int maxsize = 64000;
371-
if (0 != ::setsockopt(
383+
if (-1 == ::setsockopt(
372384
m_iSocket, SOL_SOCKET, SO_RCVBUF, (const char*)&m_mcfg.iUDPRcvBufSize, sizeof m_mcfg.iUDPRcvBufSize))
373385
::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF, (const char*)&maxsize, sizeof maxsize);
374-
if (0 != ::setsockopt(
386+
if (-1 == ::setsockopt(
375387
m_iSocket, SOL_SOCKET, SO_SNDBUF, (const char*)&m_mcfg.iUDPSndBufSize, sizeof m_mcfg.iUDPSndBufSize))
376388
::setsockopt(m_iSocket, SOL_SOCKET, SO_SNDBUF, (const char*)&maxsize, sizeof maxsize);
377389
#else
378390
// for other systems, if requested is greated than maximum, the maximum value will be automactally used
379-
if ((0 !=
380-
::setsockopt(
381-
m_iSocket, SOL_SOCKET, SO_RCVBUF, (const char*)&m_mcfg.iUDPRcvBufSize, sizeof m_mcfg.iUDPRcvBufSize)) ||
382-
(0 != ::setsockopt(
383-
m_iSocket, SOL_SOCKET, SO_SNDBUF, (const char*)&m_mcfg.iUDPSndBufSize, sizeof m_mcfg.iUDPSndBufSize)))
391+
if ((-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVBUF,
392+
(const char*)&m_mcfg.iUDPRcvBufSize, sizeof m_mcfg.iUDPRcvBufSize))
393+
||
394+
(-1 == ::setsockopt( m_iSocket, SOL_SOCKET, SO_SNDBUF,
395+
(const char*)&m_mcfg.iUDPSndBufSize, sizeof m_mcfg.iUDPSndBufSize)))
384396
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
385397
#endif
386398

399+
bool is_set = false;
400+
bool adr_unspec = false, adr_mapped = false, adr_v6 = false;
401+
if (m_BindAddr.family() == AF_INET)
402+
{
403+
adr_unspec = m_BindAddr.isany();
404+
}
405+
else
406+
{
407+
adr_unspec = IN6_IS_ADDR_UNSPECIFIED(&m_BindAddr.sin6.sin6_addr);
408+
adr_mapped = IN6_IS_ADDR_V4MAPPED(&m_BindAddr.sin6.sin6_addr);
409+
adr_v6 = true;
410+
}
411+
387412
if (m_mcfg.iIpTTL != -1)
388413
{
389-
if (m_BindAddr.family() == AF_INET)
414+
if (!adr_v6)
390415
{
391-
if (0 != ::setsockopt(m_iSocket, IPPROTO_IP, IP_TTL, (const char*)&m_mcfg.iIpTTL, sizeof m_mcfg.iIpTTL))
416+
if (-1 == ::setsockopt(m_iSocket, IPPROTO_IP, IP_TTL, (const char*)&m_mcfg.iIpTTL, sizeof m_mcfg.iIpTTL))
417+
{
418+
LOGC(kmlog.Error, log << "setsockopt(IP_TTL): " << SysStrError(NET_ERROR));
392419
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
420+
}
421+
is_set = true;
393422
}
394423
else
395424
{
396425
// If IPv6 address is unspecified, set BOTH IP_TTL and IPV6_UNICAST_HOPS.
397426

398427
// For specified IPv6 address, set IPV6_UNICAST_HOPS ONLY UNLESS it's an IPv4-mapped-IPv6
399-
if (IN6_IS_ADDR_UNSPECIFIED(&m_BindAddr.sin6.sin6_addr) ||
400-
!IN6_IS_ADDR_V4MAPPED(&m_BindAddr.sin6.sin6_addr))
428+
if (adr_unspec || !adr_mapped)
401429
{
402-
if (0 !=
403-
::setsockopt(
404-
m_iSocket, IPPROTO_IPV6, IPV6_UNICAST_HOPS, (const char*)&m_mcfg.iIpTTL, sizeof m_mcfg.iIpTTL))
430+
if (-1 == ::setsockopt( m_iSocket, IPPROTO_IPV6, IPV6_UNICAST_HOPS,
431+
(const char*)&m_mcfg.iIpTTL, sizeof m_mcfg.iIpTTL))
405432
{
433+
LOGC(kmlog.Error, log << "setsockopt(IPV6_UNICAST_HOPS): " << SysStrError(NET_ERROR));
406434
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
407435
}
436+
is_set = true;
408437
}
409438
// For specified IPv6 address, set IP_TTL ONLY WHEN it's an IPv4-mapped-IPv6
410-
if (IN6_IS_ADDR_UNSPECIFIED(&m_BindAddr.sin6.sin6_addr) || IN6_IS_ADDR_V4MAPPED(&m_BindAddr.sin6.sin6_addr))
439+
if (!is_set) // adr_mapped (because adr_unspec was handled above)
411440
{
412-
if (0 != ::setsockopt(m_iSocket, IPPROTO_IP, IP_TTL, (const char*)&m_mcfg.iIpTTL, sizeof m_mcfg.iIpTTL))
441+
if (-1 == ::setsockopt(m_iSocket, IPPROTO_IP, IP_TTL, (const char*)&m_mcfg.iIpTTL, sizeof m_mcfg.iIpTTL))
413442
{
443+
LOGC(kmlog.Error, log << "setsockopt(IP_TTL): " << SysStrError(NET_ERROR)
444+
<< fmt_alt(adr_unspec, " (v6 unspec)", " (v6 mapped v4)"));
414445
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
415446
}
447+
is_set = true;
416448
}
417449
}
450+
451+
if (!is_set)
452+
{
453+
LOGC(kmlog.Error, log << "srt_setsockflag(SRTO_IPTTL): No suitable condition for adr=" << m_BindAddr.str()
454+
<< " : " << fmt_opt(adr_v6, "v6 ") << fmt_opt(adr_unspec, "unspec ") << fmt_opt(adr_mapped, "mapped"));
455+
throw CUDTException(MJ_SETUP, MN_INVAL, 0);
456+
}
418457
}
419458

459+
is_set = false;
420460
if (m_mcfg.iIpToS != -1)
421461
{
422462
if (m_BindAddr.family() == AF_INET)
423463
{
424-
if (0 != ::setsockopt(m_iSocket, IPPROTO_IP, IP_TOS, (const char*)&m_mcfg.iIpToS, sizeof m_mcfg.iIpToS))
464+
if (-1 == ::setsockopt(m_iSocket, IPPROTO_IP, IP_TOS, (const char*)&m_mcfg.iIpToS, sizeof m_mcfg.iIpToS))
465+
{
466+
LOGC(kmlog.Error, log << "setsockopt(IP_TOS): " << SysStrError(NET_ERROR));
425467
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
468+
}
469+
is_set = true;
426470
}
427471
else
428472
{
429473
// If IPv6 address is unspecified, set BOTH IP_TOS and IPV6_TCLASS.
430474

475+
SRT_ATR_UNUSED bool using_tclass = false;
431476
#ifdef IPV6_TCLASS
477+
using_tclass = true;
432478
// For specified IPv6 address, set IPV6_TCLASS ONLY UNLESS it's an IPv4-mapped-IPv6
433-
if (IN6_IS_ADDR_UNSPECIFIED(&m_BindAddr.sin6.sin6_addr) ||
434-
!IN6_IS_ADDR_V4MAPPED(&m_BindAddr.sin6.sin6_addr))
479+
if (adr_unspec || !adr_mapped)
435480
{
436-
if (0 != ::setsockopt(
437-
m_iSocket, IPPROTO_IPV6, IPV6_TCLASS, (const char*)&m_mcfg.iIpToS, sizeof m_mcfg.iIpToS))
481+
if (-1 == ::setsockopt(m_iSocket, IPPROTO_IPV6, IPV6_TCLASS,
482+
(const char*)&m_mcfg.iIpToS, sizeof m_mcfg.iIpToS))
438483
{
484+
LOGC(kmlog.Error, log << "setsockopt(IPV6_TCLASS): " << SysStrError(NET_ERROR));
439485
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
440486
}
487+
is_set = true;
441488
}
442489
#endif
443490

444491
// For specified IPv6 address, set IP_TOS ONLY WHEN it's an IPv4-mapped-IPv6
445-
if (IN6_IS_ADDR_UNSPECIFIED(&m_BindAddr.sin6.sin6_addr) || IN6_IS_ADDR_V4MAPPED(&m_BindAddr.sin6.sin6_addr))
492+
if (!is_set && (adr_unspec || adr_mapped))
446493
{
447-
if (0 != ::setsockopt(m_iSocket, IPPROTO_IP, IP_TOS, (const char*)&m_mcfg.iIpToS, sizeof m_mcfg.iIpToS))
494+
if (-1 == ::setsockopt(m_iSocket, IPPROTO_IP, IP_TOS, (const char*)&m_mcfg.iIpToS, sizeof m_mcfg.iIpToS))
448495
{
496+
LOGC(kmlog.Error, log << "setsockopt(IP_TOS): " << SysStrError(NET_ERROR)
497+
<< (adr_unspec ? " (v6 unspecified)" : " (v6 mapped v4)")
498+
<< (using_tclass ? "(fallback to IP_TOS)" : ""));
449499
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
450500
}
501+
is_set = true;
451502
}
452503
}
504+
505+
if (!is_set)
506+
{
507+
LOGC(kmlog.Error, log << "srt_setsockflag(SRTO_IPTOS): No suitable condition for adr=" << m_BindAddr.str()
508+
<< " : " << fmt_opt(adr_v6, "v6 ") << fmt_opt(adr_unspec, "unspec ") << fmt_opt(adr_mapped, "mapped"));
509+
throw CUDTException(MJ_SETUP, MN_INVAL, 0);
510+
}
453511
}
454512

513+
455514
#ifdef SRT_ENABLE_BINDTODEVICE
456515
if (!m_mcfg.sBindToDevice.empty())
457516
{
@@ -461,14 +520,10 @@ void srt::CChannel::setUDPSockOpt()
461520
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
462521
}
463522

464-
if (0 != ::setsockopt(
465-
m_iSocket, SOL_SOCKET, SO_BINDTODEVICE, m_mcfg.sBindToDevice.c_str(), m_mcfg.sBindToDevice.size()))
523+
if (-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_BINDTODEVICE,
524+
m_mcfg.sBindToDevice.c_str(), m_mcfg.sBindToDevice.size()))
466525
{
467-
#if ENABLE_LOGGING
468-
char buf[255];
469-
const char* err = SysStrError(NET_ERROR, buf, 255);
470-
LOGC(kmlog.Error, log << "setsockopt(SRTO_BINDTODEVICE): " << err);
471-
#endif // ENABLE_LOGGING
526+
LOGC(kmlog.Error, log << "setsockopt(SRTO_BINDTODEVICE): " << SysStrError(NET_ERROR));
472527
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
473528
}
474529
}
@@ -482,7 +537,7 @@ void srt::CChannel::setUDPSockOpt()
482537
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
483538
#elif defined(_WIN32)
484539
u_long nonBlocking = 1;
485-
if (0 != ioctlsocket(m_iSocket, FIONBIO, &nonBlocking))
540+
if (-1 == ioctlsocket(m_iSocket, FIONBIO, &nonBlocking))
486541
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
487542
#else
488543
timeval tv;
@@ -495,7 +550,7 @@ void srt::CChannel::setUDPSockOpt()
495550
tv.tv_usec = 100;
496551
#endif
497552
// Set receiving time-out value
498-
if (0 != ::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVTIMEO, (char*)&tv, sizeof(timeval)))
553+
if (-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVTIMEO, (char*)&tv, sizeof(timeval)))
499554
throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR);
500555
#endif
501556

‎test/test_env.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ class TestEnv: public testing::Environment
4040
// All must be static, return bool. Arguments allowed.
4141
// The name must start with Allowed_.
4242
static bool Allowed_IPv6();
43+
44+
template<typename... Args>
45+
static bool Allowed_Platform(const std::string& first, const Args&... follow)
46+
{
47+
if (first == SRT_TEST_SYSTEM_NAME)
48+
return true;
49+
return Allowed_Platform(follow...);
50+
}
51+
52+
static bool Allowed_Platform() { return false; }
4353
};
4454

4555
#define SRTST_REQUIRES(feature,...) if (!srt::TestEnv::Allowed_##feature(__VA_ARGS__)) { return; }

‎test/test_reuseaddr.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,29 @@ TEST_F(ReuseAddr, DiffAddr)
531531
shutdownListener(bindsock_2);
532532
}
533533

534+
TEST_F(ReuseAddr, UDPOptions)
535+
{
536+
// IP_TOS and IP_TTL don't work on Windows and Mac
537+
SRTST_REQUIRES(Platform, "Linux", "GNU");
538+
539+
// Travis doesn't work with IPv6
540+
SRTST_REQUIRES(IPv6);
541+
542+
MAKE_UNIQUE_SOCK(bs1, "general ipv6", prepareServerSocket());
543+
MAKE_UNIQUE_SOCK(bs2, "mapped ipv4", prepareServerSocket());
544+
545+
int val_TOS = 4; // IPTOS_RELIABILITY per <netinet/ip.h>, but not available on Windows
546+
int val_TTL = 10;
547+
548+
EXPECT_NE(srt_setsockflag(bs1, SRTO_IPTOS, &val_TOS, sizeof val_TOS), SRT_ERROR);
549+
EXPECT_NE(srt_setsockflag(bs1, SRTO_IPTTL, &val_TTL, sizeof val_TTL), SRT_ERROR);
550+
EXPECT_NE(srt_setsockflag(bs2, SRTO_IPTOS, &val_TOS, sizeof val_TOS), SRT_ERROR);
551+
EXPECT_NE(srt_setsockflag(bs2, SRTO_IPTTL, &val_TTL, sizeof val_TTL), SRT_ERROR);
552+
553+
bindSocket(bs1, "::1", 5000, true);
554+
bindSocket(bs2, "::FFFF:127.0.0.1", 5001, true);
555+
}
556+
534557
TEST_F(ReuseAddr, Wildcard)
535558
{
536559
#if defined(_WIN32) || defined(CYGWIN)

0 commit comments

Comments
 (0)