From: Tomek Mrugalski <tomasz@isc.org>
Date: Tue, 11 Oct 2011 14:43:30 +0000 (+0200)
Subject: [1186] Part 2 of review changes
X-Git-Tag: perftcpdns_before_epoll~86^2~26
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=223b19a30e4897c7281bb40c9f366a01c8f449ca;p=thirdparty%2Fkea.git

[1186] Part 2 of review changes

- Dhcpv6Srv::process*() methods does not use references anymore
- Dhcpv6Srv::shutdown variable added for ordered shutdown procedure
- Dhcpv6Srv is now in isc::dhcp namespace
- Dhcpv6Srv is now derived from boot/noncopyable
- IfaceMgr methods and fields are now commented appropriately
- IfaceMgr
- Dhcpv6Srv friend declaration removed
- Dhcpv6SrvTest now uses derived class to access protected methods
- Dhcpv6SrvTest Solicit_Basic test now includes client-id, checks for
  received client-id, server-id and content of ia
- IfaceMgrTest minor improvements

See comments in #1186 ticket for extensive list of smaller changes
and reasons behind them.
---

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 05e3c77dba..ad77621100 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -35,6 +35,8 @@ Dhcpv6Srv::Dhcpv6Srv() {
     /// @todo: instantiate LeaseMgr here once it is imlpemented.
 
     setServerID();
+
+    shutdown = false;
 }
 
 Dhcpv6Srv::~Dhcpv6Srv() {
@@ -43,7 +45,7 @@ Dhcpv6Srv::~Dhcpv6Srv() {
 
 bool
 Dhcpv6Srv::run() {
-    while (true) {
+    while (!shutdown) {
         boost::shared_ptr<Pkt6> query; // client's message
         boost::shared_ptr<Pkt6> rsp;   // server's response
 
@@ -132,12 +134,14 @@ Dhcpv6Srv::setServerID() {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6>& solicit) {
+Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6> solicit) {
 
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_ADVERTISE,
                                            solicit->getTransid(),
                                            Pkt6::UDP));
 
+    /// TODO Rewrite this once LeaseManager is implemented.
+
     // answer client's IA (this is mostly a dummy,
     // so let's answer only first IA and hope there is only one)
     boost::shared_ptr<Option> ia_opt = solicit->getOption(D6O_IA_NA);
@@ -171,7 +175,7 @@ Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6>& solicit) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6>& request) {
+Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6> request) {
     /// TODO: Implement processRequest() for real
     boost::shared_ptr<Pkt6> reply = processSolicit(request);
     reply->setType(DHCPV6_REPLY);
@@ -179,7 +183,7 @@ Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6>& request) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6>& renew) {
+Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6> renew) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            renew->getTransid(),
                                            Pkt6::UDP));
@@ -187,7 +191,7 @@ Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6>& renew) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6>& rebind) {
+Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6> rebind) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            rebind->getTransid(),
                                            Pkt6::UDP));
@@ -195,7 +199,7 @@ Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6>& rebind) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6>& confirm) {
+Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6> confirm) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            confirm->getTransid(),
                                            Pkt6::UDP));
@@ -203,7 +207,7 @@ Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6>& confirm) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6>& release) {
+Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6> release) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            release->getTransid(),
                                            Pkt6::UDP));
@@ -211,7 +215,7 @@ Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6>& release) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6>& decline) {
+Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6> decline) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            decline->getTransid(),
                                            Pkt6::UDP));
@@ -219,7 +223,7 @@ Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6>& decline) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processInfRequest(boost::shared_ptr<Pkt6>& infRequest) {
+Dhcpv6Srv::processInfRequest(boost::shared_ptr<Pkt6> infRequest) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            infRequest->getTransid(),
                                            Pkt6::UDP));
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index c51a6d32b1..90ecb8aa7a 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -16,15 +16,14 @@
 #define DHCPV6_SRV_H
 
 #include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
 #include "dhcp/pkt6.h"
 #include "dhcp/option.h"
 #include <iostream>
 
-namespace test {
-class Dhcpv6SrvTest_Solicit_basic_Test;
-}
-
 namespace isc {
+
+    namespace dhcp {
     /// @brief DHCPv6 server service.
     ///
     /// This singleton class represents DHCPv6 server. It contains all
@@ -33,8 +32,8 @@ namespace isc {
     /// that is going to be used as server-identifier, receives incoming
     /// packets, processes them, manages leases assignment and generates
     /// appropriate responses.
-    class Dhcpv6Srv {
-    private:
+    class Dhcpv6Srv : public boost::noncopyable {
+        private:
 
         /// @brief A private copy constructor.
         ///
@@ -54,13 +53,6 @@ namespace isc {
         ///
         Dhcpv6Srv& operator=(const Dhcpv6Srv& src);
 
-        /// @brief Returns server-intentifier option
-        ///
-        /// @return reference to server-id option
-        ///
-        boost::shared_ptr<isc::dhcp::Option>&
-        getServerID() { return serverid_; }
-
         /// @brief Sets server-identifier.
         ///
         /// This method attempts to set server-identifier DUID. It loads it
@@ -88,6 +80,13 @@ namespace isc {
         /// @brief Destructor. Shuts down DHCPv6 service.
         ~Dhcpv6Srv();
 
+        /// @brief Returns server-intentifier option
+        ///
+        /// @return reference to server-id option
+        ///
+        boost::shared_ptr<isc::dhcp::Option>&
+        getServerID() { return serverid_; }
+
         /// @brief Main server processing loop.
         ///
         /// Main server processing loop. Receives incoming packets, verifies
@@ -115,7 +114,7 @@ namespace isc {
         /// @return ADVERTISE, REPLY message or NULL
         ///
         boost::shared_ptr<Pkt6>
-        processSolicit(boost::shared_ptr<Pkt6>& solicit);
+        processSolicit(boost::shared_ptr<Pkt6> solicit);
 
         /// @brief Processes incoming REQUEST and returns REPLY response.
         ///
@@ -129,36 +128,38 @@ namespace isc {
         ///
         /// @return REPLY message or NULL
         boost::shared_ptr<Pkt6>
-        processRequest(boost::shared_ptr<Pkt6>& request);
+        processRequest(boost::shared_ptr<Pkt6> request);
 
         /// @brief Stub function that will handle incoming RENEW messages.
         boost::shared_ptr<Pkt6>
-        processRenew(boost::shared_ptr<Pkt6>& renew);
+        processRenew(boost::shared_ptr<Pkt6> renew);
 
          /// @brief Stub function that will handle incoming REBIND messages.
         boost::shared_ptr<Pkt6>
-        processRebind(boost::shared_ptr<Pkt6>& rebind);
+        processRebind(boost::shared_ptr<Pkt6> rebind);
 
         /// @brief Stub function that will handle incoming CONFIRM messages.
         boost::shared_ptr<Pkt6>
-        processConfirm(boost::shared_ptr<Pkt6>& confirm);
+        processConfirm(boost::shared_ptr<Pkt6> confirm);
 
         /// @brief Stub function that will handle incoming RELEASE messages.
         boost::shared_ptr<Pkt6>
-        processRelease(boost::shared_ptr<Pkt6>& release);
+        processRelease(boost::shared_ptr<Pkt6> release);
 
         /// @brief Stub function that will handle incoming DECLINE messages.
         boost::shared_ptr<Pkt6>
-        processDecline(boost::shared_ptr<Pkt6>& decline);
+        processDecline(boost::shared_ptr<Pkt6> decline);
 
         /// @brief Stub function that will handle incoming INF-REQUEST messages.
         boost::shared_ptr<Pkt6>
-        processInfRequest(boost::shared_ptr<Pkt6>& infRequest);
+        processInfRequest(boost::shared_ptr<Pkt6> infRequest);
 
-        bool shutdown;
-
-        friend class test::Dhcpv6SrvTest_Solicit_basic_Test;
+        /// indicates if shutdown is in progress. Setting it to true will
+        /// initiate server shutdown procedure.
+        volatile bool shutdown;
     };
-};
+
+    }; // namespace isc::dhcp
+}; // namespace isc
 
 #endif // DHCP6_SRV_H
diff --git a/src/bin/dhcp6/iface_mgr.cc b/src/bin/dhcp6/iface_mgr.cc
index e108317091..79b3c612bb 100644
--- a/src/bin/dhcp6/iface_mgr.cc
+++ b/src/bin/dhcp6/iface_mgr.cc
@@ -25,6 +25,7 @@
 using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
+using namespace isc::dhcp;
 
 namespace isc {
 
@@ -44,14 +45,16 @@ IfaceMgr::instanceCreate() {
 
 IfaceMgr&
 IfaceMgr::instance() {
-    if (instance_ == 0)
+    if (instance_ == 0) {
         instanceCreate();
+    }
     return (*instance_);
 }
 
 IfaceMgr::Iface::Iface(const std::string& name, int ifindex)
     :name_(name), ifindex_(ifindex), mac_len_(0) {
-    memset(mac_, 0, 20);
+
+    memset(mac_, 0, sizeof(mac_));
 }
 
 std::string
@@ -64,10 +67,11 @@ IfaceMgr::Iface::getFullName() const {
 std::string
 IfaceMgr::Iface::getPlainMac() const {
     ostringstream tmp;
+    tmp.fill('0');
+    tmp << hex;
     for (int i=0; i<mac_len_; i++) {
-        tmp.fill('0');
         tmp.width(2);
-        tmp << (hex) << (int) mac_[i];
+        tmp << mac_[i];
         if (i<mac_len_-1) {
             tmp << ":";
         }
@@ -108,6 +112,7 @@ IfaceMgr::~IfaceMgr() {
         control_buf_ = 0;
         control_buf_len_ = 0;
     }
+    control_buf_len_ = 0;
 }
 
 void
@@ -225,18 +230,6 @@ IfaceMgr::getIface(const std::string& ifname) {
     return (NULL); // not found
 }
 
-
-/**
- * Opens UDP/IPv6 socket and binds it to specific address, interface and port.
- *
- * @param ifname name of the interface
- * @param addr address to be bound.
- * @param port UDP port.
- * @param mcast Should multicast address also be bound?
- *
- * @return socket descriptor, if socket creation, binding and multicast
- * group join were all successful. -1 otherwise.
- */
 int
 IfaceMgr::openSocket(const std::string& ifname,
                      const IOAddress& addr,
diff --git a/src/bin/dhcp6/iface_mgr.h b/src/bin/dhcp6/iface_mgr.h
index 0f070b35d5..cc89045986 100644
--- a/src/bin/dhcp6/iface_mgr.h
+++ b/src/bin/dhcp6/iface_mgr.h
@@ -17,34 +17,51 @@
 
 #include <list>
 #include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
 #include "asiolink/io_address.h"
 #include "dhcp/pkt6.h"
 
 namespace isc {
 
-    /**
-     * IfaceMgr is an interface manager class that detects available network
-     * interfaces, configured addresses, link-local addresses, and provides
-     * API for using sockets.
-     *
-     */
-    class IfaceMgr {
+    namespace dhcp {
+    /// @brief handles network interfaces, transmission and reception
+    ///
+    /// IfaceMgr is an interface manager class that detects available network
+    /// interfaces, configured addresses, link-local addresses, and provides
+    /// API for using sockets.
+    ///
+    class IfaceMgr : public boost::noncopyable {
     public:
+        /// type that defines list of addresses
         typedef std::list<isc::asiolink::IOAddress> Addr6Lst;
-        struct Iface { // TODO: could be a class as well
-            std::string name_; // network interface name
-            int ifindex_; // interface index (a value that uniquely indentifies
-                          // an interface
-            Addr6Lst addrs_;
-            char mac_[20]; // Infiniband used 20 bytes indentifiers
-            int mac_len_;
 
+        /// maximum MAC address length (Infiniband uses 20 bytes)
+        static const unsigned int MAX_MAC_LEN = 20;
+
+        /// @brief represents a single network interface
+        ///
+        /// Iface structure represents network interface with all useful
+        /// information, like name, interface index, MAC address and
+        /// list of assigned addresses
+        struct Iface {
+            /// constructor
             Iface(const std::string& name, int ifindex);
+
+            /// returns full interface name in format ifname/ifindex
             std::string getFullName() const;
+
+            /// returns link-layer address a plain text
             std::string getPlainMac() const;
 
-            int sendsock_; // socket used to sending data
-            int recvsock_; // socket used for receiving data
+            std::string name_; /// network interface name
+            int ifindex_;      /// interface index (a value that uniquely
+                               /// indentifies an interface
+            Addr6Lst addrs_;   /// list of assigned addresses
+            uint8_t mac_[MAX_MAC_LEN]; /// link-layer address
+            int mac_len_;      /// length of link-layer address (usually 6)
+
+            int sendsock_; /// socket used to sending data
+            int recvsock_; /// socket used for receiving data
 
             // next field is not needed, let's keep it in cointainers
         };
@@ -54,31 +71,99 @@ namespace isc {
         //      also hide it (make it public make tests easier for now)
         typedef std::list<Iface> IfaceLst;
 
+        /// IfaceMgr is a singleton class. This method returns reference
+        /// to its sole instance.
+        ///
+        /// @return the only existing instance of interface manager
         static IfaceMgr& instance();
 
-        Iface * getIface(int ifindex);
-        Iface * getIface(const std::string& ifname);
-
-        void printIfaces(std::ostream& out = std::cout);
-
-        bool send(boost::shared_ptr<Pkt6> pkt);
+        /// @brief Returns interface with specified interface index
+        ///
+        /// @param ifindex index of searched interface
+        ///
+        /// @return interface with requested index (or NULL if no such
+        ///         interface is present)
+        ///
+        Iface*
+        getIface(int ifindex);
+
+        /// @brief Returns interface with specified interface name
+        ///
+        /// @param ifname name of searched interface
+        ///
+        /// @return interface with requested name (or NULL if no such
+        ///         interface is present)
+        ///
+        Iface*
+        getIface(const std::string& ifname);
+
+        /// debugging method that prints out all available interfaces
+        ///
+        /// @param out specifies stream to print list of interfaces to
+        void
+        printIfaces(std::ostream& out = std::cout);
+
+        /// @brief Sends a packet.
+        ///
+        /// Sends a packet. All parameters regarding interface, destination
+        /// address are set in pkt object.
+        ///
+        /// @param pkt packet to be sent
+        ///
+        /// @return true if sending was successful
+        ///
+        bool
+        send(boost::shared_ptr<Pkt6> pkt);
+
+        /// @brief Tries to receive packet over open sockets.
+        ///
+        /// Attempts to receive a single packet of any of the open sockets.
+        /// If reception is successful and all information about its sender
+        /// are obtained, Pkt6 object is created and returned.
+        ///
+        /// @return Pkt6 object representing received packet (or NULL)
+        ///
         boost::shared_ptr<Pkt6> receive();
 
         // don't use private, we need derived classes in tests
     protected:
-        IfaceMgr(); // don't create IfaceMgr directly, use instance() method
-        ~IfaceMgr();
 
-        void detectIfaces();
+        /// @brief Protected constructor.
+        ///
+        /// Protected constructor. This is a singleton class. We don't want
+        /// anyone to create instances of IfaceMgr. Use instance() method
+        IfaceMgr();
+
+        ~IfaceMgr();
 
+        /// @brief Detects network interfaces.
+        ///
+        /// This method will eventually detect available interfaces. For now
+        /// it offers stub implementation. First interface name and link-local
+        /// IPv6 address is read from intefaces.txt file.
+        void
+        detectIfaces();
+
+        ///
+        /// Opens UDP/IPv6 socket and binds it to address, interface nad port.
+        ///
+        /// @param ifname name of the interface
+        /// @param addr address to be bound.
+        /// @param port UDP port.
+        ///
+        /// @return socket descriptor, if socket creation, binding and multicast
+        /// group join were all successful. -1 otherwise.
         int openSocket(const std::string& ifname,
                        const isc::asiolink::IOAddress& addr,
                        int port);
 
         // TODO: having 2 maps (ifindex->iface and ifname->iface would)
         //      probably be better for performance reasons
+
+        /// List of available interfaces
         IfaceLst ifaces_;
 
+        /// a pointer to a sole instance of this class (a singleton)
         static IfaceMgr * instance_;
 
         // TODO: Also keep this interface on Iface once interface detection
@@ -90,15 +175,38 @@ namespace isc {
         // is bound to multicast address. And we all know what happens
         // to people who try to use multicast as source address.
 
+        /// control-buffer, used in transmission and reception
         char * control_buf_;
         int control_buf_len_;
 
     private:
-        bool openSockets();
-        static void instanceCreate();
-        bool joinMcast(int sock, const std::string& ifname,
-                       const std::string& mcast);
+        /// Opens sockets on detected interfaces.
+        bool
+        openSockets();
+
+        /// creates a single instance of this class (a singleton implementation)
+        static void
+        instanceCreate();
+
+        /// @brief Joins IPv6 multicast group on a socket.
+        ///
+        /// Socket must be created and bound to an address. Note that this
+        /// address is different than the multicast address. For example DHCPv6
+        /// server should bind its socket to link-local address (fe80::1234...)
+        /// and later join ff02::1:2 multicast group.
+        ///
+        /// @param sock socket fd (socket must be bound)
+        /// @param ifname interface name (for link-scoped multicast groups)
+        /// @param mcast multicast address to join (e.g. "ff02::1:2")
+        ///
+        /// @return true if multicast join was successful
+        ///
+        bool
+        joinMcast(int sock, const std::string& ifname,
+                  const std::string& mcast);
     };
-};
+
+    }; // namespace isc::dhcp
+}; // namespace isc
 
 #endif
diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc
index 95d2261277..ae3ff06e9e 100644
--- a/src/bin/dhcp6/main.cc
+++ b/src/bin/dhcp6/main.cc
@@ -43,6 +43,7 @@ using namespace isc::config;
 using namespace isc::util;
 
 using namespace isc;
+using namespace isc::dhcp;
 
 namespace {
 
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 842ad3150b..22511683a6 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -31,6 +31,21 @@ using namespace isc::dhcp;
 // Maybe it should be isc::test?
 namespace test {
 
+class NakedDhcpv6Srv: public Dhcpv6Srv {
+    // "naked" Interface Manager, exposes internal fields
+public:
+    NakedDhcpv6Srv() { }
+
+    boost::shared_ptr<Pkt6>
+    processSolicit(boost::shared_ptr<Pkt6>& request) {
+        return Dhcpv6Srv::processSolicit(request);
+    }
+    boost::shared_ptr<Pkt6>
+    processRequest(boost::shared_ptr<Pkt6>& request) {
+        return Dhcpv6Srv::processRequest(request);
+    }
+};
+
 class Dhcpv6SrvTest : public ::testing::Test {
 public:
     Dhcpv6SrvTest() {
@@ -45,7 +60,6 @@ TEST_F(Dhcpv6SrvTest, basic) {
     // srv has stubbed interface detection. It will read
     // interfaces.txt instead. It will pretend to have detected
     // fe80::1234 link-local address on eth0 interface. Obviously
-
     // an attempt to bind this socket will fail.
     EXPECT_NO_THROW( {
         Dhcpv6Srv * srv = new Dhcpv6Srv();
@@ -55,15 +69,21 @@ TEST_F(Dhcpv6SrvTest, basic) {
 
 }
 
-TEST_F(Dhcpv6SrvTest,Solicit_basic) {
-    Dhcpv6Srv * srv = 0;
-    EXPECT_NO_THROW( srv = new Dhcpv6Srv(); );
+TEST_F(Dhcpv6SrvTest, Solicit_basic) {
+    NakedDhcpv6Srv * srv = 0;
+    EXPECT_NO_THROW( srv = new NakedDhcpv6Srv(); );
+
+    // a dummy content for client-id
+    boost::shared_array<char> clntDuid(new char[32]);
+    for (int i=0; i<32; i++)
+        clntDuid[i] = 100+i;
 
     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));
+    boost::shared_ptr<Option6IA> ia =
+        boost::shared_ptr<Option6IA>(new Option6IA(Option::V6, D6O_IA_NA, 234));
     ia->setT1(1501);
     ia->setT2(2601);
     sol->addOption(ia);
@@ -74,6 +94,20 @@ TEST_F(Dhcpv6SrvTest,Solicit_basic) {
     // ia->addOption(addr);
     // sol->addOption(ia);
 
+    // constructed very simple SOLICIT message with:
+    // - client-id option (mandatory)
+    // - IA option (a request for address, without any addresses)
+
+    // expected returned ADVERTISE message:
+    // - copy of client-id
+    // - server-id
+    // - IA that includes IAADDR
+
+    boost::shared_ptr<Option> clientid =
+        boost::shared_ptr<Option>(new Option(Option::V6, D6O_CLIENTID,
+                                             clntDuid, 0, 16));
+    sol->addOption(clientid);
+
     boost::shared_ptr<Pkt6> reply = srv->processSolicit(sol);
 
     // check if we get response at all
@@ -83,10 +117,26 @@ TEST_F(Dhcpv6SrvTest,Solicit_basic) {
     EXPECT_EQ( 1234, reply->getTransid() );
 
     boost::shared_ptr<Option> tmp = reply->getOption(D6O_IA_NA);
-    ASSERT_TRUE( tmp != boost::shared_ptr<Option>() );
+    ASSERT_TRUE( tmp );
 
     Option6IA * reply_ia = dynamic_cast<Option6IA*> ( tmp.get() );
-    EXPECT_EQ( 2345, reply_ia->getIAID() );
+    EXPECT_EQ( 234, reply_ia->getIAID() );
+
+    // check that there's an address included
+    EXPECT_TRUE( reply_ia->getOption(D6O_IAADDR));
+
+    // check that server included our own client-id
+    tmp = reply->getOption(D6O_CLIENTID);
+    ASSERT_TRUE( tmp );
+    EXPECT_EQ(clientid->getType(), tmp->getType() );
+    ASSERT_EQ(clientid->len(), tmp->len() );
+    EXPECT_FALSE(memcmp(clientid->getData(), tmp->getData(), tmp->len() ) );
+    // check that server included its server-id
+    tmp = reply->getOption(D6O_SERVERID);
+    EXPECT_EQ(tmp->getType(), srv->getServerID()->getType() );
+    ASSERT_EQ(tmp->len(),  srv->getServerID()->len() );
+    EXPECT_FALSE( memcmp(tmp->getData(), srv->getServerID()->getData(),
+                      tmp->len()) );
 
     // more checks to be implemented
     delete srv;
diff --git a/src/bin/dhcp6/tests/iface_mgr_unittest.cc b/src/bin/dhcp6/tests/iface_mgr_unittest.cc
index 0f260b5742..3559c81b81 100644
--- a/src/bin/dhcp6/tests/iface_mgr_unittest.cc
+++ b/src/bin/dhcp6/tests/iface_mgr_unittest.cc
@@ -27,6 +27,7 @@
 using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
+using namespace isc::dhcp;
 
 // name of loopback interface detection
 char LOOPBACK[32] = "lo";
@@ -61,6 +62,9 @@ public:
 // is named lo on Linux and lo0 on BSD boxes. We need to find out
 // which is available. This is not a real test, but rather a workaround
 // that will go away when interface detection is implemented.
+
+// NOTE: At this stage of development, write access to current directory
+// during running tests is required.
 TEST_F(IfaceMgrTest, loDetect) {
 
     unlink("interfaces.txt");
@@ -365,7 +369,7 @@ TEST_F(IfaceMgrTest, DISABLED_sendReceive) {
 
     rcvPkt = ifacemgr->receive();
 
-    ASSERT_TRUE( rcvPkt != NULL ); // received our own packet
+    ASSERT_TRUE( rcvPkt ); // received our own packet
 
     // let's check that we received what was sent
     EXPECT_EQ(sendPkt->data_len_, rcvPkt->data_len_);