[1955] Added remaining unit tests for PerfPkt4 and cleaned up the code.

tests/tools/perfdhcp/perf_pkt4.cc

@@ -12,8 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <iostream>
-#include <exceptions/exceptions.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/dhcp6.h>
 
@@ -27,13 +25,21 @@ using namespace dhcp;
 namespace isc {
 namespace perfdhcp {
 
-    PerfPkt4::PerfPkt4(const uint8_t* buf, uint32_t len, size_t transid_offset, uint32_t transid) :
+PerfPkt4::PerfPkt4(const uint8_t* buf, size_t len) :
+    Pkt4(buf, len),
+    transid_offset_(1) {
+}
+
+PerfPkt4::PerfPkt4(const uint8_t* buf,
+                   size_t len,
+                   size_t transid_offset,
+                   uint32_t transid) :
     Pkt4(buf, len),
     transid_offset_(transid_offset) {
     transid_ = transid;
 }
 
-PerfPkt4::PerfPkt4(const uint8_t* buf, uint32_t len, size_t transid_offset) :
+PerfPkt4::PerfPkt4(const uint8_t* buf, size_t len, size_t transid_offset) :
     Pkt4(buf, len),
     transid_offset_(transid_offset) {
 }

tests/tools/perfdhcp/perf_pkt4.h

@@ -56,6 +56,16 @@ public:
     /// Localized option pointer type.
     typedef boost::shared_ptr<LocalizedOption> LocalizedOptionPtr;
 
+    /// \brief Constructor, used for outgoing and incoming messages
+    ///
+    /// This constructor initializes transaction id and
+    /// transaction id offset of the packet with default
+    /// values.
+    ///
+    /// \param buf buffer holding contents of the message.
+    /// \param len length of the data in the buffer.
+    PerfPkt4(const uint8_t* buf, size_t len);
+
     /// \brief Constructor, used for outgoing DHCP messages.
     ///
     /// Creates new DHCPv4 message using provided buffer.
@@ -73,7 +83,7 @@ public:
     /// \param transid_offset transaction id offset in outgoing message.
     /// \param transid transaction id to be stored in outgoing message.
     PerfPkt4(const uint8_t* buf,
-             uint32_t len,
+             size_t len,
              size_t transid_offset,
              uint32_t transid);
 
@@ -100,7 +110,7 @@ public:
     /// \param len size of buffer of packet content.
     /// \param transid_offset transaction id offset in a message.
     PerfPkt4(const uint8_t* buf,
-             uint32_t len,
+             size_t len,
              size_t transid_offset);
 
     /// \brief Returns transaction id offset in packet buffer

tests/tools/perfdhcp/pkt_transform.cc

@@ -57,7 +57,7 @@ PktTransform::pack(const Option::Universe universe,
     out_buffer.skip(transid_offset);
 
     try {
-        if (universe == Option::V6) {
+        if (universe == Option::V4) {
             out_buffer.writeUint8(transid >> 24 & 0xFF);
         }
         out_buffer.writeUint8(transid >> 16 & 0xFF);
@@ -111,7 +111,7 @@ PktTransform::unpack(const Option::Universe universe,
     }
 
     try {
-        PktTransform::unpackOptions(universe, in_buffer, options);
+        PktTransform::unpackOptions(in_buffer, options);
     } catch (const isc::BadValue& e) {
         cout << "Packet parsing failed: " << e.what() << endl;
         return (false);
@@ -156,8 +156,7 @@ PktTransform::packOptions(const OptionBuffer& in_buffer,
 }
 
 void
-PktTransform::unpackOptions(const Option::Universe universe,
-                            const OptionBuffer& in_buffer,
+PktTransform::unpackOptions(const OptionBuffer& in_buffer,
                             const Option::OptionCollection& options) {
     for (Option::OptionCollection::const_iterator it = options.begin();
          it != options.end(); ++it) {
@@ -168,7 +167,7 @@ PktTransform::unpackOptions(const Option::Universe universe,
         if (opt_pos == 0) {
             isc_throw(isc::BadValue, "failed to unpack packet from raw buffer "
                       "(Option position not specified)");
-        } else if (opt_pos + 4 > in_buffer.size()) {
+        } else if (opt_pos + option->getHeaderLen() > in_buffer.size()) {
             isc_throw(isc::BadValue,
                       "failed to unpack options from from raw buffer "
                       "(Option position out of bounds)");
@@ -177,7 +176,7 @@ PktTransform::unpackOptions(const Option::Universe universe,
         size_t offset = opt_pos;
         size_t offset_step = 1;
         uint16_t opt_type = 0;
-        if (universe == Option::V6) {
+        if (option->getUniverse() == Option::V6) {
             offset_step = 2;
             // For DHCPv6 option type is in first two octets.
             opt_type = in_buffer[offset] * 256 + in_buffer[offset + 1];
@@ -195,14 +194,14 @@ PktTransform::unpackOptions(const Option::Universe universe,
         // Get option length which is supposed to be after option type.
         offset += offset_step;
         uint16_t opt_len = in_buffer[offset] * 256 + in_buffer[offset + 1];
-        if (universe == Option::V6) {
+        if (option->getUniverse() == Option::V6) {
             opt_len = in_buffer[offset] * 256 + in_buffer[offset + 1];
         } else {
             opt_len = in_buffer[offset];
         }
 
         // Check if packet is not truncated.
-        if (offset + opt_len > in_buffer.size()) {
+        if (offset + option->getHeaderLen() + opt_len > in_buffer.size()) {
             isc_throw(isc::BadValue,
                       "failed to unpack option from raw buffer "
                       "(option truncated)");

tests/tools/perfdhcp/pkt_transform.h

@@ -15,7 +15,6 @@
 #ifndef __PKT_TRANSFORM_H
 #define __PKT_TRANSFORM_H
 
-#include <boost/shared_ptr.hpp>
 #include <dhcp/option.h>
 
 #include "localized_option.h"
@@ -139,8 +138,7 @@ private:
     /// \param options oprions collection with their offsets
     /// in input buffer specified.
     /// \throw isc::Unexpected if options unpack failed.
-    static void unpackOptions(const dhcp::Option::Universe universe,
-                              const dhcp::OptionBuffer& in_buffer,
+    static void unpackOptions(const dhcp::OptionBuffer& in_buffer,
                               const dhcp::Option::OptionCollection& options);
 };
 

tests/tools/perfdhcp/tests/perf_pkt4_unittest.cc

@@ -35,23 +35,6 @@ typedef PerfPkt4::LocalizedOptionPtr LocalizedOptionPtr;
 
 namespace {
 
-// a sample data
-const uint8_t dummyOp = BOOTREQUEST;
-const uint8_t dummyHtype = 6;
-const uint8_t dummyHlen = 6;
-const uint8_t dummyHops = 13;
-const uint32_t dummyTransid = 0x12345678;
-const uint16_t dummySecs = 42;
-const uint16_t dummyFlags = BOOTP_BROADCAST;
-
-const IOAddress dummyCiaddr("192.0.2.1");
-const IOAddress dummyYiaddr("1.2.3.4");
-const IOAddress dummySiaddr("192.0.2.255");
-const IOAddress dummyGiaddr("255.255.255.255");
-
-// a dummy MAC address
-const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
-
 // a dummy MAC address, padded with 0s
 const uint8_t dummyChaddr[16] = {0, 1, 2, 3, 4, 5, 0, 0,
                                  0, 0, 0, 0, 0, 0, 0, 0 };
@@ -70,16 +53,14 @@ public:
     PerfPkt4Test() {
     }
 
-    /// \brief Returns captured SOLICIT packet.
+    /// \brief Returns buffer with sample DHCPDISCOVER message.
     ///
-    /// Captured SOLICIT packet with transid=0x3d79fb and options: client-id,
-    /// in_na, dns-server, elapsed-time, option-request
-    /// This code was autogenerated
-    /// (see src/bin/dhcp6/tests/iface_mgr_unittest.c),
-    /// but we spent some time to make is less ugly than it used to be.
+    /// This method creates buffer containing on-wire data of
+    /// DHCPDICOSVER message. This buffer is used by tests below
+    /// to create DHCPv4 test packets.
     ///
-    /// \return pointer to Pkt6 that represents received SOLICIT
-    std::vector<uint8_t> capture() {
+    /// \return vector containing on-wire data
+    std::vector<uint8_t>& capture() {
 
         // That is only part of the header. It contains all "short" fields,
         // larger fields are constructed separately.
@@ -103,12 +84,20 @@ public:
         };
 
         // Initialize the vector with the header fields defined above.
-        vector<uint8_t> buf(hdr, hdr + sizeof(hdr));
+        static std::vector<uint8_t> buf(hdr, hdr + sizeof(hdr));
+
+        // If this is a first call to this function. Initialize
+        // remaining data.
+        if (buf.size() == sizeof(hdr))
+        {
 
             // Append the large header fields.
-        std::copy(dummyChaddr, dummyChaddr + Pkt4::MAX_CHADDR_LEN, back_inserter(buf));
-        std::copy(dummySname, dummySname + Pkt4::MAX_SNAME_LEN, back_inserter(buf));
-        std::copy(dummyFile, dummyFile + Pkt4::MAX_FILE_LEN, back_inserter(buf));
+            std::copy(dummyChaddr, dummyChaddr + Pkt4::MAX_CHADDR_LEN,
+                      back_inserter(buf));
+            std::copy(dummySname, dummySname + Pkt4::MAX_SNAME_LEN,
+                      back_inserter(buf));
+            std::copy(dummyFile, dummyFile + Pkt4::MAX_FILE_LEN,
+                      back_inserter(buf));
 
             // Append magic cookie.
             buf.push_back(0x63);
@@ -118,7 +107,7 @@ public:
 
             // Append options.
             std::copy(v4Opts, v4Opts + sizeof(v4Opts), back_inserter(buf));
-
+        }
         return buf;
     }
 };
@@ -137,21 +126,27 @@ TEST_F(PerfPkt4Test, Constructor) {
 
     // Test constructor to be used for outgoing messages.
     // Use non-zero offset and specify transaction id.
-    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(data, sizeof(data), 10, 0x010203));
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(data, sizeof(data),
+                                                  10, 0x010203));
     EXPECT_EQ(0x010203, pkt2->getTransid());
     EXPECT_EQ(10, pkt2->getTransIdOffset());
+
+    // Test default constructor. Transaction id offset is expected to be 1.
+    boost::scoped_ptr<PerfPkt4> pkt3(new PerfPkt4(data, sizeof(data)));
+    EXPECT_EQ(1, pkt3->getTransIdOffset());
 }
 
 TEST_F(PerfPkt4Test, RawPack) {
     // Create new packet.
     std::vector<uint8_t> buf = capture();
-    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size(), 0x1, 1));
+    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size()));
 
     // Initialize options data.
     uint8_t buf_hostname[] = { 12, 3, 4, 5, 6 };
     uint8_t buf_boot_filesize[] = { 13,  3, 1, 2, 3 };
     OptionBuffer vec_hostname(buf_hostname + 2, buf_hostname + 5);
-    OptionBuffer vec_boot_filesize(buf_boot_filesize + 2, buf_boot_filesize + 5);
+    OptionBuffer vec_boot_filesize(buf_boot_filesize + 2,
+                                   buf_boot_filesize + 5);
 
     // Create options objects.
     LocalizedOptionPtr pkt_hostname(new LocalizedOption(Option::V4,
@@ -175,7 +170,8 @@ TEST_F(PerfPkt4Test, RawPack) {
     // DHO_BOOT_SIZE options.
     util::OutputBuffer pkt_output = pkt->getBuffer();
     ASSERT_EQ(buf.size(), pkt_output.getLength());
-    const uint8_t* out_buf_data = static_cast<const uint8_t*>(pkt_output.getData());
+    const uint8_t* out_buf_data =
+        static_cast<const uint8_t*>(pkt_output.getData());
 
     // Check if options we read from buffer is valid.
     EXPECT_EQ(0, memcmp(buf_hostname, out_buf_data + 240, 5));
@@ -185,7 +181,7 @@ TEST_F(PerfPkt4Test, RawPack) {
 TEST_F(PerfPkt4Test, RawUnpack) {
     // Create new packet.
     std::vector<uint8_t> buf = capture();
-    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size(), 0x1, 1));    
+    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size()));
 
     // Create options (existing in the packet) and specify their offsets.
     LocalizedOptionPtr opt_merit(new LocalizedOption(Option::V4,
@@ -207,8 +203,10 @@ TEST_F(PerfPkt4Test, RawUnpack) {
 
     // At this point we should have updated options data (read from buffer).
     // Let's try to retrieve them.
-    opt_merit = boost::dynamic_pointer_cast<LocalizedOption>(pkt->getOption(DHO_MERIT_DUMP));
-    opt_msg_type = boost::dynamic_pointer_cast<LocalizedOption>(pkt->getOption(DHO_DHCP_MESSAGE_TYPE));
+    opt_merit = boost::dynamic_pointer_cast<LocalizedOption>
+        (pkt->getOption(DHO_MERIT_DUMP));
+    opt_msg_type = boost::dynamic_pointer_cast<LocalizedOption>
+        (pkt->getOption(DHO_DHCP_MESSAGE_TYPE));
     ASSERT_TRUE(opt_merit);
     ASSERT_TRUE(opt_msg_type);
 
@@ -220,7 +218,9 @@ TEST_F(PerfPkt4Test, RawUnpack) {
 
     // Validate first option data.
     ASSERT_EQ(sizeof(buf_merit), opt_merit_data.size());
-    EXPECT_TRUE(std::equal(opt_merit_data.begin(), opt_merit_data.end(), buf_merit));
+    EXPECT_TRUE(std::equal(opt_merit_data.begin(),
+                           opt_merit_data.end(),
+                           buf_merit));
 
     // Get second option payload.
     OptionBuffer opt_msg_type_data = opt_msg_type->getData();
@@ -230,115 +230,133 @@ TEST_F(PerfPkt4Test, RawUnpack) {
     EXPECT_EQ(1, opt_msg_type_data[0]);
 }
 
-    /*
 TEST_F(PerfPkt4Test, InvalidOptions) {
-    // Create packet.
-    boost::scoped_ptr<PerfPkt4> pkt1(capture());
-    OptionBuffer vec_server_id;
-    vec_server_id.resize(10);
-    // Testing invalid offset of the option (greater than packet size)
-    LocalizedOptionPtr pkt1_serverid(new LocalizedOption(Option::V6,
-                                                         D6O_SERVERID,
-                                                         vec_server_id,
-                                                         150));
-    pkt1->addOption(pkt1_serverid);
-    // Pack has to fail due to invalid offset.
-    EXPECT_FALSE(pkt1->rawPack());
-
-    // Create packet.
-    boost::scoped_ptr<PerfPkt4> pkt2(capture());
-    // Testing offset of the option (lower than pakcet size but
-    // tail of the option out of bounds).
-    LocalizedOptionPtr pkt2_serverid(new LocalizedOption(Option::V6,
-                                                         D6O_SERVERID,
-                                                         vec_server_id,
-                                                         85));
-    pkt2->addOption(pkt2_serverid);
-    // Pack must fail due to invalid offset.
-    EXPECT_FALSE(pkt2->rawPack());
-}
+    // Create new packet.
+    std::vector<uint8_t> buf = capture();
+    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(&buf[0], buf.size()));
 
+    // Create option with invalid offset.
+    // This option is at offset 250 (not 251).
+    LocalizedOptionPtr opt_merit(new LocalizedOption(Option::V4,
+                                                     DHO_MERIT_DUMP,
+                                                     OptionBuffer(),
+                                                     251));
+    ASSERT_NO_THROW(pkt1->addOption(opt_merit));
 
-TEST_F(PerfPkt4Test, TruncatedPacket) {
-    cout << "Testing parsing options from truncated packet."
-         << "This may produce spurious errors" << endl;
+    cout << "Testing unpack of invalid options. "
+         << "This may produce spurious errors." << endl;
+
+    // Unpack is expected to fail because it is supposed to read
+    // option type from buffer and match it with DHO_MERIT_DUMP.
+    // It will not match because option is shifted by on byte.
+    ASSERT_FALSE(pkt1->rawUnpack());
 
-    // Create truncated (in the middle of duid options)
-    boost::scoped_ptr<PerfPkt4> pkt1(captureTruncated());
-    OptionBuffer vec_duid;
-    vec_duid.resize(30);
-    LocalizedOptionPtr pkt1_duid(new LocalizedOption(Option::V6,
-                                                     D6O_CLIENTID,
-                                                     vec_duid,
-                                                     4));
-    pkt1->addOption(pkt1_duid);
-    // Pack/unpack must fail because length of the option read from buffer
-    // will extend over the actual packet length.
-    EXPECT_FALSE(pkt1->rawUnpack());
-    EXPECT_FALSE(pkt1->rawPack());
+    // Crete another packet.
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(&buf[0], buf.size()));
+
+    // Create DHO_DHCP_MESSAGE_TYPE option that has wrong offset.
+    // With this offset, option goes beyond packet size (268).
+    LocalizedOptionPtr opt_msg_type(new LocalizedOption(Option::V4,
+                                                        DHO_DHCP_MESSAGE_TYPE,
+                                                        OptionBuffer(1, 2),
+                                                        266));
+    // Adding option is expected to be successful because no
+    // offset validation takes place at this point.
+    ASSERT_NO_THROW(pkt2->addOption(opt_msg_type));
+
+    // This is expected to fail because option is out of bounds.
+    ASSERT_FALSE(pkt2->rawPack());
 }
 
-TEST_F(PerfPkt4Test, PackTransactionId) {
-    uint8_t data[100] = { 0 };
+TEST_F(PerfPkt4Test, TruncatedPacket) {
+    // Get the whole packet and truncate it to 249 bytes.
+    std::vector<uint8_t> buf = capture();
+    buf.resize(249);
+    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size()));
 
-    // Create dummy packet that is simply filled with zeros.
-    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(data,
-                                                  sizeof(data),
-                                                  0x010203,
-                                                  PerfPkt4::Offset(50)));
+    // Option DHO_BOOT_SIZE is now truncated because whole packet
+    // is truncated. This option ends at 249 while last index of
+    // truncated packet is now 248.
+    LocalizedOptionPtr opt_boot_filesize(new LocalizedOption(Option::V4,
+                                                             DHO_BOOT_SIZE,
+                                                             OptionBuffer(3, 1),
+                                                             245));
+    ASSERT_NO_THROW(pkt->addOption(opt_boot_filesize));
 
-    // Reference data are non zero so we can detect them in dummy packet.
-    uint8_t ref_data[3] = { 1, 2, 3 };
+    cout << "Testing pack and unpack of options in truncated "
+         << "packet. This may produce spurious errors." << endl;
 
-    // This will store given transaction id in the packet data at
-    // offset of 50.
+    // Both pack and unpack are expected to fail because
+    // added option is out of bounds.
+    EXPECT_FALSE(pkt->rawUnpack());
+    EXPECT_FALSE(pkt->rawPack());
+}
+
+TEST_F(PerfPkt4Test, PackTransactionId) {
+    // Create dummy packet that consists of zeros.
+    std::vector<uint8_t> buf(268, 0);
+    // Initialize transaction id 0x00000102 at offset 10.
+    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(&buf[0], buf.size(),
+                                                  10, 0x0102));
+
+    // Pack will inject transaction id at offset 10 into the
+    // packet buffer.
     ASSERT_TRUE(pkt1->rawPack());
 
-    // Get the output buffer so we can validate it.
+    // Get packet's output buffer and make sure it has valid size.
     util::OutputBuffer out_buf = pkt1->getBuffer();
-    ASSERT_EQ(sizeof(data), out_buf.getLength());
-    const uint8_t *out_buf_data = static_cast<const uint8_t*>
-        (out_buf.getData());
-
-    // Validate transaction id.
-    EXPECT_EQ(0, memcmp(out_buf_data + 50, ref_data, 3));
-
-    // Out of bounds transaction id offset.
-    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(data,
-                                                  sizeof(data),
-                                                  0x010202,
-                                                  PerfPkt4::Offset(100)));
-    cout << "Testing out of bounds offset. "
-        "This may produce spurious errors ..." << endl;
+    ASSERT_EQ(buf.size(), out_buf.getLength());
+    const uint8_t *out_buf_data =
+        static_cast<const uint8_t*>(out_buf.getData());
+
+    // Initialize reference data for transaction id.
+    const uint8_t ref_data[] = { 0, 0, 1, 2 };
+
+    // Expect that reference transaction id matches what we have
+    // read from buffer.
+    EXPECT_EQ(0, memcmp(ref_data, out_buf_data + 10, 4));
+
+    cout << "Testing pack with invalid transaction id offset. "
+         << "This may produce spurious errors" << endl;
+
+    // Create packet with invalid transaction id offset.
+    // Packet length is 268, transaction id is 4 bytes long so last byte of
+    // transaction id is out of bounds.
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(&buf[0], buf.size(),
+                                                  265, 0x0102));
     EXPECT_FALSE(pkt2->rawPack());
 }
 
 TEST_F(PerfPkt4Test, UnpackTransactionId) {
-    // Initialize data for dummy packet (zeros only).
-    uint8_t data[100] = { 0 };
+    // Initialize packet data, lebgth 268, zeros only.
+    std::vector<uint8_t> in_data(268, 0);
 
-    // Generate transaction id = 0x010203 and inject at offset = 50.
-    for (int i = 50; i <  53; ++i) {
+    // Assume that transaction id is at offset 100.