From 027e009005167faae29272ff363ee37f74739f53 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Mon, 29 Aug 2016 09:31:26 -0700 Subject: [PATCH] Simplify ICMPv6 Echo Request/Reply implementation. (#481) - Only support a single ICMPv6 Echo Reply handler. - Remove large static ICMPv6 Echo Request buffer. --- src/cli/cli.cpp | 29 ++++++++---- src/cli/cli.hpp | 3 +- src/core/net/icmp6.cpp | 94 +++++++++++++++---------------------- src/core/net/icmp6.hpp | 103 ++++++++++++++++++----------------------- 4 files changed, 103 insertions(+), 126 deletions(-) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index e697a92b7..523b12682 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -104,13 +104,10 @@ const struct Command Interpreter::sCommands[] = otNetifAddress Interpreter::sAddress; -static otDEFINE_ALIGNED_VAR(sIcmpEchoBuf, sizeof(Ip6::IcmpEcho), uint64_t); -Ip6::IcmpEcho *Interpreter::sIcmpEcho; - static otDEFINE_ALIGNED_VAR(sPingTimerBuf, sizeof(Timer), uint64_t); Timer *Interpreter::sPingTimer; -Ip6::SockAddr Interpreter::sSockAddr; +Ip6::MessageInfo Interpreter::sMessageInfo; Server *Interpreter::sServer; uint8_t Interpreter::sEchoRequest[1500]; uint16_t Interpreter::sLength; @@ -121,7 +118,7 @@ static otNetifAddress sAutoAddresses[8]; void Interpreter::Init(void) { - sIcmpEcho = new(&sIcmpEchoBuf) Ip6::IcmpEcho(&HandleEchoResponse, NULL); + Ip6::Icmp::SetEchoReplyHandler(&HandleEchoResponse, NULL); sPingTimer = new(&sPingTimerBuf) Timer(&HandlePingTimer, NULL); sLength = 8; sCount = 1; @@ -1002,9 +999,9 @@ void Interpreter::ProcessPing(int argc, char *argv[]) VerifyOrExit(argc > 0, error = kThreadError_Parse); VerifyOrExit(!sPingTimer->IsRunning(), error = kThreadError_Busy); - memset(&sSockAddr, 0, sizeof(sSockAddr)); - SuccessOrExit(error = sSockAddr.GetAddress().FromString(argv[0])); - sSockAddr.mScopeId = 1; + memset(&sMessageInfo, 0, sizeof(sMessageInfo)); + SuccessOrExit(error = sMessageInfo.GetPeerAddr().FromString(argv[0])); + sMessageInfo.mInterfaceId = 1; sLength = 8; sCount = 1; @@ -1046,12 +1043,24 @@ exit: void Interpreter::HandlePingTimer(void *aContext) { + ThreadError error = kThreadError_None; uint32_t timestamp = HostSwap32(Timer::GetNow()); + Message *message; - memcpy(sEchoRequest, ×tamp, sizeof(timestamp)); - sIcmpEcho->SendEchoRequest(sSockAddr, sEchoRequest, sLength); + VerifyOrExit((message = Ip6::Icmp::NewMessage(0)) != NULL, error = kThreadError_NoBufs); + SuccessOrExit(error = message->Append(×tamp, sizeof(timestamp))); + SuccessOrExit(error = message->SetLength(sLength)); + + SuccessOrExit(error = Ip6::Icmp::SendEchoRequest(*message, sMessageInfo)); sCount--; +exit: + + if (error != kThreadError_None && message != NULL) + { + Message::Free(*message); + } + if (sCount) { sPingTimer->Start(sInterval); diff --git a/src/cli/cli.hpp b/src/cli/cli.hpp index 72401088e..087fac63f 100644 --- a/src/cli/cli.hpp +++ b/src/cli/cli.hpp @@ -194,8 +194,7 @@ private: static const struct Command sCommands[]; static otNetifAddress sAddress; - static Ip6::SockAddr sSockAddr; - static Ip6::IcmpEcho *sIcmpEcho; + static Ip6::MessageInfo sMessageInfo; static Server *sServer; static uint8_t sEchoRequest[]; static uint16_t sLength; diff --git a/src/core/net/icmp6.cpp b/src/core/net/icmp6.cpp index b9401aed2..06f9136e4 100644 --- a/src/core/net/icmp6.cpp +++ b/src/core/net/icmp6.cpp @@ -47,54 +47,14 @@ namespace Ip6 { bool Icmp::sIsEchoEnabled = true; -uint16_t IcmpEcho::sNextId = 1; -IcmpEcho *IcmpEcho::sEchoClients = NULL; +uint16_t Icmp::sEchoSequence = 1; IcmpHandler *IcmpHandler::sHandlers = NULL; +Icmp::EchoReplyHandler Icmp::sEchoReplyHandler = NULL; +void *Icmp::sEchoReplyContext = NULL; -IcmpEcho::IcmpEcho(EchoReplyHandler aHandler, void *aContext) +Message *Icmp::NewMessage(uint16_t aReserved) { - mHandler = aHandler; - mContext = aContext; - mId = sNextId++; - mSeq = 0; - mNext = sEchoClients; - sEchoClients = this; -} - -ThreadError IcmpEcho::SendEchoRequest(const SockAddr &aDestination, - const void *aPayload, uint16_t aPayloadLength) -{ - ThreadError error = kThreadError_None; - MessageInfo messageInfo; - Message *message = NULL; - IcmpHeader icmp6Header; - - VerifyOrExit((message = Ip6::NewMessage(0)) != NULL, error = kThreadError_NoBufs); - SuccessOrExit(error = message->SetLength(sizeof(icmp6Header) + aPayloadLength)); - - message->Write(sizeof(icmp6Header), aPayloadLength, aPayload); - - icmp6Header.Init(); - icmp6Header.SetType(IcmpHeader::kTypeEchoRequest); - icmp6Header.SetId(mId); - icmp6Header.SetSequence(mSeq++); - message->Write(0, sizeof(icmp6Header), &icmp6Header); - - memset(&messageInfo, 0, sizeof(messageInfo)); - messageInfo.GetPeerAddr() = aDestination.GetAddress(); - messageInfo.mInterfaceId = aDestination.mScopeId; - - SuccessOrExit(error = Ip6::SendDatagram(*message, messageInfo, kProtoIcmp6)); - otLogInfoIcmp("Sent echo request\n"); - -exit: - - if (error != kThreadError_None && message != NULL) - { - Message::Free(*message); - } - - return error; + return Ip6::NewMessage(sizeof(IcmpHeader) + aReserved); } ThreadError Icmp::RegisterCallbacks(IcmpHandler &aHandler) @@ -116,6 +76,35 @@ exit: return error; } +void Icmp::SetEchoReplyHandler(EchoReplyHandler aHandler, void *aContext) +{ + sEchoReplyHandler = aHandler; + sEchoReplyContext = aContext; +} + +ThreadError Icmp::SendEchoRequest(Message &aMessage, const MessageInfo &aMessageInfo) +{ + ThreadError error = kThreadError_None; + MessageInfo messageInfoLocal; + IcmpHeader icmpHeader; + + messageInfoLocal = aMessageInfo; + + icmpHeader.Init(); + icmpHeader.SetType(IcmpHeader::kTypeEchoRequest); + icmpHeader.SetId(1); + icmpHeader.SetSequence(sEchoSequence++); + + SuccessOrExit(error = aMessage.Prepend(&icmpHeader, sizeof(icmpHeader))); + aMessage.SetOffset(0); + SuccessOrExit(error = Ip6::SendDatagram(aMessage, messageInfoLocal, kProtoIcmp6)); + + otLogInfoIcmp("Sent echo request\n"); + +exit: + return error; +} + ThreadError Icmp::SendError(const Address &aDestination, IcmpHeader::Type aType, IcmpHeader::Code aCode, const Header &aHeader) { @@ -176,7 +165,7 @@ ThreadError Icmp::HandleMessage(Message &aMessage, MessageInfo &aMessageInfo) return HandleEchoRequest(aMessage, aMessageInfo); case IcmpHeader::kTypeEchoReply: - return HandleEchoReply(aMessage, aMessageInfo, icmp6Header); + return HandleEchoReply(aMessage, aMessageInfo); case IcmpHeader::kTypeDstUnreach: return HandleDstUnreach(aMessage, aMessageInfo, icmp6Header); @@ -246,18 +235,11 @@ exit: return error; } -ThreadError Icmp::HandleEchoReply(Message &aMessage, const MessageInfo &aMessageInfo, - const IcmpHeader &aIcmpHeader) +ThreadError Icmp::HandleEchoReply(Message &aMessage, const MessageInfo &aMessageInfo) { - VerifyOrExit(sIsEchoEnabled, ;); + VerifyOrExit(sIsEchoEnabled && sEchoReplyHandler, ;); - for (IcmpEcho *client = IcmpEcho::sEchoClients; client; client = client->mNext) - { - if (client->mId == aIcmpHeader.GetId()) - { - client->HandleEchoReply(aMessage, aMessageInfo); - } - } + sEchoReplyHandler(sEchoReplyContext, aMessage, aMessageInfo); exit: return kThreadError_None; diff --git a/src/core/net/icmp6.hpp b/src/core/net/icmp6.hpp index 954075903..1c360b406 100644 --- a/src/core/net/icmp6.hpp +++ b/src/core/net/icmp6.hpp @@ -206,62 +206,6 @@ public: } OT_TOOL_PACKED_END; -/** - * This class implements an ICMPv6 echo client. - * - */ -class IcmpEcho -{ - friend class Icmp; - -public: - /** - * This function pointer is called when receiving an ICMPv6 Echo Reply in response to an Echo Request. - * - * @param[in] aContext A pointer to arbitrary context information. - * @param[in] aMessage A reference to the received message. - * @param[in] aMessageInfo A reference to message information associated with @p aMessage. - * - */ - typedef void (*EchoReplyHandler)(void *aContext, Message &aMessage, const MessageInfo &aMessageInfo); - - /** - * This constructor creates an ICMPv6 echo client. - * - * @param[in] aHandler A pointer to a function that is called when receiving an ICMPv6 Echo Reply. - * @param[in] aContext A pointer to arbitrary context information. - * - */ - IcmpEcho(EchoReplyHandler aHandler, void *aContext); - - /** - * This method sends an ICMPv6 Echo Request message. - * - * @param[in] aDestination The socket address of the destination. - * @param[in] aPayload A pointer to the data payload to send. - * @param[in] aPayloadLength The number of data payload bytes. - * - * @retval kThreadError_None An ICMPv6 Echo Request message was enqueued. - * @retval kThreadError_NoBufs Insufficient buffers available to generate an ICMPv6 Echo Request message. - * - */ - ThreadError SendEchoRequest(const SockAddr &aDestination, const void *aPayload, uint16_t aPayloadLength); - -private: - void HandleEchoReply(Message &message, const MessageInfo &messageInfo) { - mHandler(mContext, message, messageInfo); - } - - EchoReplyHandler mHandler; - void *mContext; - uint16_t mId; - uint16_t mSeq; - IcmpEcho *mNext; - - static uint16_t sNextId; - static IcmpEcho *sEchoClients; -}; - /** * This class implements ICMPv6 message handlers. * @@ -316,6 +260,16 @@ private: class Icmp { public: + /** + * This static method returns a new ICMP message with sufficient header space reserved. + * + * @param[in] aReserved The number of header bytes to reserve after the ICMP header. + * + * @returns A pointer to the message or NULL if no buffers are available. + * + */ + static Message *NewMessage(uint16_t aReserved); + /** * This static method registers ICMPv6 handlers. * @@ -327,6 +281,37 @@ public: */ static ThreadError RegisterCallbacks(IcmpHandler &aHandler); + /** + * This function pointer is called when receiving an ICMPv6 Echo Reply in response to an Echo Request. + * + * @param[in] aContext A pointer to arbitrary context information. + * @param[in] aMessage A reference to the received message. + * @param[in] aMessageInfo A reference to message information associated with @p aMessage. + * + */ + typedef void (*EchoReplyHandler)(void *aContext, Message &aMessage, const MessageInfo &aMessageInfo); + + /** + * This static method sets the Echo Reply handler. + * + * @param[in] aHandler A pointer to a function that is called when receiving an ICMPv6 Echo Reply. + * @param[in] aContext A pointer to arbitrary context information. + * + */ + static void SetEchoReplyHandler(EchoReplyHandler aHandler, void *aContext); + + /** + * This static method sends an ICMPv6 Echo Request message. + * + * @param[in] aMessage A reference to the Echo Request payload. + * @param[in] aMessageInfo A reference to the message info associated with @p aMessage. + * + * @retval kThreadError_None Successfully enqueued the ICMPv6 Echo Request message. + * @retval kThreadError_NoBufs Insufficient buffers available to generate an ICMPv6 Echo Request message. + * + */ + static ThreadError SendEchoRequest(Message &aMessage, const MessageInfo &aMessageInfo); + /** * This static method sends an ICMPv6 error message. * @@ -388,9 +373,11 @@ private: static ThreadError HandleDstUnreach(Message &aMessage, const MessageInfo &aMessageInfo, const IcmpHeader &aIcmpHeader); static ThreadError HandleEchoRequest(Message &aMessage, const MessageInfo &aMessageInfo); - static ThreadError HandleEchoReply(Message &aMessage, const MessageInfo &aMessageInfo, - const IcmpHeader &aIcmpHeader); + static ThreadError HandleEchoReply(Message &aMessage, const MessageInfo &aMessageInfo); + static uint16_t sEchoSequence; + static EchoReplyHandler sEchoReplyHandler; + static void *sEchoReplyContext; static bool sIsEchoEnabled; };