aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKrystian Zlomek <kzlomek@affinegy.com>2017-08-02 11:03:02 +0200
committerKrystian Zlomek <kzlomek@affinegy.com>2017-08-03 16:08:11 +0000
commit350c901d87aa75891a99bb28302aaed1be7018c2 (patch)
treee991f0ab2d18a3e0594afd0439629d0121fea3ff
parent55c484e27fa3d3c9b87a9c1439d31f3ac613b824 (diff)
ASACORE-3274, ASACORE-2370: Cleanup around IPNS
- Updated IsSameIPv6Network() to handle any mask - Added QCC_VERIFY around IPNS::Enabled()/Enable() calls - Removed in IPNS not needed locks acquisition Change-Id: I64fd986368778be2e48220623038dcb0e6dd7a0d Signed-off-by: Krystian Zlomek <kzlomek@affinegy.com>
-rw-r--r--alljoyn_core/router/TCPTransport.cc14
-rw-r--r--alljoyn_core/router/UDPTransport.cc25
-rw-r--r--alljoyn_core/router/ns/IpNameServiceImpl.cc37
-rw-r--r--common/inc/qcc/IPAddress.h16
-rw-r--r--common/src/IPAddress.cc23
-rw-r--r--common/unit_test/IPAddressTest.cc13
6 files changed, 82 insertions, 46 deletions
diff --git a/alljoyn_core/router/TCPTransport.cc b/alljoyn_core/router/TCPTransport.cc
index 0f95ebd..b246595 100644
--- a/alljoyn_core/router/TCPTransport.cc
+++ b/alljoyn_core/router/TCPTransport.cc
@@ -1448,7 +1448,7 @@ QStatus TCPTransport::GetListenAddresses(const SessionOpts& opts, std::vector<qc
qcc::String ipv6address;
std::map<qcc::String, uint16_t> reliableIpv4PortMap, unreliablePortMap;
uint16_t reliableIpv6Port;
- IpNameService::Instance().Enabled(TRANSPORT_TCP, reliableIpv4PortMap, reliableIpv6Port, unreliablePortMap);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enabled(TRANSPORT_TCP, reliableIpv4PortMap, reliableIpv6Port, unreliablePortMap));
/*
* If no listening port corresponding to this network interface is found in the map,
* then it hasn't been set and this implies that there is no listener for this transport
@@ -2299,7 +2299,7 @@ void TCPTransport::EnableAdvertisementInstance(ListenRequest& listenRequest)
*/
if (m_isListening) {
if (!m_isNsEnabled) {
- IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), true, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), true, false, false));
m_isNsEnabled = true;
}
}
@@ -2390,7 +2390,7 @@ void TCPTransport::DisableAdvertisementInstance(ListenRequest& listenRequest)
* name service. We do this by telling it we don't want it to be
* enabled on any of the possible ports.
*/
- IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), false, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), false, false, false));
m_isNsEnabled = false;
/*
@@ -2474,7 +2474,7 @@ void TCPTransport::EnableDiscoveryInstance(ListenRequest& listenRequest)
*/
if (m_isListening) {
if (!m_isNsEnabled) {
- IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), true, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), true, false, false));
m_isNsEnabled = true;
}
}
@@ -2549,7 +2549,7 @@ void TCPTransport::DisableDiscoveryInstance(ListenRequest& listenRequest)
*/
if (isEmpty && !m_isAdvertising) {
- IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), false, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), false, false, false));
m_isNsEnabled = false;
/*
@@ -4458,7 +4458,7 @@ void TCPTransport::HandleNetworkEventInstance(ListenRequest& listenRequest)
* a zero port). Remember the port we enabled so we can re-enable the name
* service if listeners come and go.
*/
- IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), true, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), true, false, false));
/*
* There is a special case in which we respond to embedded AllJoyn bus
@@ -4556,7 +4556,7 @@ void TCPTransport::HandleNetworkEventInstance(ListenRequest& listenRequest)
for (list<pair<qcc::String, SocketFd> >::iterator it = addedList.begin(); it != addedList.end(); it++) {
replacedList.push_back(it->first);
}
- IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), false, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_TCP, m_listenPortMap, 0, std::map<qcc::String, uint16_t>(), false, false, false));
m_isListening = false;
m_isNsEnabled = false;
m_listenPortMap.clear();
diff --git a/alljoyn_core/router/UDPTransport.cc b/alljoyn_core/router/UDPTransport.cc
index 380e67a..34cfe6a 100644
--- a/alljoyn_core/router/UDPTransport.cc
+++ b/alljoyn_core/router/UDPTransport.cc
@@ -5790,7 +5790,7 @@ QStatus UDPTransport::GetListenAddresses(const SessionOpts& opts, std::vector<qc
qcc::String ipv6address;
std::map<qcc::String, uint16_t> reliableIpv4PortMap, unreliablePortMap;
uint16_t reliableIpv6Port;
- IpNameService::Instance().Enabled(TRANSPORT_UDP, reliableIpv4PortMap, reliableIpv6Port, unreliablePortMap);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enabled(TRANSPORT_UDP, reliableIpv4PortMap, reliableIpv6Port, unreliablePortMap));
/*
* If no listening port corresponding to this network interface is found in the map,
@@ -9048,7 +9048,7 @@ void UDPTransport::EnableAdvertisementInstance(ListenRequest& listenRequest)
if (m_isListening) {
if (!m_isNsEnabled) {
QCC_ASSERT(!m_listenPortMap.empty());
- IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, true);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, true));
m_isNsEnabled = true;
}
}
@@ -9149,7 +9149,7 @@ void UDPTransport::DisableAdvertisementInstance(ListenRequest& listenRequest)
* enabled on any of the possible ports.
*/
QCC_DbgPrintf(("UDPTransport::DisableAdvertisementInstance(): Disabling NS"));
- IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, false));
m_isNsEnabled = false;
/*
@@ -9314,7 +9314,7 @@ void UDPTransport::DisableDiscoveryInstance(ListenRequest& listenRequest)
isEmpty ? "true" : "false", m_isAdvertising ? "true" : "false"));
if (isEmpty && !m_isAdvertising) {
QCC_DbgPrintf(("UDPTransport::DisableDiscoveryInstance(): Disabling NS"));
- IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, false));
m_isNsEnabled = false;
/*
@@ -10011,6 +10011,7 @@ QStatus UDPTransport::Connect(const char* connectSpec, const SessionOpts& opts,
for (uint32_t j = 0; j < entries.size(); ++j) {
if (entries[j].m_addr == listenAddr.ToString()) {
prefixLen = entries[j].m_prefixlen;
+ break;
}
}
@@ -10050,7 +10051,17 @@ QStatus UDPTransport::Connect(const char* connectSpec, const SessionOpts& opts,
break;
}
- if (listenAddr.IsSameIPv6Network(ipAddr)) {
+ uint32_t prefixLen = 0;
+ for (uint32_t j = 0; j < entries.size(); ++j) {
+ if (entries[j].m_addr == listenAddr.ToString()) {
+ prefixLen = entries[j].m_prefixlen;
+ break;
+ }
+ }
+
+ QCC_DbgPrintf(("UDPTransport::Connect(): prefixlen=%d.", prefixLen));
+
+ if (listenAddr.IsSameIPv6Network(ipAddr, prefixLen)) {
QCC_DbgPrintf(("UDPTransport::Connect(): network of \"%s\" matches network of \"%s\"",
listenAddr.ToString().c_str(), ipAddr.ToString().c_str()));
/*
@@ -11813,7 +11824,7 @@ void UDPTransport::HandleNetworkEventInstance(ListenRequest& listenRequest)
*/
QCC_DbgPrintf(("UDPTransport::HandleNetworkEventInstance(): IpNameService::Instance().Enable()"));
QCC_ASSERT(!m_listenPortMap.empty());
- IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, true);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, true));
/*
* There is a special case in which we respond to embedded AllJoyn bus
@@ -11926,7 +11937,7 @@ void UDPTransport::HandleNetworkEventInstance(ListenRequest& listenRequest)
}
addedList.clear();
QCC_DbgPrintf(("UDPTransport::HandleNetworkEventInstance(): Disable NS"));
- IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, false);
+ QCC_VERIFY(ER_OK == IpNameService::Instance().Enable(TRANSPORT_UDP, std::map<qcc::String, uint16_t>(), 0, m_listenPortMap, false, false, false));
m_isListening = false;
m_isNsEnabled = false;
m_listenPortMap.clear();
diff --git a/alljoyn_core/router/ns/IpNameServiceImpl.cc b/alljoyn_core/router/ns/IpNameServiceImpl.cc
index c230af4..1086d86 100644
--- a/alljoyn_core/router/ns/IpNameServiceImpl.cc
+++ b/alljoyn_core/router/ns/IpNameServiceImpl.cc
@@ -1203,7 +1203,7 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
std::vector<qcc::IfConfigEntry> entries;
QStatus status = qcc::IfConfig(entries);
if (status != ER_OK) {
- QCC_LogError(status, ("LazyUpdateInterfaces: IfConfig() failed"));
+ QCC_LogError(status, ("IpNameServiceImpl::LazyUpdateInterfaces(): IfConfig() failed"));
ClearUnicastSocketAndEvent();
return;
}
@@ -1359,8 +1359,12 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
// If we aren't configured to use this entry, or have no idea how to use
// this entry (not AF_INET or AF_INET6), try the next one.
//
- if (useEntry == false || (entries[i].m_family != qcc::QCC_AF_INET && entries[i].m_family != qcc::QCC_AF_INET6)) {
- QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): Won't use this IfConfig entry"));
+ if (useEntry == false || ((entries[i].m_family != qcc::QCC_AF_INET) && (entries[i].m_family != qcc::QCC_AF_INET6))) {
+ if (useEntry == false) {
+ QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): Won't use this IfConfig entry"));
+ } else {
+ QCC_ASSERT(!"IpNameServiceImpl::LazyUpdateInterfaces(): Unexpected value in m_family (not AF_INET or AF_INET6");
+ }
continue;
}
@@ -1397,7 +1401,7 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
bool af_inet = entries[i].m_family == qcc::QCC_AF_INET;
if (!loopback && !multicast && (!broadcast || !m_broadcast || !af_inet)) {
- QCC_DbgPrintf(("LazyUpdateInterfaces: !loopback && !multicast && (!broadcast || !m_broadcast || !af_inet). Ignoring family %d flags %d", entries[i].m_family, entries[i].m_flags));
+ QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): !loopback && !multicast && (!broadcast || !m_broadcast || !af_inet). Ignoring family %d flags %d", entries[i].m_family, entries[i].m_flags));
continue;
}
@@ -1414,22 +1418,17 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
qcc::SocketFd multicastMDNSsockFd = qcc::INVALID_SOCKET_FD;
qcc::SocketFd multicastsockFd = qcc::INVALID_SOCKET_FD;
- if ((entries[i].m_family != qcc::QCC_AF_INET) && (entries[i].m_family != qcc::QCC_AF_INET6)) {
- QCC_ASSERT(!"IpNameServiceImpl::LazyUpdateInterfaces(): Unexpected value in m_family (not AF_INET or AF_INET6");
- continue;
- }
-
status = CreateMulticastSocket(entries[i], IPV4_MDNS_MULTICAST_GROUP, IPV6_MDNS_MULTICAST_GROUP, MULTICAST_MDNS_PORT,
m_broadcast, multicastMDNSsockFd);
if (status != ER_OK) {
- QCC_DbgPrintf(("Failed to create multicast socket for MDNS packets."));
+ QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): Failed to create multicast socket for MDNS packets."));
continue;
}
status = CreateMulticastSocket(entries[i], IPV4_ALLJOYN_MULTICAST_GROUP, IPV6_ALLJOYN_MULTICAST_GROUP, MULTICAST_PORT,
m_broadcast, multicastsockFd);
if (status != ER_OK) {
- QCC_DbgPrintf(("Failed to create multicast socket for NS packets."));
+ QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): Failed to create multicast socket for NS packets."));
qcc::Close(multicastMDNSsockFd);
continue;
}
@@ -1482,7 +1481,8 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
if (m_ipv4UnicastSockFd != qcc::INVALID_SOCKET_FD) {
status = qcc::GetLocalAddress(m_ipv4UnicastSockFd, listenAddr, listenPort);
if (status != ER_OK) {
- QCC_LogError(status, ("GetLocalAddress(%d, %s, %u) failed", m_ipv4UnicastSockFd, listenAddr.ToString().c_str(), listenPort));
+ QCC_LogError(status, ("IpNameServiceImpl::LazyUpdateInterfaces(): GetLocalAddress(%d, %s, %u) failed",
+ m_ipv4UnicastSockFd, listenAddr.ToString().c_str(), listenPort));
}
}
} else if (live.m_interfaceAddr.IsIPv6()) {
@@ -1498,14 +1498,15 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
if (m_ipv6UnicastSockFd != qcc::INVALID_SOCKET_FD) {
status = qcc::GetLocalAddress(m_ipv6UnicastSockFd, listenAddr, listenPort);
if (status != ER_OK) {
- QCC_LogError(status, ("GetLocalAddress(%d, %s, %u) failed", m_ipv6UnicastSockFd, listenAddr.ToString().c_str(), listenPort));
+ QCC_LogError(status, ("IpNameServiceImpl::LazyUpdateInterfaces(): GetLocalAddress(%d, %s, %u) failed",
+ m_ipv6UnicastSockFd, listenAddr.ToString().c_str(), listenPort));
}
}
}
live.m_unicastPort = listenPort;
- QCC_DbgPrintf(("Pushing back interface %s addr %s", live.m_interfaceName.c_str(), entries[i].m_addr.c_str()));
+ QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): Pushing back interface %s addr %s", live.m_interfaceName.c_str(), entries[i].m_addr.c_str()));
//
// Lazy update is called with the mutex taken, so this is safe here.
//
@@ -1589,7 +1590,7 @@ void IpNameServiceImpl::LazyUpdateInterfaces(const qcc::NetworkEventSet& network
}
#endif
if (refreshAdvertisements || processAnyTransport) {
- QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces() is calling m_packetScheduler.Alert(), refreshAdvertisements: %d, processAnyTransport: %d\n", refreshAdvertisements, processAnyTransport));
+ QCC_DbgPrintf(("IpNameServiceImpl::LazyUpdateInterfaces(): Calling m_packetScheduler.Alert(), refreshAdvertisements: %d, processAnyTransport: %d\n", refreshAdvertisements, processAnyTransport));
m_packetScheduler.Alert();
}
}
@@ -5507,7 +5508,7 @@ void* IpNameServiceImpl::Run(void* arg)
void IpNameServiceImpl::GetResponsePackets(std::list<Packet>& packets, bool quietly, const qcc::IPEndpoint destination, uint8_t type,
TransportMask completeTransportMask, const int32_t interfaceIndex, const qcc::AddressFamily family)
{
- m_mutex.Lock(MUTEX_CONTEXT);
+ m_mutex.AssertOwnedByCurrentThread();
bool tcpProcessed = false;
bool udpProcessed = false;
for (uint32_t transportIndex = 0; transportIndex < N_TRANSPORTS; ++transportIndex) {
@@ -5819,12 +5820,11 @@ void IpNameServiceImpl::GetResponsePackets(std::list<Packet>& packets, bool quie
udpProcessed = true;
}
}
- m_mutex.Unlock(MUTEX_CONTEXT);
}
void IpNameServiceImpl::GetQueryPackets(std::list<Packet>& packets, const uint8_t type, const int32_t interfaceIndex, const qcc::AddressFamily family)
{
- m_mutex.Lock(MUTEX_CONTEXT);
+ m_mutex.AssertOwnedByCurrentThread();
for (uint32_t transportIndex = 0; transportIndex < N_TRANSPORTS; ++transportIndex) {
if (m_enableV1 && (type & TRANSMIT_V0_V1) && !m_v0_v1_queries[transportIndex].empty()) {
@@ -6044,7 +6044,6 @@ void IpNameServiceImpl::GetQueryPackets(std::list<Packet>& packets, const uint8_
}
}
}
- m_mutex.Unlock(MUTEX_CONTEXT);
}
void IpNameServiceImpl::Retransmit(uint32_t transportIndex, bool exiting, bool quietly, const qcc::IPEndpoint& destination, const qcc::IPEndpoint& source, uint8_t type, TransportMask completeTransportMask, vector<qcc::String>& wkns, const int32_t interfaceIndex, const qcc::AddressFamily family)
diff --git a/common/inc/qcc/IPAddress.h b/common/inc/qcc/IPAddress.h
index 405fe72..b631d35 100644
--- a/common/inc/qcc/IPAddress.h
+++ b/common/inc/qcc/IPAddress.h
@@ -196,22 +196,12 @@ class IPAddress {
* Test if two IPv6 addresses belong to the same network by comparing
* their most significant 64 bits (routing prefix and subnet id).
*
- * @param other The other IPAddress to compare against.
+ * @param other The other IPAddress to compare against.
+ * @param prefixLen The network prefix length of both networks.
*
* @return "true" if two IPv6 addresses belong to the same network.
*/
- bool IsSameIPv6Network(const IPAddress& other) const
- {
- if ((addrSize != IPv6_SIZE) || (other.addrSize != IPv6_SIZE)) {
- return false;
- }
- for (size_t i = 0; i < (IPv6_SIZE / 2); ++i) {
- if (addr[i] != other.addr[i]) {
- return false;
- }
- }
- return true;
- }
+ bool IsSameIPv6Network(const IPAddress& other, uint32_t prefixLen) const;
/**
* Test if IP address is a loopback address.
diff --git a/common/src/IPAddress.cc b/common/src/IPAddress.cc
index fc9330f..679eba8 100644
--- a/common/src/IPAddress.cc
+++ b/common/src/IPAddress.cc
@@ -90,6 +90,29 @@ IPAddress::IPAddress(const qcc::String& addrString)
}
}
+bool IPAddress::IsSameIPv6Network(const IPAddress& other, uint32_t prefixLen) const
+{
+ if (!this->IsIPv6() || !other.IsIPv6()) {
+ return false;
+ }
+ if (prefixLen == 0) {
+ return true;
+ }
+
+ for (size_t i = 0; i < IPv6_SIZE; ++i) {
+ for (int j = 7; j >= 0; --j) {
+ uint8_t mask = (1 << j);
+ if ((addr[i] & mask) != (other.addr[i] & mask)) {
+ return false;
+ }
+ if (--prefixLen == 0) {
+ return true;
+ }
+ }
+ }
+ return true;
+}
+
qcc::String AJ_CALL IPAddress::IPv4ToString(const uint8_t addr[])
{
qcc::String result = "";
diff --git a/common/unit_test/IPAddressTest.cc b/common/unit_test/IPAddressTest.cc
index c0e9a09..b1a23a7 100644
--- a/common/unit_test/IPAddressTest.cc
+++ b/common/unit_test/IPAddressTest.cc
@@ -218,3 +218,16 @@ TEST(IPAddressTest, string_to_ipv4_negative_test_cases) {
delete [] address_buffer;
address_buffer = NULL;
}
+
+TEST(IPAddressTest, comparing_ipv6_addresses_test_cases) {
+ IPAddress v6ULA1 = qcc::String("2001:db8::ff00:42:8329");
+ IPAddress v6ULA2 = qcc::String("2001:db8:fb0b:12f0::1");
+
+ EXPECT_TRUE(v6ULA1.IsSameIPv6Network(v6ULA1, 128));
+ EXPECT_TRUE(v6ULA2.IsSameIPv6Network(v6ULA2, 128));
+ EXPECT_TRUE(v6ULA1.IsSameIPv6Network(v6ULA1, 256));
+
+ EXPECT_TRUE(v6ULA1.IsSameIPv6Network(v6ULA2, 32));
+ EXPECT_FALSE(v6ULA1.IsSameIPv6Network(v6ULA2, 33));
+}
+