BIND 10 master, updated. 50d91e4c069c6de13680bfaaee3c56b68d6e4ab1 [master] Merge branch 'trac3200'
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Oct 23 05:59:19 UTC 2013
The branch, master has been updated
via 50d91e4c069c6de13680bfaaee3c56b68d6e4ab1 (commit)
via 45eb1980e4238717e3aee354587eb857759d74e8 (commit)
via c936de6a262f1509d14c3ce457c5c0b27e86a4e2 (commit)
via e135eb799810f5671b622a2e5d5e4bedf545c049 (commit)
via 1bbab8c3c1d12e1a7aaa2e2fc36ab4fc5653c92e (commit)
via 5baa1aeb9435170663bcce936e53fbac6d55eef8 (commit)
via 9a9b9d8e466efb4a3a2e363a65c84ac0a37f2226 (commit)
via e9f4d7e510ae5392cfa25c40c5aebe1a49222c5b (commit)
via beac4c1be1f4063d1b8420d0241f5f384564bba3 (commit)
from 78cb0f4ff0cfba61cf775c178632131f21432304 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 50d91e4c069c6de13680bfaaee3c56b68d6e4ab1
Merge: 78cb0f4 45eb198
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Oct 23 07:12:24 2013 +0200
[master] Merge branch 'trac3200'
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 4 +-
src/bin/dhcp4/dhcp4_srv.cc | 33 +++--
src/bin/dhcp4/tests/dhcp4_test_utils.cc | 219 +++++++++++++++++++++++++-----
src/bin/dhcp4/tests/dhcp4_test_utils.h | 62 ++++++++-
src/lib/dhcp/option_definition.cc | 6 +-
src/lib/dhcp/pkt4.cc | 4 +
src/lib/dhcp/pkt4.h | 1 +
src/lib/dhcp/pkt6.cc | 32 +++--
src/lib/dhcp/pkt6.h | 7 +-
src/lib/dhcp/tests/libdhcp++_unittest.cc | 2 +-
tests/tools/perfdhcp/perf_pkt6.cc | 2 +-
11 files changed, 293 insertions(+), 79 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index fca75bf..1bfce66 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -118,7 +118,7 @@ a lease. It is up to the client to choose one server out of othe advertised
and continue allocation with that server. This is a normal behavior and
indicates successful operation.
-% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id %1, hwaddr %2
+% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id %1, hwaddr %2, client sent yiaddr %3
This message indicates that the server has failed to offer a lease to
the specified client after receiving a DISCOVER message from it. There are
many possible reasons for such a failure.
@@ -128,7 +128,7 @@ This debug message indicates that the server successfully granted a lease
in response to client's REQUEST message. This is a normal behavior and
indicates successful operation.
-% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2
+% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2, client sent yiaddr %3
This message indicates that the server failed to grant a lease to the
specified client after receiving a REQUEST message from it. There are many
possible reasons for such a failure. Additional messages will indicate the
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index f2903ba..1735a42 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -754,13 +754,6 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
opt->setUint32(lease->valid_lft_);
answer->addOption(opt);
- // Router (type 3)
- Subnet::OptionDescriptor opt_routers =
- subnet->getOptionDescriptor("dhcp4", DHO_ROUTERS);
- if (opt_routers.option) {
- answer->addOption(opt_routers.option);
- }
-
// Subnet mask (type 1)
answer->addOption(getNetmaskOption(subnet));
@@ -857,14 +850,17 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
copyDefaultFields(discover, offer);
appendDefaultOptions(offer, DHCPOFFER);
- appendRequestedOptions(discover, offer);
assignLease(discover, offer);
- // There are a few basic options that we always want to
- // include in the response. If client did not request
- // them we append them for him.
- appendBasicOptions(discover, offer);
+ // Adding any other options makes sense only when we got the lease.
+ if (offer->getYiaddr() != IOAddress("0.0.0.0")) {
+ appendRequestedOptions(discover, offer);
+ // There are a few basic options that we always want to
+ // include in the response. If client did not request
+ // them we append them for him.
+ appendBasicOptions(discover, offer);
+ }
return (offer);
}
@@ -880,17 +876,20 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
copyDefaultFields(request, ack);
appendDefaultOptions(ack, DHCPACK);
- appendRequestedOptions(request, ack);
// Note that we treat REQUEST message uniformly, regardless if this is a
// first request (requesting for new address), renewing existing address
// or even rebinding.
assignLease(request, ack);
- // There are a few basic options that we always want to
- // include in the response. If client did not request
- // them we append them for him.
- appendBasicOptions(request, ack);
+ // Adding any other options makes sense only when we got the lease.
+ if (ack->getYiaddr() != IOAddress("0.0.0.0")) {
+ appendRequestedOptions(request, ack);
+ // There are a few basic options that we always want to
+ // include in the response. If client did not request
+ // them we append them for him.
+ appendBasicOptions(request, ack);
+ }
return (ack);
}
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index d085316..56e96f1 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
@@ -22,6 +22,7 @@
#include <dhcp/option_custom.h>
#include <dhcp/iface_mgr.h>
#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/lease.h>
#include <dhcpsrv/lease_mgr.h>
#include <dhcpsrv/lease_mgr_factory.h>
@@ -123,37 +124,101 @@ void Dhcpv4SrvTest::messageCheck(const Pkt4Ptr& q, const Pkt4Ptr& a) {
EXPECT_TRUE(q->getLocalHWAddr() == a->getLocalHWAddr());
EXPECT_TRUE(q->getRemoteHWAddr() == a->getRemoteHWAddr());
- // Check that bare minimum of required options are there.
- // We don't check options requested by a client. Those
- // are checked elsewhere.
- EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
- EXPECT_TRUE(a->getOption(DHO_ROUTERS));
+ // Check that the server identifier is present in the response.
+ // Presence (or absence) of other options is checked elsewhere.
EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
- EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
- EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
- EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
- EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
// Check that something is offered
EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
}
-void Dhcpv4SrvTest::optionsCheck(const Pkt4Ptr& pkt) {
- // Check that the requested and configured options are returned
- // in the ACK message.
- EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME))
- << "domain-name not present in the response";
- EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME_SERVERS))
- << "dns-servers not present in the response";
- EXPECT_TRUE(pkt->getOption(DHO_LOG_SERVERS))
- << "log-servers not present in the response";
- EXPECT_TRUE(pkt->getOption(DHO_COOKIE_SERVERS))
- << "cookie-servers not present in the response";
- // Check that the requested but not configured options are not
- // returned in the ACK message.
- EXPECT_FALSE(pkt->getOption(DHO_LPR_SERVERS))
- << "domain-name present in the response but it is"
- << " expected not to be present";
+::testing::AssertionResult
+Dhcpv4SrvTest::basicOptionsPresent(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option missing in the response";
+ if (!pkt->getOption(DHO_DOMAIN_NAME)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "domain-name " << errmsg));
+
+ } else if (!pkt->getOption(DHO_DOMAIN_NAME_SERVERS)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "dns-servers " << errmsg));
+
+ } else if (!pkt->getOption(DHO_SUBNET_MASK)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "subnet-mask " << errmsg));
+
+ } else if (!pkt->getOption(DHO_ROUTERS)) {
+ return (::testing::AssertionFailure(::testing::Message() << "routers "
+ << errmsg));
+
+ } else if (!pkt->getOption(DHO_DHCP_LEASE_TIME)) {
+ return (::testing::AssertionFailure(::testing::Message() <<
+ "dhcp-lease-time " << errmsg));
+
+ }
+ return (::testing::AssertionSuccess());
+
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::noBasicOptions(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option present in the response";
+ if (pkt->getOption(DHO_DOMAIN_NAME)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "domain-name " << errmsg));
+
+ } else if (pkt->getOption(DHO_DOMAIN_NAME_SERVERS)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "dns-servers " << errmsg));
+
+ } else if (pkt->getOption(DHO_SUBNET_MASK)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "subnet-mask " << errmsg));
+
+ } else if (pkt->getOption(DHO_ROUTERS)) {
+ return (::testing::AssertionFailure(::testing::Message() << "routers "
+ << errmsg));
+
+ } else if (pkt->getOption(DHO_DHCP_LEASE_TIME)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "dhcp-lease-time " << errmsg));
+
+ }
+ return (::testing::AssertionSuccess());
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::requestedOptionsPresent(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option missing in the response";
+ if (!pkt->getOption(DHO_LOG_SERVERS)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "log-servers " << errmsg));
+
+ } else if (!pkt->getOption(DHO_COOKIE_SERVERS)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "cookie-servers " << errmsg));
+
+ }
+ return (::testing::AssertionSuccess());
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::noRequestedOptions(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option present in the response";
+ if (pkt->getOption(DHO_LOG_SERVERS)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "log-servers " << errmsg));
+
+ } else if (pkt->getOption(DHO_COOKIE_SERVERS)) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "cookie-servers " << errmsg));
+
+ }
+ return (::testing::AssertionSuccess());
}
OptionPtr Dhcpv4SrvTest::generateClientId(size_t size /*= 4*/) {
@@ -274,6 +339,48 @@ void Dhcpv4SrvTest::checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_
EXPECT_TRUE(expected_clientid->getData() == opt->getData());
}
+::testing::AssertionResult
+Dhcpv4SrvTest::createPacketFromBuffer(const Pkt4Ptr& src_pkt,
+ Pkt4Ptr& dst_pkt) {
+ // Create on-wire format of the packet. If pack() has been called
+ // on this instance of the packet already, the next call to pack()
+ // should remove all contents of the output buffer.
+ try {
+ src_pkt->pack();
+ } catch (const Exception& ex) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "Failed to parse source packet: "
+ << ex.what()));
+ }
+ // Get the output buffer from the source packet.
+ const util::OutputBuffer& buf = src_pkt->getBuffer();
+ // Create a copy of the packet using the output buffer from the source
+ // packet.
+ try {
+ dst_pkt.reset(new Pkt4(static_cast<const uint8_t*>(buf.getData()),
+ buf.getLength()));
+ } catch (const Exception& ex) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "Failed to create a"
+ " destination packet from"
+ " the buffer: "
+ << ex.what()));
+ }
+
+ try {
+ // Parse the new packet and return to the caller.
+ dst_pkt->unpack();
+ } catch (const Exception& ex) {
+ return (::testing::AssertionFailure(::testing::Message()
+ << "Failed to parse a"
+ << " destination packet: "
+ << ex.what()));
+ }
+
+ return (::testing::AssertionSuccess());
+}
+
+
void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
// Create an instance of the tested class.
boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
@@ -318,17 +425,22 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
// are returned when requested.
configureRequestedOptions();
+ // Create a copy of the original packet by parsing its wire format.
+ // This simulates the real life scenario when we process the packet
+ // which was parsed from its wire format.
+ Pkt4Ptr received;
+ ASSERT_TRUE(createPacketFromBuffer(req, received));
if (msg_type == DHCPDISCOVER) {
ASSERT_NO_THROW(
- rsp = srv->processDiscover(req);
- );
+ rsp = srv->processDiscover(received);
+ );
// Should return OFFER
ASSERT_TRUE(rsp);
EXPECT_EQ(DHCPOFFER, rsp->getType());
} else {
- ASSERT_NO_THROW(rsp = srv->processRequest(req););
+ ASSERT_NO_THROW(rsp = srv->processRequest(received));
// Should return ACK
ASSERT_TRUE(rsp);
@@ -336,27 +448,30 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
}
- messageCheck(req, rsp);
+ messageCheck(received, rsp);
+ // Basic options should be present when we got the lease.
+ EXPECT_TRUE(basicOptionsPresent(rsp));
// We did not request any options so these should not be present
// in the RSP.
- EXPECT_FALSE(rsp->getOption(DHO_LOG_SERVERS));
- EXPECT_FALSE(rsp->getOption(DHO_COOKIE_SERVERS));
- EXPECT_FALSE(rsp->getOption(DHO_LPR_SERVERS));
+ EXPECT_TRUE(noRequestedOptions(rsp));
// Repeat the test but request some options.
// Add 'Parameter Request List' option.
addPrlOption(req);
+ ASSERT_TRUE(createPacketFromBuffer(req, received));
+ ASSERT_TRUE(received->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+
if (msg_type == DHCPDISCOVER) {
- ASSERT_NO_THROW(rsp = srv->processDiscover(req););
+ ASSERT_NO_THROW(rsp = srv->processDiscover(received));
// Should return non-NULL packet.
ASSERT_TRUE(rsp);
EXPECT_EQ(DHCPOFFER, rsp->getType());
} else {
- ASSERT_NO_THROW(rsp = srv->processRequest(req););
+ ASSERT_NO_THROW(rsp = srv->processRequest(received));
// Should return non-NULL packet.
ASSERT_TRUE(rsp);
@@ -364,7 +479,41 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
}
// Check that the requested options are returned.
- optionsCheck(rsp);
+ EXPECT_TRUE(basicOptionsPresent(rsp));
+ EXPECT_TRUE(requestedOptionsPresent(rsp));
+
+ // The following part of the test will test that the NAK is sent when
+ // there is no address pool configured. In the same time, we expect
+ // that the requested options are not included in NAK message, but that
+ // they are only included when yiaddr is set to non-zero value.
+ ASSERT_NO_THROW(subnet_->delPools(Lease::TYPE_V4));
+
+ // There has been a lease allocated for the particular client. So,
+ // even though we deleted the subnet, the client would get the
+ // existing lease (not a NAK). Therefore, we have to change the chaddr
+ // in the packet so as the existing lease is not returned.
+ req->setHWAddr(1, 6, std::vector<uint8_t>(2, 6));
+ ASSERT_TRUE(createPacketFromBuffer(req, received));
+ ASSERT_TRUE(received->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+
+ if (msg_type == DHCPDISCOVER) {
+ ASSERT_NO_THROW(rsp = srv->processDiscover(received));
+ // Should return non-NULL packet.
+ ASSERT_TRUE(rsp);
+ } else {
+ ASSERT_NO_THROW(rsp = srv->processRequest(received));
+ // Should return non-NULL packet.
+ ASSERT_TRUE(rsp);
+ }
+ // We should get the NAK packet with yiaddr set to 0.
+ EXPECT_EQ(DHCPNAK, rsp->getType());
+ ASSERT_EQ("0.0.0.0", rsp->getYiaddr().toText());
+
+ // Make sure that none of the requested options is returned in NAK.
+ // Also options such as Routers or Subnet Mask should not be there,
+ // because lease hasn't been acquired.
+ EXPECT_TRUE(noRequestedOptions(rsp));
+ EXPECT_TRUE(noBasicOptions(rsp));
}
/// @brief This function cleans up after the test.
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index 2e39a8b..ee19728 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -109,10 +109,32 @@ public:
/// @param a answer (server's message)
void messageCheck(const Pkt4Ptr& q, const Pkt4Ptr& a);
- /// @brief Check that requested options are present.
+ /// @brief Check that certain basic (always added when lease is acquired)
+ /// are present in a message.
///
- /// @param pkt packet to be checked.
- void optionsCheck(const Pkt4Ptr& pkt);
+ /// @param pkt A message to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult basicOptionsPresent(const Pkt4Ptr& pkt);
+
+ /// @brief Check that certain basic (always added when lease is acquired)
+ /// are not present.
+ ///
+ /// @param pkt A packet to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult noBasicOptions(const Pkt4Ptr& pkt);
+
+ /// @brief Check that certain requested options are present in the message.
+ ///
+ /// @param pkt A message to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult requestedOptionsPresent(const Pkt4Ptr& pkt);
+
+ /// @brief Check that certain options (requested with PRL option)
+ /// are not present.
+ ///
+ /// @param pkt A packet to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult noRequestedOptions(const Pkt4Ptr& pkt);
/// @brief generates client-id option
///
@@ -183,6 +205,32 @@ public:
/// @return relayed DISCOVER
Pkt4Ptr captureRelayedDiscover();
+ /// @brief Create packet from output buffer of another packet.
+ ///
+ /// This function creates a packet using an output buffer from another
+ /// packet. This imitates reception of a packet from the wire. The
+ /// unpack function is then called to parse packet contents and to
+ /// create a collection of the options carried by this packet.
+ ///
+ /// This function is useful for unit tests which verify that the received
+ /// packet is parsed correctly. In those cases it is usually inappropriate
+ /// to create an instance of the packet, add options, set packet
+ /// fields and use such packet as an input to processDiscover or
+ /// processRequest function. This is because, such a packet has certain
+ /// options already initialized and there is no way to verify that they
+ /// have been initialized when packet instance was created or wire buffer
+ /// processing. By creating a packet from the buffer we guarantee that the
+ /// new packet is entirely initialized during wire data parsing.
+ ///
+ /// @param src_pkt A source packet, to be copied.
+ /// @param [out] dst_pkt A destination packet.
+ ///
+ /// @return assertion result indicating if a function completed with
+ /// success or failure.
+ static ::testing::AssertionResult
+ createPacketFromBuffer(const Pkt4Ptr& src_pkt,
+ Pkt4Ptr& dst_pkt);
+
/// @brief generates a DHCPv4 packet based on provided hex string
///
/// @return created packet
@@ -190,6 +238,14 @@ public:
/// @brief Tests if Discover or Request message is processed correctly
///
+ /// This test verifies that the Parameter Request List option is handled
+ /// correctly, i.e. it checks that certain options are present in the
+ /// server's response when they are requested and that they are not present
+ /// when they are not requested or NAK occurs.
+ ///
+ /// @todo We need an additional test for PRL option using real traffic
+ /// capture.
+ ///
/// @param msg_type DHCPDISCOVER or DHCPREQUEST
void testDiscoverRequest(const uint8_t msg_type);
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 9b93a8e..8c1cad0 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -124,12 +124,14 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
return (factoryGeneric(u, type, begin, end));
case OPT_UINT8_TYPE:
- return (array_type_ ? factoryGeneric(u, type, begin, end) :
+ return (array_type_ ?
+ factoryIntegerArray<uint8_t>(u, type, begin, end) :
factoryInteger<uint8_t>(u, type, getEncapsulatedSpace(),
begin, end, callback));
case OPT_INT8_TYPE:
- return (array_type_ ? factoryGeneric(u, type, begin, end) :
+ return (array_type_ ?
+ factoryIntegerArray<int8_t>(u, type, begin, end) :
factoryInteger<int8_t>(u, type, getEncapsulatedSpace(),
begin, end, callback));
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 4fc1c42..468037a 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -108,6 +108,10 @@ Pkt4::pack() {
isc_throw(InvalidOperation, "Can't build Pkt4 packet. HWAddr not set.");
}
+ // Clear the output buffer to make sure that consecutive calls to pack()
+ // will not result in concatenation of multiple packet copies.
+ buffer_out_.clear();
+
try {
size_t hw_len = hwaddr_->hwaddr_.size();
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index be82e13..c8015cd 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -72,6 +72,7 @@ public:
/// Prepares on-wire format of message and all its options.
/// Options must be stored in options_ field.
/// Output buffer will be stored in buffer_out_.
+ /// The buffer_out_ is cleared before writting to the buffer.
///
/// @throw InvalidOperation if packing fails
void
diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index 9d194a5..57daee0 100644
--- a/src/lib/dhcp/pkt6.cc
+++ b/src/lib/dhcp/pkt6.cc
@@ -43,7 +43,7 @@ Pkt6::Pkt6(const uint8_t* buf, uint32_t buf_len, DHCPv6Proto proto /* = UDP */)
remote_addr_("::"),
local_port_(0),
remote_port_(0),
- bufferOut_(0) {
+ buffer_out_(0) {
data_.resize(buf_len);
memcpy(&data_[0], buf, buf_len);
}
@@ -58,7 +58,7 @@ Pkt6::Pkt6(uint8_t msg_type, uint32_t transid, DHCPv6Proto proto /*= UDP*/) :
remote_addr_("::"),
local_port_(0),
remote_port_(0),
- bufferOut_(0) {
+ buffer_out_(0) {
}
uint16_t Pkt6::len() {
@@ -199,6 +199,8 @@ Pkt6::pack() {
void
Pkt6::packUDP() {
try {
+ // Make sure that the buffer is empty before we start writting to it.
+ buffer_out_.clear();
// is this a relayed packet?
if (!relay_info_.empty()) {
@@ -215,11 +217,11 @@ Pkt6::packUDP() {
relay != relay_info_.end(); ++relay) {
// build relay-forw/relay-repl header (see RFC3315, section 7)
- bufferOut_.writeUint8(relay->msg_type_);
- bufferOut_.writeUint8(relay->hop_count_);
- bufferOut_.writeData(&(relay->linkaddr_.toBytes()[0]),
+ buffer_out_.writeUint8(relay->msg_type_);
+ buffer_out_.writeUint8(relay->hop_count_);
+ buffer_out_.writeData(&(relay->linkaddr_.toBytes()[0]),
isc::asiolink::V6ADDRESS_LEN);
- bufferOut_.writeData(&relay->peeraddr_.toBytes()[0],
+ buffer_out_.writeData(&relay->peeraddr_.toBytes()[0],
isc::asiolink::V6ADDRESS_LEN);
// store every option in this relay scope. Usually that will be
@@ -230,28 +232,28 @@ Pkt6::packUDP() {
for (OptionCollection::const_iterator opt =
relay->options_.begin();
opt != relay->options_.end(); ++opt) {
- (opt->second)->pack(bufferOut_);
+ (opt->second)->pack(buffer_out_);
}
// and include header relay-msg option. Its payload will be
// generated in the next iteration (if there are more relays)
// or outside the loop (if there are no more relays and the
// payload is a direct message)
- bufferOut_.writeUint16(D6O_RELAY_MSG);
- bufferOut_.writeUint16(relay->relay_msg_len_);
+ buffer_out_.writeUint16(D6O_RELAY_MSG);
+ buffer_out_.writeUint16(relay->relay_msg_len_);
}
}
// DHCPv6 header: message-type (1 octect) + transaction id (3 octets)
- bufferOut_.writeUint8(msg_type_);
+ buffer_out_.writeUint8(msg_type_);
// store 3-octet transaction-id
- bufferOut_.writeUint8( (transid_ >> 16) & 0xff );
- bufferOut_.writeUint8( (transid_ >> 8) & 0xff );
- bufferOut_.writeUint8( (transid_) & 0xff );
+ buffer_out_.writeUint8( (transid_ >> 16) & 0xff );
+ buffer_out_.writeUint8( (transid_ >> 8) & 0xff );
+ buffer_out_.writeUint8( (transid_) & 0xff );
// the rest are options
- LibDHCP::packOptions(bufferOut_, options_);
+ LibDHCP::packOptions(buffer_out_, options_);
}
catch (const Exception& e) {
// An exception is thrown and message will be written to Logger
@@ -502,7 +504,7 @@ Pkt6::delOption(uint16_t type) {
}
void Pkt6::repack() {
- bufferOut_.writeData(&data_[0], data_.size());
+ buffer_out_.writeData(&data_[0], data_.size());
}
void
diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index 207f576..702c424 100644
--- a/src/lib/dhcp/pkt6.h
+++ b/src/lib/dhcp/pkt6.h
@@ -115,6 +115,7 @@ public:
/// Options must be stored in options_ field.
/// Output buffer will be stored in data_. Length
/// will be set in data_len_.
+ /// The output buffer is cleared before new data is written to it.
///
/// @throw BadValue if packet protocol is invalid, InvalidOperation
/// if packing fails, or NotImplemented if protocol is TCP (IPv6 over TCP is
@@ -139,7 +140,7 @@ public:
/// zero length
///
/// @return reference to output buffer
- const isc::util::OutputBuffer& getBuffer() const { return (bufferOut_); };
+ const isc::util::OutputBuffer& getBuffer() const { return (buffer_out_); };
/// @brief Returns protocol of this packet (UDP or TCP).
///
@@ -530,7 +531,7 @@ protected:
/// remote TCP or UDP port
uint16_t remote_port_;
- /// output buffer (used during message transmission)
+ /// Output buffer (used during message transmission)
///
/// @warning This protected member is accessed by derived
/// classes directly. One of such derived classes is
@@ -538,7 +539,7 @@ protected:
/// behavior must be taken into consideration before making
/// changes to this member such as access scope restriction or
/// data format change etc.
- isc::util::OutputBuffer bufferOut_;
+ isc::util::OutputBuffer buffer_out_;
/// packet timestamp
boost::posix_time::ptime timestamp_;
diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index 085ff0b..776b084 100644
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc
@@ -706,7 +706,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
typeid(OptionCustom));
LibDhcpTest::testStdOptionDefs4(DHO_DHCP_PARAMETER_REQUEST_LIST, begin, end,
- typeid(Option));
+ typeid(OptionUint8Array));
LibDhcpTest::testStdOptionDefs4(DHO_DHCP_MESSAGE, begin, end,
typeid(OptionString));
diff --git a/tests/tools/perfdhcp/perf_pkt6.cc b/tests/tools/perfdhcp/perf_pkt6.cc
index 56fe9df..0ede077 100644
--- a/tests/tools/perfdhcp/perf_pkt6.cc
+++ b/tests/tools/perfdhcp/perf_pkt6.cc
@@ -43,7 +43,7 @@ PerfPkt6::rawPack() {
options_,
getTransidOffset(),
getTransid(),
- bufferOut_));
+ buffer_out_));
}
bool
More information about the bind10-changes
mailing list