[1956] Cleanup of the code in PerfPkt6.

Options moved away from PerfPkt6 to different class. Raw pack/unpack functions resturn bool.

tests/tools/perfdhcp/localized_option.h

@@ -0,0 +1,101 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __LOCALIZED_OPTION_H
+#define __LOCALIZED_OPTION_H
+
+#include <dhcp/pkt6.h>
+
+namespace isc {
+namespace perfdhcp {
+
+/// \brief DHCPv6 option at specific offset
+///
+/// This class represents DHCPv6 option at specified
+/// offset in DHCPv6 message.
+///
+class LocalizedOption : public dhcp::Option {
+public:
+    /// \brief Constructor, sets default (0) option offset
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param data content of the option
+    LocalizedOption(dhcp::Option::Universe u, uint16_t type, const dhcp::OptionBuffer& data) : 
+        dhcp::Option(u, type, data),
+        offset_(0) {
+    }
+
+
+    /// \brief Constructor, used to create localized option from buffer
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param data content of the option
+    /// \param offset location of option in a packet (zero is default)
+    LocalizedOption(dhcp::Option::Universe u, uint16_t type, const dhcp::OptionBuffer& data,
+                    const size_t offset) :
+        dhcp::Option(u, type, data),
+        offset_(offset) {
+    }
+
+    /// \brief Constructor, sets default (0) option offset
+    ///
+    /// This contructor is similar to the previous one, but it does not take
+    /// the whole vector<uint8_t>, but rather subset of it.
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param first iterator to the first element that should be copied
+    /// \param last iterator to the next element after the last one
+    ///        to be copied.
+    LocalizedOption(dhcp::Option::Universe u, uint16_t type, dhcp::OptionBufferConstIter first,
+                    dhcp::OptionBufferConstIter last) :
+        dhcp::Option(u, type, first, last),
+        offset_(0) {
+    }
+
+
+    /// \brief Constructor, used to create positioned option from buffer iterators
+    ///
+    /// This contructor is similar to the previous one, but it does not take
+    /// the whole vector<uint8_t>, but rather subset of it.
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param first iterator to the first element that should be copied
+    /// \param last iterator to the next element after the last one
+    ///        to be copied.
+    /// \param offset location of option in a packet (zero is default)
+    LocalizedOption(dhcp::Option::Universe u, uint16_t type, dhcp::OptionBufferConstIter first,
+                    dhcp::OptionBufferConstIter last, const size_t offset) :
+        dhcp::Option(u, type, first, last),
+        offset_(offset) {
+    }
+
+    /// \brief Returns absolute position (offset) of an option in a
+    /// DHCPv6 packet.
+    ///
+    /// \return option offset in a packet
+    size_t getOffset() const { return offset_; };
+    
+private:
+    size_t offset_;   ///< Offset of DHCPv6 option in a packet
+};
+
+
+} // namespace perfdhcp
+} // namespace isc
+
+#endif // __LOCALIZED_OPTION_H

tests/tools/perfdhcp/perf_pkt6.cc

@@ -12,6 +12,7 @@
 // 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>
@@ -24,39 +25,6 @@ using namespace isc;
 namespace isc {
 namespace perfdhcp {
 
-
-PerfPkt6::PositionedOption::PositionedOption(dhcp::Option::Universe u,
-                                             uint16_t type,
-                                             const dhcp::OptionBuffer& data) :
-    dhcp::Option(u, type, data),
-    position_(0) {
-}
-
-PerfPkt6::PositionedOption::PositionedOption(dhcp::Option::Universe u,
-                                             uint16_t type,
-                                             const dhcp::OptionBuffer& data,
-                                             const size_t position) :
-    dhcp::Option(u, type, data),
-    position_(position) {
-}
-
-PerfPkt6::PositionedOption::PositionedOption(dhcp::Option::Universe u,
-                                             uint16_t type,
-                                             dhcp::OptionBufferConstIter first,
-                                             dhcp::OptionBufferConstIter last) :
-    dhcp::Option(u, type, first, last),
-    position_(0) {
-}
-
-PerfPkt6::PositionedOption::PositionedOption(dhcp::Option::Universe u,
-                                             uint16_t type,
-                                             dhcp::OptionBufferConstIter first,
-                                             dhcp::OptionBufferConstIter last,
-                                             const size_t position) :
-    dhcp::Option(u, type, first, last),
-    position_(position) {
-}
-
 PerfPkt6::PerfPkt6(const uint8_t* buf, uint32_t len, size_t transid_offset) :
     Pkt6(buf, len, dhcp::Pkt6::UDP),
     transid_offset_(transid_offset) {
@@ -65,32 +33,8 @@ PerfPkt6::PerfPkt6(const uint8_t* buf, uint32_t len, size_t transid_offset) :
     memset(static_cast<void*>(&time_stamp_), 0, sizeof(time_stamp_));
 }
 
-void
-PerfPkt6::stampedPack() {
-    // This function simply wraps pack() function from parent class
-    // and then updates timestamp.
-    if (!pack()) {
-        isc_throw(isc::Unexpected, "Failed to prepare packet to send");
-    }
-    // Update pack time for RTT calculations.
-    updateTimestamp();
-}
-
-void
-PerfPkt6::stampedUnpack() {
-    // Update timestamp of the packet for RTT calculations.
-    // We do this before unpacking the packet so as unpack
-    // time will not affect the timestamp.
-    updateTimestamp();
-
-    // Unpack the incoming packet.
-    if (!unpack()) {
-        isc_throw(isc::Unexpected, "Failed to unpack incoming packet");
-    }
-}
-
-void
-PerfPkt6::stampedRawPack() {
+bool
+PerfPkt6::rawPack() {
     try {
         // Always override the packet if function is called.
         bufferOut_.clear();
@@ -99,19 +43,16 @@ PerfPkt6::stampedRawPack() {
         // We already have packet template stored in out buffer
         // but still some options have to be updated if client
         // specified them along with their offsets in the buffer.
-        updateOptions();
-    } catch (Exception& e) {
-        isc_throw(isc::Unexpected, "Failed to build packet from buffer");
+        rawPackOptions();
+    } catch (isc::Unexpected& e) {
+        cout << "Failed to build packet: " << e.what() << endl;
+        return (false);
     }
-    // Update packet timestamp for RTT calculations - do not include pack time.
-    updateTimestamp();
+    return (true);
 }
 
-void
-PerfPkt6::stampedRawUnpack() {
-    // Update packet timestamp for RTT calculations but do not include unpack time.
-    updateTimestamp();
-
+bool
+PerfPkt6::rawUnpack() {
     // Read transaction id from the packet at the given offset.
     transid_ = ( (data_[transid_offset_]) << 16 ) +
         ((data_[transid_offset_ + 1]) << 8) + (data_[transid_offset_ + 2]);
@@ -119,22 +60,26 @@ PerfPkt6::stampedRawUnpack() {
 
     // Get message type - assume it is first octet in the packet.
     msg_type_ = data_[0];
-
-    rawUnpackOptions();
+    try {
+        rawUnpackOptions();
+    } catch (isc::Unexpected& e) {
+        cout << "Packet parsing failed: " << e.what() << endl;
+        return (false);
+    }
+    return (true);
 }
 
 
 void
-PerfPkt6::updateOptions() {
+PerfPkt6::rawPackOptions() {
     try {
         // If there are any options on the list we will use provided options offsets
         // to override them in the output buffer with new contents.
         for (dhcp::Option::OptionCollection::const_iterator it = options_.begin();
              it != options_.end(); ++it) {
             // Get options with their position (offset).
-            boost::shared_ptr<PositionedOption> option = boost::dynamic_pointer_cast<PositionedOption>(it->second);
-            uint32_t position = option->getOptionPosition();
-            // If position is specified w need to seek to it.
+            boost::shared_ptr<LocalizedOption> option = boost::dynamic_pointer_cast<LocalizedOption>(it->second);
+            uint32_t position = option->getOffset();
             if (position > 0) {
                 bufferOut_.clear();
                 bufferOut_.skip(position);
@@ -142,6 +87,9 @@ PerfPkt6::updateOptions() {
             // Replace existing option with new value.
             option->pack(bufferOut_);
         }
+        // Seek to the end of the end of the buffer
+        bufferOut_.clear();
+        bufferOut_.skip(data_.size());
     }
     catch (const Exception&) {
         isc_throw(isc::Unexpected, "Failed to build packet (Option build failed");
@@ -153,8 +101,8 @@ PerfPkt6::rawUnpackOptions() {
     for (dhcp::Option::OptionCollection::const_iterator it = options_.begin();
          it != options_.end(); ++it) {
 
-        PositionedOptionPtr option = boost::dynamic_pointer_cast<PositionedOption>(it->second);
-        size_t opt_pos = option->getOptionPosition();
+        LocalizedOptionPtr option = boost::dynamic_pointer_cast<LocalizedOption>(it->second);
+        size_t opt_pos = option->getOffset();
 
         if (opt_pos == 0) {
             isc_throw(isc::BadValue, "Failed to unpack packet from raw buffer "

tests/tools/perfdhcp/perf_pkt6.h

@@ -19,6 +19,8 @@
 #include <boost/shared_ptr.hpp>
 #include <dhcp/pkt6.h>
 
+#include "localized_option.h"
+
 namespace isc {
 namespace perfdhcp {
 
@@ -43,7 +45,7 @@ namespace perfdhcp {
 /// in a template packet, we need to add these selected options
 /// to packet object using addOption() method. Please note
 /// that options must be of the
-/// \ref isc::perfdhcp::PerfPkt6::PositionedOption type.
+/// \ref isc::perfdhcp::LocalizedOption type.
 ///
 /// This class also records timestamps of last pack/unpack
 /// operation on the packet. This is to track DHCP server
@@ -54,74 +56,8 @@ namespace perfdhcp {
 ///
 class PerfPkt6 : public dhcp::Pkt6 {
 public:
-
-    /// \brief DHCPv6 option at specific offset
-    ///
-    /// This class represents DHCPv6 option at specified
-    /// offset in DHCPv6 message.
-    ///
-    class PositionedOption : public dhcp::Option {
-    public:
-
-        /// \brief Constructor, sets default (0) option position
-        ///
-        ///
-        /// \param u specifies universe (V4 or V6)
-        /// \param type option type (0-255 for V4 and 0-65535 for V6)
-        /// \param data content of the option
-        PositionedOption(dhcp::Option::Universe u, uint16_t type, const dhcp::OptionBuffer& data);
-
-
-        /// \brief Constructor, used to create positioned option from buffer
-        ///
-        ///
-        /// \param u specifies universe (V4 or V6)
-        /// \param type option type (0-255 for V4 and 0-65535 for V6)
-        /// \param data content of the option
-        /// \param position absolute position of option in a packet (zero is default)
-        PositionedOption(dhcp::Option::Universe u, uint16_t type, const dhcp::OptionBuffer& data,
-                         const size_t position);
-
-        /// \brief Constructor, sets default (0) option position
-        ///
-        /// This contructor is similar to the previous one, but it does not take
-        /// the whole vector<uint8_t>, but rather subset of it.
-        ///
-        /// \param u specifies universe (V4 or V6)
-        /// \param type option type (0-255 for V4 and 0-65535 for V6)
-        /// \param first iterator to the first element that should be copied
-        /// \param last iterator to the next element after the last one
-        ///        to be copied.
-        /// \param position absolute position of option in a packet (zero is default)
-        PositionedOption(dhcp::Option::Universe u, uint16_t type, dhcp::OptionBufferConstIter first,
-                         dhcp::OptionBufferConstIter last);
-
-        /// \brief Constructor, used to create positioned option from buffer iterators
-        ///
-        /// This contructor is similar to the previous one, but it does not take
-        /// the whole vector<uint8_t>, but rather subset of it.
-        ///
-        /// \param u specifies universe (V4 or V6)
-        /// \param type option type (0-255 for V4 and 0-65535 for V6)
-        /// \param first iterator to the first element that should be copied
-        /// \param last iterator to the next element after the last one
-        ///        to be copied.
-        /// \param position absolute position of option in a packet (zero is default)
-        PositionedOption(dhcp::Option::Universe u, uint16_t type, dhcp::OptionBufferConstIter first,
-                         dhcp::OptionBufferConstIter last, const size_t position);
-
-
-        /// \brief Returns absolute position (offset) of an option in a
-        /// DHCPv6 packet.
-        ///
-        /// \return absolute position (offset) of an option in a packet
-        size_t getOptionPosition() const { return position_; };
-
-    private:
-        size_t position_;   ///< Absolute position of DHCPv6 option in a packet
-    };
-
-    typedef boost::shared_ptr<PositionedOption> PositionedOptionPtr;
+    /// Localized option pointer type.
+    typedef boost::shared_ptr<LocalizedOption> LocalizedOptionPtr;
 
     /// Constructor, used in message transmission
     ///
@@ -130,7 +66,7 @@ public:
     /// options at custom offsets (e.g. if packet was read from
     /// template file) additional information about options'
     /// offsets has to be provided - see
-    /// \ref isc::perfdhcp::PositionedOption for details.
+    /// \ref isc::perfdhcp::LocalizedOption for details.
     ///
     /// Transaction id is not considered DHCP option so
     /// we pass it to constructor as extra argument. This is
@@ -159,29 +95,6 @@ public:
     /// \return current packet timestamp
     timespec getTimestamp() const { return time_stamp_; }
 
-    /// \brief Prepare on-wire packet format and record timestamp
-    ///
-    /// Prepares on-wire format of packet and all its options.
-    /// This method wraps dhcp::Pkt6::pack() function to
-    /// update packet timestamp.
-    ///
-    /// \note Use this function if you don't use template files
-    /// to construct DHCPv6 packets.
-    ///
-    /// \throw isc::Unexpected if pack and timestamp update failed
-    void stampedPack();
-
-    /// \brief Handles binary packet parsing and updates timestamp
-    ///
-    /// This method wraps dhcp::Pkt6::unpack() function to
-    /// update packet timestamp.
-    ///
-    /// \note Use this function if you don't use template files
-    /// and custom options offsets to construct DHCPv6 packets.
-    ///
-    /// \throw isc::Unexpected if function failed
-    void stampedUnpack();
-
     /// \brief Prepares on-wire format from raw buffer
     ///
     /// The method copies user buffer to output buffer and
@@ -193,8 +106,8 @@ public:
     /// when you use template packets that require replacement
     /// of selected options contents before sending.
     ///
-    /// \throw isc::Unexepected if function failed
-    void stampedRawPack();
+    /// \retrun false, id pack operation failed.
+    bool rawPack();
 
     /// \brief Handles limited binary packet parsing for packets with
     /// custom offsets of options and transaction id
@@ -202,11 +115,16 @@ public:
     /// Function handles reception of packets that have non-default values
     /// of options or transaction id offsets. Use
     /// \ref isc::dhcp::Pkt6::addOption to specify which options to parse.
-    /// Each option should be of the: isc::perfdhcp::PerfPkt6::PositionedOption
+    /// Each option should be of the: isc::perfdhcp::LocalizedOption
     /// type with offset value indicated.
     ///
-    /// \throw isc::Unexpected if function failed
-    void stampedRawUnpack();
+    /// \return false, if unpack operation failed.
+    bool rawUnpack();
+
+    /// \brief Update packet timestamp with current time
+    ///
+    /// \throw isc::Unexpected if timestamp update failed
+    void updateTimestamp();
 
 private:
 
@@ -216,19 +134,14 @@ private:
     /// with the ones provided with
     /// \ref isc::dhcp::Pkt6::addOption. It is expected
     /// that these options will be of the
-    /// \ref isc::perfdhcp::PerfPkt6::PositionedOption type
+    /// \ref isc::perfdhcp::LocalizedOption type
     /// with their position (offset) specified.
     ///
-    /// throw isc::Unexpected if options update fails
-    void updateOptions();
+    /// \throw isc::Unexpected if options update failed.
+    void rawPackOptions();
 
     void rawUnpackOptions();
 
-    /// \brief Update packet timestamp with current time
-    ///
-    /// \throw isc::Unexpected if timestamp update failed
-    void updateTimestamp();
-
     size_t transid_offset_;      ///< transaction id offset
     timespec time_stamp_;        ///< time stamp of last pack or unpack