Browse Source

[4498] Removed server specific method for unpacking DHCPv4 options.

Marcin Siodelski 9 years ago
parent
commit
baa4290727

+ 0 - 105
src/bin/dhcp4/dhcp4_srv.cc

@@ -646,15 +646,6 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
     isc::stats::StatsMgr::instance().addValue("pkt4-received",
                                               static_cast<int64_t>(1));
 
-    // In order to parse the DHCP options, the server needs to use some
-    // configuration information such as: existing option spaces, option
-    // definitions etc. This is the kind of information which is not
-    // available in the libdhcp, so we need to supply our own implementation
-    // of the option parsing function here, which would rely on the
-    // configuration data.
-    query->setCallback(boost::bind(&Dhcpv4Srv::unpackOptions, this,
-                                   _1, _2, _3));
-
     bool skip_unpack = false;
 
     // The packet has just been received so contains the uninterpreted wire
@@ -2339,102 +2330,6 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& query, RequirementLevel serverid) {
     }
 }
 
-size_t
-Dhcpv4Srv::unpackOptions(const OptionBuffer& buf,
-                         const std::string& option_space,
-                         isc::dhcp::OptionCollection& options) {
-    size_t offset = 0;
-
-    OptionDefContainer option_defs;
-    if (option_space == "dhcp4") {
-        // Get the list of standard option definitions.
-        option_defs = LibDHCP::getOptionDefs(Option::V4);
-    } else if (!option_space.empty()) {
-        OptionDefContainerPtr option_defs_ptr = CfgMgr::instance()
-            .getCurrentCfg()->getCfgOptionDef()->getAll(option_space);
-        if (option_defs_ptr != NULL) {
-            option_defs = *option_defs_ptr;
-        }
-    }
-    // Get the search index #1. It allows to search for option definitions
-    // using option code.
-    const OptionDefContainerTypeIndex& idx = option_defs.get<1>();
-
-    // The buffer being read comprises a set of options, each starting with
-    // a one-byte type code and a one-byte length field.
-    while (offset + 1 <= buf.size()) {
-        uint8_t opt_type = buf[offset++];
-
-        // DHO_END is a special, one octet long option
-        if (opt_type == DHO_END)
-            return (offset); // just return. Don't need to add DHO_END option
-
-        // DHO_PAD is just a padding after DHO_END. Let's continue parsing
-        // in case we receive a message without DHO_END.
-        if (opt_type == DHO_PAD)
-            continue;
-
-        if (offset + 1 > buf.size()) {
-            // opt_type must be cast to integer so as it is not treated as
-            // unsigned char value (a number is presented in error message).
-            isc_throw(OutOfRange, "Attempt to parse truncated option "
-                      << static_cast<int>(opt_type));
-        }
-
-        uint8_t opt_len =  buf[offset++];
-        if (offset + opt_len > buf.size()) {
-
-            // We peeked at the option header of the next option, but discovered
-            // that it would end up beyond buffer end, so the option is
-            // truncated. Hence we can't parse it. Therefore we revert
-            // back by two bytes (as if we never parsed them).
-            return (offset - 2);
-
-            // isc_throw(OutOfRange, "Option parse failed. Tried to parse "
-            //          << offset + opt_len << " bytes from " << buf.size()
-            //          << "-byte long buffer.");
-        }
-
-        // Get all definitions with the particular option code. Note that option code
-        // is non-unique within this container however at this point we expect
-        // to get one option definition with the particular code. If more are
-        // returned we report an error.
-        const OptionDefContainerTypeRange& range = idx.equal_range(opt_type);
-        // Get the number of returned option definitions for the option code.
-        size_t num_defs = distance(range.first, range.second);
-
-        OptionPtr opt;
-        if (num_defs > 1) {
-            // Multiple options of the same code are not supported right now!
-            isc_throw(isc::Unexpected, "Internal error: multiple option definitions"
-                      " for option type " << static_cast<int>(opt_type)
-                      << " returned. Currently it is not supported to initialize"
-                      << " multiple option definitions for the same option code."
-                      << " This will be supported once support for option spaces"
-                      << " is implemented");
-        } else if (num_defs == 0) {
-            opt = OptionPtr(new Option(Option::V4, opt_type,
-                                       buf.begin() + offset,
-                                       buf.begin() + offset + opt_len));
-            opt->setEncapsulatedSpace("dhcp4");
-        } else {
-            // The option definition has been found. Use it to create
-            // the option instance from the provided buffer chunk.
-            const OptionDefinitionPtr& def = *(range.first);
-            assert(def);
-            opt = def->optionFactory(Option::V4, opt_type,
-                                     buf.begin() + offset,
-                                     buf.begin() + offset + opt_len,
-                                     boost::bind(&Dhcpv4Srv::unpackOptions,
-                                                 this, _1, _2, _3));
-        }
-
-        options.insert(std::make_pair(opt_type, opt));
-        offset += opt_len;
-    }
-    return (offset);
-}
-
 void Dhcpv4Srv::classifyByVendor(const Pkt4Ptr& pkt, std::string& classes) {
     // Built-in vendor class processing
     boost::shared_ptr<OptionString> vendor_class =

+ 0 - 12
src/bin/dhcp4/dhcp4_srv.h

@@ -729,18 +729,6 @@ protected:
     /// simulates transmission of a packet. For that purpose it is protected.
     virtual void sendPacket(const Pkt4Ptr& pkt);
 
-    /// @brief Implements a callback function to parse options in the message.
-    ///
-    /// @param buf a A buffer holding options in on-wire format.
-    /// @param option_space A name of the option space which holds definitions
-    /// of to be used to parse options in the packets.
-    /// @param [out] options A reference to the collection where parsed options
-    /// will be stored.
-    /// @return An offset to the first byte after last parsed option.
-    size_t unpackOptions(const OptionBuffer& buf,
-                         const std::string& option_space,
-                         isc::dhcp::OptionCollection& options);
-
     /// @brief Assigns incoming packet to zero or more classes.
     ///
     /// @note This is done in two phases: first the content of the

+ 2 - 182
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -15,6 +15,7 @@
 #include <dhcp/tests/pkt_captures.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/iface_mgr.h>
+#include <dhcp/libdhcp++.h>
 #include <dhcp/option.h>
 #include <dhcp/option_int.h>
 #include <dhcp/option4_addrlst.h>
@@ -1142,187 +1143,6 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsis) {
 /// See ticket #3057
 
 
-// This test verifies that the following option structure can be parsed:
-// - option (option space 'foobar')
-//   - sub option (option space 'foo')
-//      - sub option (option space 'bar')
-// @todo Add more thorough unit tests for unpackOptions.
-TEST_F(Dhcpv4SrvTest, unpackSubOptions) {
-    // Create option definition for each level of encapsulation. Each option
-    // definition is for the option code 1. Options may have the same
-    // option code because they belong to different option spaces.
-
-    // Top level option encapsulates options which belong to 'space-foo'.
-    OptionDefinitionPtr opt_def(new OptionDefinition("option-foobar", 1, "uint32",
-                                                      "space-foo"));\
-    // Middle option encapsulates options which belong to 'space-bar'
-    OptionDefinitionPtr opt_def2(new OptionDefinition("option-foo", 1, "uint16",
-                                                      "space-bar"));
-    // Low level option doesn't encapsulate any option space.
-    OptionDefinitionPtr opt_def3(new OptionDefinition("option-bar", 1,
-                                                      "uint8"));
-
-    // Add option definitions to the Configuration Manager. Each goes under
-    // different option space.
-    CfgOptionDefPtr cfg_option_def =
-        CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def, "space-foobar"));
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def2, "space-foo"));
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def3, "space-bar"));
-    CfgMgr::instance().commit();
-
-    // Create the buffer holding the structure of options.
-    const uint8_t raw_data[] = {
-        // First option starts here.
-        0x01,                   // option code = 1
-        0x0B,                   // option length = 11
-        0x00, 0x01, 0x02, 0x03, // This option carries uint32 value
-        // Sub option starts here.
-        0x01,                   // option code = 1
-        0x05,                   // option length = 5
-        0x01, 0x02,             // this option carries uint16 value
-        // Last option starts here.
-        0x01,                   // option code = 1
-        0x01,                   // option length = 1
-        0x00                    // This option carries a single uint8
-                                // value and has no sub options.
-    };
-    size_t raw_data_len = sizeof(raw_data) / sizeof(uint8_t);
-    OptionBuffer buf(raw_data, raw_data + raw_data_len);
-
-    // Parse options.
-    NakedDhcpv4Srv srv(0);
-    OptionCollection options;
-    ASSERT_NO_THROW(srv.unpackOptions(buf, "space-foobar", options));
-
-    // There should be one top level option.
-    ASSERT_EQ(1, options.size());
-    boost::shared_ptr<OptionInt<uint32_t> > option_foobar =
-        boost::dynamic_pointer_cast<OptionInt<uint32_t> >(options.begin()->
-                                                          second);
-    ASSERT_TRUE(option_foobar);
-    EXPECT_EQ(1, option_foobar->getType());
-    EXPECT_EQ(0x00010203, option_foobar->getValue());
-    // There should be a middle level option held in option_foobar.
-    boost::shared_ptr<OptionInt<uint16_t> > option_foo =
-        boost::dynamic_pointer_cast<OptionInt<uint16_t> >(option_foobar->
-                                                          getOption(1));
-    ASSERT_TRUE(option_foo);
-    EXPECT_EQ(1, option_foo->getType());
-    EXPECT_EQ(0x0102, option_foo->getValue());
-    // Finally, there should be a low level option under option_foo.
-    boost::shared_ptr<OptionInt<uint8_t> > option_bar =
-        boost::dynamic_pointer_cast<OptionInt<uint8_t> >(option_foo->getOption(1));
-    ASSERT_TRUE(option_bar);
-    EXPECT_EQ(1, option_bar->getType());
-    EXPECT_EQ(0x0, option_bar->getValue());
-}
-
-// Check parsing of an empty option
-TEST_F(Dhcpv4SrvTest, unpackEmptyOption) {
-    // Create option definition for the option code 1 without fields.
-    OptionDefinitionPtr opt_def(new OptionDefinition("option-empty", 1,
-                                                     "empty", false));
-
-    // Add it to the Configuration Manager.
-    CfgOptionDefPtr cfg_option_def =
-        CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def, "space-empty"));
-    CfgMgr::instance().commit();
-
-    // Create the buffer holding the structure of the empty option.
-    const uint8_t raw_data[] = {
-      0x01,                     // option code = 1
-      0x00                      // option length = 0
-    };
-    size_t raw_data_len = sizeof(raw_data) / sizeof(uint8_t);
-    OptionBuffer buf(raw_data, raw_data + raw_data_len);
-
-    // Parse options.
-    NakedDhcpv4Srv srv(0);
-    OptionCollection options;
-    ASSERT_NO_THROW(srv.unpackOptions(buf, "space-empty", options));
-
-    // There should be one option.
-    ASSERT_EQ(1, options.size());
-    OptionPtr option_empty = options.begin()->second;
-    ASSERT_TRUE(option_empty);
-    EXPECT_EQ(1, option_empty->getType());
-    EXPECT_EQ(2, option_empty->len());
-}
-
-// Check parsing of an empty VSI sub option
-TEST_F(Dhcpv4SrvTest, unpackVSIOption) {
-    // Create the buffer holding the structure of the Vendor-Specific Info
-    const uint8_t raw_data[] = {
-      43,     // option code = DHO_VENDOR_ENCAPSULATED_OPTIONS
-      2,      // option length
-      0xdc,   // suboption code
-      0       // suboption length
-    };
-    size_t raw_data_len = sizeof(raw_data) / sizeof(uint8_t);
-    OptionBuffer buf(raw_data, raw_data + raw_data_len);
-
-    // Parse options.
-    NakedDhcpv4Srv srv(0);
-    OptionCollection options;
-    ASSERT_NO_THROW(srv.unpackOptions(buf, "dhcp4", options));
-
-    // There should be one option: the VSI
-    ASSERT_EQ(1, options.size());
-    OptionPtr vsi = options.begin()->second;
-    ASSERT_TRUE(vsi);
-    EXPECT_EQ(DHO_VENDOR_ENCAPSULATED_OPTIONS, vsi->getType());
-    OptionCollection suboptions = vsi->getOptions();
-
-    // There should be one suboption
-    ASSERT_EQ(1, suboptions.size());
-    OptionPtr eso = suboptions.begin()->second;
-    ASSERT_TRUE(eso);
-    EXPECT_EQ(0xdc, eso->getType());
-    EXPECT_EQ(2, eso->len());
-}
-
-// Check parsing of an empty VIVSI sub option
-TEST_F(Dhcpv4SrvTest, unpackVIVSIOption) {
-    // Create the buffer holding the structure of the Vendor-Identifying
-    // Vendor-Specific Info
-    const uint8_t raw_data[] = {
-      125,            // option code = DHO_VIVSO_SUBOPTIONS
-      7,              // option length
-      0, 0, 9, 0xbf,  // ISC enterprise number (2495)
-      2,              // option data length
-      0xdc,           // suboption code
-      0               // suboption length
-    };
-    size_t raw_data_len = sizeof(raw_data) / sizeof(uint8_t);
-    OptionBuffer buf(raw_data, raw_data + raw_data_len);
-
-    // Parse options.
-    NakedDhcpv4Srv srv(0);
-    OptionCollection options;
-    ASSERT_NO_THROW(srv.unpackOptions(buf, "dhcp4", options));
-
-    // There should be one option: the VIVSI
-    ASSERT_EQ(1, options.size());
-    OptionPtr vivsi = options.begin()->second;
-    ASSERT_TRUE(vivsi);
-    EXPECT_EQ(DHO_VIVSO_SUBOPTIONS, vivsi->getType());
-
-    // Cast to OptionVendor
-    OptionVendorPtr vsi = boost::dynamic_pointer_cast<OptionVendor>(vivsi);
-    ASSERT_TRUE(vsi);
-    EXPECT_EQ(2495, vsi->getVendorId());
-    OptionCollection suboptions = vsi->getOptions();
-
-    // There should be one suboption
-    ASSERT_EQ(1, suboptions.size());
-    OptionPtr eso = suboptions.begin()->second;
-    ASSERT_TRUE(eso);
-    EXPECT_EQ(0xdc, eso->getType());
-    EXPECT_EQ(2, eso->len());
-}
-
 // Checks whether the server uses default (0.0.0.0) siaddr value, unless
 // explicitly specified
 TEST_F(Dhcpv4SrvTest, siaddrDefault) {

+ 5 - 0
src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -11,6 +11,7 @@
 #include <cc/command_interpreter.h>
 #include <dhcp4/json_config_parser.h>
 #include <dhcp4/tests/dhcp4_test_utils.h>
+#include <dhcp/libdhcp++.h>
 #include <dhcp/option4_addrlst.h>
 #include <dhcp/option_int.h>
 #include <dhcp/option_int_array.h>
@@ -67,6 +68,8 @@ Dhcpv4SrvTest::Dhcpv4SrvTest()
     CfgMgr::instance().getStagingCfg()->getCfgSubnets4()->add(subnet_);
     CfgMgr::instance().commit();
 
+    LibDHCP::clearRuntimeOptionDefs();
+
     // Let's wipe all existing statistics.
     isc::stats::StatsMgr::instance().removeAll();
 }
@@ -77,6 +80,8 @@ Dhcpv4SrvTest::~Dhcpv4SrvTest() {
     CfgMgr::instance().clear();
     CfgMgr::instance().echoClientId(true);
 
+    LibDHCP::clearRuntimeOptionDefs();
+
     // Let's wipe all existing statistics.
     isc::stats::StatsMgr::instance().removeAll();
 }

+ 0 - 1
src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -194,7 +194,6 @@ public:
     using Dhcpv4Srv::acceptServerId;
     using Dhcpv4Srv::sanityCheck;
     using Dhcpv4Srv::srvidToString;
-    using Dhcpv4Srv::unpackOptions;
     using Dhcpv4Srv::classifyPacket;
     using Dhcpv4Srv::accept;
     using Dhcpv4Srv::acceptMessageType;

+ 10 - 1
src/lib/dhcp/libdhcp++.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -15,6 +15,7 @@
 #include <dhcp/option6_iaaddr.h>
 #include <dhcp/option_definition.h>
 #include <dhcp/option_int_array.h>
+#include <dhcp/option_space.h>
 #include <dhcp/std_option_defs.h>
 #include <dhcp/docsis3_option_defs.h>
 #include <exceptions/exceptions.h>
@@ -447,7 +448,14 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
     OptionDefContainer option_defs;
     if (option_space == "dhcp4") {
         option_defs = LibDHCP::getOptionDefs(Option::V4);
+    } else {
+        OptionDefContainerPtr option_defs_ptr =
+            LibDHCP::getRuntimeOptionDefs(option_space);
+        if (option_defs_ptr) {
+            option_defs = *option_defs_ptr;
+        }
     }
+
     // @todo Once we implement other option spaces we should add else clause
     // here and gather option definitions for them. For now leaving option_defs
     // empty will imply creation of generic Option.
@@ -521,6 +529,7 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
             opt = OptionPtr(new Option(Option::V4, opt_type,
                                        buf.begin() + offset,
                                        buf.begin() + offset + opt_len));
+            opt->setEncapsulatedSpace(DHCP4_OPTION_SPACE);
         } else {
             // The option definition has been found. Use it to create
             // the option instance from the provided buffer chunk.

+ 183 - 20
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -51,7 +51,19 @@ const uint16_t OPTION_CM_MAC = 1026;
 
 class LibDhcpTest : public ::testing::Test {
 public:
-    LibDhcpTest() { }
+    /// @brief Constructor.
+    ///
+    /// Removes runtime option definitions.
+    LibDhcpTest() {
+        LibDHCP::clearRuntimeOptionDefs();
+    }
+
+    /// @brief Destructor.
+    ///
+    /// Removes runtime option definitions.
+    virtual ~LibDhcpTest() {
+        LibDHCP::clearRuntimeOptionDefs();
+    }
 
     /// @brief Generic factory function to create any option.
     ///
@@ -510,16 +522,20 @@ TEST_F(LibDhcpTest, unpackOptions6) {
 /// For simplicity, we assign data of the length 3 for each
 /// of them.
 static uint8_t v4_opts[] = {
-    12,  3, 0,   1,  2, // Hostname
-    60,  3, 10, 11, 12, // Class Id
-    14,  3, 20, 21, 22, // Merit Dump File
-    254, 3, 30, 31, 32, // Reserved
-    128, 3, 40, 41, 42, // Vendor specific
-    0x52, 0x19,         // RAI
+    12,  3, 0,   1,  2,    // Hostname
+    60,  3, 10, 11, 12,    // Class Id
+    14,  3, 20, 21, 22,    // Merit Dump File
+    254, 3, 30, 31, 32,    // Reserved
+    128, 3, 40, 41, 42,    // Vendor specific
+    125, 7, 0, 0, 9, 0xBF, // V-I Vendor-Specific Information
+    2, 0xDC, 0,            // Suboption of VIVSI
+    43, 2,                 // Vendor Specific Information
+    0xDC, 0,               // VSI suboption
+    0x52, 0x19,            // RAI
     0x01, 0x04, 0x20, 0x00, 0x00, 0x02, // Agent Circuit ID
     0x02, 0x06, 0x20, 0xE5, 0x2A, 0xB8, 0x15, 0x14, // Agent Remote ID
     0x09, 0x09, 0x00, 0x00, 0x11, 0x8B, 0x04, // Vendor Specific Information
-    0x01, 0x02, 0x03, 0x00  // Vendor Specific Information continued
+    0x01, 0x02, 0x03, 0x00 // Vendor Specific Information continued
 };
 
 // This test verifies that pack options for v4 is working correctly.
@@ -539,6 +555,13 @@ TEST_F(LibDhcpTest, packOptions4) {
     OptionPtr opt4(new Option(Option::V4,254, payload[3]));
     OptionPtr opt5(new Option(Option::V4,128, payload[4]));
 
+    OptionVendorPtr vivsi(new OptionVendor(Option::V4, 0x9BF));
+    vivsi->addOption(OptionPtr(new Option(Option::V4, 0xDC, OptionBuffer())));
+
+    OptionPtr vsi(new Option(Option::V4, DHO_VENDOR_ENCAPSULATED_OPTIONS,
+                              OptionBuffer()));
+    vsi->addOption(OptionPtr(new Option(Option::V4, 0xDC, OptionBuffer())));
+
     // Add RAI option, which comprises 3 sub-options.
 
     // Get the option definition for RAI option. This option is represented
@@ -556,19 +579,19 @@ TEST_F(LibDhcpTest, packOptions4) {
 
     // Create Ciruit ID sub-option and add to RAI.
     OptionPtr circuit_id(new Option(Option::V4, RAI_OPTION_AGENT_CIRCUIT_ID,
-                                    OptionBuffer(v4_opts + 29,
-                                                 v4_opts + 33)));
+                                    OptionBuffer(v4_opts + 42,
+                                                 v4_opts + 46)));
     rai->addOption(circuit_id);
 
     // Create Remote ID option and add to RAI.
     OptionPtr remote_id(new Option(Option::V4, RAI_OPTION_REMOTE_ID,
-                                   OptionBuffer(v4_opts + 35, v4_opts + 41)));
+                                   OptionBuffer(v4_opts + 48, v4_opts + 54)));
     rai->addOption(remote_id);
 
     // Create Vendor Specific Information and add to RAI.
-    OptionPtr vsi(new Option(Option::V4, RAI_OPTION_VSI,
-                             OptionBuffer(v4_opts + 43, v4_opts + 52)));
-    rai->addOption(vsi);
+    OptionPtr rai_vsi(new Option(Option::V4, RAI_OPTION_VSI,
+                                 OptionBuffer(v4_opts + 56, v4_opts + 65)));
+    rai->addOption(rai_vsi);
 
     isc::dhcp::OptionCollection opts; // list of options
     // Note that we insert each option under the same option code into
@@ -580,6 +603,8 @@ TEST_F(LibDhcpTest, packOptions4) {
     opts.insert(make_pair(opt1->getType(), opt3));
     opts.insert(make_pair(opt1->getType(), opt4));
     opts.insert(make_pair(opt1->getType(), opt5));
+    opts.insert(make_pair(opt1->getType(), vivsi));
+    opts.insert(make_pair(opt1->getType(), vsi));
     opts.insert(make_pair(opt1->getType(), rai));
 
     OutputBuffer buf(100);
@@ -659,19 +684,50 @@ TEST_F(LibDhcpTest, unpackOptions4) {
     EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4_opts + 12, 3)); // data len=3
 
     x = options.find(254);
-    ASSERT_FALSE(x == options.end()); // option 3 should exist
+    ASSERT_FALSE(x == options.end()); // option 4 should exist
     EXPECT_EQ(254, x->second->getType());  // this should be option 254
     ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->second->len()); // total option length 5
     EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 17, 3)); // data len=3
 
     x = options.find(128);
-    ASSERT_FALSE(x == options.end()); // option 3 should exist
-    EXPECT_EQ(128, x->second->getType());  // this should be option 254
+    ASSERT_FALSE(x == options.end()); // option 5 should exist
+    EXPECT_EQ(128, x->second->getType());  // this should be option 128
     ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->second->len()); // total option length 5
     EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 22, 3)); // data len=3
 
+    // Verify that V-I Vendor Specific Information option is parsed correctly.
+    x = options.find(125);
+    ASSERT_FALSE(x == options.end());
+    OptionVendorPtr vivsi = boost::dynamic_pointer_cast<OptionVendor>(x->second);
+    ASSERT_TRUE(vivsi);
+    EXPECT_EQ(DHO_VIVSO_SUBOPTIONS, vivsi->getType());
+    EXPECT_EQ(2495, vivsi->getVendorId());
+    OptionCollection suboptions = vivsi->getOptions();
+
+    // There should be one suboption of V-I VSI.
+    ASSERT_EQ(1, suboptions.size());
+    OptionPtr eso = suboptions.begin()->second;
+    ASSERT_TRUE(eso);
+    EXPECT_EQ(0xdc, eso->getType());
+    EXPECT_EQ(2, eso->len());
+
+    // Vendor Specific Information option
+    x = options.find(43);
+    ASSERT_FALSE(x == options.end());
+    OptionPtr vsi = x->second;
+    ASSERT_TRUE(vsi);
+    EXPECT_EQ(DHO_VENDOR_ENCAPSULATED_OPTIONS, vsi->getType());
+    suboptions = vsi->getOptions();
+
+    // There should be one suboption of VSI.
+    ASSERT_EQ(1, suboptions.size());
+    eso = suboptions.begin()->second;
+    ASSERT_TRUE(eso);
+    EXPECT_EQ(0xdc, eso->getType());
+    EXPECT_EQ(2, eso->len());
+
     // Checking DHCP Relay Agent Information Option.
     x = options.find(DHO_DHCP_AGENT_OPTIONS);
     ASSERT_FALSE(x == options.end());
@@ -694,21 +750,21 @@ TEST_F(LibDhcpTest, unpackOptions4) {
     ASSERT_TRUE(rai_option);
     EXPECT_EQ(RAI_OPTION_AGENT_CIRCUIT_ID, rai_option->getType());
     ASSERT_EQ(6, rai_option->len());
-    EXPECT_EQ(0, memcmp(&rai_option->getData()[0], v4_opts + 29, 4));
+    EXPECT_EQ(0, memcmp(&rai_option->getData()[0], v4_opts + 42, 4));
 
     // Check that Remote ID option is among parsed options.
     rai_option = rai->getOption(RAI_OPTION_REMOTE_ID);
     ASSERT_TRUE(rai_option);
     EXPECT_EQ(RAI_OPTION_REMOTE_ID, rai_option->getType());
     ASSERT_EQ(8, rai_option->len());
-    EXPECT_EQ(0, memcmp(&rai_option->getData()[0], v4_opts + 35, 6));
+    EXPECT_EQ(0, memcmp(&rai_option->getData()[0], v4_opts + 48, 6));
 
     // Check that Vendor Specific Information option is among parsed options.
     rai_option = rai->getOption(RAI_OPTION_VSI);
     ASSERT_TRUE(rai_option);
     EXPECT_EQ(RAI_OPTION_VSI, rai_option->getType());
     ASSERT_EQ(11, rai_option->len());
-    EXPECT_EQ(0, memcmp(&rai_option->getData()[0], v4_opts + 43, 9));
+    EXPECT_EQ(0, memcmp(&rai_option->getData()[0], v4_opts + 56, 9));
 
     // Make sure, that option other than those above is not present.
     EXPECT_FALSE(rai->getOption(10));
@@ -725,6 +781,113 @@ TEST_F(LibDhcpTest, unpackOptions4) {
 
 }
 
+// Check parsing of an empty option
+TEST_F(LibDhcpTest, unpackEmptyOption) {
+    // Create option definition for the option code 1 without fields.
+    OptionDefinitionPtr opt_def(new OptionDefinition("option-empty", 1,
+                                                     "empty", false));
+
+    // Use it as runtime option definition.
+    OptionDefSpaceContainer defs;
+    defs.addItem(opt_def, "space-empty");
+    LibDHCP::setRuntimeOptionDefs(defs);
+    LibDHCP::commitRuntimeOptionDefs();
+
+    // Create the buffer holding the structure of the empty option.
+    const uint8_t raw_data[] = {
+      0x01,                     // option code = 1
+      0x00                      // option length = 0
+    };
+    size_t raw_data_len = sizeof(raw_data) / sizeof(uint8_t);
+    OptionBuffer buf(raw_data, raw_data + raw_data_len);
+
+    // Parse options.
+    OptionCollection options;
+    ASSERT_NO_THROW(LibDHCP::unpackOptions4(buf, "space-empty",
+                                            options));
+
+    // There should be one option.
+    ASSERT_EQ(1, options.size());
+    OptionPtr option_empty = options.begin()->second;
+    ASSERT_TRUE(option_empty);
+    EXPECT_EQ(1, option_empty->getType());
+    EXPECT_EQ(2, option_empty->len());
+}
+
+// This test verifies that the following option structure can be parsed:
+// - option (option space 'foobar')
+//   - sub option (option space 'foo')
+//      - sub option (option space 'bar')
+// @todo Add more thorough unit tests for unpackOptions.
+TEST_F(LibDhcpTest, unpackSubOptions) {
+    // Create option definition for each level of encapsulation. Each option
+    // definition is for the option code 1. Options may have the same
+    // option code because they belong to different option spaces.
+
+    // Top level option encapsulates options which belong to 'space-foo'.
+    OptionDefinitionPtr opt_def(new OptionDefinition("option-foobar", 1, "uint32",
+                                                      "space-foo"));\
+    // Middle option encapsulates options which belong to 'space-bar'
+    OptionDefinitionPtr opt_def2(new OptionDefinition("option-foo", 1, "uint16",
+                                                      "space-bar"));
+    // Low level option doesn't encapsulate any option space.
+    OptionDefinitionPtr opt_def3(new OptionDefinition("option-bar", 1,
+                                                      "uint8"));
+
+    // Register created option definitions as runtime option definitions.
+    OptionDefSpaceContainer defs;
+    ASSERT_NO_THROW(defs.addItem(opt_def, "space-foobar"));
+    ASSERT_NO_THROW(defs.addItem(opt_def2, "space-foo"));
+    ASSERT_NO_THROW(defs.addItem(opt_def3, "space-bar"));
+    LibDHCP::setRuntimeOptionDefs(defs);
+    LibDHCP::commitRuntimeOptionDefs();
+
+    // Create the buffer holding the structure of options.
+    const uint8_t raw_data[] = {
+        // First option starts here.
+        0x01,                   // option code = 1
+        0x0B,                   // option length = 11
+        0x00, 0x01, 0x02, 0x03, // This option carries uint32 value
+        // Sub option starts here.
+        0x01,                   // option code = 1
+        0x05,                   // option length = 5
+        0x01, 0x02,             // this option carries uint16 value
+        // Last option starts here.
+        0x01,                   // option code = 1
+        0x01,                   // option length = 1
+        0x00                    // This option carries a single uint8
+                                // value and has no sub options.
+    };
+    size_t raw_data_len = sizeof(raw_data) / sizeof(uint8_t);
+    OptionBuffer buf(raw_data, raw_data + raw_data_len);
+
+    // Parse options.
+    OptionCollection options;
+    ASSERT_NO_THROW(LibDHCP::unpackOptions4(buf, "space-foobar", options));
+
+    // There should be one top level option.
+    ASSERT_EQ(1, options.size());
+    boost::shared_ptr<OptionInt<uint32_t> > option_foobar =
+        boost::dynamic_pointer_cast<OptionInt<uint32_t> >(options.begin()->
+                                                          second);
+    ASSERT_TRUE(option_foobar);
+    EXPECT_EQ(1, option_foobar->getType());
+    EXPECT_EQ(0x00010203, option_foobar->getValue());
+    // There should be a middle level option held in option_foobar.
+    boost::shared_ptr<OptionInt<uint16_t> > option_foo =
+        boost::dynamic_pointer_cast<OptionInt<uint16_t> >(option_foobar->
+                                                          getOption(1));
+    ASSERT_TRUE(option_foo);
+    EXPECT_EQ(1, option_foo->getType());
+    EXPECT_EQ(0x0102, option_foo->getValue());
+    // Finally, there should be a low level option under option_foo.
+    boost::shared_ptr<OptionInt<uint8_t> > option_bar =
+        boost::dynamic_pointer_cast<OptionInt<uint8_t> >(option_foo->getOption(1));
+    ASSERT_TRUE(option_bar);
+    EXPECT_EQ(1, option_bar->getType());
+    EXPECT_EQ(0x0, option_bar->getValue());
+}
+
 TEST_F(LibDhcpTest, isStandardOption4) {
     // Get all option codes that are not occupied by standard options.
     const uint16_t unassigned_codes[] = { 84, 96, 102, 103, 104, 105, 106, 107, 108,