[1186] Added test for solicit;

Loopback interface name is now constant;
Function comments added;

src/bin/dhcp6/dhcp6_srv.cc

@@ -160,7 +160,9 @@ Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6> solicit) {
 
     // add client-id
     boost::shared_ptr<Option> clientid = solicit->getOption(D6O_CLIENTID);
-    reply->addOption(clientid);
+    if (clientid) {
+        reply->addOption(clientid);
+    }
 
     // add server-id
     reply->addOption(getServerID());

src/bin/dhcp6/dhcp6_srv.h

@@ -20,6 +20,10 @@
 #include "dhcp/option.h"
 #include <iostream>
 
+namespace test {
+class Dhcpv6SrvTest_Solicit_basic_Test;
+}
+
 namespace isc {
     class Dhcpv6Srv {
     private:
@@ -66,6 +70,8 @@ namespace isc {
         processDecline(boost::shared_ptr<Pkt6> solicit);
 
         bool shutdown;
+
+        friend class test::Dhcpv6SrvTest_Solicit_basic_Test;
     };
 };
 

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -19,21 +19,26 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
-
+#include "dhcp/dhcp6.h"
 #include "dhcp6/dhcp6_srv.h"
+#include "dhcp/option6_ia.h"
 
 using namespace std;
 using namespace isc;
+using namespace isc::dhcp;
+
+// namespace has to be named, because friends are defined in Dhcpv6Srv class
+// Maybe it should be isc::test?
+namespace test {
 
-namespace {
 class Dhcpv6SrvTest : public ::testing::Test {
 public:
     Dhcpv6SrvTest() {
     }
 };
 
-TEST_F(Dhcpv6SrvTest, basic) {
-    // there's almost no code now. What's there provides echo capability 
+Test_F(Dhcpv6SrvTest, basic) {
+    // there's almost no code now. What's there provides echo capability
     // that is just a proof of concept and will be removed soon
     // No need to thoroughly test it
 
@@ -45,9 +50,47 @@ TEST_F(Dhcpv6SrvTest, basic) {
     EXPECT_NO_THROW( {
         Dhcpv6Srv * srv = new Dhcpv6Srv();
 
-	delete srv;
-	});
-    
+        delete srv;
+        });
+
+}
+
+TEST_F(Dhcpv6SrvTest,Solicit_basic) {
+    Dhcpv6Srv * srv = 0;
+    EXPECT_NO_THROW( srv = new Dhcpv6Srv(); );
+
+    boost::shared_ptr<Pkt6> sol =
+        boost::shared_ptr<Pkt6>(new Pkt6(DHCPV6_SOLICIT,
+                                         1234, Pkt6::UDP));
+
+    boost::shared_ptr<Option6IA> ia(new Option6IA(Option::V6, D6O_IA_NA, 2345));
+    ia->setT1(1501);
+    ia->setT2(2601);
+    sol->addOption(ia);
+
+    // Let's not send address in solicit yet
+    // boost::shared_ptr<Option6IAAddr> addr(new Option6IAAddr(D6O_IAADDR,
+    //    IOAddress("2001:db8:1234:ffff::ffff"), 5001, 7001));
+    // ia->addOption(addr);
+    // sol->addOption(ia);
+
+    boost::shared_ptr<Pkt6> reply = srv->processSolicit(sol);
+
+    // check if we get response at all
+    ASSERT_TRUE( reply != boost::shared_ptr<Pkt6>() );
+
+    EXPECT_EQ( DHCPV6_ADVERTISE, reply->getType() );
+    EXPECT_EQ( 1234, reply->getTransid() );
+
+    boost::shared_ptr<Option> tmp = reply->getOption(D6O_IA_NA);
+    ASSERT_TRUE( tmp != boost::shared_ptr<Option>() );
+
+    Option6IA * reply_ia = dynamic_cast<Option6IA*> ( tmp.get() );
+    EXPECT_EQ( 2345, reply_ia->getIAID() );
+
+    // more checks to be implemented
+    delete srv;
+
 }
 
 }

src/bin/dhcp6/tests/iface_mgr_unittest.cc

@@ -28,6 +28,8 @@ using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 
+#define LOOPBACK "lo0"
+
 namespace {
 const char* const INTERFACE_FILE = TEST_DATA_BUILDDIR "/interfaces.txt";
 
@@ -79,19 +81,22 @@ TEST_F(IfaceMgrTest, dhcp6Sniffer) {
     cout << "---8X-----------------------------------------" << endl;
     while (true) {
         pkt = ifacemgr->receive();
-        
+
         cout << "// Received " << pkt->data_len_ << " bytes packet:" << endl;
         cout << "Pkt6 *capture" << cnt++ << "() {" << endl;
         cout << "    Pkt6* pkt;" << endl;
         cout << "    pkt = new Pkt6(" << pkt->data_len_ << ");" << endl;
         cout << "    pkt->remote_port_ = " << pkt-> remote_port_ << ";" << endl;
-        cout << "    pkt->remote_addr_ = IOAddress(\"" << pkt->remote_addr_.toText() << "\");" << endl;
+        cout << "    pkt->remote_addr_ = IOAddress(\""
+             << pkt->remote_addr_.toText() << "\");" << endl;
         cout << "    pkt->local_port_ = " << pkt-> local_port_ << ";" << endl;
-        cout << "    pkt->local_addr_ = IOAddress(\"" << pkt->local_addr_.toText() << "\");" << endl;
+        cout << "    pkt->local_addr_ = IOAddress(\""
+             << pkt->local_addr_.toText() << "\");" << endl;
         cout << "    pkt->ifindex_ = " << pkt->ifindex_ << ";" << endl;
         cout << "    pkt->iface_ = \"" << pkt->iface_ << "\";" << endl;
         for (int i=0; i< pkt->data_len_; i++) {
-            cout << "    pkt->data_[" << i << "]=" << (int)(unsigned char)pkt->data_[i] << "; ";
+            cout << "    pkt->data_[" << i << "]="
+                 << (int)(unsigned char)pkt->data_[i] << "; ";
             if (!(i%4))
                 cout << endl;
         }
@@ -102,7 +107,7 @@ TEST_F(IfaceMgrTest, dhcp6Sniffer) {
         delete pkt;
     }
     cout << "---8X-----------------------------------------" << endl;
-        
+
     // never happens. Infinite loop is infinite
     delete pkt;
     delete ifacemgr;
@@ -127,23 +132,34 @@ TEST_F(IfaceMgrTest, ifaceClass) {
 
 }
 
-// TODO: Implement getPlainMac() test as soon as interface detection is implemented.
+// TODO: Implement getPlainMac() test as soon as interface detection
+// is implemented.
 TEST_F(IfaceMgrTest, getIface) {
 
     cout << "Interface checks. Please ignore socket binding errors." << endl;
     NakedIfaceMgr * ifacemgr = new NakedIfaceMgr();
 
     // interface name, ifindex
-    IfaceMgr::Iface iface1("lo", 1);
+    IfaceMgr::Iface iface1("lo1", 1);
     IfaceMgr::Iface iface2("eth5", 2);
     IfaceMgr::Iface iface3("en3", 5);
     IfaceMgr::Iface iface4("e1000g0", 3);
 
+    // note: real interfaces may be detected as well
     ifacemgr->getIfacesLst().push_back(iface1);
     ifacemgr->getIfacesLst().push_back(iface2);
     ifacemgr->getIfacesLst().push_back(iface3);
     ifacemgr->getIfacesLst().push_back(iface4);
 
+    cout << "There are " << ifacemgr->getIfacesLst().size()
+         << " interfaces." << endl;
+    for (IfaceMgr::IfaceLst::iterator iface=ifacemgr->getIfacesLst().begin();
+         iface != ifacemgr->getIfacesLst().end();
+         ++iface) {
+        cout << "  " << iface->name_ << "/" << iface->ifindex_ << endl;
+    }
+
+
     // check that interface can be retrieved by ifindex
     IfaceMgr::Iface * tmp = ifacemgr->getIface(5);
     // ASSERT_NE(NULL, tmp); is not supported. hmmmm.
@@ -153,10 +169,10 @@ TEST_F(IfaceMgrTest, getIface) {
     EXPECT_EQ(5, tmp->ifindex_);
 
     // check that interface can be retrieved by name
-    tmp = ifacemgr->getIface("lo");
+    tmp = ifacemgr->getIface("lo1");
     ASSERT_TRUE( tmp != NULL );
 
-    EXPECT_STREQ( "lo", tmp->name_.c_str() );
+    EXPECT_STREQ( "lo1", tmp->name_.c_str() );
     EXPECT_EQ(1, tmp->ifindex_);
 
     // check that non-existing interfaces are not returned
@@ -208,17 +224,22 @@ TEST_F(IfaceMgrTest, DISABLED_sockets) {
     IOAddress loAddr("::1");
 
     // bind multicast socket to port 10547
-    int socket1 = ifacemgr->openSocket("lo", loAddr, 10547);
+    int socket1 = ifacemgr->openSocket(LOOPBACK, loAddr, 10547);
     EXPECT_GT(socket1, 0); // socket > 0
 
     // bind unicast socket to port 10548
-    int socket2 = ifacemgr->openSocket("lo", loAddr, 10548);
+    int socket2 = ifacemgr->openSocket(LOOPBACK, loAddr, 10548);
     EXPECT_GT(socket2, 0);
 
     // expect success. This address/port is already bound, but
     // we are using SO_REUSEADDR, so we can bind it twice
-    int socket3 = ifacemgr->openSocket("lo", loAddr, 10547);
-    EXPECT_GT(socket3, 0); // socket > 0
+    int socket3 = ifacemgr->openSocket(LOOPBACK, loAddr, 10547);
+
+    // rebinding succeeds on Linux, fails on BSD
+    // TODO: add OS-specific defines here (or modify code to
+    // behave the same way on all OSes, but that may not be
+    // possible
+    // EXPECT_GT(socket3, 0); // socket > 0
 
     // we now have 3 sockets open at the same time. Looks good.
 
@@ -241,12 +262,12 @@ TEST_F(IfaceMgrTest, DISABLED_socketsMcast) {
     IOAddress mcastAddr("ff02::1:2");
 
     // bind multicast socket to port 10547
-    int socket1 = ifacemgr->openSocket("lo", mcastAddr, 10547);
+    int socket1 = ifacemgr->openSocket(LOOPBACK, mcastAddr, 10547);
     EXPECT_GT(socket1, 0); // socket > 0
 
     // expect success. This address/port is already bound, but
     // we are using SO_REUSEADDR, so we can bind it twice
-    int socket2 = ifacemgr->openSocket("lo", mcastAddr, 10547);
+    int socket2 = ifacemgr->openSocket(LOOPBACK, mcastAddr, 10547);
     EXPECT_GT(socket2, 0);
 
     // there's no good way to test negative case here.
@@ -269,15 +290,15 @@ TEST_F(IfaceMgrTest, DISABLED_sendReceive) {
     // without interface detection implemented
 
     fstream fakeifaces(INTERFACE_FILE, ios::out|ios::trunc);
-    fakeifaces << "lo ::1";
+    fakeifaces << LOOPBACK << " ::1";
     fakeifaces.close();
 
     NakedIfaceMgr * ifacemgr = new NakedIfaceMgr();
 
     // let's assume that every supported OS have lo interface
     IOAddress loAddr("::1");
-    int socket1 = ifacemgr->openSocket("lo", loAddr, 10547);
-    int socket2 = ifacemgr->openSocket("lo", loAddr, 10546);
+    int socket1 = ifacemgr->openSocket(LOOPBACK, loAddr, 10547);
+    int socket2 = ifacemgr->openSocket(LOOPBACK, loAddr, 10546);
 
     ifacemgr->setSendSock(socket2);
     ifacemgr->setRecvSock(socket1);
@@ -292,7 +313,7 @@ TEST_F(IfaceMgrTest, DISABLED_sendReceive) {
     sendPkt->remote_port_ = 10547;
     sendPkt->remote_addr_ = IOAddress("::1");
     sendPkt->ifindex_ = 1;
-    sendPkt->iface_ = "lo";
+    sendPkt->iface_ = LOOPBACK;
 
     boost::shared_ptr<Pkt6> rcvPkt;
 

src/lib/dhcp/libdhcp.cc

@@ -33,19 +33,6 @@ LibDHCP::version() {
     return PACKAGE_VERSION;
 }
 
-/**
- * Parses provided buffer and creates Option objects.
- *
- * Parses provided buf array and stores created Option objects
- * in options container.
- *
- * @param buf Buffer to be parsed.
- * @param offset Specifies offset for the first option.
- * @param options Reference to option container. Options will be
- *        put here.
- *
- * @return offset to first byte after last parsed option
- */
 unsigned int
 LibDHCP::unpackOptions6(boost::shared_array<char> buf, unsigned int buf_len,
                         unsigned int offset, unsigned int parse_len,
@@ -126,9 +113,9 @@ LibDHCP::packOptions6(boost::shared_array<char> data,
 }
 
 bool
-LibDHCP::OptionFactorRegister(Option::Universe u,
-                              unsigned short opt_type,
-                              Option::Factory * factory) {
+LibDHCP::OptionFactoryRegister(Option::Universe u,
+                               unsigned short opt_type,
+                               Option::Factory * factory) {
     switch (u) {
     case Option::V6: {
         if (v6factories_.find(opt_type)!=v6factories_.end()) {

src/lib/dhcp/libdhcp.h

@@ -24,24 +24,60 @@ namespace dhcp {
 class LibDHCP {
 
 public:
-    LibDHCP();
-    static std::string version();
-
-    bool parsePkt6(Pkt6& pkt);
-    bool builtPkt6(Pkt6& pkt);
+    /// Returns version of the library.
+    ///
+    /// @return string that contains libdhcp version.
+    ///
+    static std::string
+    version();
 
+    /// Builds collection of options.
+    ///
+    /// Builds raw (on-wire) data for provided collection of options.
+    ///
+    /// @param buf shared pointer to buffer. Data will be stored there.
+    /// @param buf_len buffer length. Used for buffer overflow protection.
+    /// @param offset Offset from beginning of the buffer, where store options
+    /// @param options collection of options to store to
+    ///
+    /// @return offset to the first unused byte in buffer (next one after last
+    ///         used byte)
+    ///
     static unsigned int
     packOptions6(boost::shared_array<char> buf, unsigned int buf_len,
                  unsigned int offset,
-                 isc::dhcp::Option::Option6Lst& options_);
+                 isc::dhcp::Option::Option6Lst& options);
+
+    ///
+    /// Parses provided buffer and creates Option objects.
+    ///
+    /// Parses provided buf array and stores created Option objects
+    /// in options container.
+    ///
+    /// @param buf Buffer to be parsed.
+    /// @param offset Specifies offset for the first option.
+    /// @param options Reference to option container. Options will be
+    ///        put here.
+    ///
+    /// @return offset to first byte after last parsed option
+    ///
     static unsigned int
     unpackOptions6(boost::shared_array<char> buf, unsigned int buf_len,
                    unsigned int offset, unsigned int parse_len,
                    isc::dhcp::Option::Option6Lst& options_);
 
-    bool OptionFactorRegister(Option::Universe u,
-                              unsigned short type,
-                              Option::Factory * factory);
+    ///
+    /// Registers factory method that produces options of specific option types.
+    ///
+    /// @param u universe of the option (V4 or V6)
+    /// @param opt_type option-type
+    /// @param factory function pointer
+    ///
+    /// @return true, if registration was successful, false otherwise
+    ///
+    bool OptionFactoryRegister(Option::Universe u,
+                               unsigned short type,
+                               Option::Factory * factory);
 protected:
     // pointers to factories that produce DHCPv6 options
     static std::map<unsigned short, Option::Factory*> v6factories_;

src/lib/dhcp/option.cc

@@ -117,16 +117,7 @@ Option::unpack4(boost::shared_array<char>,
     isc_throw(Unexpected, "IPv4 support not implemented yet.");
     return 0;
 }
-/**
- * Parses buffer and creates collection of Option objects.
- *
- * @param buf pointer to buffer
- * @param buf_len length of buf
- * @param offset offset, where start parsing option
- * @param parse_len how many bytes should be parsed
- *
- * @return offset after last parsed option
- */
+
 unsigned int
 Option::unpack6(boost::shared_array<char> buf,
                 unsigned int buf_len,
@@ -178,12 +169,6 @@ isc::dhcp::Option::addOption(boost::shared_ptr<isc::dhcp::Option> opt) {
 
 }
 
-
-/**
- * Converts generic option to string.
- *
- * @return string that represents option.
- */
 std::string Option::toText() {
     std::stringstream tmp;
     tmp << type_ << "(len=" << data_len_ << "):";

src/lib/dhcp/option.h

@@ -60,60 +60,137 @@ public:
 
     // parses received buffer, returns offset to the first unused byte after
     // parsed option
-    virtual unsigned int
-        unpack(boost::shared_array<char> buf,
-               unsigned int buf_len,
-               unsigned int offset,
-               unsigned int parse_len);
-
-    virtual std::string toText();
-
-    unsigned short getType();
 
-    // returns length of the complete option (data length + DHCPv4/DHCPv6 option header)
-    virtual unsigned short len();
+    ///
+    /// Parses buffer and creates collection of Option objects.
+    ///
+    /// @param buf pointer to buffer
+    /// @param buf_len length of buf
+    /// @param offset offset, where start parsing option
+    /// @param parse_len how many bytes should be parsed
+    ///
+    /// @return offset after last parsed option
+    ///
+    virtual unsigned int
+    unpack(boost::shared_array<char> buf,
+           unsigned int buf_len,
+           unsigned int offset,
+           unsigned int parse_len);
+
+    ///
+    /// Returns string representation of the option.
+    ///
+    /// @return string with text representation.
+    ///
+    virtual std::string
+    toText();
+
+    ///
+    /// Returns option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
+    ///
+    /// @return option type
+    ///
+    unsigned short
+    getType();
+
+    /// Returns length of the complete option (data length + DHCPv4/DHCPv6
+    /// option header)
+    ///
+    /// @return length of the option
+    ///
+    virtual unsigned short
+    len();
 
     // returns if option is valid (e.g. option may be truncated)
-    virtual bool valid();
+    virtual bool
+    valid();
 
-    void addOption(boost::shared_ptr<Option> opt);
+    /// Adds a sub-option.
+    ///
+    /// @param opt shared pointer to a suboption that is going to be added.
+    ///
+    void
+    addOption(boost::shared_ptr<Option> opt);
 
     // just to force that every option has virtual dtor
-    virtual ~Option();
+    virtual
+    ~Option();
 
 protected:
+
+    ///
+    /// Builds raw (over-wire) buffer of this option, including all
+    /// defined suboptions. Version for building DHCPv4 options.
+    ///
+    /// @param buf output buffer (built options will be stored here)
+    /// @param buf_len buffer length (used for buffer overflow checks)
+    /// @param offset offset from start of the buf buffer
+    ///
+    /// @return offset to the next byte after last used byte
+    ///
     virtual unsigned int
     pack4(boost::shared_array<char> buf,
           unsigned int buf_len,
           unsigned int offset);
+
+    ///
+    /// Builds raw (over-wire) buffer of this option, including all
+    /// defined suboptions. Version for building DHCPv4 options.
+    ///
+    /// @param buf output buffer (built options will be stored here)
+    /// @param buf_len buffer length (used for buffer overflow checks)
+    /// @param offset offset from start of the buf buffer
+    ///
+    /// @return offset to the next byte after last used byte
+    ///
     virtual unsigned int
     pack6(boost::shared_array<char> buf,
           unsigned int buf_len,
           unsigned int offset);
-    virtual unsigned int unpack4(boost::shared_array<char> buf,
-                                 unsigned int buf_len,
-                                 unsigned int offset,
-                                 unsigned int parse_len);
-    virtual unsigned int unpack6(boost::shared_array<char> buf,
-                                 unsigned int buf_len,
-                                 unsigned int offset,
-                                 unsigned int parse_len);
-
-    Universe universe_;
-    unsigned short type_;
+
+
+    ///
+    /// Parses provided buffer and creates DHCPv4 options.
+    ///
+    /// @param buf buffer that contains raw buffer to parse (on-wire format)
+    /// @param buf_len buffer length (used for buffer overflow checks)
+    /// @param offset offset from start of the buf buffer
+    ///
+    /// @return offset to the next byte after last parsed byte
+    ///
+    virtual unsigned int
+    unpack4(boost::shared_array<char> buf,
+            unsigned int buf_len,
+            unsigned int offset,
+            unsigned int parse_len);
+
+    ///
+    /// Parses provided buffer and creates DHCPv6 options.
+    ///
+    /// @param buf buffer that contains raw buffer to parse (on-wire format)
+    /// @param buf_len buffer length (used for buffer overflow checks)
+    /// @param offset offset from start of the buf buffer
+    ///
+    /// @return offset to the next byte after last parsed byte
+    ///
+    virtual unsigned int
+    unpack6(boost::shared_array<char> buf,
+            unsigned int buf_len,
+            unsigned int offset,
+            unsigned int parse_len);
+
+    Universe universe_; // option universe (V4 or V6)
+    unsigned short type_; // option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
 
     boost::shared_array<char> data_;
-    unsigned int data_len_; // length of data only. Use len() if you want to know
-                            // proper length with option header overhead
+    unsigned int data_len_; // length of data only. Use len() if you want to
+                            // know proper length with option header overhead
     unsigned int offset_; // data is a shared_pointer that points out to the
                           // whole packet. offset_ specifies where data for
                           // this option begins.
-    char * value_;
 
-    // 2 different containers are used, because v4 options are unique
-    // and v6 allows multiple instances of the same option types
-    // originally 2 separate containers were planned. Let's try if we
-    // can use a single apporach
+    // TODO: probably 2 different containers have to be used for v4 (unique
+    // options) and v6 (options with the same type can repeat)
     Option6Lst optionLst_;
 };