From 53a19d961ccda6b3e124d7fd396c4a03a4f7486c Mon Sep 17 00:00:00 2001 From: Simon Lin Date: Thu, 6 Jan 2022 03:58:51 +0800 Subject: [PATCH] [posix-netif] refactor address changing logs (#7271) --- include/openthread/instance.h | 2 +- include/openthread/ip6.h | 4 +- src/core/net/netif.cpp | 2 +- src/core/net/netif.hpp | 3 +- src/posix/platform/netif.cpp | 76 +++++++++++------------------------ tests/unit/test_netif.cpp | 2 +- 6 files changed, 31 insertions(+), 58 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 30797de4e..c9aaf0b4b 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (181) +#define OPENTHREAD_API_VERSION (182) /** * @addtogroup api-instance diff --git a/include/openthread/ip6.h b/include/openthread/ip6.h index e802359ce..62f18e9b8 100644 --- a/include/openthread/ip6.h +++ b/include/openthread/ip6.h @@ -329,8 +329,8 @@ const otNetifAddress *otIp6GetUnicastAddresses(otInstance *aInstance); * * @retval OT_ERROR_NONE Successfully subscribed to the Network Interface Multicast Address. * @retval OT_ERROR_ALREADY The multicast address is already subscribed. - * @retval OT_ERROR_INVALID_ARGS The IP Address indicated by @p aAddress is invalid address. - * @retval OT_ERROR_INVALID_STATE The Network Interface is not up. + * @retval OT_ERROR_INVALID_ARGS The IP Address indicated by @p aAddress is an invalid multicast address. + * @retval OT_ERROR_REJECTED The IP Address indicated by @p aAddress is an internal multicast address. * @retval OT_ERROR_NO_BUFS The Network Interface is already storing the maximum allowed external multicast * addresses. * diff --git a/src/core/net/netif.cpp b/src/core/net/netif.cpp index 00489e3ee..543ccca3d 100644 --- a/src/core/net/netif.cpp +++ b/src/core/net/netif.cpp @@ -399,7 +399,7 @@ Error Netif::SubscribeExternalMulticast(const Address &aAddress) for (const MulticastAddress *cur = &linkLocalAllRoutersAddress; cur; cur = cur->GetNext()) { - VerifyOrExit(cur->GetAddress() != aAddress, error = kErrorInvalidArgs); + VerifyOrExit(cur->GetAddress() != aAddress, error = kErrorRejected); } entry = mExtMulticastAddressPool.Allocate(); diff --git a/src/core/net/netif.hpp b/src/core/net/netif.hpp index df3c9766e..76cb923d5 100644 --- a/src/core/net/netif.hpp +++ b/src/core/net/netif.hpp @@ -548,7 +548,8 @@ public: * * @retval kErrorNone Successfully subscribed to @p aAddress. * @retval kErrorAlready The multicast address is already subscribed. - * @retval kErrorInvalidArgs The address indicated by @p aAddress is an internal multicast address. + * @retval kErrorInvalidArgs The IP Address indicated by @p aAddress is an invalid multicast address. + * @retval kErrorRejected The IP Address indicated by @p aAddress is an internal multicast address. * @retval kErrorNoBufs The maximum number of allowed external multicast addresses are already added. * */ diff --git a/src/posix/platform/netif.cpp b/src/posix/platform/netif.cpp index e093dddff..b0243617f 100644 --- a/src/posix/platform/netif.cpp +++ b/src/posix/platform/netif.cpp @@ -858,28 +858,24 @@ exit: } } -static void logAddrEvent(bool isAdd, bool isUnicast, struct sockaddr_in6 &addr6, otError error) +static void logAddrEvent(bool isAdd, const ot::Ip6::Address &aAddress, otError error) { - char addressString[INET6_ADDRSTRLEN + 1]; + OT_UNUSED_VARIABLE(aAddress); - // these parameters may not be used if logging is disabled at compile time - OT_UNUSED_VARIABLE(isUnicast); - OT_UNUSED_VARIABLE(addr6); - OT_UNUSED_VARIABLE(addressString); - - if ((error == OT_ERROR_NONE) || ((isAdd) && (error == OT_ERROR_ALREADY)) || + if ((error == OT_ERROR_NONE) || ((isAdd) && (error == OT_ERROR_ALREADY || error == OT_ERROR_REJECTED)) || ((!isAdd) && (error == OT_ERROR_NOT_FOUND))) { - otLogNotePlat("[netif] %s [%s] %s%s", isAdd ? "ADD" : "DEL", isUnicast ? "U" : "M", - inet_ntop(AF_INET6, addr6.sin6_addr.s6_addr, addressString, sizeof(addressString)), - error == OT_ERROR_ALREADY ? " (already subscribed, ignored)" - : error == OT_ERROR_NOT_FOUND ? " (not found, ignored)" : ""); + otLogInfoPlat("[netif] %s [%s] %s%s", isAdd ? "ADD" : "DEL", aAddress.IsMulticast() ? "M" : "U", + aAddress.ToString().AsCString(), + error == OT_ERROR_ALREADY + ? " (already subscribed, ignored)" + : error == OT_ERROR_REJECTED ? " (rejected)" + : error == OT_ERROR_NOT_FOUND ? " (not found, ignored)" : ""); } else { - otLogWarnPlat("[netif] %s [%s] %s failed (%s)", isAdd ? "ADD" : "DEL", isUnicast ? "U" : "M", - inet_ntop(AF_INET6, addr6.sin6_addr.s6_addr, addressString, sizeof(addressString)), - otThreadErrorToString(error)); + otLogWarnPlat("[netif] %s [%s] %s failed (%s)", isAdd ? "ADD" : "DEL", aAddress.IsMulticast() ? "M" : "U", + aAddress.ToString().AsCString(), otThreadErrorToString(error)); } } @@ -934,8 +930,8 @@ static void processNetifAddrEvent(otInstance *aInstance, struct nlmsghdr *aNetli error = otIp6SubscribeMulticastAddress(aInstance, &addr); } - logAddrEvent(/* isAdd */ true, !addr.IsMulticast(), addr6, error); - if (error == OT_ERROR_ALREADY) + logAddrEvent(/* isAdd */ true, addr, error); + if (error == OT_ERROR_ALREADY || error == OT_ERROR_REJECTED) { error = OT_ERROR_NONE; } @@ -953,7 +949,7 @@ static void processNetifAddrEvent(otInstance *aInstance, struct nlmsghdr *aNetli error = otIp6UnsubscribeMulticastAddress(aInstance, &addr); } - logAddrEvent(/* isAdd */ false, !addr.IsMulticast(), addr6, error); + logAddrEvent(/* isAdd */ false, addr, error); if (error == OT_ERROR_NOT_FOUND) { error = OT_ERROR_NONE; @@ -1133,7 +1129,7 @@ static void processNetifAddrEvent(otInstance *aInstance, struct rt_msghdr *rtm) if (subscribed) { - logAddrEvent(/* isAdd */ true, /* isUnicast */ true, addr6, OT_ERROR_ALREADY); + logAddrEvent(/* isAdd */ true, addr, OT_ERROR_ALREADY); error = OT_ERROR_NONE; } else @@ -1183,7 +1179,7 @@ static void processNetifAddrEvent(otInstance *aInstance, struct rt_msghdr *rtm) else { error = otIp6AddUnicastAddress(aInstance, &netAddr); - logAddrEvent(/* isAdd */ true, /* isUnicast */ true, addr6, error); + logAddrEvent(/* isAdd */ true, addr, error); if (error == OT_ERROR_ALREADY) { error = OT_ERROR_NONE; @@ -1198,8 +1194,8 @@ static void processNetifAddrEvent(otInstance *aInstance, struct rt_msghdr *rtm) netAddr.mAddress = addr; error = otIp6SubscribeMulticastAddress(aInstance, &addr); - logAddrEvent(/* isAdd */ true, /* isUnicast */ false, addr6, error); - if (error == OT_ERROR_ALREADY) + logAddrEvent(/* isAdd */ true, addr, error); + if (error == OT_ERROR_ALREADY || error == OT_ERROR_REJECTED) { error = OT_ERROR_NONE; } @@ -1215,7 +1211,7 @@ static void processNetifAddrEvent(otInstance *aInstance, struct rt_msghdr *rtm) if (!addr.IsMulticast()) { error = otIp6RemoveUnicastAddress(aInstance, &addr); - logAddrEvent(/* isAdd */ false, /* isUnicast */ true, addr6, error); + logAddrEvent(/* isAdd */ false, addr, error); if (error == OT_ERROR_NOT_FOUND) { error = OT_ERROR_NONE; @@ -1224,7 +1220,7 @@ static void processNetifAddrEvent(otInstance *aInstance, struct rt_msghdr *rtm) else { error = otIp6UnsubscribeMulticastAddress(aInstance, &addr); - logAddrEvent(/* isAdd */ false, /* isUnicast */ false, addr6, error); + logAddrEvent(/* isAdd */ false, addr, error); if (error == OT_ERROR_NOT_FOUND) { error = OT_ERROR_NONE; @@ -1413,42 +1409,20 @@ static void processMLDEvent(otInstance *aInstance) { MLDv2Record *record = reinterpret_cast(&buffer[offset]); - otError err; - otIp6Address address; + otError err; + ot::Ip6::Address address; memcpy(&address.mFields.m8, &record->mMulticastAddress, sizeof(address.mFields.m8)); inet_ntop(AF_INET6, &record->mMulticastAddress, addressString, sizeof(addressString)); if (record->mRecordType == kICMPv6MLDv2RecordChangeToIncludeType) { err = otIp6SubscribeMulticastAddress(aInstance, &address); - if (err == OT_ERROR_ALREADY) - { - otLogNotePlat( - "[netif] Will not subscribe duplicate multicast address %s", - inet_ntop(AF_INET6, &record->mMulticastAddress, addressString, sizeof(addressString))); - } - else if (err != OT_ERROR_NONE) - { - otLogWarnPlat("[netif] Failed to subscribe multicast address %s: %s", addressString, - otThreadErrorToString(err)); - } - else - { - otLogDebgPlat("[netif] Subscribed multicast address %s", addressString); - } + logAddrEvent(/* isAdd */ true, address, err); } else if (record->mRecordType == kICMPv6MLDv2RecordChangeToExcludeType) { err = otIp6UnsubscribeMulticastAddress(aInstance, &address); - if (err != OT_ERROR_NONE) - { - otLogWarnPlat("[netif] Failed to unsubscribe multicast address %s: %s", addressString, - otThreadErrorToString(err)); - } - else - { - otLogDebgPlat("[netif] Unsubscribed multicast address %s", addressString); - } + logAddrEvent(/* isAdd */ false, address, err); } offset += sizeof(MLDv2Record) + sizeof(in6_addr) * ntohs(record->mNumSources); @@ -1460,8 +1434,6 @@ exit: { freeifaddrs(ifAddrs); } - - return; } #endif diff --git a/tests/unit/test_netif.cpp b/tests/unit/test_netif.cpp index 121f4acb6..9ff41f328 100644 --- a/tests/unit/test_netif.cpp +++ b/tests/unit/test_netif.cpp @@ -200,7 +200,7 @@ void TestNetifMulticastAddresses(void) for (uint8_t i = 0; i < 5; i++) { - VerifyOrQuit(netif.SubscribeExternalMulticast(addresses[i]) == kErrorInvalidArgs, + VerifyOrQuit(netif.SubscribeExternalMulticast(addresses[i]) == kErrorRejected, "SubscribeExternalMulticast() did not fail when address was a default/fixed address"); } }