From: Tomek Mrugalski <tomasz@isc.org>
Date: Wed, 14 Feb 2018 13:36:57 +0000 (+0000)
Subject: [github55] Changes after review:
X-Git-Tag: trac5502_base~5^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a4ffb5ada32e7956508ad281261af48e75d2f63;p=thirdparty%2Fkea.git

[github55] Changes after review:

 - lateExit renamed to waitToExit, logic updated
 - option renamed to exit-wait-time (-W)
---

diff --git a/src/bin/perfdhcp/command_options.cc b/src/bin/perfdhcp/command_options.cc
index 1cf3c43f5f..7d6babffc8 100644
--- a/src/bin/perfdhcp/command_options.cc
+++ b/src/bin/perfdhcp/command_options.cc
@@ -124,7 +124,7 @@ CommandOptions::reset() {
     mac_list_file_.clear();
     mac_list_.clear();
     num_request_.clear();
-    late_exit_delay_ = 0;
+    exit_wait_time_ = 0;
     period_ = 0;
     drop_time_set_ = 0;
     drop_time_.assign(dt, dt + 2);
@@ -217,8 +217,8 @@ CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
 
     // In this section we collect argument values from command line
     // they will be tuned and validated elsewhere
-    while((opt = getopt(argc, argv, "hv46A:r:t:R:b:n:p:d:D:l:P:a:L:M:m:"
-                        "s:iBc1T:X:O:E:S:I:x:w:e:f:F:")) != -1) {
+    while((opt = getopt(argc, argv, "hv46A:r:t:R:b:n:p:d:D:l:P:a:L:M:"
+                        "s:iBc1T:X:O:E:S:I:x:W:w:e:f:F:")) != -1) {
         stream << " -" << static_cast<char>(opt);
         if (optarg) {
             stream << " " << optarg;
@@ -369,9 +369,9 @@ CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
             loadMacs();
             break;
 
-        case 'm':
+        case 'W':
             // 'm' for moratorium
-            late_exit_delay_ = nonNegativeInteger("value of late exit delay: "
+            exit_wait_time_ = nonNegativeInteger("value of exist wait time: "
                                                   "-m<value> must not be a "
                                                   "negative integer");
             break;
@@ -1074,6 +1074,9 @@ CommandOptions::usage() const {
         "-T<template-file>: The name of a file containing the template to use\n"
         "    as a stream of hexadecimal digits.\n"
         "-v: Report the version number of this program.\n"
+        "-W<time>: Specifies exit-wait-time parameter, that makes perfdhcp wait\n"
+        "    for <time> us after an exit condition has been met to receive all\n"
+        "    packets without sending any new packets. Expressed in microseconds.\n"
         "-w<wrapped>: Command to call with start/stop at the beginning/end of\n"
         "    the program.\n"
         "-x<diagnostic-selector>: Include extended diagnostics in the output.\n"
diff --git a/src/bin/perfdhcp/command_options.h b/src/bin/perfdhcp/command_options.h
index 73a74c7761..0c95030439 100644
--- a/src/bin/perfdhcp/command_options.h
+++ b/src/bin/perfdhcp/command_options.h
@@ -250,7 +250,7 @@ public:
     /// @brief Returns the time in microseconds to delay the program by.
     ///
     /// @return the time in microseconds to delay the program by.
-    int getLateExitDelay() const { return late_exit_delay_; }
+    int getExitWaitTime() const { return exit_wait_time_; }
 
     /// \brief Checks if seed provided.
     ///
@@ -493,110 +493,148 @@ private:
     /// IP protocol version to be used, expected values are:
     /// 4 for IPv4 and 6 for IPv6, default value 0 means "not set"
     uint8_t ipversion_;
+
     /// Packet exchange mode (e.g. DORA/SARR)
     ExchangeMode exchange_mode_;
+
     /// Lease Type to be obtained: address only, IPv6 prefix only.
     LeaseType lease_type_;
+
     /// Rate in exchange per second
     int rate_;
+
     /// A rate at which DHCPv6 Renew messages are sent.
     int renew_rate_;
+
     /// A rate at which DHCPv6 Release messages are sent.
     int release_rate_;
-    /// Delay between generation of two consecutive
-    /// performance reports
+
+    /// Delay between generation of two consecutive performance reports
     int report_delay_;
+
     /// Number of simulated clients (aka randomization range).
     uint32_t clients_num_;
+
     /// MAC address template used to generate unique MAC
     /// addresses for simulated clients.
     std::vector<uint8_t> mac_template_;
+
     /// DUID template used to generate unique DUIDs for
     /// simulated clients
     std::vector<uint8_t> duid_template_;
+
     /// Collection of base values specified with -b<value>
     /// options. Supported "bases" are mac=<mac> and duid=<duid>
     std::vector<std::string> base_;
+
     /// Number of microseconds by which you should delay the exit
-    int late_exit_delay_;
+    int exit_wait_time_;
+
     /// Number of 2 or 4-way exchanges to perform.
     std::vector<int> num_request_;
+
     /// Test period in seconds
     int period_;
+
     /// Indicates number of -d<value> parameters specified by user.
     /// If this value goes above 2, command line parsing fails.
     uint8_t drop_time_set_;
+
     /// Time to elapse before request is lost. The first value of
     /// two-element vector refers to DO/SA exchanges,
     /// second value refers to RA/RR. Default values are { 1, 1 }
     std::vector<double> drop_time_;
+
     /// Maximum number of drops request before aborting test.
     /// First value of two-element vector specifies maximum
     /// number of drops for DO/SA exchange, second value
     /// specifies maximum number of drops for RA/RR.
     std::vector<int> max_drop_;
+
     /// Maximal percentage of lost requests before aborting test.
     /// First value of two-element vector specifies percentage for
     /// DO/SA exchanges, second value for RA/RR.
     std::vector<double> max_pdrop_;
+
     /// Local address or interface specified with -l<value> option.
     std::string localname_;
+
     /// Indicates that specified value with -l<value> is
     /// rather interface (not address)
     bool is_interface_;
+
     /// Number of preload packets. Preload packets are used to
     /// initiate communication with server before doing performance
     /// measurements.
     int preload_;
+
     /// Number of exchanges sent before next pause.
     int aggressivity_;
+
     /// Local port number (host endian)
     int local_port_;
+
     /// Randomization seed.
     uint32_t seed_;
+
     /// Indicates that randomization seed was provided.
     bool seeded_;
+
     /// Indicates that we use broadcast address.
     bool broadcast_;
+
     /// Indicates that we do rapid commit option.
     bool rapid_commit_;
+
     /// Indicates that we take server id from first received packet.
     bool use_first_;
+
     /// Packet template file names. These files store template packets
     /// that are used for initiating exchanges. Template packets
     /// read from files are later tuned with variable data.
     std::vector<std::string> template_file_;
+
     /// Location of a file containing a list of MAC addresses, one per line.
     /// This can be used if you don't want to generate MAC address from a
     /// base MAC address, but rather provide the file with a list of MAC
     /// addresses to be randomly picked. Note that in DHCPv6 those MAC
     /// addresses will be used to generate DUID-LL.
     std::string mac_list_file_;
+
     /// List of MAC addresses loaded from a file.
     std::vector<std::vector<uint8_t> > mac_list_;
+
     /// Offset of transaction id in template files. First vector
     /// element points to offset for DISCOVER/SOLICIT messages,
     /// second element points to transaction id offset for
     /// REQUEST messages
     std::vector<int> xid_offset_;
+
     /// Random value offset in templates. Random value offset
     /// points to last octet of DUID. Up to 4 last octets of
     /// DUID are randomized to simulate different clients.
     std::vector<int> rnd_offset_;
+
     /// Offset of elapsed time option in template packet.
     int elp_offset_;
+
     /// Offset of server id option in template packet.
     int sid_offset_;
+
     /// Offset of requested ip data in template packet
     int rip_offset_;
+
     /// String representing diagnostic selectors specified
     /// by user with -x<value>.
     std::string diags_;
+
     /// Command to be executed at the beginning/end of the test.
     /// This command is expected to expose start and stop argument.
     std::string wrapped_;
+
     /// Server name specified as last argument of command line.
     std::string server_name_;
+
     /// Indicates how many DHCPv6 relay agents are simulated.
     uint8_t v6_relay_encapsulation_level_;
 };
diff --git a/src/bin/perfdhcp/test_control.cc b/src/bin/perfdhcp/test_control.cc
index 20b11c1fe0..011894b6f9 100644
--- a/src/bin/perfdhcp/test_control.cc
+++ b/src/bin/perfdhcp/test_control.cc
@@ -50,20 +50,27 @@ TestControl::hasLateExitCommenced() const {
 }
 
 bool
-TestControl::lateExit() const {
-    if (haveAllPacketsBeenReceived()) {
-        return true;
-    }
-    const ptime now = microsec_clock::universal_time();
-    if (late_exit_target_time_.is_not_a_date_time()) {
-        CommandOptions& options = CommandOptions::instance();
-        late_exit_target_time_ =
-            now + time_duration(microseconds(options.getLateExitDelay()));
-    }
-    if (late_exit_target_time_ <= now) {
-        return true;
+TestControl::waitToExit() const {
+    static ptime exit_time = ptime(not_a_date_time);
+    CommandOptions& options = CommandOptions::instance();
+    uint32_t wait_time = options.getExitWaitTime();
+
+    // If we care and not all packets are in yet
+    if (wait_time && !haveAllPacketsBeenReceived()) {
+        const ptime now = microsec_clock::universal_time();
+
+        // Init the end time if it hasn't started yet
+        if (exit_time.is_not_a_date_time()) {
+            CommandOptions& options = CommandOptions::instance();
+            exit_time = now + time_duration(microseconds(wait_time));
+        }
+
+        // If we're not at end time yet, return true
+        return (now < exit_time);
     }
-    return false;
+
+    // No need to wait, return false;
+    return (false);
 }
 
 bool
@@ -77,31 +84,21 @@ TestControl::haveAllPacketsBeenReceived() const {
         return false;
     }
 
-    const uint32_t& request_count_DO_SA = num_request[0];
-    uint32_t request_count_RA_RR;
+    uint32_t responses = 0;
+    uint32_t requests = num_request[0];
     if (num_request_size >= 2) {
-        request_count_RA_RR = num_request[1];
-    } else {
-        request_count_RA_RR = num_request[0];
+        requests += num_request[1];
     }
 
     if (ipversion == 4) {
-        if (stats_mgr4_->getRcvdPacketsNum(StatsMgr4::XCHG_DO) !=
-                request_count_DO_SA ||
-            stats_mgr4_->getRcvdPacketsNum(StatsMgr4::XCHG_RA) !=
-                request_count_RA_RR) {
-            return false;
-        }
-    } else if (ipversion == 6) {
-        if (stats_mgr6_->getRcvdPacketsNum(StatsMgr6::XCHG_SA) !=
-                request_count_DO_SA ||
-            stats_mgr6_->getRcvdPacketsNum(StatsMgr6::XCHG_RR) !=
-                request_count_RA_RR) {
-            return false;
-        }
+        responses = stats_mgr4_->getRcvdPacketsNum(StatsMgr4::XCHG_DO) +
+                    stats_mgr4_->getRcvdPacketsNum(StatsMgr4::XCHG_RA);
+    } else {
+        responses = stats_mgr6_->getRcvdPacketsNum(StatsMgr6::XCHG_SA) +
+                    stats_mgr6_->getRcvdPacketsNum(StatsMgr6::XCHG_RR);
     }
 
-    return true;
+    return (responses == requests);
 }
 
 TestControl::TestControlSocket::TestControlSocket(const int socket) :
@@ -258,7 +255,7 @@ TestControl::checkExitConditions() const {
         if (testDiags('e')) {
             std::cout << "reached test-period." << std::endl;
         }
-        if (lateExit()) {
+        if (!waitToExit()) {
             return true;
         }
     }
@@ -296,7 +293,7 @@ TestControl::checkExitConditions() const {
         if (testDiags('e')) {
             std::cout << "Reached max requests limit." << std::endl;
         }
-        if (lateExit()) {
+        if (!waitToExit()) {
             return true;
         }
     }
@@ -334,7 +331,7 @@ TestControl::checkExitConditions() const {
         if (testDiags('e')) {
             std::cout << "Reached maximum drops number." << std::endl;
         }
-        if (lateExit()) {
+        if (!waitToExit()) {
             return true;
         }
     }
@@ -381,7 +378,7 @@ TestControl::checkExitConditions() const {
         if (testDiags('e')) {
             std::cout << "Reached maximum percentage of drops." << std::endl;
         }
-        if (lateExit()) {
+        if (!waitToExit()) {
             return true;
         }
     }
diff --git a/src/bin/perfdhcp/test_control.h b/src/bin/perfdhcp/test_control.h
index b7ddb3f9b7..39ce8e28d4 100644
--- a/src/bin/perfdhcp/test_control.h
+++ b/src/bin/perfdhcp/test_control.h
@@ -138,14 +138,14 @@ public:
 
     /// @brief Delay the exit by a fixed given time to catch up to all exchanges
     ///     that were already started.
-    bool lateExit() const;
+    /// @return true if need to wait, false = ok to exit now
+    bool waitToExit() const;
 
     /// @brief Check if the program is in that period where the program was
     ///     bound to exit, but was delayed by lateExit().
     bool hasLateExitCommenced() const;
 
-    /// @brief Delay the exit by a fixed given time to catch up to all exchanges
-    ///     that were already started.
+    /// @brief Checks if all expected packets were already received
     bool haveAllPacketsBeenReceived() const;
 
     /// \brief Socket wrapper structure.