[1956] Code cleanup - many comments updated.

tests/tools/perfdhcp/localized_option.h

@@ -20,10 +20,25 @@
 namespace isc {
 namespace perfdhcp {
 
-/// \brief DHCPv6 option at specific offset
+/// \brief DHCP option at specific offset
 ///
-/// This class represents DHCPv6 option at specified
-/// offset in DHCPv6 message.
+/// This class represents DHCP option at specified
+/// offset in DHCP message.
+/// Objects of this type are intended to be used when DHCP packets
+/// are created from templates (e.g. read from template file).
+/// Such packets have number of that have to be replaced before
+/// sending: e.g. DUID can be randomized. If option of this type
+/// is added to \ref PerfPkt6 options collection,
+/// \ref perfdhcp::PerfPkt6 will call \ref getOffset on this object
+/// to retrieve user-defined option position and replace contents of
+/// the output buffer at this offset before packet is sent to the server.
+/// (\see perfdhcp::PerfPkt6::rawPack).
+/// In order to read on-wire data from incoming packet client class
+/// has to specify options of \ref perfdhcp::LocalizedOption type
+/// with expected offsets of these options in a packet. The
+/// \ret perfdhcp::PerfPkt6 will use offsets to read fragments
+/// of packet and store them in options' buffers.
+/// (\see perfdhcp::PerfPkt6::rawUnpack).
 ///
 class LocalizedOption : public dhcp::Option {
 public:
@@ -32,7 +47,7 @@ public:
     /// \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) : 
+    LocalizedOption(dhcp::Option::Universe u, uint16_t type, const dhcp::OptionBuffer& data) :
         dhcp::Option(u, type, data),
         offset_(0) {
     }
@@ -77,21 +92,20 @@ public:
     /// \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)
+    /// \param offset offset 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.
+    /// \brief Returns offset of an option in a DHCP packet.
     ///
     /// \return option offset in a packet
     size_t getOffset() const { return offset_; };
-    
+
 private:
-    size_t offset_;   ///< Offset of DHCPv6 option in a packet
+    size_t offset_;   ///< Offset of DHCP option in a packet
 };
 
 

tests/tools/perfdhcp/perf_pkt6.cc

@@ -87,9 +87,8 @@ PerfPkt6::rawPackOptions() {
             // 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());
+        // Seek to the end of the buffer to make sure size is correct.
+        bufferOut_.skip(data_.size() - bufferOut_.getLength());
     }
     catch (const Exception&) {
         isc_throw(isc::Unexpected, "Failed to build packet (Option build failed");
@@ -113,7 +112,11 @@ PerfPkt6::rawUnpackOptions() {
         }
 
         size_t offset = opt_pos;
-        //        uint16_t opt_type = data_[offset] * 256 + data_[offset + 1];
+        uint16_t opt_type = data_[offset] * 256 + data_[offset + 1];
+        if (opt_type != option->getType()) {
+            isc_throw(isc::BadValue,
+                      "Failed to unpack option from raw buffer (Option type mismatch)");
+        }
         offset += 2;
         uint16_t opt_len = data_[offset] * 256 + data_[offset + 1];
         offset += 2;

tests/tools/perfdhcp/perf_pkt6.h

@@ -47,10 +47,6 @@ namespace perfdhcp {
 /// that options must be of the
 /// \ref isc::perfdhcp::LocalizedOption type.
 ///
-/// This class also records timestamps of last pack/unpack
-/// operation on the packet. This is to track DHCP server
-/// performance based on packet's send/receive duration.
-///
 /// \note: if you don't use template files simply use constructors
 /// inherited from parent class and isc::dhcp::Option type instead
 ///
@@ -100,11 +96,10 @@ public:
     /// The method copies user buffer to output buffer and
     /// extracts transaction id from it based on transaction id
     /// offset provided in constructor.
-    /// Eventually, this method updates packet timestamp.
     ///
     /// \note: Use this method to prepare on-wire DHCPv6 message
     /// when you use template packets that require replacement
-    /// of selected options contents before sending.
+    /// of selected options' contents before sending.
     ///
     /// \retrun false, id pack operation failed.
     bool rawPack();
@@ -130,16 +125,32 @@ private:
 
     /// \brief Updates options in the output buffer
     ///
-    /// This method updates options in the output buffer
-    /// with the ones provided with
-    /// \ref isc::dhcp::Pkt6::addOption. It is expected
-    /// that these options will be of the
-    /// \ref isc::perfdhcp::LocalizedOption type
-    /// with their position (offset) specified.
+    /// The method uses options collection added to object
+    /// of this class with \ref dhcp::Pkt6::addOption to
+    /// on-wire data. Option objects has to be of
+    /// \ref perfdhcp::LocalizedOption type and should
+    /// have non-zero values of offsets specified.
+    /// This method will use these offsets to seek to
+    /// given position in output buffer and update option
+    /// on-wire data with contents of option's buffer.
     ///
     /// \throw isc::Unexpected if options update failed.
     void rawPackOptions();
 
+    /// \brief Reads contents of specified options from buffer
+    ///
+    /// The method reads options data from the copy of the buffer
+    /// provided in constructor and stores data in options
+    /// objects that belong to options collection.
+    /// Client class that constructs this object has to create
+    /// options collection prior to calling \ref rawUnpack
+    /// method that in turn calls this method.
+    /// If option is not added to options collection, it will
+    /// not be added by this method. This method will rather
+    /// skip update of such an option even if it is present
+    /// in packet's buffer.
+    ///
+    /// \throw isc::Unexpected if options unpack failed.
     void rawUnpackOptions();
 
     size_t transid_offset_;      ///< transaction id offset