BIND 10 trac3181, updated. 840afe672b9002ddecba485aec09ad085e09baeb [3181] Addressed review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Dec 10 13:10:55 UTC 2013
The branch, trac3181 has been updated
via 840afe672b9002ddecba485aec09ad085e09baeb (commit)
from 7a96b9bb8c38e0e45b44db09c5c2b015c2609cb6 (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 840afe672b9002ddecba485aec09ad085e09baeb
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Dec 10 14:10:40 2013 +0100
[3181] Addressed review comments.
-----------------------------------------------------------------------
Summary of changes:
tests/tools/perfdhcp/command_options.cc | 22 ++++-----
tests/tools/perfdhcp/rate_control.cc | 24 +++++++++-
tests/tools/perfdhcp/rate_control.h | 47 +++++++++++++-------
tests/tools/perfdhcp/stats_mgr.h | 2 +-
tests/tools/perfdhcp/test_control.cc | 21 +++++++++
.../perfdhcp/tests/command_options_unittest.cc | 8 +++-
.../tools/perfdhcp/tests/rate_control_unittest.cc | 28 ++++++++++--
tests/tools/perfdhcp/tests/stats_mgr_unittest.cc | 20 +++------
8 files changed, 126 insertions(+), 46 deletions(-)
-----------------------------------------------------------------------
diff --git a/tests/tools/perfdhcp/command_options.cc b/tests/tools/perfdhcp/command_options.cc
index db3305a..26255e8 100644
--- a/tests/tools/perfdhcp/command_options.cc
+++ b/tests/tools/perfdhcp/command_options.cc
@@ -743,8 +743,8 @@ CommandOptions::validate() const {
"-r<rate> must be set to use -D<max-drop>");
check((getRate() != 0) && (getRenewRate() + getReleaseRate() > getRate()),
"The sum of Renew rate (-f<renew-rate>) and Release rate"
- " (-F<release-rate>) must not be greater than the rate specified"
- " as -r<rate>");
+ " (-F<release-rate>) must not be greater than the exchange"
+ " rate specified as -r<rate>");
check((getRate() == 0) && (getRenewRate() != 0),
"Renew rate specified as -f<renew-rate> must not be specified"
" when -r<rate> parameter is not specified");
@@ -1014,14 +1014,16 @@ CommandOptions::usage() const {
"\n"
"DHCPv6 only options:\n"
"-c: Add a rapid commit option (exchanges will be SA).\n"
- "-f<renew-rate>: A rate at which IPv6 Renew requests are sent to\n"
- " a server. The sum of this value and release-rate must be equal\n"
- " or lower than the rate specified as -r<rate>. If -r<rate> is\n"
- " not specified, this parameter must not be specified too.\n"
- "-F<release-rate>: A rate at which IPv6 Release requests are sent to\n"
- " a server. The sum of this value and renew-rate must be equal or\n"
- " lower than the rate specified as -r<rate>. If -r<rate> is not\n"
- " specified, this parameter must not be specified too.\n"
+ "-f<renew-rate>: Rate at which IPv6 Renew requests are sent to\n"
+ " a server. This value is only valid when used in conjunction with\n"
+ " the exchange rate (given by -r<rate>). Furthermore the sum of\n"
+ " this value and the release-rate (given by -F<rate) must be equal\n"
+ " to or less than the exchange rate.\n"
+ "-F<release-rate>: Rate at which IPv6 Release requests are sent to\n"
+ " a server. This value is only valid when used in conjunction with\n"
+ " the exchange rate (given by -r<rate>). Furthermore the sum of\n"
+ " this value and the renew-rate (given by -f<rate) must be equal\n"
+ " to or less than the exchange rate.\n"
"\n"
"The remaining options are used only in conjunction with -r:\n"
"\n"
diff --git a/tests/tools/perfdhcp/rate_control.cc b/tests/tools/perfdhcp/rate_control.cc
index 04cfa81..06cdd4e 100644
--- a/tests/tools/perfdhcp/rate_control.cc
+++ b/tests/tools/perfdhcp/rate_control.cc
@@ -28,10 +28,14 @@ RateControl::RateControl()
RateControl::RateControl(const int rate, const int aggressivity)
: send_due_(currentTime()), last_sent_(currentTime()),
aggressivity_(aggressivity), rate_(rate), late_sent_(false) {
- if (aggressivity < 1) {
+ if (aggressivity_ < 1) {
isc_throw(isc::BadValue, "invalid value of aggressivity "
<< aggressivity << ", expected value is greater than 0");
}
+ if (rate_ < 0) {
+ isc_throw(isc::BadValue, "invalid value of rate " << rate
+ << ", expected non-negative value");
+ }
}
uint64_t
@@ -121,6 +125,24 @@ RateControl::updateSendDue() {
}
void
+RateControl::setAggressivity(const int aggressivity) {
+ if (aggressivity < 1) {
+ isc_throw(isc::BadValue, "invalid value of aggressivity "
+ << aggressivity << ", expected value is greater than 0");
+ }
+ aggressivity_ = aggressivity;
+}
+
+void
+RateControl::setRate(const int rate) {
+ if (rate < 0) {
+ isc_throw(isc::BadValue, "invalid value of rate " << rate
+ << ", expected non-negative value");
+ }
+ rate_ = rate;
+}
+
+void
RateControl::setRelativeDue(const int offset) {
send_due_ = offset > 0 ?
currentTime() + seconds(abs(offset)) :
diff --git a/tests/tools/perfdhcp/rate_control.h b/tests/tools/perfdhcp/rate_control.h
index c52f264..cd49c2c 100644
--- a/tests/tools/perfdhcp/rate_control.h
+++ b/tests/tools/perfdhcp/rate_control.h
@@ -26,7 +26,7 @@ namespace perfdhcp {
/// of the specific type are sent by perfdhcp. Each message type,
/// for which the desired rate can be specified, has a corresponding
/// \c RateControl object. So, the perfdhcp is using up to three objects
-/// of this type in the same time, to control the rate of the following
+/// of this type at the same time, to control the rate of the following
/// messages being sent:
/// - Discover(DHCPv4) or Solicit (DHCPv6)
/// - Renew (DHCPv6) or Request (DHCPv4) to renew leases.
@@ -34,16 +34,15 @@ namespace perfdhcp {
///
/// The purpose of the RateControl class is to track the due time for
/// sending next message (or bunch of messages) to keep outbound rate
-/// of particular messages on the desired level. The due time is calculated
+/// of particular messages at the desired level. The due time is calculated
/// using the desired rate value and the timestamp when the last message of
/// the particular type has been sent. That puts the responsibility on the
/// \c TestControl class to invoke the \c RateControl::updateSendDue, every
/// time the message is sent.
///
/// The \c RateControl object returns the number of messages to be sent at
-/// the time. Typically the number returned is 0, if perfdhcp shouldn't send
-/// any messages yet, or 1 (sometimes more) if the send due time has been
-/// reached.
+/// the time. The number returned is 0, if perfdhcp shouldn't send any messages
+/// yet, or 1 (sometimes more) if the send due time has been reached.
class RateControl {
public:
@@ -68,7 +67,7 @@ public:
/// \brief Returns number of messages to be sent "now".
///
- /// This function calculates how many masseges of the given type should
+ /// This function calculates how many messages of the given type should
/// be sent immediately when the call to the function returns, to catch
/// up with the desired message rate.
///
@@ -77,6 +76,25 @@ public:
/// the due time has been hit, the non-zero number of messages is returned.
/// If the due time hasn't been hit, the number returned is 0.
///
+ /// If the rate is non-zero, the number of messages to be sent is calculated
+ /// as follows:
+ /// \code
+ /// num = duration * rate
+ /// \endcode
+ /// where <b>duration</b> is a time period between the due time to send
+ /// next set of messages and current time. The duration is expressed in
+ /// seconds with the fractional part having 6 or 9 digits (depending on
+ /// the timer resolution). If the calculated value is equal to 0, it is
+ /// rounded to 1, so as at least one message is sent.
+ ///
+ /// The value of aggressivity limits the maximal number of messages to
+ /// be sent one after another. If the number of messages calculated with
+ /// the equation above exceeds the aggressivity, this function will return
+ /// the value equal to aggressivity.
+ ///
+ /// If the rate is not specified (equal to 0), the value calculated by
+ /// this function is equal to aggressivity.
+ ///
/// \return A number of messages to be sent immediately.
uint64_t getOutboundMessageCount();
@@ -88,7 +106,7 @@ public:
/// \brief Returns the value of the late send flag.
///
/// The flag returned by this function indicates whether the new due time
- /// calculated by the \c RateControl::updateSendDue has been in the past.
+ /// calculated by the \c RateControl::updateSendDue is in the past.
/// This value is used by the \c TestControl object to increment the counter
/// of the late sent messages in the \c StatsMgr.
bool isLateSent() const {
@@ -97,17 +115,16 @@ public:
/// \brief Sets the value of aggressivity.
///
- /// \param aggressivity A new value of aggressivity.
- void setAggressivity(const int aggressivity) {
- aggressivity_ = aggressivity;
- }
+ /// \param aggressivity A new value of aggressivity. This value must be
+ /// a positive integer.
+ /// \throw isc::BadValue if new value is not a positive integer.
+ void setAggressivity(const int aggressivity);
/// \brief Sets the new rate.
///
- /// \param rate A new value of rate.
- void setRate(const int rate) {
- rate_ = rate;
- }
+ /// \param rate A new value of rate. This value must not be negative.
+ /// \throw isc::BadValue if new rate is negative.
+ void setRate(const int rate);
/// \brief Sets the value of the due time.
///
diff --git a/tests/tools/perfdhcp/stats_mgr.h b/tests/tools/perfdhcp/stats_mgr.h
index 0ab4ce1..7486b9e 100644
--- a/tests/tools/perfdhcp/stats_mgr.h
+++ b/tests/tools/perfdhcp/stats_mgr.h
@@ -1181,7 +1181,7 @@ public:
///
/// \param xchg_type exchange type.
/// \return string representing name of the exchange.
- std::string exchangeToString(ExchangeType xchg_type) const {
+ static std::string exchangeToString(ExchangeType xchg_type) {
switch(xchg_type) {
case XCHG_DO:
return("DISCOVER-OFFER");
diff --git a/tests/tools/perfdhcp/test_control.cc b/tests/tools/perfdhcp/test_control.cc
index 6e9e1b9..01ceb89 100644
--- a/tests/tools/perfdhcp/test_control.cc
+++ b/tests/tools/perfdhcp/test_control.cc
@@ -1117,14 +1117,35 @@ TestControl::processReceivedPacket6(const TestControlSocket& socket,
}
}
} else if (packet_type == DHCPV6_REPLY) {
+ // If the received message is Reply, we have to find out which exchange
+ // type the Reply message belongs to. It is doable by matching the Reply
+ // transaction id with the transaction id of the sent Request, Renew
+ // or Release. First we start with the Request.
if (stats_mgr6_->passRcvdPacket(StatsMgr6::XCHG_RR, pkt6)) {
+ // The Reply belongs to Request-Reply exchange type. So, we may need
+ // to keep this Reply in the storage if Renews or/and Releases are
+ // being sent. Note that, Reply messages hold the information about
+ // leases assigned. We use this information to construct Renew and
+ // Release messages.
if (stats_mgr6_->hasExchangeStats(StatsMgr6::XCHG_RN) ||
stats_mgr6_->hasExchangeStats(StatsMgr6::XCHG_RL)) {
+ // Renew or Release messages are sent, because StatsMgr has the
+ // specific exchange type specified. Let's append the Reply
+ // message to a storage.
reply_storage_.append(pkt6);
}
+ // The Reply message is not a server's response to the Request message
+ // sent within the 4-way exchange. It may be a response to the Renew
+ // or Release message. In the if clause we first check if StatsMgr
+ // has exchange type for Renew specified, and if it has, if there is
+ // a corresponding Renew message for the received Reply. If not,
+ // we check that StatsMgr has exchange type for Release specified,
+ // as possibly the Reply has been sent in response to Release.
} else if (!(stats_mgr6_->hasExchangeStats(StatsMgr6::XCHG_RN) &&
stats_mgr6_->passRcvdPacket(StatsMgr6::XCHG_RN, pkt6)) &&
stats_mgr6_->hasExchangeStats(StatsMgr6::XCHG_RL)) {
+ // At this point, it is only possible that the Reply has been sent
+ // in response to a Release. Try to match the Reply with Release.
stats_mgr6_->passRcvdPacket(StatsMgr6::XCHG_RL, pkt6);
}
}
diff --git a/tests/tools/perfdhcp/tests/command_options_unittest.cc b/tests/tools/perfdhcp/tests/command_options_unittest.cc
index 0696783..3d2ce2e 100644
--- a/tests/tools/perfdhcp/tests/command_options_unittest.cc
+++ b/tests/tools/perfdhcp/tests/command_options_unittest.cc
@@ -351,7 +351,10 @@ TEST_F(CommandOptionsTest, RenewRate) {
EXPECT_THROW(process("perfdhcp -6 -r 10 -f 11 -l ethx all"),
isc::InvalidParameter);
// The renew-rate of 0 is invalid.
- EXPECT_THROW(process("perfdhcp -6 -r 10 -f 0 - l ethx all"),
+ EXPECT_THROW(process("perfdhcp -6 -r 10 -f 0 -l ethx all"),
+ isc::InvalidParameter);
+ // The negative renew-rate is invalid.
+ EXPECT_THROW(process("perfdhcp -6 -r 10 -f -5 -l ethx all"),
isc::InvalidParameter);
// If -r<rate> is not specified the -f<renew-rate> should not
// be accepted.
@@ -387,6 +390,9 @@ TEST_F(CommandOptionsTest, ReleaseRate) {
// The release-rate of 0 is invalid.
EXPECT_THROW(process("perfdhcp -6 -r 10 -F 0 -l ethx all"),
isc::InvalidParameter);
+ // The negative rlease-rate is invalid.
+ EXPECT_THROW(process("perfdhcp -6 -r 10 -F -5 -l ethx all"),
+ isc::InvalidParameter);
// If -r<rate> is not specified the -F<release-rate> should not
// be accepted.
EXPECT_THROW(process("perfdhcp -6 -F 10 -l ethx all"),
diff --git a/tests/tools/perfdhcp/tests/rate_control_unittest.cc b/tests/tools/perfdhcp/tests/rate_control_unittest.cc
index ae1897a..99fc35e 100644
--- a/tests/tools/perfdhcp/tests/rate_control_unittest.cc
+++ b/tests/tools/perfdhcp/tests/rate_control_unittest.cc
@@ -80,6 +80,8 @@ TEST(RateControl, constructor) {
// The 0 value of aggressivity < 1 is not acceptable.
EXPECT_THROW(RateControl(3, 0), isc::BadValue);
+ // The negative value of rate is not acceptable.
+ EXPECT_THROW(RateControl(-1, 3), isc::BadValue);
}
// Check the aggressivity accessor.
@@ -98,7 +100,8 @@ TEST(RateControl, getDue) {
ASSERT_FALSE(rc.getDue().is_not_a_date_time());
rc.send_due_ = NakedRateControl::currentTime();
EXPECT_TRUE(NakedRateControl::currentTime() >= rc.getDue());
- rc.send_due_ = NakedRateControl::currentTime() + boost::posix_time::seconds(10);
+ rc.send_due_ = NakedRateControl::currentTime() +
+ boost::posix_time::seconds(10);
EXPECT_TRUE(NakedRateControl::currentTime() < rc.getDue());
}
@@ -126,7 +129,7 @@ TEST(RateControl, isLateSent) {
// it is quite hard to fully test this function as its behaviour strongly
// depends on time.
TEST(RateControl, getOutboundMessageCount) {
- NakedRateControl rc1;
+ NakedRateControl rc1(1000, 1);
// Set the timestamp of the last sent message well to the past.
// The resulting due time will be in the past too.
rc1.last_sent_ =
@@ -146,12 +149,29 @@ TEST(RateControl, getOutboundMessageCount) {
// to the future. If the resulting due time is well in the future too,
// the number of messages to be sent must be 0.
NakedRateControl rc3(10, 3);
- rc3.last_sent_ = NakedRateControl::currentTime() + boost::posix_time::seconds(5);
+ rc3.last_sent_ = NakedRateControl::currentTime() +
+ boost::posix_time::seconds(5);
ASSERT_NO_THROW(count = rc3.getOutboundMessageCount());
EXPECT_EQ(0, count);
}
+// Test the aggressivity modifier for valid and invalid values.
+TEST(RateControl, setAggressivity) {
+ NakedRateControl rc;
+ ASSERT_NO_THROW(rc.setAggressivity(1));
+ EXPECT_THROW(rc.setAggressivity(0), isc::BadValue);
+ EXPECT_THROW(rc.setAggressivity(-1), isc::BadValue);
+}
+
+// Test the rate modifier for valid and invalid rate values.
+TEST(RateControl, setRate) {
+ NakedRateControl rc;
+ EXPECT_NO_THROW(rc.setRate(1));
+ EXPECT_NO_THROW(rc.setRate(0));
+ EXPECT_THROW(rc.setRate(-1), isc::BadValue);
+}
+
// Test the function which calculates the due time to send next set of
// messages.
TEST(RateControl, updateSendDue) {
@@ -170,7 +190,7 @@ TEST(RateControl, updateSendDue) {
last_send_due = rc.send_due_;
ASSERT_NO_THROW(rc.updateSendDue());
// The value should be modified to the new value.
- EXPECT_TRUE(rc.send_due_ != last_send_due);
+ EXPECT_TRUE(rc.send_due_ > last_send_due);
}
diff --git a/tests/tools/perfdhcp/tests/stats_mgr_unittest.cc b/tests/tools/perfdhcp/tests/stats_mgr_unittest.cc
index 348ab42..6147bf7 100644
--- a/tests/tools/perfdhcp/tests/stats_mgr_unittest.cc
+++ b/tests/tools/perfdhcp/tests/stats_mgr_unittest.cc
@@ -266,24 +266,16 @@ TEST_F(StatsMgrTest, MultipleExchanges) {
TEST_F(StatsMgrTest, ExchangeToString) {
// Test DHCPv4 specific exchange names.
- StatsMgr4 stats_mgr4;
- stats_mgr4.addExchangeStats(StatsMgr4::XCHG_DO);
- stats_mgr4.addExchangeStats(StatsMgr4::XCHG_RA);
EXPECT_EQ("DISCOVER-OFFER",
- stats_mgr4.exchangeToString(StatsMgr4::XCHG_DO));
- EXPECT_EQ("REQUEST-ACK", stats_mgr4.exchangeToString(StatsMgr4::XCHG_RA));
+ StatsMgr4::exchangeToString(StatsMgr4::XCHG_DO));
+ EXPECT_EQ("REQUEST-ACK", StatsMgr4::exchangeToString(StatsMgr4::XCHG_RA));
// Test DHCPv6 specific exchange names.
- StatsMgr6 stats_mgr6;
- stats_mgr6.addExchangeStats(StatsMgr6::XCHG_SA);
- stats_mgr6.addExchangeStats(StatsMgr6::XCHG_RR);
- stats_mgr6.addExchangeStats(StatsMgr6::XCHG_RN);
- stats_mgr6.addExchangeStats(StatsMgr6::XCHG_RL);
EXPECT_EQ("SOLICIT-ADVERTISE",
- stats_mgr6.exchangeToString(StatsMgr6::XCHG_SA));
- EXPECT_EQ("REQUEST-REPLY", stats_mgr6.exchangeToString(StatsMgr6::XCHG_RR));
- EXPECT_EQ("RENEW-REPLY", stats_mgr6.exchangeToString(StatsMgr6::XCHG_RN));
- EXPECT_EQ("RELEASE-REPLY", stats_mgr6.exchangeToString(StatsMgr6::XCHG_RL));
+ StatsMgr6::exchangeToString(StatsMgr6::XCHG_SA));
+ EXPECT_EQ("REQUEST-REPLY", StatsMgr6::exchangeToString(StatsMgr6::XCHG_RR));
+ EXPECT_EQ("RENEW-REPLY", StatsMgr6::exchangeToString(StatsMgr6::XCHG_RN));
+ EXPECT_EQ("RELEASE-REPLY", StatsMgr6::exchangeToString(StatsMgr6::XCHG_RL));
}
More information about the bind10-changes
mailing list