From: Marcin Siodelski Date: Fri, 18 Jul 2014 16:32:59 +0000 (+0200) Subject: [3390] Addressed review comments. X-Git-Tag: trac3482_base~79^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cc0fc02c624cfc7cb3ec1f15006b3abd6570831e;p=thirdparty%2Fkea.git [3390] Addressed review comments. --- diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index b1d7c10a28..28766dab6d 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -1078,8 +1078,8 @@ temporarily override a list of interface names and listen on all interfaces. { "subnet": "192.0.2.0/24" "option-data": [ {" - "name": "routers", - "code": 3, + "name": "domain-name-servers", + "code": 6, "data": "192.0.2.200,192.0.2.201", "csv-format": true, "space": "dhcp4" @@ -1760,8 +1760,8 @@ temporarily override a list of interface names and listen on all interfaces. The following standards and draft standards are currently supported: - RFC 2131: Supported messages are DISCOVER, OFFER, - REQUEST, RELEASE, INFORM, ACK, and NAK. + RFC 2131: Supported messages are DISCOVER (1), OFFER (2), + REQUEST (3), RELEASE (7), INFORM (8), ACK (5), and NAK(6). RFC 2132: diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 78dddab86c..bbb8c8f0d0 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1164,6 +1164,13 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) { // If received relayed message, server responds to the relay address. if (question->isRelayed()) { + // The client should set the ciaddr when sending the DHCPINFORM + // but in case he didn't, the relay may not be able to determine the + // address of the client, because yiaddr is not set when responding + // to Confirm and the only address available was the source address + // of the client. The source address is however not used here because + // the message is relayed. Therefore, we set the BROADCAST flag so + // as the relay can broadcast the packet. if ((question->getType() == DHCPINFORM) && (question->getCiaddr() == zero_addr)) { response->setFlags(BOOTP_BROADCAST); @@ -1418,6 +1425,9 @@ Dhcpv4Srv::processDecline(Pkt4Ptr& /* decline */) { Pkt4Ptr Dhcpv4Srv::processInform(Pkt4Ptr& inform) { + // DHCPINFORM MUST not include server identifier. + sanityCheck(inform, FORBIDDEN); + Pkt4Ptr ack = Pkt4Ptr(new Pkt4(DHCPACK, inform->getTransid())); copyDefaultFields(inform, ack); appendRequestedOptions(inform, ack); diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc index 42900ba172..9d9428c82d 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.cc +++ b/src/bin/dhcp4/tests/dhcp4_client.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -165,11 +166,9 @@ Dhcp4Client::generateHWAddr(const uint8_t htype) const { "The harware address type " << static_cast(htype) << " is currently not supported"); } - std::vector hwaddr; + std::vector hwaddr(HWAddr::ETHERNET_HWADDR_LEN); // Generate ethernet hardware address by assigning random byte values. - for (int i = 0; i < HWAddr::ETHERNET_HWADDR_LEN; ++i) { - hwaddr.push_back(static_cast(rand() % 255)); - } + isc::util::fillRandom(hwaddr.begin(), hwaddr.end()); return (HWAddrPtr(new HWAddr(hwaddr, htype))); } diff --git a/src/bin/dhcp4/tests/dhcp4_client.h b/src/bin/dhcp4/tests/dhcp4_client.h index 07457fb7df..258d35e44e 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.h +++ b/src/bin/dhcp4/tests/dhcp4_client.h @@ -57,15 +57,24 @@ public: /// @brief Holds the configuration of the client received from the /// DHCP server. struct Configuration { + /// @brief Holds IP addresses received in the Routers option. Option4AddrLst::AddressContainer routers_; + /// @brief Holds IP addresses received in the DNS Servers option. Option4AddrLst::AddressContainer dns_servers_; + /// @brief Holds IP addresses received in the Log Servers option. Option4AddrLst::AddressContainer log_servers_; + /// @brief Holds IP addresses received in the Quotes Servers option. Option4AddrLst::AddressContainer quotes_servers_; + /// @brief Holds a lease obtained by the client. Lease4 lease_; + /// @brief Holds server id of the server which responded to the client's + /// request. asiolink::IOAddress serverid_; + /// @brief Constructor. Configuration(); + /// @brief Sets configuration values to defaults. void reset(); }; @@ -95,7 +104,12 @@ public: /// @brief Sends DHCPINFORM message to the server and receives response. /// /// This function simulates sending the DHCPINFORM message to the server - /// and receiving server's response (if any). + /// and receiving server's response (if any). The server's response and the + /// message sent to the server is stored in the context structure and can + /// be accessed using @c getContext function. + /// + /// The configuration returned by the server is stored in the + /// @c config_ public member and can be accessed directly. /// /// @param set_ciaddr Indicates if the ciaddr should be set for an /// outgoing message and defaults to true. Note, that the RFC2131 mandates @@ -186,6 +200,7 @@ public: /// @param use Parameter which 'true' value indicates that client should /// simulate sending messages via relay. /// @param relay_addr Relay address + /// @param sf_relay_addr Server facing relay address. void useRelay(const bool use = true, const asiolink::IOAddress& relay_addr = asiolink::IOAddress("192.0.2.2"), diff --git a/src/bin/dhcp4/tests/inform_unittest.cc b/src/bin/dhcp4/tests/inform_unittest.cc index 494ea77420..ee8dd5e5aa 100644 --- a/src/bin/dhcp4/tests/inform_unittest.cc +++ b/src/bin/dhcp4/tests/inform_unittest.cc @@ -149,9 +149,10 @@ TEST_F(InformTest, directClientBroadcast) { client.requestOptions(DHO_LOG_SERVERS, DHO_COOKIE_SERVERS); // Preconfigure the client with the IP address. client.createLease(IOAddress("10.0.0.56"), 600); + // Send DHCPINFORM message to the server. - client.doInform(); - // ASSERT_NO_THROW(client.doInform()); + ASSERT_NO_THROW(client.doInform()); + // Make sure that the server responded. ASSERT_TRUE(client.getContext().response_); Pkt4Ptr resp = client.getContext().response_; @@ -163,6 +164,7 @@ TEST_F(InformTest, directClientBroadcast) { EXPECT_FALSE(resp->isRelayed()); // Make sure that the server id is present. EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the Routers option has been received. ASSERT_EQ(2, client.config_.routers_.size()); EXPECT_EQ("10.0.0.200", client.config_.routers_[0].toText()); @@ -186,16 +188,20 @@ TEST_F(InformTest, directClientBroadcast) { // This time do not request Quotes Servers option and it should not // be returned. client.requestOptions(DHO_LOG_SERVERS); + // Send DHCPINFORM. ASSERT_NO_THROW(client.doInform()); - ASSERT_TRUE(client.getContext().response_); + + // Make sure that the server responded. resp = client.getContext().response_; + ASSERT_TRUE(resp); // Make sure that the server has responded with DHCPACK. ASSERT_EQ(DHCPACK, static_cast(resp->getType())); // Response should have been unicast to the ciaddr. EXPECT_EQ(IOAddress("10.0.0.12"), resp->getLocalAddr()); // Response must not be relayed. EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); // Make sure that the Routers option has been received.