BIND 10 trac1959, updated. c2fa97e23503ea00f82e0adff38d14473f475da8 [1959] Implemented changes from the 3rd part of code review.

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Sep 4 14:39:32 UTC 2012


The branch, trac1959 has been updated
       via  c2fa97e23503ea00f82e0adff38d14473f475da8 (commit)
      from  ad297b10887895eae9cb6e3c5e00e38f7d1f5fda (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 c2fa97e23503ea00f82e0adff38d14473f475da8
Author: Marcin Siodelski <marcin at isc.org>
Date:   Tue Sep 4 16:39:23 2012 +0200

    [1959] Implemented changes from the 3rd part of code review.

-----------------------------------------------------------------------

Summary of changes:
 tests/tools/perfdhcp/test_control.cc               |  118 ++++++++----
 tests/tools/perfdhcp/test_control.h                |  192 ++++++++++----------
 .../tools/perfdhcp/tests/test_control_unittest.cc  |   18 +-
 3 files changed, 183 insertions(+), 145 deletions(-)

-----------------------------------------------------------------------
diff --git a/tests/tools/perfdhcp/test_control.cc b/tests/tools/perfdhcp/test_control.cc
index 3c0f6d1..6e7fa05 100644
--- a/tests/tools/perfdhcp/test_control.cc
+++ b/tests/tools/perfdhcp/test_control.cc
@@ -45,14 +45,21 @@ namespace perfdhcp {
 
 bool TestControl::interrupted_ = false;
 
-TestControl::TestControlSocket::TestControlSocket(int socket) :
-    socket_(socket),
-    addr_("127.0.0.1") {
-    initSocketData();
+TestControl::TestControlSocket::TestControlSocket(const int socket) :
+    SocketInfo(socket, asiolink::IOAddress("127.0.0.1"), 0),
+    ifindex_(0), valid_(true) {
+    try {
+        initSocketData();
+    } catch (const Exception&) {
+        valid_ = false;
+    }
 }
 
 TestControl::TestControlSocket::~TestControlSocket() {
-    IfaceMgr::instance().closeSockets();
+    IfaceMgr::Iface* iface = IfaceMgr::instance().getIface(ifindex_);
+    if (iface) {
+        iface->delSocket(sockfd_);
+    }
 }
 
 void
@@ -68,8 +75,7 @@ TestControl::TestControlSocket::initSocketData() {
                  socket_collection.begin();
              s != socket_collection.end();
              ++s) {
-            if (s->sockfd_ == socket_) {
-                iface_ = it->getName();
+            if (s->sockfd_ == sockfd_) {
                 ifindex_ = it->getIndex();
                 addr_ = s->addr_;
                 return;
@@ -269,6 +275,7 @@ TestControl::factoryGeneric(Option::Universe u, uint16_t type,
 OptionPtr
 TestControl::factoryIana6(Option::Universe, uint16_t,
                           const OptionBuffer& buf) {
+    // @todo allow different values of T1, T2 and IAID.
     const uint8_t buf_array[] = {
         0, 0, 0, 1,                     // IAID = 1
         0, 0, 3600 >> 8, 3600 && 0xff,  // T1 = 3600
@@ -333,7 +340,7 @@ TestControl::generateMacAddress(uint8_t& randomized) const {
         isc_throw(BadValue, "invalid MAC address template specified");
     }
     uint32_t r = random();
-    // The random number must be in the range 0..clients_num. This
+    // The random number must be in the range 0..clients_num-1. This
     // will guarantee that every client has exactly one random MAC
     // address assigned.
     r %= clients_num;
@@ -370,13 +377,31 @@ TestControl::generateDuid(uint8_t& randomized) const {
     std::vector<uint8_t> duid(options.getDuidTemplate());
     // @todo: add support for DUIDs of different sizes.
     std::vector<uint8_t> mac_addr(generateMacAddress(randomized));
-    duid.resize(duid.size() - mac_addr.size());
-    for (int i = 0; i < mac_addr.size(); ++i) {
-        duid.push_back(mac_addr[i]);
-    }
+    duid.resize(duid.size());
+    std::copy(mac_addr.begin(), mac_addr.end(),
+              duid.begin() + duid.size() - mac_addr.size());
     return (duid);
 }
 
+template<class T>
+uint32_t
+TestControl::getElapsedTime(const T& pkt1, const T& pkt2) {
+    using namespace boost::posix_time;
+    ptime pkt1_time = pkt1->getTimestamp();
+    ptime pkt2_time = pkt2->getTimestamp();
+    if (pkt1_time.is_not_a_date_time() ||
+        pkt2_time.is_not_a_date_time()) {
+        isc_throw(InvalidOperation, "packet timestamp not set");;
+    }
+    time_period elapsed_period(pkt1_time, pkt2_time);
+    if (elapsed_period.is_null()) {
+        isc_throw(InvalidOperation, "unable to calculate time elapsed"
+                  " between packets");
+    }
+    return(elapsed_period.length().total_milliseconds());
+}
+
+
 uint64_t
 TestControl::getNextExchangesNum() const {
     CommandOptions& options = CommandOptions::instance();
@@ -399,7 +424,9 @@ TestControl::getNextExchangesNum() const {
             // of exchanges to be initiated.
             due_exchanges = static_cast<uint64_t>(due_factor * options.getRate());
             // We want to make sure that at least one packet goes out.
-            due_exchanges += 1;
+            if (due_exchanges == 0) {
+                due_exchanges = 1;
+            }
             // We should not exceed aggressivity as it could have been
             // restricted from command line.
             if (due_exchanges > options.getAggressivity()) {
@@ -441,7 +468,7 @@ TestControl::getTemplateBuffer(const size_t idx) const {
     if (template_buffers_.size() > idx) {
         return (template_buffers_[idx]);
     }
-    return (TemplateBuffer());
+    isc_throw(OutOfRange, "invalid buffer index");
 }
 
 void
@@ -455,8 +482,7 @@ TestControl::initPacketTemplates() {
     CommandOptions& options = CommandOptions::instance();
     std::vector<std::string> template_files = options.getTemplateFiles();
     for (std::vector<std::string>::const_iterator it = template_files.begin();
-         it != template_files.end();
-         ++it) {
+         it != template_files.end(); ++it) {
         readPacketTemplate(*it);
     }
 }
@@ -738,10 +764,13 @@ TestControl::receivePacket6(const TestControlSocket& socket,
         CommandOptions::ExchangeMode xchg_mode =
             CommandOptions::instance().getExchangeMode();
         if ((xchg_mode == CommandOptions::DORA_SARR) && solicit_pkt6) {
+            // \todo check whether received ADVERTISE packet is sane.
+            // We might want to check if STATUS_CODE option is non-zero
+            // and if there is IAADR option in IA_NA.
             if (template_buffers_.size() < 2) {
-                sendRequest6(socket, solicit_pkt6, pkt6);
+                sendRequest6(socket, pkt6);
             } else {
-                sendRequest6(socket, template_buffers_[1], solicit_pkt6, pkt6);
+                sendRequest6(socket, template_buffers_[1], pkt6);
             }
         }
     } else if (packet_type == DHCPV6_REPLY) {
@@ -880,16 +909,25 @@ TestControl::run() {
                   "command options must be parsed before running a test");
     }
 
-    // Diagnostics is command line options mainly.
+    // Diagnostics are command line options mainly.
     printDiagnostics();
     // Option factories have to be registered.
     registerOptionFactories();
     TestControlSocket socket(openSocket());
+    if (!socket.valid_) {
+        isc_throw(Unexpected, "invalid socket descriptor");
+    }
     // Initialize packet templates.
     initPacketTemplates();
     // Initialize randomization seed.
     if (options.isSeeded()) {
         srandom(options.getSeed());
+    } else {
+        // Seed with current time.
+        time_period duration(from_iso_string("20111231T235959"),
+                             microsec_clock::universal_time());
+        srandom(duration.length().total_seconds()
+                + duration.length().fractional_seconds());
     }
     // If user interrupts the program we will exit gracefully.
     signal(SIGINT, TestControl::handleInterrupt);
@@ -898,10 +936,12 @@ TestControl::run() {
     for (int i = 0; i < options.getPreload(); ++i) {
         if (options.getIpVersion() == 4) {
             // No template buffer means no -T option specified.
-            // We will build packet ourselfs.
+            // We will build packet ourselves.
             if (template_buffers_.size() == 0) {
                 sendDiscover4(socket, do_preload);
             } else {
+                // Pick template #0 if Discover is being sent.
+                // For Request it would be #1.
                 const uint8_t template_idx = 0;
                 sendDiscover4(socket, template_buffers_[template_idx],
                               do_preload);
@@ -912,6 +952,8 @@ TestControl::run() {
             if (template_buffers_.size() == 0) {
                 sendSolicit6(socket, do_preload);
             } else {
+                // Pick template #0 if Solicit is being sent.
+                // For Request it would be #1.
                 const uint8_t template_idx = 0;
                 sendSolicit6(socket, template_buffers_[template_idx],
                              do_preload);
@@ -1187,7 +1229,7 @@ TestControl::sendRequest4(const TestControlSocket& socket,
     std::vector<uint8_t> mac_address(chaddr, chaddr + HW_ETHER_LEN);
     pkt4->writeAt(rand_offset, mac_address.begin(), mac_address.end());
 
-   // Set elapsed time.
+    // Set elapsed time.
     size_t elp_offset = 0;
     if (options.getElapsedTimeOffset() > 0) {
         elp_offset = options.getElapsedTimeOffset();
@@ -1266,16 +1308,12 @@ TestControl::sendRequest4(const TestControlSocket& socket,
 
 void
 TestControl::sendRequest6(const TestControlSocket& socket,
-                          const Pkt6Ptr& solicit_pkt6,
                           const Pkt6Ptr& advertise_pkt6) {
     const uint32_t transid = generateTransid();
     Pkt6Ptr pkt6(new Pkt6(DHCPV6_REQUEST, transid));
     // Set elapsed time.
-    const uint32_t elapsed_time =
-        getElapsedTime<Pkt6Ptr>(solicit_pkt6, advertise_pkt6);
     OptionPtr opt_elapsed_time =
         Option::factory(Option::V6, D6O_ELAPSED_TIME);
-    opt_elapsed_time->setUint16(static_cast<uint16_t>(elapsed_time / 10));
     pkt6->addOption(opt_elapsed_time);
     // Set client id.
     OptionPtr opt_clientid = advertise_pkt6->getOption(D6O_CLIENTID);
@@ -1323,7 +1361,6 @@ TestControl::sendRequest6(const TestControlSocket& socket,
 void
 TestControl::sendRequest6(const TestControlSocket& socket,
                           const std::vector<uint8_t>& template_buf,
-                          const Pkt6Ptr& solicit_pkt6,
                           const Pkt6Ptr& advertise_pkt6) {
     CommandOptions& options = CommandOptions::instance();
     // Get the second argument if multiple the same arguments specified
@@ -1343,12 +1380,9 @@ TestControl::sendRequest6(const TestControlSocket& socket,
     if (options.getElapsedTimeOffset() > 0) {
         elp_offset = options.getElapsedTimeOffset();
     }
-    uint32_t elapsed_time =
-        getElapsedTime<Pkt6Ptr>(solicit_pkt6, advertise_pkt6);
     boost::shared_ptr<LocalizedOption>
         opt_elapsed_time(new LocalizedOption(Option::V6, D6O_ELAPSED_TIME,
                                              OptionBuffer(), elp_offset));
-    opt_elapsed_time->setUint16(static_cast<uint16_t>(elapsed_time / 10));
     pkt6->addOption(opt_elapsed_time);
 
     // Get the actual server id offset.
@@ -1537,9 +1571,13 @@ TestControl::setDefaults4(const TestControlSocket& socket,
                           const Pkt4Ptr& pkt) {
     CommandOptions& options = CommandOptions::instance();
     // Interface name.
-    pkt->setIface(socket.getIface());
+    IfaceMgr::Iface* iface = IfaceMgr::instance().getIface(socket.ifindex_);
+    if (iface == NULL) {
+        isc_throw(BadValue, "unable to find interface with given index");
+    }
+    pkt->setIface(iface->getName());
     // Interface index.
-    pkt->setIndex(socket.getIfIndex());
+    pkt->setIndex(socket.ifindex_);
     // Local client's port (68)
     pkt->setLocalPort(DHCP4_CLIENT_PORT);
     // Server's port (67)
@@ -1547,9 +1585,9 @@ TestControl::setDefaults4(const TestControlSocket& socket,
     // The remote server's name or IP.
     pkt->setRemoteAddr(IOAddress(options.getServerName()));
     // Set local addresss.
-    pkt->setLocalAddr(IOAddress(socket.getAddress()));
+    pkt->setLocalAddr(IOAddress(socket.addr_));
     // Set relay (GIADDR) address to local address.
-    pkt->setGiaddr(IOAddress(socket.getAddress()));
+    pkt->setGiaddr(IOAddress(socket.addr_));
     // Pretend that we have one relay (which is us).
     pkt->setHops(1);
 }
@@ -1559,15 +1597,19 @@ TestControl::setDefaults6(const TestControlSocket& socket,
                           const Pkt6Ptr& pkt) {
     CommandOptions& options = CommandOptions::instance();
     // Interface name.
-    pkt->setIface(socket.getIface());
+    IfaceMgr::Iface* iface = IfaceMgr::instance().getIface(socket.ifindex_);
+    if (iface == NULL) {
+        isc_throw(BadValue, "unable to find interface with given index");
+    }
+    pkt->setIface(iface->getName());
     // Interface index.
-    pkt->setIndex(socket.getIfIndex());
+    pkt->setIndex(socket.ifindex_);
     // Local client's port (547)
     pkt->setLocalPort(DHCP6_CLIENT_PORT);
     // Server's port (548)
     pkt->setRemotePort(DHCP6_SERVER_PORT);
     // Set local address.
-    pkt->setLocalAddr(socket.getAddress());
+    pkt->setLocalAddr(socket.addr_);
     // The remote server's name or IP.
     pkt->setRemoteAddr(IOAddress(options.getServerName()));
 }
@@ -1606,6 +1648,10 @@ TestControl::updateSendDue() {
     send_due_ = last_sent_ + time_duration(0, 0, 0, duration);
     // Check if it is already due.
     ptime now(microsec_clock::universal_time());
+    // \todo verify if this condition is not too tight. In other words
+    // verify if this will not produce too many late sends.
+    // We might want to look at this once we are done implementing
+    // microsecond timeouts in IfaceMgr.
     if (now > send_due_) {
         if (testDiags('i')) {
             if (options.getIpVersion() == 4) {
diff --git a/tests/tools/perfdhcp/test_control.h b/tests/tools/perfdhcp/test_control.h
index 5994006..cf83113 100644
--- a/tests/tools/perfdhcp/test_control.h
+++ b/tests/tools/perfdhcp/test_control.h
@@ -23,6 +23,7 @@
 #include <boost/function.hpp>
 #include <boost/date_time/posix_time/posix_time.hpp>
 
+#include <dhcp/iface_mgr.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/pkt6.h>
@@ -48,7 +49,7 @@ namespace perfdhcp {
 class TestControl : public boost::noncopyable {
 public:
 
-    /// Default transaction id offset. 
+    /// Default transaction id offset.
     static const size_t DHCPV4_TRANSID_OFFSET = 4;
     /// Default offset of MAC's last octet.
     static const size_t DHCPV4_RANDOMIZATION_OFFSET = 35;
@@ -84,55 +85,38 @@ public:
     /// Packet template buffers list.
     typedef std::vector<TemplateBuffer> TemplateBufferCollection;
 
-    /// \brief Socket wrapper class.
-    ///
-    /// This is wrapper class that holds descriptor of the socket
-    /// used to run DHCP test. All sockets created with \ref IfaceMgr
-    /// are closed in the destructor. This ensures that socket is
-    /// closed when the function that created the socket ends
-    /// (normally or when exception occurs).
-    class TestControlSocket {
-    public:
+    /// \brief Socket wrapper structure.
+    ///
+    /// This is the wrapper that holds descriptor of the socket
+    /// used to run DHCP test. The wrapped socket is closed in
+    /// the destructor. This prevents resource leaks when when
+    /// function that created the socket ends (normally or
+    /// when exception occurs). This structure extends parent
+    /// structure with new field ifindex_ that holds interface
+    /// index where socket is bound to.
+    struct TestControlSocket : public dhcp::IfaceMgr::SocketInfo {
+        /// Interface index.
+        uint16_t ifindex_;
+        /// Is socket valid. It will not be valid if the provided socket
+        /// descriptor does not point to valid socket.
+        bool valid_;
 
         /// \brief Constructor of socket wrapper class.
         ///
         /// This constructor uses provided socket descriptor to
         /// find the name of the interface where socket has been
-        /// bound to.
+        /// bound to. If provided socket descriptor is invalid then
+        /// valid_ field is set to false;
         ///
         /// \param socket socket descriptor.
-        /// \throw isc::BadValue if interface for specified
-        /// socket descriptor does not exist.
         TestControlSocket(const int socket);
 
         /// \brief Destriuctor of the socket wrapper class.
         ///
-        /// Destructor closes all open sockets on all interfaces.
-        /// \todo close only the socket being wrapped by this class.
+        /// Destructor closes wrapped socket.
         ~TestControlSocket();
 
-        /// \brief Return name of the interface where socket is bound to.
-        ///
-        /// \return name of the interface where socket is bound to.
-        const std::string& getIface() const { return(iface_); }
-
-        /// \brief Return interface index where socket is bound to.
-        ///
-        /// \return index fo the interface where sockert is bound to.
-        int getIfIndex() const { return(ifindex_); }
-
-        /// \brief Return address where socket is bound to.
-        ///
-        /// \return address where socket is bound to.
-        const asiolink::IOAddress& getAddress() const { return(addr_); }
-
     private:
-        /// \brief Private default constructor.
-        ///
-        /// Default constructor is private to make sure that valid
-        /// socket descriptor is passed to create object.
-        TestControlSocket();
-
         /// \brief Initialize socket data.
         ///
         /// This method initializes members of the class that Interface
@@ -141,11 +125,6 @@ public:
         /// \throw isc::BadValue if interface for specified socket
         /// descriptor does not exist.
         void initSocketData();
-
-        int socket_;               ///< Socket descirptor.
-        std::string iface_;        ///< Name of the interface.
-        int ifindex_;              ///< Index of the interface.
-        asiolink::IOAddress addr_; ///< Address bound.
     };
 
     /// \brief Default transaction id generator class.
@@ -169,6 +148,9 @@ public:
     typedef boost::shared_ptr<TransidGenerator> TransidGeneratorPtr;
 
     /// \brief Length of the Ethernet HW address (MAC) in bytes.
+    ///
+    /// \todo Make this variable length as there are cases when HW
+    /// address is longer than this (e.g. 20 bytes).
     static const uint8_t HW_ETHER_LEN = 6;
 
     /// TestControl is a singleton class. This method returns reference
@@ -195,22 +177,20 @@ public:
         transid_gen_ = generator;
     }
 
-protected:
-
-    // We would really like these methods and members to be private but
+    // We would really like following methods and members to be private but
     // they have to be accessible for unit-testing. Another, possibly better,
     // solution is to make this class friend of test class but this is not
     // what's followed in other classes.
-
+protected:
     /// \brief Default constructor.
     ///
     /// Default constructor is protected as the object can be created
     /// only via \ref instance method.
     TestControl();
 
-    /// \brief Check if test exit condtitions fulfiled.
+    /// \brief Check if test exit condtitions fulfilled.
     ///
-    /// Method checks if test exit conditions are fulfiled.
+    /// Method checks if the test exit conditions are fulfiled.
     /// Exit conditions are checked periodically from the
     /// main loop. Program should break the main loop when
     /// this method returns true. It is calling function
@@ -227,9 +207,10 @@ protected:
     /// to length 2 and values will be initialized to zeros. Otherwise
     /// function will initialize option buffer with values in passed buffer.
     ///
-    /// \param u universe (V6 or V4).
-    /// \param type option-type.
-    /// \param buf option-buffer.
+    /// \param u universe (ignored)
+    /// \param type option-type (ignored).
+    /// \param buf option-buffer containing option content (2 bytes) or
+    /// empty buffer if option content has to be set to default (0) value.
     /// \throw if elapsed time buffer size is neither 2 nor 0.
     /// \return instance o the option.
     static dhcp::OptionPtr
@@ -244,7 +225,7 @@ protected:
     /// the buffer contents, size  etc.
     ///
     /// \param u universe (V6 or V4).
-    /// \param type option-type.
+    /// \param type option-type (ignored).
     /// \param buf option-buffer.
     /// \return instance o the option.
     static dhcp::OptionPtr factoryGeneric(dhcp::Option::Universe u,
@@ -257,9 +238,9 @@ protected:
     ///
     /// \todo add support for IA Address options.
     ///
-    /// \param u universe (V6 or V4).
-    /// \param type option-type.
-    /// \param buf option-buffer.
+    /// \param u universe (ignored).
+    /// \param type option-type (ignored).
+    /// \param buf option-buffer carrying IANA suboptions.
     /// \return instance of IA_NA option.
     static dhcp::OptionPtr factoryIana6(dhcp::Option::Universe u,
                                         uint16_t type,
@@ -272,9 +253,9 @@ protected:
     /// - D6O_NAME_SERVERS
     /// - D6O_DOMAIN_SEARCH
     ///
-    /// \param u universe (V6 or V4).
-    /// \param type option-type.
-    /// \param buf option-buffer (ignored and should be empty).
+    /// \param u universe (ignored).
+    /// \param type option-type (ignored).
+    /// \param buf option-buffer (ignored).
     /// \return instance of ORO option.
     static dhcp::OptionPtr
     factoryOptionRequestOption6(dhcp::Option::Universe u,
@@ -287,9 +268,9 @@ protected:
     /// The buffer passed to this option must be empty because option does
     /// not have any payload.
     ///
-    /// \param u universe (V6 or V4).
-    /// \param type option-type.
-    /// \param buf option-buffer (ignored and should be empty).
+    /// \param u universe (ignored).
+    /// \param type option-type (ignored).
+    /// \param buf option-buffer (ignored).
     /// \return instance of RAPID_COMMIT option..
     static dhcp::OptionPtr factoryRapidCommit6(dhcp::Option::Universe u,
                                                uint16_t type,
@@ -308,9 +289,9 @@ protected:
     /// - DHO_DOMAIN_NAME_SERVERS,
     /// - DHO_HOST_NAME.
     ///
-    /// \param u universe (V6 or V4).
-    /// \param type option-type.
-    /// \param buf option-buffer (ignored and should be empty).
+    /// \param u universe (ignored).
+    /// \param type option-type (ignored).
+    /// \param buf option-buffer (ignored).
     /// \return instance o the generic option.
      static dhcp::OptionPtr factoryRequestList4(dhcp::Option::Universe u,
                                                uint16_t type,
@@ -319,15 +300,16 @@ protected:
     /// \brief Generate DUID.
     ///
     /// Method generates unique DUID. The number of DUIDs it can generate
-    /// depends on the number of simulated clinets, which is specified
+    /// depends on the number of simulated clients, which is specified
     /// from the command line. It uses \ref CommandOptions object to retrieve
-    /// number of clinets. Since the last six octets of DUID are constructed
+    /// number of clients. Since the last six octets of DUID are constructed
     /// from the MAC address, this function uses \ref generateMacAddress
     /// internally to randomize the DUID.
     ///
     /// \todo add support for other types of DUID.
     ///
-    /// \param randomized number of bytes randomized.
+    /// \param [out] randomized number of bytes randomized (initial value
+    /// is ignored).
     /// \throw isc::BadValue if \ref generateMacAddress throws.
     /// \return vector representing DUID.
     std::vector<uint8_t> generateDuid(uint8_t& randomized) const;
@@ -341,7 +323,8 @@ protected:
     /// Based on this the random value is generated and added to
     /// the MAC address template (default MAC address).
     ///
-    /// \param randomized number of bytes randomized.
+    /// \param [out] randomized number of bytes randomized (initial
+    /// value is ignored).
     /// \throw isc::BadValue if MAC address template (default or specified
     /// from the command line) has invalid size (expected 6 octets).
     /// \return generated MAC address.
@@ -349,6 +332,9 @@ protected:
 
     /// \brief generate transaction id.
     ///
+    /// Generate transaction id value (32-bit for DHCPv4,
+    /// 24-bit for DHCPv6).
+    ///
     /// \return generated transaction id.
     uint32_t generateTransid() {
         return(transid_gen_->generate());
@@ -361,7 +347,7 @@ protected:
     /// is based on current time, due time calculated with
     /// \ref updateSendTime function and expected rate.
     ///
-    /// \return number of exchanges to be started immediatelly.
+    /// \return number of exchanges to be started immediately.
     uint64_t getNextExchangesNum() const;
 
     /// \brief Return template buffer.
@@ -369,14 +355,15 @@ protected:
     /// Method returns template buffer at specified index.
     ///
     /// \param idx index of template buffer.
-    /// \return reference to template buffer or empty buffer if index
-    /// is out of bounds.
+    /// \throw isc::OutOfRange if buffer index out of bounds.
+    /// \return reference to template buffer.
     TemplateBuffer getTemplateBuffer(const size_t idx) const;
 
     /// \brief Reads packet templates from files.
     ///
     /// Method iterates through all specified template files, reads
-    /// their content and stores it in class internal buffers
+    /// their content and stores it in class internal buffers. Template
+    /// file names are specified from the command line with -T option.
     ///
     /// \throw isc::BadValue if any of the template files does not exist
     void initPacketTemplates();
@@ -390,13 +377,13 @@ protected:
     /// \brief Open socket to communicate with DHCP server.
     ///
     /// Method opens socket and binds it to local address. Function will
-    /// can use either interface name, local address or server address
+    /// use either interface name, local address or server address
     /// to create a socket, depending on what is available (specified
     /// from the command line). If socket can't be created for any
     /// reason, exception is thrown.
     /// If destination address is broadcast (for DHCPv4) or multicast
     /// (for DHCPv6) than broadcast or multicast option is set on
-    /// the socket.
+    /// the socket. Opened socket is registered and managed by IfaceMgr.
     ///
     /// \throw isc::BadValue if socket can't be created for given
     /// interface, local address or remote address.
@@ -432,8 +419,11 @@ protected:
     /// when OFFER packet arrives, this function will initiate
     /// REQUEST message to the server.
     ///
-    /// \param socket socket to be used.
-    /// \param pkt4 object representing DHCPv4 packet received.
+    /// \warning this method does not check if provided socket is
+    /// valid (specifically if v4 socket for received v4 packet).
+    ///
+    /// \param [in] socket socket to be used.
+    /// \param [in] pkt4 object representing DHCPv4 packet received.
     /// \throw isc::BadValue if unknown message type received.
     /// \throw isc::Unexpected if unexpected error occured.
     void receivePacket4(const TestControlSocket& socket,
@@ -446,8 +436,11 @@ protected:
     /// when ADVERTISE packet arrives, this function will initiate
     /// REQUEST message to the server.
     ///
-    /// \param socket socket to be used.
-    /// \param pkt6 object representing DHCPv6 packet received.
+    /// \warning this method does not check if provided socket is
+    /// valid (specifically if v4 socket for received v4 packet).
+    ///
+    /// \param [in] socket socket to be used.
+    /// \param [in] pkt6 object representing DHCPv6 packet received.
     /// \throw isc::BadValue if unknown message type received.
     /// \throw isc::Unexpected if unexpected error occured.
     void receivePacket6(const TestControlSocket& socket,
@@ -460,6 +453,9 @@ protected:
     /// \ref receivePacket6 depending if DHCPv4 or DHCPv6 packet
     /// has arrived.
     ///
+    /// \warning this method does not check if provided socket is
+    /// valid. Ensure that it is valid prior to calling it.
+    ///
     /// \param socket socket to be used.
     /// \throw::BadValue if unknown message type received.
     /// \throw::Unexpected if unexpected error occured.
@@ -506,6 +502,8 @@ protected:
     /// The transaction id and MAC address are randomly generated for
     /// the message. Range of unique MAC addresses generated depends
     /// on the number of clients specified from the command line.
+    /// Copy of sent packet is stored in the stats_mgr4_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send the message.
     /// \param preload preload mode, packets not included in statistics.
@@ -520,6 +518,8 @@ protected:
     /// template data is exepcted to be in binary format. Provided
     /// buffer is copied and parts of it are replaced with actual
     /// data (e.g. MAC address, transaction id etc.).
+    /// Copy of sent packet is stored in the stats_mgr4_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send the message.
     /// \param template_buf buffer holding template packet.
@@ -532,6 +532,8 @@ protected:
     /// \brief Send DHCPv4 REQUEST message.
     ///
     /// Method creates and sends DHCPv4 REQUEST message to the server.
+    /// Copy of sent packet is stored in the stats_mgr4_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send message.
     /// \param discover_pkt4 DISCOVER packet sent.
@@ -546,6 +548,8 @@ protected:
     /// \brief Send DHCPv4 REQUEST message from template.
     ///
     /// Method sends DHCPv4 REQUEST message from template.
+    /// Copy of sent packet is stored in the stats_mgr4_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send message.
     /// \param template_buf buffer holding template packet.
@@ -563,32 +567,28 @@ protected:
     /// - D6O_ELAPSED_TIME
     /// - D6O_CLIENTID
     /// - D6O_SERVERID
-    /// The elapsed time is calculated based on the duration between
-    /// sending a SOLICIT and receiving the ADVERTISE packet prior.
-    /// For this reason both solicit and advertise packet objects have
-    /// to be passed when calling this function.
+    /// Copy of sent packet is stored in the stats_mgr6_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send message.
-    /// \param solicit_pkt6 SOLICIT packet object.
     /// \param advertise_pkt6 ADVERTISE packet object.
     /// \throw isc::Unexpected if unexpected error occured.
     /// \throw isc::InvalidOperation if Statistics Manager has not been
     /// initialized.
     void sendRequest6(const TestControlSocket& socket,
-                      const dhcp::Pkt6Ptr& solicit_pkt6,
                       const dhcp::Pkt6Ptr& advertise_pkt6);
 
     /// \brief Send DHCPv6 REQUEST message from template.
     ///
     /// Method sends DHCPv6 REQUEST message from template.
+    /// Copy of sent packet is stored in the stats_mgr6_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send message.
     /// \param template_buf packet template buffer.
-    /// \param solicit_pkt6 SOLICIT packet object.
     /// \param advertise_pkt6 ADVERTISE packet object.
     void sendRequest6(const TestControlSocket& socket,
                       const std::vector<uint8_t>& template_buf,
-                      const dhcp::Pkt6Ptr& solicit_pkt6,
                       const dhcp::Pkt6Ptr& advertise_pkt6);
 
     /// \brief Send DHCPv6 SOLICIT message.
@@ -600,6 +600,8 @@ protected:
     /// - D6O_CLIENTID,
     /// - D6O_ORO (Option Request Option),
     /// - D6O_IA_NA.
+    /// Copy of sent packet is stored in the stats_mgr6_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send the message.
     /// \param preload mode, packets not included in statistics.
@@ -610,6 +612,8 @@ protected:
     /// \brief Send DHCPv6 SOLICIT message from template.
     ///
     /// Method sends DHCPv6 SOLICIT message from template.
+    /// Copy of sent packet is stored in the stats_mgr6_ object to
+    /// update statistics.
     ///
     /// \param socket socket to be used to send the message.
     /// \param template_buf packet template buffer.
@@ -648,7 +652,7 @@ protected:
     void setDefaults6(const TestControlSocket& socket,
                       const dhcp::Pkt6Ptr& pkt);
 
-    /// \brief Find of diagnostic flag has been set.
+    /// \brief Find if diagnostic flag has been set.
     ///
     /// \param diag diagnostic flag (a,e,i,s,r,t,T).
     /// \return true if diagnostics flag has been set.
@@ -674,22 +678,10 @@ private:
     /// \param T Pkt4Ptr or Pkt6Ptr class.
     /// \param pkt1 first packet.
     /// \param pkt2 second packet.
+    /// \throw InvalidOperation if packet timestamps are invalid.
     /// \return elapsed time in milliseconds between pkt1 and pkt2.
     template<class T>
-    uint32_t getElapsedTime(const T& pkt1, const T& pkt2) {
-        using namespace boost::posix_time;
-        ptime pkt1_time = pkt1->getTimestamp();
-        ptime pkt2_time = pkt2->getTimestamp();
-        if (pkt1_time.is_not_a_date_time() || 
-            pkt2_time.is_not_a_date_time()) {
-            return (0);
-        }
-        time_period elapsed_period(pkt1_time, pkt2_time);
-        if (elapsed_period.is_null()) {
-            return (0);
-        }
-        return(elapsed_period.length().total_milliseconds());
-    }
+    uint32_t getElapsedTime(const T& pkt1, const T& pkt2);
 
     /// \brief Get number of received packets.
     ///
diff --git a/tests/tools/perfdhcp/tests/test_control_unittest.cc b/tests/tools/perfdhcp/tests/test_control_unittest.cc
index 2ea3667..f75c310 100644
--- a/tests/tools/perfdhcp/tests/test_control_unittest.cc
+++ b/tests/tools/perfdhcp/tests/test_control_unittest.cc
@@ -321,8 +321,8 @@ public:
         // Use templates files to crate packets.
         if (use_templates) {
             tc.initPacketTemplates();
-            ASSERT_GT(tc.getTemplateBuffer(0).size(), 0);
-            ASSERT_GT(tc.getTemplateBuffer(1).size(), 0);
+            ASSERT_NO_THROW(tc.getTemplateBuffer(0));
+            ASSERT_NO_THROW(tc.getTemplateBuffer(1));
         }
 
         // Incremental transaction id generator will generate
@@ -384,8 +384,8 @@ public:
         // Use templates files to crate packets.
         if (use_templates) {
             tc.initPacketTemplates();
-            ASSERT_GT(tc.getTemplateBuffer(0).size(), 0);
-            ASSERT_GT(tc.getTemplateBuffer(1).size(), 0);
+            ASSERT_NO_THROW(tc.getTemplateBuffer(0));
+            ASSERT_NO_THROW(tc.getTemplateBuffer(1));
         }
 
         // Incremental transaction id generator will generate
@@ -720,14 +720,14 @@ TEST_F(TestControlTest, Packet4) {
         ASSERT_NO_THROW(tc.setDefaults4(sock, pkt4));
         // Validate that packet has been setup correctly.
         EXPECT_EQ(loopback_iface, pkt4->getIface());
-        EXPECT_EQ(sock.getIfIndex(), pkt4->getIndex());
+        EXPECT_EQ(sock.ifindex_, pkt4->getIndex());
         EXPECT_EQ(DHCP4_CLIENT_PORT, pkt4->getLocalPort());
         EXPECT_EQ(DHCP4_SERVER_PORT, pkt4->getRemotePort());
         EXPECT_EQ(1, pkt4->getHops());
         EXPECT_EQ(asiolink::IOAddress("255.255.255.255"),
                   pkt4->getRemoteAddr());
-        EXPECT_EQ(asiolink::IOAddress(sock.getAddress()), pkt4->getLocalAddr());
-        EXPECT_EQ(asiolink::IOAddress(sock.getAddress()), pkt4->getGiaddr());
+        EXPECT_EQ(asiolink::IOAddress(sock.addr_), pkt4->getLocalAddr());
+        EXPECT_EQ(asiolink::IOAddress(sock.addr_), pkt4->getGiaddr());
     } else {
         std::cout << "Unable to find the loopback interface. Skip test. "
                   << std::endl;
@@ -753,10 +753,10 @@ TEST_F(TestControlTest, Packet6) {
         ASSERT_NO_THROW(tc.setDefaults6(sock, pkt6));
         // Validate if parameters have been set correctly.
         EXPECT_EQ(loopback_iface, pkt6->getIface());
-        EXPECT_EQ(sock.getIfIndex(), pkt6->getIndex());
+        EXPECT_EQ(sock.ifindex_, pkt6->getIndex());
         EXPECT_EQ(DHCP6_CLIENT_PORT, pkt6->getLocalPort());
         EXPECT_EQ(DHCP6_SERVER_PORT, pkt6->getRemotePort());
-        EXPECT_EQ(sock.getAddress(), pkt6->getLocalAddr());
+        EXPECT_EQ(sock.addr_, pkt6->getLocalAddr());
         EXPECT_EQ(asiolink::IOAddress("FF05::1:3"), pkt6->getRemoteAddr());
     } else {
         std::cout << "Unable to find the loopback interface. Skip test. "



More information about the bind10-changes mailing list