From: Tomek Mrugalski <tomasz@isc.org>
Date: Fri, 17 Feb 2012 18:27:42 +0000 (+0100)
Subject: [1540] Changes after review:
X-Git-Tag: trac2351_base~240^2~8^2~10
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=526f8713fb6e13456f7008b5cd075f6fbdb89161;p=thirdparty%2Fkea.git

[1540] Changes after review:

- port is now specified using uint16_t in IfaceMgr
- support for proper DUID generation implemented
- implemented new tests
- new HWTYPEs defines
- unpack methods now return size_t type
- comments updated in option*.h headers
---

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 735c39f7fa..638ae051cd 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -59,10 +59,10 @@ Dhcpv4Srv::~Dhcpv4Srv() {
 bool
 Dhcpv4Srv::run() {
     while (!shutdown_) {
-        Pkt4Ptr query; // client's message
-        Pkt4Ptr rsp;   // server's response
 
-        query = IfaceMgr::instance().receive4();
+        // client's message
+        Pkt4Ptr query = IfaceMgr::instance().receive4();
+        Pkt4Ptr rsp;   // server's response
 
         if (query) {
             try {
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index b2bd5f54ee..a0e2b08e51 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <time.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/pkt6.h>
 #include <dhcp/iface_mgr.h>
@@ -21,11 +22,13 @@
 #include <dhcp/option6_addrlst.h>
 #include <asiolink/io_address.h>
 #include <exceptions/exceptions.h>
+#include <util/io_utilities.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
+using namespace isc::util;
 
 const std::string HARDCODED_LEASE = "2001:db8:1::1234:abcd";
 const uint32_t HARDCODED_T1 = 1500; // in seconds
@@ -67,13 +70,11 @@ Dhcpv6Srv::~Dhcpv6Srv() {
     IfaceMgr::instance().closeSockets();
 }
 
-bool
-Dhcpv6Srv::run() {
+bool Dhcpv6Srv::run() {
     while (!shutdown) {
-        Pkt6Ptr query; // client's message
-        Pkt6Ptr rsp;   // server's response
 
-        query = IfaceMgr::instance().receive6();
+        Pkt6Ptr query = IfaceMgr::instance().receive6(); // client's message
+        Pkt6Ptr rsp;   // server's response
 
         if (query) {
             if (!query->unpack()) {
@@ -138,24 +139,91 @@ Dhcpv6Srv::run() {
     return (true);
 }
 
-void
-Dhcpv6Srv::setServerID() {
-    /// TODO implement this for real once interface detection is done.
-    /// Use hardcoded server-id for now
-
-    OptionBuffer srvid(14);
-    srvid[0] = 0;
-    srvid[1] = 1; // DUID type 1 = DUID-LLT (see section 9.2 of RFC3315)
-    srvid[2] = 0;
-    srvid[3] = 6; // HW type = ethernet (I think. I'm typing this from my head
-                  // in hotel, without Internet connection)
-    for (int i=4; i<14; i++) {
-        srvid[i]=i-4;
+void Dhcpv6Srv::setServerID() {
+
+    /// @todo: DUID should be generated once and then stored, rather
+    /// than generated each time
+
+    /// @todo: This code implements support for DUID-LLT (the recommended one).
+    /// We should eventually add support for other DUID types: DUID-LL, DUID-EN
+    /// and DUID-UUID
+
+    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+
+    // let's find suitable interface
+    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
+         iface != ifaces.end(); ++iface) {
+        // all those conditions could be merged into one multi-condition
+        // statement, but let's keep them separated as perhaps one day
+        // we will grow knobs to selectively turn them on or off. Also,
+        // this code is used only *once* during first start on a new machine
+        // and then server-id is stored. (or at least it will be once
+        // DUID storage is implemente
+
+        // I wish there was a this_is_a_real_physical_interface flag...
+
+        // mac at least 6 bytes. All decent physical interfaces (Ethernet,
+        // WiFi, Infiniband, etc.) have 6 bytes long MAC address
+        if (iface->mac_len_ < 6) {
+            continue;
+        }
+
+        // let's don't use loopback
+        if (iface->flag_loopback_) {
+            continue;
+        }
+
+        // let's skip downed interfaces. It is better to use working ones.
+        if (!iface->flag_up_) {
+            continue;
+        }
+
+        uint8_t zeros[IfaceMgr::MAX_MAC_LEN];
+        memset(zeros, 0, IfaceMgr::MAX_MAC_LEN);
+
+        // some interfaces (like lo on Linux) report 6-bytes long
+        // MAC adress 00:00:00:00:00:00. Let's not use such weird interfaces
+        // to generate DUID.
+        if (!memcmp(iface->mac_, zeros, iface->mac_len_)) {
+            continue;
+        }
+
+        // Ok, we have useful MAC. Let's generate DUID-LLT based on
+        // it. See RFC3315, Section 9.2 for details.
+
+        // DUID uses seconds since midnight of 01-01-2000, time() returns
+        // seconds since 01-01-1970. DUID_TIME_EPOCH substution corrects that.
+        time_t seconds = time(NULL);
+        seconds -= DUID_TIME_EPOCH;
+
+        OptionBuffer srvid(8 + iface->mac_len_);
+        writeUint16(DUID_LLT, &srvid[0]);
+        writeUint16(HWTYPE_ETHERNET, &srvid[2]);
+        writeUint32(static_cast<uint32_t>(seconds), &srvid[4]);
+        memcpy(&srvid[0]+8, iface->mac_, iface->mac_len_);
+
+        serverid_ = OptionPtr(new Option(Option::V6, D6O_SERVERID,
+                                         srvid.begin(), srvid.end()));
+        return;
     }
-    serverid_ = boost::shared_ptr<Option>(new Option(Option::V6,
-                                                     D6O_SERVERID,
-                                                     srvid.begin(),
-                                                     srvid.end()));
+
+    // if we reached here, there are no suitable interfaces found.
+    // Either interface detection is not supported on this platform or
+    // this is really weird box. Let's use DUID-EN instead.
+    // See Section 9.3 of RFC3315 for details.
+
+    OptionBuffer srvid(12);
+    srand(time(NULL));
+    writeUint16(DUID_EN, &srvid[0]);
+    writeUint32(ENTERPRISE_ID_ISC, &srvid[2]);
+
+    // length of the identifier is company specific. I hereby declare
+    // ISC standard of 6 bytes long random numbers.
+    for (int i = 6; i < 12; i++) {
+        srvid[i] = static_cast<uint8_t>(rand());
+    }
+    serverid_ = OptionPtr(new Option(Option::V6, D6O_SERVERID,
+                                     srvid.begin(), srvid.end()));
 }
 
 void Dhcpv6Srv::copyDefaultOptions(const Pkt6Ptr& question, Pkt6Ptr& answer) {
@@ -214,7 +282,6 @@ void Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer) {
 }
 
 Pkt6Ptr Dhcpv6Srv::processSolicit(const Pkt6Ptr& solicit) {
-
     Pkt6Ptr advertise(new Pkt6(DHCPV6_ADVERTISE, solicit->getTransid()));
 
     copyDefaultOptions(solicit, advertise);
@@ -227,7 +294,6 @@ Pkt6Ptr Dhcpv6Srv::processSolicit(const Pkt6Ptr& solicit) {
 }
 
 Pkt6Ptr Dhcpv6Srv::processRequest(const Pkt6Ptr& request) {
-
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, request->getTransid()));
 
     copyDefaultOptions(request, reply);
@@ -240,33 +306,37 @@ Pkt6Ptr Dhcpv6Srv::processRequest(const Pkt6Ptr& request) {
 }
 
 Pkt6Ptr Dhcpv6Srv::processRenew(const Pkt6Ptr& renew) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, renew->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processRebind(const Pkt6Ptr& rebind) {
-    Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY,
-                                           rebind->getTransid(),
-                                           Pkt6::UDP));
+    /// @todo: Implement this
+    Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, rebind->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processConfirm(const Pkt6Ptr& confirm) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, confirm->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processRelease(const Pkt6Ptr& release) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, release->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processDecline(const Pkt6Ptr& decline) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, decline->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processInfRequest(const Pkt6Ptr& infRequest) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, infRequest->getTransid()));
     return reply;
 }
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 33267a1b14..eb351ac6aa 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -20,13 +20,15 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
-#include "dhcp/dhcp6.h"
-#include "dhcp6/dhcp6_srv.h"
-#include "dhcp/option6_ia.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp6/dhcp6_srv.h>
+#include <util/buffer.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 // namespace has to be named, because friends are defined in Dhcpv6Srv class
 // Maybe it should be isc::test?
@@ -79,16 +81,84 @@ TEST_F(Dhcpv6SrvTest, basic) {
     delete srv;
 }
 
+TEST_F(Dhcpv6SrvTest, DUID) {
+    // tests that DUID is generated properly
+
+    Dhcpv6Srv* srv = NULL;
+    ASSERT_NO_THROW( {
+        srv = new Dhcpv6Srv(DHCP6_SERVER_PORT + 10000);
+    });
+
+    OptionPtr srvid = srv->getServerID();
+    ASSERT_TRUE(srvid);
+
+    EXPECT_EQ(D6O_SERVERID, srvid->getType());
+
+    OutputBuffer buf(32);
+    srvid->pack(buf);
+
+    // length of the actual DUID
+    size_t len = buf.getLength() - srvid->getHeaderLen();
+
+    InputBuffer data(buf.getData(), buf.getLength());
+
+    // ignore first four bytes (standard DHCPv6 header)
+    data.readUint32();
+
+    uint16_t duid_type = data.readUint16();
+    cout << "Duid-type=" << duid_type << endl;
+    switch(duid_type) {
+    case DUID_LLT: {
+        // DUID must contain at least 6 bytes long MAC
+        // + 8 bytes of fixed header
+        EXPECT_GE(14, len);
+
+        uint16_t hw_type = data.readUint16();
+        // there's no real way to find out "correct"
+        // hardware type
+        EXPECT_GT(hw_type, 0);
+
+        // check that timer is counted since 1.1.2000,
+        // not from 1.1.1970.
+        uint32_t seconds = data.readUint32();
+        EXPECT_LE(seconds, DUID_TIME_EPOCH);
+        // this test will start failing after 2030.
+        // Hopefully we will be at BIND12 by then.
+
+        // MAC must not be zeros
+        vector<uint8_t> mac(len-8);
+        vector<uint8_t> zeros(len-8, 0);
+        data.readVector(mac, len-8);
+        EXPECT_NE(mac, zeros);
+        break;
+    }
+    case DUID_EN: {
+        // there's not much we can check. Just simple
+        // check if it is not all zeros
+        vector<uint8_t> content(len-2);
+        vector<uint8_t> zeros(0, len-2);
+        data.readVector(content, len-2);
+        EXPECT_NE(content, zeros);
+    }
+    case DUID_LL: // not supported yet
+    case DUID_UUID: // not supported yet
+    default:
+        cout << "Not supported duid type=" << duid_type << endl;
+        FAIL();
+    }
+}
+
 TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     NakedDhcpv6Srv* srv = NULL;
     ASSERT_NO_THROW( srv = new NakedDhcpv6Srv(); );
 
     // a dummy content for client-id
     OptionBuffer clntDuid(32);
-    for (int i = 0; i < 32; i++)
+    for (int i = 0; i < 32; i++) {
         clntDuid[i] = 100 + i;
+    }
 
-    Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234, Pkt6::UDP));
+    Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
 
     boost::shared_ptr<Option6IA> ia =
         boost::shared_ptr<Option6IA>(new Option6IA(D6O_IA_NA, 234));
@@ -113,7 +183,7 @@ TEST_F(Dhcpv6SrvTest, Solicit_basic) {
 
     OptionPtr clientid = OptionPtr(new Option(Option::V6, D6O_CLIENTID,
                                               clntDuid.begin(),
-                                              clntDuid.begin()+16));
+                                              clntDuid.begin() + 16));
     sol->addOption(clientid);
 
     boost::shared_ptr<Pkt6> reply = srv->processSolicit(sol);
diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h
index 6012003b96..15c306da66 100644
--- a/src/lib/dhcp/dhcp6.h
+++ b/src/lib/dhcp/dhcp6.h
@@ -108,6 +108,15 @@ extern const int dhcpv6_type_name_max;
 #define DUID_LLT        1
 #define DUID_EN         2
 #define DUID_LL         3
+#define DUID_UUID       4
+
+// Define hardware types
+// Taken from http://www.iana.org/assignments/arp-parameters/
+#define HWTYPE_ETHERNET    0x0001
+#define HWTYPE_INIFINIBAND 0x0020
+
+// Taken from http://www.iana.org/assignments/enterprise-numbers
+#define ENTERPRISE_ID_ISC 2495
 
 /* Offsets into IA_*'s where Option spaces commence.  */
 #define IA_NA_OFFSET 12 /* IAID, T1, T2, all 4 octets each */
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index f0df4626a6..e328cdadbe 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -178,6 +178,12 @@ IfaceMgr::stubDetectIfaces() {
         cout << "Detected interface " << ifaceName << "/" << linkLocal << endl;
 
         Iface iface(ifaceName, if_nametoindex( ifaceName.c_str() ) );
+        iface.flag_up_ = true;
+        iface.flag_running_ = true;
+        iface.flag_loopback_ = false;
+        iface.flag_multicast_ = true;
+        iface.flag_broadcast_ = true;
+        iface.hardware_type_ = HWTYPE_ETHERNET;
         IOAddress addr(linkLocal);
         iface.addAddress(addr);
         addInterface(iface);
@@ -347,9 +353,7 @@ IfaceMgr::getIface(const std::string& ifname) {
     return (NULL); // not found
 }
 
-int IfaceMgr::openSocket(const std::string& ifname,
-                     const IOAddress& addr,
-                     int port) {
+int IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr, uint16_t port) {
     Iface* iface = getIface(ifname);
     if (!iface) {
         isc_throw(BadValue, "There is no " << ifname << " interface present.");
@@ -365,7 +369,7 @@ int IfaceMgr::openSocket(const std::string& ifname,
     }
 }
 
-int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
+int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, uint16_t port) {
 
     cout << "Creating UDP4 socket on " << iface.getFullName()
          << " " << addr.toText() << "/port=" << port << endl;
@@ -409,7 +413,7 @@ int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     return (sock);
 }
 
-int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
+int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, uint16_t port) {
 
     cout << "Creating UDP6 socket on " << iface.getFullName()
          << " " << addr.toText() << "/port=" << port << endl;
@@ -515,7 +519,7 @@ const std::string & mcast) {
 }
 
 bool
-IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
+IfaceMgr::send(const Pkt6Ptr& pkt) {
     struct msghdr m;
     struct iovec v;
     int result;
@@ -549,7 +553,7 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
     // Set the data buffer we're sending. (Using this wacky
     // "scatter-gather" stuff... we only have a single chunk
     // of data to send, so we declare a single vector entry.)
-    v.iov_base = (char *) pkt->getBuffer().getData();
+    v.iov_base = const_cast<void *>(pkt->getBuffer().getData());
     v.iov_len = pkt->getBuffer().getLength();
     m.msg_iov = &v;
     m.msg_iovlen = 1;
@@ -585,11 +589,10 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
 }
 
 bool
-IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
+IfaceMgr::send(const Pkt4Ptr& pkt)
 {
     struct msghdr m;
     struct iovec v;
-    int result;
 
     Iface* iface = getIface(pkt->getIface());
     if (!iface) {
@@ -620,7 +623,6 @@ IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
     m.msg_iov = &v;
     m.msg_iovlen = 1;
 
-// OS_LINUX defines are part of ticket #1237
 #if defined(OS_LINUX)
     // Setting the interface is a bit more involved.
     //
@@ -628,16 +630,14 @@ IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
     // define the IPv4 packet information. We could set the
     // source address if we wanted, but we can safely let the
     // kernel decide what that should be.
-    struct in_pktinfo *pktinfo;
-    struct cmsghdr *cmsg;
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
-    cmsg = CMSG_FIRSTHDR(&m);
+    struct cmsghdr* cmsg = CMSG_FIRSTHDR(&m);
     cmsg->cmsg_level = IPPROTO_IP;
     cmsg->cmsg_type = IP_PKTINFO;
-    cmsg->cmsg_len = CMSG_LEN(sizeof(*pktinfo));
-    pktinfo = (struct in_pktinfo *)CMSG_DATA(cmsg);
-    memset(pktinfo, 0, sizeof(*pktinfo));
+    cmsg->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo));
+    struct in_pktinfo* pktinfo =(struct in_pktinfo *)CMSG_DATA(cmsg);
+    memset(pktinfo, 0, sizeof(struct in_pktinfo));
     pktinfo->ipi_ifindex = pkt->getIndex();
     m.msg_controllen = cmsg->cmsg_len;
 #endif
@@ -647,7 +647,7 @@ IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
          << " over socket " << getSocket(*pkt) << " on interface "
          << getIface(pkt->getIface())->getFullName() << endl;
 
-    result = sendmsg(getSocket(*pkt), &m, 0);
+        int result = sendmsg(getSocket(*pkt), &m, 0);
     if (result < 0) {
         isc_throw(Unexpected, "Pkt4 send failed.");
     }
@@ -669,22 +669,12 @@ IfaceMgr::receive4() {
     IfaceCollection::const_iterator iface;
     for (iface = ifaces_.begin(); iface != ifaces_.end(); ++iface) {
 
-#if 0
-        // uncomment this once #1237 is merged
-        // Let's skip loopback and downed interfaces.
-        if (iface->flag_loopback_ ||
-            !iface->flag_up_ ||
-            !iface->flag_running_) {
-            continue;
-        }
-#endif
-
-        SocketCollection::const_iterator s = iface->sockets_.begin();
-        while (s != iface->sockets_.end()) {
+        /// @todo: rewrite this as part of #1555
+        for (SocketCollection::const_iterator s = iface->sockets_.begin();
+             s != iface->sockets_.end(); ++s) {
 
             // We don't want IPv6 addresses here.
             if (s->addr_.getFamily() != AF_INET) {
-                ++s;
                 continue;
             }
 
@@ -693,8 +683,6 @@ IfaceMgr::receive4() {
                 candidate = &(*s);
                 break;
             }
-
-            ++s;
         }
 
         if (candidate) {
@@ -711,13 +699,8 @@ IfaceMgr::receive4() {
          << iface->getFullName() << endl;
 
     // Now we have a socket, let's get some data from it!
-
-    struct msghdr m;
-    struct iovec v;
-    int result;
     struct sockaddr_in from_addr;
     struct in_addr to_addr;
-    boost::shared_ptr<Pkt4> pkt;
     const uint32_t RCVBUFSIZE = 1500;
     static uint8_t buf[RCVBUFSIZE];
 
@@ -726,13 +709,15 @@ IfaceMgr::receive4() {
     memset(&to_addr, 0, sizeof(to_addr));
 
     // Initialize our message header structure.
+    struct msghdr m;
     memset(&m, 0, sizeof(m));
 
     // Point so we can get the from address.
     m.msg_name = &from_addr;
     m.msg_namelen = sizeof(from_addr);
 
-    v.iov_base = (void*)buf;
+    struct iovec v;
+    v.iov_base = static_cast<void*>(buf);
     v.iov_len = RCVBUFSIZE;
     m.msg_iov = &v;
     m.msg_iovlen = 1;
@@ -746,16 +731,14 @@ IfaceMgr::receive4() {
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
 
-    result = recvmsg(candidate->sockfd_, &m, 0);
-
+    int result = recvmsg(candidate->sockfd_, &m, 0);
     if (result < 0) {
         cout << "Failed to receive UDP4 data." << endl;
-        return (boost::shared_ptr<Pkt4>()); // NULL
+        return (Pkt4Ptr()); // NULL
     }
 
     unsigned int ifindex = iface->getIndex();
 
-// OS_LINUX defines are part of ticket #1237
 #if defined(OS_LINUX)
     struct cmsghdr* cmsg;
     struct in_pktinfo* pktinfo;
@@ -800,7 +783,7 @@ IfaceMgr::receive4() {
          << iface->getFullName() << endl;
 
     // we have all data let's create Pkt4 object
-    pkt = boost::shared_ptr<Pkt4>(new Pkt4(buf, result));
+    Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(buf, result));
 
     pkt->setIface(iface->getName());
     pkt->setIndex(ifindex);
@@ -820,7 +803,7 @@ Pkt6Ptr IfaceMgr::receive6() {
     struct in6_pktinfo* pktinfo;
     struct sockaddr_in6 from;
     struct in6_addr to_addr;
-    int ifindex;
+    int ifindex = -1;
     Pkt6Ptr pkt;
 
     // RFC3315 states that server responses may be
@@ -866,10 +849,9 @@ Pkt6Ptr IfaceMgr::receive6() {
     IfaceCollection::const_iterator iface = ifaces_.begin();
     const SocketInfo* candidate = 0;
     while (iface != ifaces_.end()) {
-        SocketCollection::const_iterator s = iface->sockets_.begin();
-        while (s != iface->sockets_.end()) {
+        for (SocketCollection::const_iterator s = iface->sockets_.begin();
+             s != iface->sockets_.end(); ++s) {
             if (s->addr_.getFamily() != AF_INET6) {
-                ++s;
                 continue;
             }
             if (s->addr_.getAddress().to_v6().is_multicast()) {
@@ -879,7 +861,6 @@ Pkt6Ptr IfaceMgr::receive6() {
             if (!candidate) {
                 candidate = &(*s); // it's not multicast, but it's better than nothing
             }
-            ++s;
         }
         if (candidate) {
             break;
@@ -887,7 +868,7 @@ Pkt6Ptr IfaceMgr::receive6() {
         ++iface;
     }
     if (iface == ifaces_.end()) {
-        isc_throw(Unexpected, "No interfaces detected. Can't receive anything.");
+        isc_throw(Unexpected, "No suitable IPv6 interfaces detected. Can't receive anything.");
     }
 
     if (!candidate) {
@@ -921,11 +902,11 @@ Pkt6Ptr IfaceMgr::receive6() {
         }
         if (!found_pktinfo) {
             cout << "Unable to find pktinfo" << endl;
-            return (boost::shared_ptr<Pkt6>()); // NULL
+            return (Pkt6Ptr()); // NULL
         }
     } else {
         cout << "Failed to receive data." << endl;
-        return (boost::shared_ptr<Pkt6>()); // NULL
+        return (Pkt6Ptr()); // NULL
     }
 
 
@@ -933,11 +914,11 @@ Pkt6Ptr IfaceMgr::receive6() {
         pkt = Pkt6Ptr(new Pkt6(buf, result));
     } catch (const std::exception& ex) {
         cout << "Failed to create new packet." << endl;
-        return (boost::shared_ptr<Pkt6>()); // NULL
+        return (Pkt6Ptr()); // NULL
     }
 
-    pkt->setLocalAddr(IOAddress::from_bytes(AF_INET6, (const uint8_t*)&to_addr));
-    pkt->setRemoteAddr(IOAddress::from_bytes(AF_INET6, (const uint8_t*)&from.sin6_addr));
+    pkt->setLocalAddr(IOAddress::from_bytes(AF_INET6, reinterpret_cast<const uint8_t*>(&to_addr)));
+    pkt->setRemoteAddr(IOAddress::from_bytes(AF_INET6, reinterpret_cast<const uint8_t*>(&from.sin6_addr)));
 
     pkt->setRemotePort(ntohs(from.sin6_port));
 
@@ -962,7 +943,7 @@ Pkt6Ptr IfaceMgr::receive6() {
     return (pkt);
 }
 
-uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt6 const& pkt) {
+uint16_t IfaceMgr::getSocket(const isc::dhcp::Pkt6& pkt) {
     Iface* iface = getIface(pkt.getIface());
     if (!iface) {
         isc_throw(BadValue, "Tried to find socket for non-existent interface "
@@ -971,19 +952,13 @@ uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt6 const& pkt) {
 
     SocketCollection::const_iterator s;
     for (s = iface->sockets_.begin(); s != iface->sockets_.end(); ++s) {
-        if (s->family_ != AF_INET6) {
-            // don't use IPv4 sockets
-            continue;
-        }
-        if (s->addr_.getAddress().to_v6().is_multicast()) {
-            // don't use IPv6 sockets bound to multicast address
-            continue;
+        if ( (s->family_ == AF_INET6) &&
+             (!s->addr_.getAddress().to_v6().is_multicast()) ) {
+            return (s->sockfd_);
         }
         /// TODO: Add more checks here later. If remote address is
         /// not link-local, we can't use link local bound socket
         /// to send data.
-
-        return (s->sockfd_);
     }
 
     isc_throw(Unexpected, "Interface " << iface->getFullName()
@@ -999,16 +974,12 @@ uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt4 const& pkt) {
 
     SocketCollection::const_iterator s;
     for (s = iface->sockets_.begin(); s != iface->sockets_.end(); ++s) {
-        if (s->family_ != AF_INET) {
-            // don't use IPv6 sockets
-            continue;
+        if (s->family_ == AF_INET) {
+            return (s->sockfd_);
         }
-
         /// TODO: Add more checks here later. If remote address is
         /// not link-local, we can't use link local bound socket
         /// to send data.
-
-        return (s->sockfd_);
     }
 
     isc_throw(Unexpected, "Interface " << iface->getFullName()
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 93e6b981e7..bb3a73a966 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -230,6 +230,11 @@ public:
     Iface*
     getIface(const std::string& ifname);
 
+    /// @brief Returns container with all interfaces.
+    ///
+    /// @return container with all interfaces.
+    const IfaceCollection& getIfaces() { return ifaces_; }
+
     /// @brief Return most suitable socket for transmitting specified IPv6 packet.
     ///
     /// This method takes Pkt6 (see overloaded implementation that takes
@@ -271,7 +276,7 @@ public:
     /// @param pkt packet to be sent
     ///
     /// @return true if sending was successful
-    bool send(boost::shared_ptr<Pkt6>& pkt);
+    bool send(const Pkt6Ptr& pkt);
 
     /// @brief Sends an IPv4 packet.
     ///
@@ -282,7 +287,7 @@ public:
     /// @param pkt a packet to be sent
     ///
     /// @return true if sending was successful
-    bool send(boost::shared_ptr<Pkt4>& pkt);
+    bool send(const Pkt4Ptr& pkt);
 
     /// @brief Tries to receive IPv6 packet over open IPv6 sockets.
     ///
@@ -325,7 +330,7 @@ public:
     /// @return socket descriptor, if socket creation, binding and multicast
     /// group join were all successful.
     int openSocket(const std::string& ifname,
-                   const isc::asiolink::IOAddress& addr, int port);
+                   const isc::asiolink::IOAddress& addr, uint16_t port);
 
     /// Opens IPv6 sockets on detected interfaces.
     ///
@@ -375,7 +380,7 @@ protected:
     /// @param port a port that created socket should be bound to
     ///
     /// @return socket descriptor
-    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
+    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
 
     /// @brief Opens IPv6 socket.
     ///
@@ -388,7 +393,7 @@ protected:
     /// @param port a port that created socket should be bound to
     ///
     /// @return socket descriptor
-    int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
+    int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
 
     /// @brief Adds an interface to list of known interfaces.
     ///
diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 9a4f4dea9e..8b4f62b391 100644
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -34,10 +34,10 @@ std::map<unsigned short, Option::Factory*> LibDHCP::v4factories_;
 std::map<unsigned short, Option::Factory*> LibDHCP::v6factories_;
 
 
-uint32_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
-                                 isc::dhcp::Option::OptionCollection& options) {
-    uint32_t offset = 0;
-    unsigned int end = buf.size();
+size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
+                               isc::dhcp::Option::OptionCollection& options) {
+    size_t offset = 0;
+    size_t end = buf.size();
 
     while (offset +4 <= end) {
         uint16_t opt_type = buf[offset]*256 + buf[offset+1];
@@ -80,17 +80,23 @@ uint32_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
     return (offset);
 }
 
-uint32_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
+size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
                                  isc::dhcp::Option::OptionCollection& options) {
     size_t offset = 0;
 
-    // 2 - header of DHCPv4 option
+    // 2 byte - header of DHCPv4 option
     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()) {
             isc_throw(OutOfRange, "Attempt to parse truncated option "
                       << opt_type);
diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h
index 658a893628..c7935c8ff3 100644
--- a/src/lib/dhcp/libdhcp++.h
+++ b/src/lib/dhcp/libdhcp++.h
@@ -57,8 +57,8 @@ public:
     /// @param buf Buffer to be parsed.
     /// @param options Reference to option container. Options will be
     ///        put here.
-    static uint32_t unpackOptions4(const OptionBuffer& buf,
-                                   isc::dhcp::Option::OptionCollection& options);
+    static size_t unpackOptions4(const OptionBuffer& buf,
+                                 isc::dhcp::Option::OptionCollection& options);
 
     /// @brief Parses provided buffer as DHCPv6 options and creates Option objects.
     ///
@@ -68,8 +68,8 @@ public:
     /// @param buf Buffer to be parsed.
     /// @param options Reference to option container. Options will be
     ///        put here.
-    static uint32_t unpackOptions6(const OptionBuffer& buf,
-                                   isc::dhcp::Option::OptionCollection& options);
+    static size_t unpackOptions6(const OptionBuffer& buf,
+                                 isc::dhcp::Option::OptionCollection& options);
 
     /// Registers factory method that produces options of specific option types.
     ///
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index f0dec3b9e2..ea273fd89b 100644
--- a/src/lib/dhcp/option.h
+++ b/src/lib/dhcp/option.h
@@ -122,9 +122,6 @@ public:
     /// TODO: Migrate DHCPv6 code to pack(OutputBuffer& buf) version
     ///
     /// @param buf pointer to a buffer
-    ///
-    /// @return offset to first unused byte after stored option
-    ///
     virtual void pack(isc::util::OutputBuffer& buf);
 
     /// @brief Writes option in a wire-format to a buffer.
@@ -137,10 +134,7 @@ public:
     /// @param buf output buffer (option will be stored there)
     virtual void pack4(isc::util::OutputBuffer& buf);
 
-    /// @brief Parses buffer.
-    ///
-    /// Parses received buffer, returns offset to the first unused byte after
-    /// parsed option.
+    /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
     /// @param end iterator to end of option data (first byte after option end)
@@ -152,32 +146,28 @@ public:
     /// @param indent number of spaces before printing text
     ///
     /// @return string with text representation.
-    virtual std::string
-    toText(int indent = 0);
+    virtual std::string toText(int indent = 0);
 
     /// Returns option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
     ///
     /// @return option type
-    unsigned short getType() { return (type_); }
+    uint16_t getType() { return (type_); }
 
     /// Returns length of the complete option (data length + DHCPv4/DHCPv6
     /// option header)
     ///
     /// @return length of the option
-    virtual uint16_t
-    len();
+    virtual uint16_t len();
 
     /// @brief Returns length of header (2 for v4, 4 for v6)
     ///
     /// @return length of option header
-    virtual uint16_t
-    getHeaderLen();
+    virtual uint16_t getHeaderLen();
 
     /// returns if option is valid (e.g. option may be truncated)
     ///
     /// @return true, if option is valid
-    virtual bool
-    valid();
+    virtual bool valid();
 
     /// Returns pointer to actual data.
     ///
diff --git a/src/lib/dhcp/option6_iaaddr.h b/src/lib/dhcp/option6_iaaddr.h
index b407b0422e..e6e2c161e7 100644
--- a/src/lib/dhcp/option6_iaaddr.h
+++ b/src/lib/dhcp/option6_iaaddr.h
@@ -52,10 +52,7 @@ public:
     /// @param buf pointer to a buffer
     void pack(isc::util::OutputBuffer& buf);
 
-    /// @brief Parses buffer.
-    ///
-    /// Parses received buffer, returns offset to the first unused byte after
-    /// parsed option.
+    /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
     /// @param end iterator to end of option data (first byte after option end)