Browse Source

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

Marcin Siodelski 12 years ago
parent
commit
c2fa97e235

+ 82 - 36
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) {

+ 92 - 100
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.
     ///

+ 9 - 9
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. "