[1186] Remaining tests for DHCPv6 Options fixed/implemented.

src/lib/dhcp/libdhcp.cc

@@ -45,9 +45,11 @@ LibDHCP::unpackOptions6(boost::shared_array<char> buf, unsigned int buf_len,
     unsigned int end = offset + parse_len;
 
     while (offset<end) {
-        unsigned int opt_type = buf[offset]*256 + buf[offset+1];
+        unsigned int opt_type = static_cast<unsigned char>(buf[offset])*256
+            + static_cast<unsigned char>(buf[offset+1]);
         offset += 2;
-        unsigned int opt_len = buf[offset]*256 + buf[offset+1];
+        unsigned int opt_len = static_cast<unsigned char>(buf[offset]*256)
+            + static_cast<unsigned char>(buf[offset+1]);
         offset += 2;
 
         if (offset + opt_len > end ) {

src/lib/dhcp/option.cc

@@ -176,6 +176,26 @@ isc::dhcp::Option::addOption(boost::shared_ptr<isc::dhcp::Option> opt) {
 
 }
 
+boost::shared_ptr<isc::dhcp::Option>
+Option::getOption(unsigned short opt_type) {
+    isc::dhcp::Option::Option6Lst::const_iterator x = optionLst_.find(opt_type);
+    if (x!=optionLst_.end()) {
+        return (*x).second;
+    }
+    return boost::shared_ptr<isc::dhcp::Option>(); // NULL
+}
+
+bool
+Option::delOption(unsigned short opt_type) {
+    isc::dhcp::Option::Option6Lst::iterator x = optionLst_.find(opt_type);
+    if (x!=optionLst_.end()) {
+        optionLst_.erase(x);
+        return true; // delete successful
+    }
+    return (false); // option not found, can't delete
+}
+
+
 std::string Option::toText(int indent /* =0 */ ) {
     std::stringstream tmp;
 
@@ -191,6 +211,13 @@ std::string Option::toText(int indent /* =0 */ ) {
         tmp << setfill('0') << setw(2) << hex
             << (unsigned short)(unsigned char)data_[offset_+i];
     }
+
+    // print suboptions
+    for (Option6Lst::const_iterator opt=optionLst_.begin();
+         opt!=optionLst_.end();
+         ++opt) {
+        tmp << (*opt).second->toText(indent+2);
+    }
     return tmp.str();
 }
 

src/lib/dhcp/option.h

@@ -53,16 +53,11 @@ public:
          unsigned int buf_len,
          unsigned int offset);
 
-    // parses received buffer, returns pointer to first unused byte
-    // after parsed option
-    // TODO: Do we need this overload? Commented out for now
-    // virtual const char* unpack(const char* buf, unsigned int len);
-
-    // parses received buffer, returns offset to the first unused byte after
-    // parsed option
-
     ///
-    /// Parses buffer and creates collection of Option objects.
+    /// @brief Parses buffer.
+    ///
+    /// Parses received buffer, returns offset to the first unused byte after
+    /// parsed option.
     ///
     /// @param buf pointer to buffer
     /// @param buf_len length of buf
@@ -128,6 +123,26 @@ public:
     void
     addOption(boost::shared_ptr<Option> opt);
 
+    /// Returns shared_ptr to suboption of specific type
+    ///
+    /// @param type type of requested suboption
+    ///
+    /// @return shared_ptr to requested suoption
+    ///
+    boost::shared_ptr<isc::dhcp::Option>
+    getOption(unsigned short type);
+
+    /// Attempts to delete first suboption of requested type
+    ///
+    /// @param type Type of option to be deleted.
+    ///
+    /// @return true if option was deleted, false if no such option existed
+    ///
+    bool
+    delOption(unsigned short type);
+
+    /// TODO Need to implement getOptions() as well
+
     // just to force that every option has virtual dtor
     virtual
     ~Option();
@@ -164,7 +179,6 @@ protected:
           unsigned int buf_len,
           unsigned int offset);
 
-
     ///
     /// Parses provided buffer and creates DHCPv4 options.
     ///

src/lib/dhcp/option6_ia.cc

@@ -94,22 +94,25 @@ std::string Option6IA::toText(int indent /* = 0*/) {
 
     for (int i=0; i<indent; i++)
         tmp << " ";
+    tmp << "type=" << type_;
 
     switch (type_) {
     case D6O_IA_NA:
-        tmp << "IA_NA";
+        tmp << "(IA_NA)";
         break;
     case D6O_IA_PD:
-        tmp << "IA_PD";
+        tmp << "(IA_PD)";
         break;
+    default:
+        tmp << "(unknown)";
     }
-    tmp << " iaid=" << iaid_ << " t1=" << t1_ << " t2=" << t2_
+    tmp << " iaid=" << iaid_ << ", t1=" << t1_ << ", t2=" << t2_
         << " " << optionLst_.size() << " sub-options:" << endl;
 
     for (Option6Lst::const_iterator opt=optionLst_.begin();
          opt!=optionLst_.end();
          ++opt) {
-        tmp << "  " << (*opt).second->toText();
+        tmp << (*opt).second->toText(indent+2);
     }
     return tmp.str();
 }

src/lib/dhcp/option6_ia.h

@@ -21,37 +21,44 @@ namespace isc {
 namespace dhcp {
 
 class Option6IA: public Option {
-        
+
 public:
     // ctor, used for options constructed, usually during transmission
-    Option6IA(Universe u, unsigned short type, unsigned int iaid); 
+    Option6IA(Universe u, unsigned short type, unsigned int iaid);
 
     // ctor, used for received options
-    // boost::shared_array allows sharing a buffer, but it requires that 
+    // boost::shared_array allows sharing a buffer, but it requires that
     // different instances share pointer to the whole array, not point
     // to different elements in shared array. Therefore we need to share
     // pointer to the whole array and remember offset where data for
     // this option begins
-    Option6IA(Universe u, unsigned short type, boost::shared_array<char> buf, 
+    Option6IA(Universe u, unsigned short type, boost::shared_array<char> buf,
               unsigned int buf_len,
-              unsigned int offset, 
+              unsigned int offset,
               unsigned int len);
-    
-    // writes option in wire-format to buf, returns pointer to first unused 
+
+    // writes option in wire-format to buf, returns pointer to first unused
     // byte after stored option
     unsigned int
-    pack(boost::shared_array<char> buf, unsigned int buf_len, 
+    pack(boost::shared_array<char> buf, unsigned int buf_len,
          unsigned int offset);
 
     // parses received buffer, returns offset to the first unused byte after
     // parsed option
-    virtual unsigned int 
-    unpack(boost::shared_array<char> buf, 
+    virtual unsigned int
+    unpack(boost::shared_array<char> buf,
            unsigned int buf_len,
-           unsigned int offset, 
+           unsigned int offset,
            unsigned int parse_len);
 
-    virtual std::string toText(int indent = 0);
+    /// Provides human readable text representation
+    ///
+    /// @param indent number of leading space characterss
+    ///
+    /// @return string with text represenation
+    ///
+    virtual std::string
+    toText(int indent = 0);
 
     void setT1(unsigned int t1) { t1_=t1; }
     void setT2(unsigned int t2) { t2_=t2; }
@@ -60,8 +67,13 @@ public:
     unsigned int getT1()   { return t1_; }
     unsigned int getT2()   { return t2_; }
 
-    // returns data length (data length + DHCPv4/DHCPv6 option header)
-    virtual unsigned short len();
+    /// @brief returns complete length of option
+    ///
+    /// Returns length of this option, including option header and suboptions
+    ///
+    /// @return length
+    virtual unsigned short
+    len();
 
 protected:
     unsigned int iaid_;
@@ -71,5 +83,5 @@ protected:
 
 } // isc::dhcp namespace
 } // isc namespace
-    
+
 #endif /* OPTION_IA_H_ */

src/lib/dhcp/option6_iaaddr.cc

@@ -27,7 +27,7 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
-Option6IAAddr::Option6IAAddr(unsigned short type, 
+Option6IAAddr::Option6IAAddr(unsigned short type,
                              isc::asiolink::IOAddress addr,
                              unsigned int pref,
                              unsigned int valid)
@@ -35,10 +35,10 @@ Option6IAAddr::Option6IAAddr(unsigned short type,
      valid_(valid) {
 }
 
-Option6IAAddr::Option6IAAddr(unsigned short type, 
-                             boost::shared_array<char> buf, 
+Option6IAAddr::Option6IAAddr(unsigned short type,
+                             boost::shared_array<char> buf,
                              unsigned int buf_len,
-                             unsigned int offset, 
+                             unsigned int offset,
                              unsigned int option_len)
     :Option(V6, type), addr_(IOAddress("::")) {
     unpack(buf, buf_len, offset, option_len);
@@ -49,10 +49,10 @@ Option6IAAddr::pack(boost::shared_array<char> buf,
                     unsigned int buf_len,
                     unsigned int offset) {
     if (len() > buf_len) {
-        isc_throw(OutOfRange, "Failed to pack IA option: len=" << len() 
+        isc_throw(OutOfRange, "Failed to pack IA option: len=" << len()
                   << ", buffer=" << buf_len << ": too small buffer.");
     }
-    
+
     *(uint16_t*)&buf[offset] = htons(type_);
     offset += 2;
     *(uint16_t*)&buf[offset] = htons(len()-4); // len() returns complete option
@@ -72,15 +72,15 @@ Option6IAAddr::pack(boost::shared_array<char> buf,
     return offset;
 }
 
-unsigned int 
+unsigned int
 Option6IAAddr::unpack(boost::shared_array<char> buf,
                   unsigned int buf_len,
-                  unsigned int offset, 
+                  unsigned int offset,
                   unsigned int parse_len) {
     if (parse_len<24 || offset+24>buf_len) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
-    
+
     // 16 bytes: IPv6 address
     /// TODO Implement fromBytes() method in IOAddress
     char addr_str[INET6_ADDRSTRLEN];
@@ -93,7 +93,7 @@ Option6IAAddr::unpack(boost::shared_array<char> buf,
 
     valid_ = ntohl(*(uint32_t*)&buf[offset]);
     offset +=4;
-    offset = LibDHCP::unpackOptions6(buf, buf_len, offset, 
+    offset = LibDHCP::unpackOptions6(buf, buf_len, offset,
                                      parse_len - 24, optionLst_);
 
     return offset;
@@ -104,25 +104,26 @@ std::string Option6IAAddr::toText(int indent /* =0 */) {
     for (int i=0; i<indent; i++)
         tmp << " ";
 
-    tmp << "addr: " << addr_.toText() << ", preferred-lft=" << preferred_ 
-        << ", valid-lft=" << valid_ << endl;
+    tmp << "type=" << type_ << "(IAADDR) addr=" << addr_.toText()
+        << ", preferred-lft=" << preferred_  << ", valid-lft="
+        << valid_ << endl;
 
     for (Option6Lst::const_iterator opt=optionLst_.begin();
          opt!=optionLst_.end();
          ++opt) {
-        tmp << "    " << (*opt).second->toText() << endl;
+        tmp << (*opt).second->toText(indent+2);
     }
     return tmp.str();
 }
 
 unsigned short Option6IAAddr::len() {
-    
+
     unsigned short length = 4/*header*/ + 24 /* content */; // header
 
     // length of all suboptions
     // TODO implement:
     // protected: unsigned short Option::lenHelper(int header_size);
-   
+
     for (Option::Option6Lst::iterator it = optionLst_.begin();
          it != optionLst_.end();
          ++it) {
@@ -130,4 +131,3 @@ unsigned short Option6IAAddr::len() {
     }
     return (length);
 }
-

src/lib/dhcp/pkt6.cc

@@ -298,8 +298,12 @@ Pkt6::addOption(boost::shared_ptr<Option> opt) {
 
 bool
 Pkt6::delOption(unsigned short type) {
-    isc_throw(Unexpected, "Not implemented yet. Not added option " << type);
-    return (false);
+    isc::dhcp::Option::Option6Lst::iterator x = options_.find(type);
+    if (x!=options_.end()) {
+        options_.erase(x);
+        return (true); // delete successful
+    }
+    return (false); // can't find option to be deleted
 }
 
 Pkt6::~Pkt6() {

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -22,10 +22,12 @@
 #include "dhcp/dhcp6.h"
 #include "dhcp/option.h"
 #include "dhcp/option6_ia.h"
+#include "dhcp/option6_iaaddr.h"
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::asiolink;
 
 namespace {
 class Option6IATest : public ::testing::Test {
@@ -54,21 +56,24 @@ TEST_F(Option6IATest, basic) {
     simple_buf[10]=0x02;
     simple_buf[11]=0x01;
 
-    // create an option (unpack content)
-    Option6IA* opt = new Option6IA(Option::V6, 
+    // create an option
+    // unpack() is called from constructor
+    Option6IA* opt = new Option6IA(Option::V6,
                                    D6O_IA_NA,
                                    simple_buf,
                                    128,
-                                   0, 
+                                   0,
                                    12);
 
     EXPECT_EQ(D6O_IA_NA, opt->getType());
     EXPECT_EQ(0xa1a2a3a4, opt->getIAID());
     EXPECT_EQ(0x81020304, opt->getT1());
     EXPECT_EQ(0x84030201, opt->getT2());
-    
+
     // pack this option again in the same buffer, but in
     // different place
+
+    // test for pack()
     int offset = opt->pack(simple_buf, 128, 60);
 
     // 4 bytes header + 4 bytes content
@@ -83,24 +88,169 @@ TEST_F(Option6IATest, basic) {
 
     // if option length is correct
     EXPECT_EQ(12, simple_buf[62]*256 + simple_buf[63]);
-    
+
     // if iaid is correct
     unsigned int iaid = htonl(*(unsigned int*)&simple_buf[64]);
     EXPECT_EQ(0xa1a2a3a4, iaid );
- 
+
    // if T1 is correct
     EXPECT_EQ(0x81020304, (simple_buf[68] << 24) +
                           (simple_buf[69] << 16) +
-                          (simple_buf[70] << 8) + 
+                          (simple_buf[70] << 8) +
                           (simple_buf[71]) );
 
     // if T1 is correct
     EXPECT_EQ(0x84030201, (simple_buf[72] << 24) +
                           (simple_buf[73] << 16) +
-                          (simple_buf[74] << 8) + 
+                          (simple_buf[74] << 8) +
                           (simple_buf[75]) );
 
     delete opt;
 }
 
+TEST_F(Option6IATest, simple) {
+    boost::shared_array<char> simple_buf(new char[128]);
+    for (int i=0; i<128; i++)
+        simple_buf[i] = 0;
+
+    Option6IA * ia = new Option6IA(Option::V6, D6O_IA_NA,
+                                   1234);
+    ia->setT1(2345);
+    ia->setT2(3456);
+
+    EXPECT_EQ(D6O_IA_NA, ia->getType());
+    EXPECT_EQ(1234, ia->getIAID());
+    EXPECT_EQ(2345, ia->getT1());
+    EXPECT_EQ(3456, ia->getT2());
+
+    delete ia;
+}
+
+// test if option can build suboptions
+TEST_F(Option6IATest, suboptions_pack) {
+    boost::shared_array<char> buf(new char[128]);
+    for (int i=0; i<128; i++)
+        buf[i] = 0;
+    buf[0] = 0xff;
+    buf[1] = 0xfe;
+    buf[2] = 0xfc;
+
+    Option6IA * ia = new Option6IA(Option::V6, D6O_IA_NA,
+                                   0x13579ace);
+    ia->setT1(0x2345);
+    ia->setT2(0x3456);
+
+    boost::shared_ptr<Option> sub1(new Option(Option::V6,
+                                              0xcafe));
+
+    boost::shared_ptr<Option6IAAddr> addr1(
+        new Option6IAAddr(D6O_IAADDR, IOAddress("2001:db8:1234:5678::abcd"),
+                          0x5000, 0x7000));
+
+    ia->addOption(sub1);
+    ia->addOption(addr1);
+
+    ASSERT_EQ(28, addr1->len());
+    ASSERT_EQ(4, sub1->len());
+    ASSERT_EQ(48, ia->len());
+
+    char expected[] = {
+        D6O_IA_NA/256, D6O_IA_NA%256, // type
+        0, 44, // length
+        0x13, 0x57, 0x9a, 0xce, // iaid
+        0, 0, 0x23, 0x45,  // T1
+        0, 0, 0x34, 0x56,  // T2
+
+        // iaaddr suboption
+        D6O_IAADDR/256, D6O_IAADDR%256, // type
+        0, 24, // len
+        0x20, 0x01, 0xd, 0xb8, 0x12,0x34, 0x56, 0x78,
+        0, 0, 0, 0, 0, 0, 0xab, 0xcd, // IP address
+        0, 0, 0x50, 0, // preferred-lifetime
+        0, 0, 0x70, 0, // valid-lifetime
+
+        // suboption
+        0xca, 0xfe, // type
+        0, 0 // len
+    };
+
+    int offset = ia->pack(buf, 128, 10);
+    ASSERT_EQ(offset, 10 + 48);
+
+    EXPECT_EQ(0, memcmp(&buf[10], expected, 48));
+
+    delete ia;
+}
+
+// test if option can parse suboptions
+TEST_F(Option6IATest, suboptions_unpack) {
+
+
+    char expected[] = {
+        D6O_IA_NA/256, D6O_IA_NA%256, // type
+        0, 28, // length
+        0x13, 0x57, 0x9a, 0xce, // iaid
+        0, 0, 0x23, 0x45,  // T1
+        0, 0, 0x34, 0x56,  // T2
+
+        // iaaddr suboption
+        D6O_IAADDR/256, D6O_IAADDR%256, // type
+        0, 24, // len
+        0x20, 0x01, 0xd, 0xb8, 0x12,0x34, 0x56, 0x78,
+        0, 0, 0, 0, 0, 0, 0xab, 0xcd, // IP address
+        0, 0, 0x50, 0, // preferred-lifetime
+        0, 0, 0x70, 0, // valid-lifetime
+
+        // suboption
+        0xca, 0xfe, // type
+        0, 0 // len
+    };
+
+    boost::shared_array<char> buf(new char[128]);
+    for (int i=0; i<128; i++)
+        buf[i] = 0;
+    memcpy(&buf[0], expected, 48);
+
+    Option6IA * ia;
+    EXPECT_NO_THROW({
+            ia = new Option6IA(Option::V6, D6O_IA_NA,
+                               buf, 128, 4, 44);
+    });
+
+    cout << "Parsed option:" << endl << ia->toText() << endl;
+
+    EXPECT_EQ(D6O_IA_NA, ia->getType());
+    EXPECT_EQ(0x13579ace, ia->getIAID());
+    EXPECT_EQ(0x2345, ia->getT1());
+    EXPECT_EQ(0x3456, ia->getT2());
+
+    boost::shared_ptr<Option> subopt = ia->getOption(D6O_IAADDR);
+    ASSERT_NE(boost::shared_ptr<Option>(), subopt); // non-NULL
+
+    // checks for address option
+    Option6IAAddr * addr = dynamic_cast<Option6IAAddr*>(subopt.get());
+    ASSERT_TRUE(NULL != addr);
+
+    EXPECT_EQ(D6O_IAADDR, addr->getType());
+    EXPECT_EQ(28, addr->len());
+    EXPECT_EQ(0x5000, addr->getPreferred());
+    EXPECT_EQ(0x7000, addr->getValid());
+    EXPECT_EQ("2001:db8:1234:5678::abcd", addr->getAddress().toText());
+
+    // checks for dummy option
+    subopt = ia->getOption(0xcafe);
+    ASSERT_FALSE(subopt == boost::shared_ptr<Option>()); // non-NULL
+
+    EXPECT_EQ(0xcafe, subopt->getType());
+    EXPECT_EQ(4, subopt->len());
+    EXPECT_EQ(NULL, subopt->getData());
+
+    subopt = ia->getOption(1); // get option 1
+    ASSERT_EQ(subopt, boost::shared_ptr<Option>()); // NULL
+
+    delete ia;
+}
+
+
+
 }

src/lib/dhcp/tests/option_unittest.cc

@@ -214,4 +214,42 @@ TEST_F(OptionTest, suboptions2) {
     delete opt1;
 }
 
+TEST_F(OptionTest, addgetdel) {
+    boost::shared_array<char> buf(new char[128]);
+    for (int i=0; i<128; i++)
+        buf[i] = 100+i;
+    Option* parent = new Option(Option::V6, 65535); //type
+    boost::shared_ptr<Option> opt1(new Option(Option::V6, 1));
+    boost::shared_ptr<Option> opt2(new Option(Option::V6, 2));
+    boost::shared_ptr<Option> opt3(new Option(Option::V6, 2));
+
+    parent->addOption(opt1);
+    parent->addOption(opt2);
+
+    // getOption() test
+    EXPECT_EQ(opt1, parent->getOption(1));
+    EXPECT_EQ(opt2, parent->getOption(2));
+
+    // expect NULL
+    EXPECT_EQ(boost::shared_ptr<Option>(), parent->getOption(4));
+
+    // now there are 2 options of type 2
+    parent->addOption(opt3);
+
+    // let's delete one of them
+    EXPECT_EQ(true, parent->delOption(2));
+
+    // there still should be the other option 2
+    EXPECT_NE(boost::shared_ptr<Option>(), parent->getOption(2));
+
+    // let's delete the other option 2
+    EXPECT_EQ(true, parent->delOption(2));
+
+    // no more options with type=2
+    EXPECT_EQ(boost::shared_ptr<Option>(), parent->getOption(2));
+
+    // let's try to delete - should fail
+    EXPECT_TRUE(false ==  parent->delOption(2));
+}
+
 }

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -15,17 +15,18 @@
 #include <config.h>
 #include <iostream>
 #include <sstream>
-
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
-#include "io_address.h"
 
+#include "io_address.h"
+#include "dhcp/option.h"
 #include "dhcp/pkt6.h"
 #include "dhcp/dhcp6.h"
 
 using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
+using namespace isc::dhcp;
 
 namespace {
 // empty class for now, but may be extended once Addr6 becomes bigger
@@ -108,4 +109,43 @@ TEST_F(Pkt6Test, parse_solicit1) {
     delete sol;
 }
 
+TEST_F(Pkt6Test, addGetDelOptions) {
+    Pkt6 * parent = new Pkt6(100);
+
+    boost::shared_ptr<Option> opt1(new Option(Option::V6, 1));
+    boost::shared_ptr<Option> opt2(new Option(Option::V6, 2));
+    boost::shared_ptr<Option> opt3(new Option(Option::V6, 2));
+
+    parent->addOption(opt1);
+    parent->addOption(opt2);
+
+    // getOption() test
+    EXPECT_EQ(opt1, parent->getOption(1));
+    EXPECT_EQ(opt2, parent->getOption(2));
+
+    // expect NULL
+    EXPECT_EQ(boost::shared_ptr<Option>(), parent->getOption(4));
+
+    // now there are 2 options of type 2
+    parent->addOption(opt3);
+
+    // let's delete one of them
+    EXPECT_EQ(true, parent->delOption(2));
+
+    // there still should be the other option 2
+    EXPECT_NE(boost::shared_ptr<Option>(), parent->getOption(2));
+
+    // let's delete the other option 2
+    EXPECT_EQ(true, parent->delOption(2));
+
+    // no more options with type=2
+    EXPECT_EQ(boost::shared_ptr<Option>(), parent->getOption(2));
+
+    // let's try to delete - should fail
+    EXPECT_TRUE(false ==  parent->delOption(2));
+
+    delete parent;
+}
+
+
 }