From 7ad13c8adb0d0d7e35071a2198bace75e7cf3740 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 2 Jun 2026 07:45:50 -0700 Subject: [PATCH] [ip4] use `Icmp4Header` directly for ICMPv4 (#13180) This commit updates the codebase to use the `Icmp4Header` type directly, replacing the nested `Ip4::Icmp::Header` type. The empty `Ip4::Icmp` wrapper class is removed to simplify the header definition. This change aligns the ICMPv4 header structure with the flat naming conventions used for other IP headers (e.g., `Ip6::Icmp6Header`, `Ip6::UpdHeader`). --- src/core/net/checksum.cpp | 2 +- src/core/net/ip4_types.hpp | 210 +++++++++++++++--------------- src/core/net/nat64_translator.cpp | 10 +- tests/nexus/test_1_4_PIC_TC_4.cpp | 16 +-- tests/unit/test_checksum.cpp | 4 +- 5 files changed, 121 insertions(+), 121 deletions(-) diff --git a/src/core/net/checksum.cpp b/src/core/net/checksum.cpp index c419acfd6..8fd83e089 100644 --- a/src/core/net/checksum.cpp +++ b/src/core/net/checksum.cpp @@ -219,7 +219,7 @@ void Checksum::UpdateMessageChecksum(Message &aMessage, break; case Ip4::kProtoIcmp: - headerOffset = Ip4::Icmp::Header::kChecksumFieldOffset; + headerOffset = Ip4::Icmp4Header::kChecksumFieldOffset; break; default: diff --git a/src/core/net/ip4_types.hpp b/src/core/net/ip4_types.hpp index 3ae9f51ae..6535e7adb 100644 --- a/src/core/net/ip4_types.hpp +++ b/src/core/net/ip4_types.hpp @@ -563,124 +563,124 @@ private: } OT_TOOL_PACKED_END; /** - * Implements ICMP(v4). - * Note: ICMP(v4) messages will only be generated / handled by NAT64. So only header definition is required. + * Represents an ICMPv4 header. */ -class Icmp +OT_TOOL_PACKED_BEGIN +class Icmp4Header : public Clearable { public: + static constexpr uint16_t kChecksumFieldOffset = 2; ///< The byte offset of the Checksum field in the ICMPv4 header. + /** - * Represents an IPv4 ICMP header. + * ICMPv4 message types. + * + * Only the ICMPv4 types used with NAT64 are listed here. */ - OT_TOOL_PACKED_BEGIN - class Header : public Clearable
+ enum Type : uint8_t { - public: - static constexpr uint16_t kChecksumFieldOffset = 2; - // A few ICMP types, only the ICMP types work with NAT64 are listed here. - enum Type : uint8_t - { - kTypeEchoReply = 0, - kTypeDestinationUnreachable = 3, - kTypeEchoRequest = 8, - kTypeTimeExceeded = 11, - }; + kTypeEchoReply = 0, ///< Echo Reply. + kTypeDestinationUnreachable = 3, ///< Destination Unreachable. + kTypeEchoRequest = 8, ///< Echo Request. + kTypeTimeExceeded = 11, ///< Time Exceeded. + }; - enum Code : uint8_t - { - kCodeNone = 0, - // Destination Unreachable codes - kCodeNetworkUnreachable = 0, - kCodeHostUnreachable = 1, - kCodeProtocolUnreachable = 2, - kCodePortUnreachable = 3, - kCodeSourceRouteFailed = 5, - kCodeNetworkUnknown = 6, - kCodeHostUnknown = 7, - }; + /** + * ICMPv4 message codes. + */ + enum Code : uint8_t + { + kCodeNone = 0, ///< None. - /** - * Returns the type of the ICMP message. - * - * @returns The type field of the ICMP message. - */ - uint8_t GetType(void) const { return mType; } + kCodeNetworkUnreachable = 0, ///< Network Unreachable. + kCodeHostUnreachable = 1, ///< Host Unreachable. + kCodeProtocolUnreachable = 2, ///< Protocol Unreachable. + kCodePortUnreachable = 3, ///< Port Unreachable. + kCodeSourceRouteFailed = 5, ///< Source Route Failed. + kCodeNetworkUnknown = 6, ///< Network Unknown. + kCodeHostUnknown = 7, ///< Host Unknown. + }; - /** - * Sets the type of the ICMP message. - * - * @param[in] aType The type of the ICMP message. - */ - void SetType(uint8_t aType) { mType = aType; } + /** + * Returns the ICMPv4 message type. + * + * @returns The ICMPv4 message type. + */ + uint8_t GetType(void) const { return mType; } - /** - * Returns the code of the ICMP message. - * - * @returns The code field of the ICMP message. - */ - uint8_t GetCode(void) const { return mCode; } + /** + * Sets the ICMPv4 message type. + * + * @param[in] aType The ICMPv4 message type. + */ + void SetType(uint8_t aType) { mType = aType; } - /** - * Sets the code of the ICMP message. - * - * @param[in] aCode The code of the ICMP message. - */ - void SetCode(uint8_t aCode) { mCode = aCode; } + /** + * Returns the ICMPv4 message code. + * + * @returns The ICMPv4 message code. + */ + uint8_t GetCode(void) const { return mCode; } - /** - * Sets the checksum field in the ICMP message. - * - * @returns The checksum of the ICMP message. - */ - uint16_t GetChecksum(void) const { return BigEndian::HostSwap16(mChecksum); } + /** + * Sets the ICMPv4 message code. + * + * @param[in] aCode The ICMPv4 message code. + */ + void SetCode(uint8_t aCode) { mCode = aCode; } - /** - * Sets the checksum field in the ICMP message. - * - * @param[in] aChecksum The checksum of the ICMP message. - */ - void SetChecksum(uint16_t aChecksum) { mChecksum = BigEndian::HostSwap16(aChecksum); } + /** + * Returns the ICMPv4 message checksum. + * + * @returns The ICMPv4 message checksum. + */ + uint16_t GetChecksum(void) const { return BigEndian::HostSwap16(mChecksum); } - /** - * Returns the ICMPv4 message ID for Echo Requests and Replies. - * - * @returns The ICMPv4 message ID. - */ - uint16_t GetId(void) const { return BigEndian::HostSwap16(mData.m16[0]); } + /** + * Sets the ICMPv4 message checksum. + * + * @param[in] aChecksum The ICMPv4 message checksum. + */ + void SetChecksum(uint16_t aChecksum) { mChecksum = BigEndian::HostSwap16(aChecksum); } - /** - * Sets the ICMPv4 message ID for Echo Requests and Replies. - * - * @param[in] aId The ICMPv4 message ID. - */ - void SetId(uint16_t aId) { mData.m16[0] = BigEndian::HostSwap16(aId); } + /** + * Returns the ICMPv4 message ID for Echo Requests and Replies. + * + * @returns The ICMPv4 message ID. + */ + uint16_t GetId(void) const { return BigEndian::HostSwap16(mData.m16[0]); } - /** - * Returns the rest of header field in the ICMP message. - * - * @returns The rest of header field in the ICMP message. The returned buffer has 4 octets. - */ - const uint8_t *GetRestOfHeader(void) const { return mData.m8; } + /** + * Sets the ICMPv4 message ID for Echo Requests and Replies. + * + * @param[in] aId The ICMPv4 message ID. + */ + void SetId(uint16_t aId) { mData.m16[0] = BigEndian::HostSwap16(aId); } - /** - * Sets the rest of header field in the ICMP message. - * - * @param[in] aRestOfHeader The rest of header field in the ICMP message. The buffer should have 4 octets. - */ - void SetRestOfHeader(const uint8_t *aRestOfHeader) { memcpy(mData.m8, aRestOfHeader, sizeof(mData)); } + /** + * Returns the "Rest of Header" field in the ICMPv4 message. + * + * @returns The "Rest of Header" field in the ICMPv4 message. The returned buffer has 4 octets. + */ + const uint8_t *GetRestOfHeader(void) const { return mData.m8; } - private: - uint8_t mType; - uint8_t mCode; - uint16_t mChecksum; - union OT_TOOL_PACKED_FIELD - { - uint8_t m8[4]; - uint16_t m16[2]; - uint32_t m32[1]; - } mData; - } OT_TOOL_PACKED_END; -}; + /** + * Sets the "Rest of Header" field in the ICMPv4 message. + * + * @param[in] aRestOfHeader The "Rest of Header" field in the ICMPv4 message. The buffer should have 4 octets. + */ + void SetRestOfHeader(const uint8_t *aRestOfHeader) { memcpy(mData.m8, aRestOfHeader, sizeof(mData)); } + +private: + uint8_t mType; + uint8_t mCode; + uint16_t mChecksum; + union OT_TOOL_PACKED_FIELD + { + uint8_t m8[4]; + uint16_t m16[2]; + uint32_t m32[1]; + } mData; +} OT_TOOL_PACKED_END; // Internet Protocol Numbers static constexpr uint8_t kProtoTcp = Ip6::kProtoTcp; ///< Transmission Control Protocol @@ -691,7 +691,7 @@ using TcpHeader = Ip6::TcpHeader; ///< TCP header (the same as in IPv6) using UdpHeader = Ip6::UdpHeader; ///< UDP header (the same as in IPv6) /** - * Represents parsed IPv4 header along with UDP/TCP/ICMP4 headers from a received message/frame. + * Represents parsed IPv4 header along with UDP/TCP/ICMPv4 headers from a received message/frame. */ class Headers : private Clearable { @@ -799,7 +799,7 @@ public: * * @returns The ICMPv4 header. */ - const Icmp::Header &GetIcmpHeader(void) const { return mHeader.mIcmp; } + const Icmp4Header &GetIcmpHeader(void) const { return mHeader.mIcmp; } /** * Returns the source port number if the header is UDP or TCP, or zero otherwise @@ -833,9 +833,9 @@ private: Header mIp4Header; union { - UdpHeader mUdp; - TcpHeader mTcp; - Icmp::Header mIcmp; + UdpHeader mUdp; + TcpHeader mTcp; + Icmp4Header mIcmp; } mHeader; }; diff --git a/src/core/net/nat64_translator.cpp b/src/core/net/nat64_translator.cpp index 332417d6f..8b4d41fe0 100644 --- a/src/core/net/nat64_translator.cpp +++ b/src/core/net/nat64_translator.cpp @@ -623,7 +623,7 @@ exit: Error Translator::TranslateIcmp4(Message &aMessage, uint16_t aOriginalId) { Error error = kErrorNone; - Ip4::Icmp::Header icmp4Header; + Ip4::Icmp4Header icmp4Header; Ip6::Icmp::Header icmp6Header; // TODO: Implement the translation of other ICMP messages. @@ -634,7 +634,7 @@ Error Translator::TranslateIcmp4(Message &aMessage, uint16_t aOriginalId) switch (icmp4Header.GetType()) { - case Ip4::Icmp::Header::Type::kTypeEchoReply: + case Ip4::Icmp4Header::Type::kTypeEchoReply: // The only difference between ICMPv6 echo and ICMP4 echo is // the message type field, so we can reinterpret it as ICMP6 // header and set the message type. @@ -656,7 +656,7 @@ exit: Error Translator::TranslateIcmp6(Message &aMessage, uint16_t aTranslatedId) { Error error = kErrorNone; - Ip4::Icmp::Header icmp4Header; + Ip4::Icmp4Header icmp4Header; Ip6::Icmp::Header icmp6Header; // TODO: Implement the translation of other ICMP messages. @@ -669,10 +669,10 @@ Error Translator::TranslateIcmp6(Message &aMessage, uint16_t aTranslatedId) { case Ip6::Icmp::Header::kTypeEchoRequest: // The only difference between ICMPv6 echo and ICMP4 echo is - // the message type field, so we can reinterpret it as ICMP6 + // the message type field, so we can reinterpret it as ICMP4 // header and set the message type. SuccessOrExit(error = aMessage.Read(0, icmp4Header)); - icmp4Header.SetType(Ip4::Icmp::Header::Type::kTypeEchoRequest); + icmp4Header.SetType(Ip4::Icmp4Header::Type::kTypeEchoRequest); icmp4Header.SetId(aTranslatedId); aMessage.Write(0, icmp4Header); break; diff --git a/tests/nexus/test_1_4_PIC_TC_4.cpp b/tests/nexus/test_1_4_PIC_TC_4.cpp index 2575e8ee6..6d8aa4042 100644 --- a/tests/nexus/test_1_4_PIC_TC_4.cpp +++ b/tests/nexus/test_1_4_PIC_TC_4.cpp @@ -120,13 +120,13 @@ static void HandleIp4Receive(otMessage *aMessage, void *aContext) if (ip4Headers.IsIcmp4()) { Log("Received ICMPv4 packet"); - Ip4::Icmp::Header icmpHeader; + Ip4::Icmp4Header icmpHeader; SuccessOrQuit(message->Read(sizeof(Ip4::Header), icmpHeader)); - if (icmpHeader.GetType() == Ip4::Icmp::Header::kTypeEchoRequest) + if (icmpHeader.GetType() == Ip4::Icmp4Header::kTypeEchoRequest) { Log("Received ICMPv4 Echo Request, sending reply"); - uint16_t payloadLen = message->GetLength() - sizeof(Ip4::Header) - sizeof(Ip4::Icmp::Header); + uint16_t payloadLen = message->GetLength() - sizeof(Ip4::Header) - sizeof(Ip4::Icmp4Header); OwnedPtr replyMessage; replyMessage.Reset(br1->Get().Allocate(Message::kTypeIp4)); @@ -135,22 +135,22 @@ static void HandleIp4Receive(otMessage *aMessage, void *aContext) Ip4::Header ip4Header; ip4Header.Clear(); ip4Header.SetVersionIhl(0x45); - ip4Header.SetTotalLength(sizeof(Ip4::Header) + sizeof(Ip4::Icmp::Header) + payloadLen); + ip4Header.SetTotalLength(sizeof(Ip4::Header) + sizeof(Ip4::Icmp4Header) + payloadLen); ip4Header.SetProtocol(Ip4::kProtoIcmp); ip4Header.SetTtl(64); ip4Header.SetSource(ip4Headers.GetDestinationAddress()); ip4Header.SetDestination(ip4Headers.GetSourceAddress()); SuccessOrQuit(replyMessage->Append(ip4Header)); - Ip4::Icmp::Header replyIcmpHeader; + Ip4::Icmp4Header replyIcmpHeader; replyIcmpHeader.Clear(); - replyIcmpHeader.SetType(Ip4::Icmp::Header::kTypeEchoReply); + replyIcmpHeader.SetType(Ip4::Icmp4Header::kTypeEchoReply); replyIcmpHeader.SetRestOfHeader(icmpHeader.GetRestOfHeader()); SuccessOrQuit(replyMessage->Append(replyIcmpHeader)); // Append the rest of the ICMP request (Id, Sequence, and Payload) - SuccessOrQuit(replyMessage->AppendBytesFromMessage( - *message, sizeof(Ip4::Header) + sizeof(Ip4::Icmp::Header), payloadLen)); + SuccessOrQuit(replyMessage->AppendBytesFromMessage(*message, sizeof(Ip4::Header) + sizeof(Ip4::Icmp4Header), + payloadLen)); replyMessage->SetOffset(sizeof(Ip4::Header)); Checksum::UpdateMessageChecksum(*replyMessage, ip4Header.GetSource(), ip4Header.GetDestination(), diff --git a/tests/unit/test_checksum.cpp b/tests/unit/test_checksum.cpp index d2ed5e908..8e409bdd2 100644 --- a/tests/unit/test_checksum.cpp +++ b/tests/unit/test_checksum.cpp @@ -432,8 +432,8 @@ void TestIcmp4MessageChecksum(void) Ip4::Address source; Ip4::Address dest; - uint8_t mPayload[sizeof(kExampleIcmpMessage)]; - Ip4::Icmp::Header icmpHeader; + uint8_t mPayload[sizeof(kExampleIcmpMessage)]; + Ip4::Icmp4Header icmpHeader; SuccessOrQuit(message->AppendBytes(kExampleIcmpMessage, sizeof(kExampleIcmpMessage)));