From 4ffbeb9f994ab0817a0741ae5c0f547d6dc59132 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 4 May 2015 18:29:54 +0200 Subject: [PATCH] [3806] Type to name conversion in Pkt4 class. --- src/bin/dhcp4/dhcp4_messages.mes | 2 +- src/bin/dhcp4/dhcp4_srv.cc | 44 ++++---------------- src/bin/dhcp4/dhcp4_srv.h | 19 --------- src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 32 --------------- src/lib/dhcp/pkt.h | 11 +++++ src/lib/dhcp/pkt4.cc | 50 ++++++++++++++++++++++- src/lib/dhcp/pkt4.h | 16 ++++++++ src/lib/dhcp/pkt6.cc | 2 +- src/lib/dhcp/pkt6.h | 4 +- src/lib/dhcp/tests/pkt4_unittest.cc | 46 +++++++++++++++++++++ 10 files changed, 133 insertions(+), 93 deletions(-) diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index dc3281ad63..442e2c5b36 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -477,7 +477,7 @@ client class has reported a failure. The response packet will not be sent. The argument contains the client and transaction identification information. -% DHCP4_RESPONSE_DATA %1: responding with packet type %2, packet details: "%3" +% DHCP4_RESPONSE_DATA %1: responding with packet %2 (type %3), packet details: "%4" A debug message including the detailed data about the packet being sent to the client. diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e8a6392a5e..f1ca156ff1 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -465,7 +465,7 @@ Dhcpv4Srv::run() { int type = query->getType(); LOG_DEBUG(packet_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_RECEIVED) .arg(query->getLabel()) - .arg(serverReceivedPacketName(type)) + .arg(query->getName()) .arg(type) .arg(query->getRemoteAddr()) .arg(query->getLocalAddr()) @@ -633,12 +633,13 @@ Dhcpv4Srv::run() { LOG_DEBUG(options_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_RESPONSE_DATA) .arg(rsp->getLabel()) + .arg(rsp->getName()) .arg(static_cast(rsp->getType())) .arg(rsp->toText()); LOG_DEBUG(packet_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_SEND) .arg(rsp->getLabel()) - .arg("fixme") + .arg(rsp->getName()) .arg(static_cast(rsp->getType())) .arg(rsp->getLocalAddr()) .arg(rsp->getLocalPort()) @@ -1138,7 +1139,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { LOG_ERROR(bad_packet_logger, DHCP4_PACKET_NAK_0001) .arg(query->getLabel()) .arg(query->getRemoteAddr().toText()) - .arg(serverReceivedPacketName(query->getType())); + .arg(query->getName()); resp->setType(DHCPNAK); resp->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS()); return; @@ -1796,37 +1797,6 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) { return (ex.getResponse()); } -const char* -Dhcpv4Srv::serverReceivedPacketName(uint8_t type) { - static const char* DISCOVER = "DISCOVER"; - static const char* REQUEST = "REQUEST"; - static const char* RELEASE = "RELEASE"; - static const char* DECLINE = "DECLINE"; - static const char* INFORM = "INFORM"; - static const char* UNKNOWN = "UNKNOWN"; - - switch (type) { - case DHCPDISCOVER: - return (DISCOVER); - - case DHCPREQUEST: - return (REQUEST); - - case DHCPRELEASE: - return (RELEASE); - - case DHCPDECLINE: - return (DECLINE); - - case DHCPINFORM: - return (INFORM); - - default: - ; - } - return (UNKNOWN); -} - bool Dhcpv4Srv::accept(const Pkt4Ptr& query) const { // Check that the message type is accepted by the server. We rely on the @@ -1983,7 +1953,7 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& query, RequirementLevel serverid) { if (server_id) { isc_throw(RFCViolation, "Server-id option was not expected, but " << "received in " - << serverReceivedPacketName(query->getType())); + << query->getName()); } break; @@ -1991,7 +1961,7 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& query, RequirementLevel serverid) { if (!server_id) { isc_throw(RFCViolation, "Server-id option was expected, but not " " received in message " - << serverReceivedPacketName(query->getType())); + << query->getName()); } break; @@ -2013,7 +1983,7 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& query, RequirementLevel serverid) { if (!client_id || client_id->len() == client_id->getHeaderLen()) { isc_throw(RFCViolation, "Missing or useless client-id and no HW address " " provided in message " - << serverReceivedPacketName(query->getType())); + << query->getName()); } } diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 86e171e447..6a05a245a7 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -199,25 +199,6 @@ public: /// @brief Instructs the server to shut down. void shutdown(); - /// @brief Return textual type of packet received by server - /// - /// Returns the name of valid packet received by the server (e.g. DISCOVER). - /// If the packet is unknown - or if it is a valid DHCP packet but not one - /// expected to be received by the server (such as an OFFER), the string - /// "UNKNOWN" is returned. This method is used in debug messages. - /// - /// As the operation of the method does not depend on any server state, it - /// is declared static. - /// - /// @todo: This should be named static Pkt4::getName() - /// - /// @param type DHCPv4 packet type - /// - /// @return Pointer to "const" string containing the packet name. - /// Note that this string is statically allocated and MUST NOT - /// be freed by the caller. - static const char* serverReceivedPacketName(uint8_t type); - /// /// @name Public accessors returning values required to (re)open sockets. /// diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 97849fc7ea..f44e7627ee 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -436,38 +436,6 @@ TEST_F(Dhcpv4SrvTest, processDecline) { EXPECT_NO_THROW(srv.processDecline(pkt)); } -TEST_F(Dhcpv4SrvTest, serverReceivedPacketName) { - // Check all possible packet types - for (int itype = 0; itype < 256; ++itype) { - uint8_t type = itype; - - switch (type) { - case DHCPDECLINE: - EXPECT_STREQ("DECLINE", Dhcpv4Srv::serverReceivedPacketName(type)); - break; - - case DHCPDISCOVER: - EXPECT_STREQ("DISCOVER", Dhcpv4Srv::serverReceivedPacketName(type)); - break; - - case DHCPINFORM: - EXPECT_STREQ("INFORM", Dhcpv4Srv::serverReceivedPacketName(type)); - break; - - case DHCPRELEASE: - EXPECT_STREQ("RELEASE", Dhcpv4Srv::serverReceivedPacketName(type)); - break; - - case DHCPREQUEST: - EXPECT_STREQ("REQUEST", Dhcpv4Srv::serverReceivedPacketName(type)); - break; - - default: - EXPECT_STREQ("UNKNOWN", Dhcpv4Srv::serverReceivedPacketName(type)); - } - } -} - // This test verifies that incoming DISCOVER can be handled properly, that an // OFFER is generated, that the response has an address and that address // really belongs to the configured pool. diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h index 5f0c5afebf..43e34879b1 100644 --- a/src/lib/dhcp/pkt.h +++ b/src/lib/dhcp/pkt.h @@ -193,6 +193,17 @@ public: /// @param type message type to be set virtual void setType(uint8_t type) = 0; + /// @brief Returns name of the DHCP message. + /// + /// For all unsupported messages the derived classes must return + /// "UNKNOWN". + /// + /// @return Ponter to "const" string containing DHCP message name. + /// The implementations in the derived classes should statically + /// allocate returned strings and the caller must not release the + /// returned pointer. + virtual const char* getName() const = 0; + /// @brief Sets transaction-id value. /// /// @param transid transaction-id to be set. diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc index 319cd3f634..778f44c460 100644 --- a/src/lib/dhcp/pkt4.cc +++ b/src/lib/dhcp/pkt4.cc @@ -282,6 +282,54 @@ void Pkt4::setType(uint8_t dhcp_type) { } } +const char* +Pkt4::getName(const uint8_t type) { + static const char* DHCPDISCOVER_NAME = "DHCPDISCOVER"; + static const char* DHCPOFFER_NAME = "DHCPOFFER"; + static const char* DHCPREQUEST_NAME = "DHCPREQUEST"; + static const char* DHCPDECLINE_NAME = "DHCPDECLINE"; + static const char* DHCPACK_NAME = "DHCPACK"; + static const char* DHCPNAK_NAME = "DHCPNAK"; + static const char* DHCPRELEASE_NAME = "DHCPRELEASE"; + static const char* DHCPINFORM_NAME = "DHCPINFORM"; + static const char* UNKNOWN_NAME = "UNKNOWN"; + + switch (type) { + case DHCPDISCOVER: + return (DHCPDISCOVER_NAME); + + case DHCPOFFER: + return (DHCPOFFER_NAME); + + case DHCPREQUEST: + return (DHCPREQUEST_NAME); + + case DHCPDECLINE: + return (DHCPDECLINE_NAME); + + case DHCPACK: + return (DHCPACK_NAME); + + case DHCPNAK: + return (DHCPNAK_NAME); + + case DHCPRELEASE: + return (DHCPRELEASE_NAME); + + case DHCPINFORM: + return (DHCPINFORM_NAME); + + default: + ; + } + return (UNKNOWN_NAME); +} + +const char* +Pkt4::getName() const { + return (Pkt4::getName(getType())); +} + std::string Pkt4::getLabel() const { @@ -320,7 +368,7 @@ Pkt4::toText() { for (isc::dhcp::OptionCollection::iterator opt=options_.begin(); opt != options_.end(); ++opt) { - tmp << " " << opt->second->toText() << std::endl; + tmp << " " << opt->second->toText(); } diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h index 71b7293751..f61fab35a8 100644 --- a/src/lib/dhcp/pkt4.h +++ b/src/lib/dhcp/pkt4.h @@ -244,6 +244,22 @@ public: /// @param type message type to be set void setType(uint8_t type); + /// @brief Returns name of the DHCP message. + /// + /// @param type DHCPv4 message type which name should be returned. + /// + /// @return Pointer to the "const" string containing DHCP message name. + /// If the message type is unsupported the "UNKNOWN" is returned. + /// The caller must not release the returned pointer. + static const char* getName(const uint8_t type); + + /// @brief Returns name of the DHCP message. + /// + /// @return Pointer to the "const" string containing DHCP message name. + /// If the message type is unsupported the "UNKNOWN" is returned. + /// The caller must not release the returned pointer. + const char* getName() const; + /// @brief Returns sname field /// /// Note: This is 64 bytes long field. It doesn't have to be diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index a8d6644c17..e8b059e345 100644 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -525,7 +525,7 @@ Pkt6::getOptions(uint16_t opt_type) { } const char* -Pkt6::getName(uint8_t type) { +Pkt6::getName(const uint8_t type) { static const char* CONFIRM = "CONFIRM"; static const char* DECLINE = "DECLINE"; static const char* INFORMATION_REQUEST = "INFORMATION_REQUEST"; diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index 1794b05a66..d76d9bd47c 100644 --- a/src/lib/dhcp/pkt6.h +++ b/src/lib/dhcp/pkt6.h @@ -232,7 +232,7 @@ public: /// @param relay structure with necessary relay information void addRelayInfo(const RelayInfo& relay); - /// @brief Return textual type of packet. + /// @brief Returns name of the DHCPv6 message. /// /// Returns the name of valid packet received by the server (e.g. SOLICIT). /// If the packet is unknown - or if it is a valid DHCP packet but not one @@ -248,7 +248,7 @@ public: /// @return Pointer to "const" string containing the packet name. /// Note that this string is statically allocated and MUST NOT /// be freed by the caller. - static const char* getName(uint8_t type); + static const char* getName(const uint8_t type); /// @brief returns textual representation of packet type. /// diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index 5ab5601291..89ddb1cdbd 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/src/lib/dhcp/tests/pkt4_unittest.cc @@ -916,4 +916,50 @@ TEST_F(Pkt4Test, getLabel) { } +// Tests that the correct DHCPv4 message name is returned for various +// message types. +TEST_F(Pkt4Test, getName) { + // Check all possible packet types + for (int itype = 0; itype < 256; ++itype) { + uint8_t type = itype; + + switch (type) { + case DHCPDISCOVER: + EXPECT_STREQ("DHCPDISCOVER", Pkt4::getName(type)); + break; + + case DHCPOFFER: + EXPECT_STREQ("DHCPOFFER", Pkt4::getName(type)); + break; + + case DHCPREQUEST: + EXPECT_STREQ("DHCPREQUEST", Pkt4::getName(type)); + break; + + case DHCPDECLINE: + EXPECT_STREQ("DHCPDECLINE", Pkt4::getName(type)); + break; + + case DHCPACK: + EXPECT_STREQ("DHCPACK", Pkt4::getName(type)); + break; + + case DHCPNAK: + EXPECT_STREQ("DHCPNAK", Pkt4::getName(type)); + break; + + case DHCPRELEASE: + EXPECT_STREQ("DHCPRELEASE", Pkt4::getName(type)); + break; + + case DHCPINFORM: + EXPECT_STREQ("DHCPINFORM", Pkt4::getName(type)); + break; + + default: + EXPECT_STREQ("UNKNOWN", Pkt4::getName(type)); + } + } +} + } // end of anonymous namespace -- 2.47.3