Merge branch 'trac1239' into trac1230

Conflicts:
	ChangeLog
	src/bin/dhcp4/dhcp4_srv.cc
	src/bin/dhcp6/dhcp6_srv.cc
	src/lib/dhcp/iface_mgr.cc
	src/lib/dhcp/iface_mgr.h

ChangeLog

@@ -4,6 +4,14 @@
 	addresses, flags and configured IPv4 and IPv6 addresses.
 	(Trac #1237, git TBD)
 
+3XX.	[func]		tomek
+	libdhcp++: Transmission and reception of DHCPv4 packets is now
+	implemented. Low-level hacks are not implemented for transmission
+	to hosts that don't have IPv4 address yet, so currently the code
+	is usable for communication with relays only, not hosts on the
+	same link.
+	(Trac #1239, #1240, git TBD)
+
 349.	[bug]		dvv
 	resolver: If an upstream server responds with FORMERR to an EDNS query,
 	try querying it without EDNS.

src/bin/dhcp4/dhcp4_srv.cc

@@ -23,6 +23,8 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
+// #define ECHO_SERVER
+
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
     cout << "Initialization: opening sockets on port " << port << endl;
 
@@ -33,10 +35,8 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
     /// @todo: instantiate LeaseMgr here once it is imlpemented.
     IfaceMgr::instance().printIfaces();
 
-#if 0
     // uncomment this once #1238, #992 and #1239 are merged
     IfaceMgr::instance().openSockets4(port);
-#endif
 
     setServerID();
 
@@ -45,6 +45,7 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
 
 Dhcpv4Srv::~Dhcpv4Srv() {
     cout << "DHCPv4 server shutdown." << endl;
+    IfaceMgr::instance().closeSockets();
 }
 
 bool
@@ -53,16 +54,23 @@ Dhcpv4Srv::run() {
         boost::shared_ptr<Pkt4> query; // client's message
         boost::shared_ptr<Pkt4> rsp;   // server's response
 
-#if 0
-        // uncomment this once ticket 1239 is merged.
         query = IfaceMgr::instance().receive4();
+
+#if defined(ECHO_SERVER)
+        query->repack();
+        IfaceMgr::instance().send(query);
+        continue;
 #endif
 
         if (query) {
-            if (!query->unpack()) {
-                cout << "Failed to parse incoming packet" << endl;
+            try {
+                query->unpack();
+            } catch (const std::exception& e) {
+                /// TODO: Printout reasons of failed parsing
+                cout << "Failed to parse incoming packet " << endl;
                 continue;
             }
+
             switch (query->getType()) {
             case DHCPDISCOVER:
                 rsp = processDiscover(query);
@@ -84,8 +92,7 @@ Dhcpv4Srv::run() {
                      << query->getType() << endl;
             }
 
-            cout << "Received " << query->len() << " bytes packet type="
-                 << query->getType() << endl;
+            cout << "Received message type " << int(query->getType()) << endl;
 
             // TODO: print out received packets only if verbose (or debug)
             // mode is enabled
@@ -99,15 +106,16 @@ Dhcpv4Srv::run() {
                 rsp->setIface(query->getIface());
                 rsp->setIndex(query->getIndex());
 
-                cout << "Replying with:" << rsp->getType() << endl;
+                cout << "Replying with message type "
+                     << static_cast<int>(rsp->getType()) << ":" << endl;
                 cout << rsp->toText();
                 cout << "----" << endl;
                 if (rsp->pack()) {
                     cout << "Packet assembled correctly." << endl;
                 }
-#if 0
+#if 1
                 // uncomment this once ticket 1240 is merged.
-                IfaceMgr::instance().send4(rsp);
+                IfaceMgr::instance().send(rsp);
 #endif
             }
         }

src/bin/dhcp4/tests/Makefile.am

@@ -41,6 +41,7 @@ dhcp4_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libdhcp++.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
+dhcp4_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -14,6 +14,7 @@
 
 #include <config.h>
 #include <iostream>
+#include <fstream>
 #include <sstream>
 
 #include <arpa/inet.h>
@@ -28,11 +29,12 @@ using namespace isc;
 using namespace isc::dhcp;
 
 namespace {
+const char* const INTERFACE_FILE = "interfaces.txt";
 
 class NakedDhcpv4Srv: public Dhcpv4Srv {
     // "naked" DHCPv4 server, exposes internal fields
 public:
-    NakedDhcpv4Srv() { }
+    NakedDhcpv4Srv():Dhcpv4Srv(DHCP4_SERVER_PORT + 10000) { }
 
     boost::shared_ptr<Pkt4> processDiscover(boost::shared_ptr<Pkt4>& discover) {
         return Dhcpv4Srv::processDiscover(discover);
@@ -54,9 +56,18 @@ public:
 class Dhcpv4SrvTest : public ::testing::Test {
 public:
     Dhcpv4SrvTest() {
+        unlink(INTERFACE_FILE);
+        fstream fakeifaces(INTERFACE_FILE, ios::out | ios::trunc);
+        if (if_nametoindex("lo") > 0) {
+            fakeifaces << "lo 127.0.0.1";
+        } else if (if_nametoindex("lo0") > 0) {
+            fakeifaces << "lo0 127.0.0.1";
+        }
+        fakeifaces.close();
     }
 
     ~Dhcpv4SrvTest() {
+        unlink(INTERFACE_FILE);
     };
 };
 
@@ -66,7 +77,7 @@ TEST_F(Dhcpv4SrvTest, basic) {
 
     Dhcpv4Srv* srv = NULL;
     ASSERT_NO_THROW({
-        srv = new Dhcpv4Srv();
+        srv = new Dhcpv4Srv(DHCP4_SERVER_PORT + 10000);
     });
 
     delete srv;

src/bin/dhcp6/dhcp6_srv.cc

@@ -44,11 +44,7 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) {
     }
 
     // Now try to open IPv6 sockets on detected interfaces.
-    cout << "Opening sockets on port " << port << endl;
-#if 0
-    // uncomment this once #1238, #992 and #1239 are merged
     IfaceMgr::instance().openSockets6(port);
-#endif
 
     /// @todo: instantiate LeaseMgr here once it is imlpemented.
 

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -14,6 +14,7 @@
 
 #include <config.h>
 #include <iostream>
+#include <fstream>
 #include <sstream>
 
 #include <arpa/inet.h>
@@ -29,7 +30,8 @@ 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 {
+const char* const INTERFACE_FILE = "interfaces.txt";
 
 class NakedDhcpv6Srv: public Dhcpv6Srv {
     // "naked" Interface Manager, exposes internal fields
@@ -49,7 +51,18 @@ public:
 class Dhcpv6SrvTest : public ::testing::Test {
 public:
     Dhcpv6SrvTest() {
+        unlink(INTERFACE_FILE);
+        fstream fakeifaces(INTERFACE_FILE, ios::out | ios::trunc);
+        if (if_nametoindex("lo") > 0) {
+            fakeifaces << "lo ::1";
+        } else if (if_nametoindex("lo0") > 0) {
+            fakeifaces << "lo0 ::1";
+        }
+        fakeifaces.close();
     }
+    ~Dhcpv6SrvTest() {
+        unlink(INTERFACE_FILE);
+    };
 };
 
 TEST_F(Dhcpv6SrvTest, basic) {
@@ -116,7 +129,7 @@ TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     boost::shared_ptr<Option> tmp = reply->getOption(D6O_IA_NA);
     ASSERT_TRUE( tmp );
 
-    Option6IA* reply_ia = dynamic_cast<Option6IA*> ( tmp.get() );
+    Option6IA* reply_ia = dynamic_cast<Option6IA*>(tmp.get());
     EXPECT_EQ( 234, reply_ia->getIAID() );
 
     // check that there's an address included

src/lib/dhcp/dhcp4.h

@@ -157,7 +157,7 @@ static const uint16_t DHCP4_SERVER_PORT = 67;
 
 /// Magic cookie validating dhcp options field (and bootp vendor
 /// extensions field).
-///static const char* DHCP_OPTIONS_COOKIE = "\143\202\123\143";
+static const uint32_t DHCP_OPTIONS_COOKIE = 0x63825363;
 
 // TODO: Following are leftovers from dhcp.h import from ISC DHCP
 // They will be converted to C++-style defines once they will start

src/lib/dhcp/iface_mgr.cc

@@ -19,6 +19,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
+#include <dhcp/dhcp4.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/iface_mgr.h>
 #include <exceptions/exceptions.h>
@@ -83,14 +84,14 @@ IfaceMgr::Iface::getPlainMac() const {
 }
 
 bool IfaceMgr::Iface::delAddress(const isc::asiolink::IOAddress& addr) {
-
-    // Let's delete all addresses that match. It really shouldn't matter
-    // if we delete first or all, as the OS should allow to add a single
-    // address to an interface only once. If OS allows multiple instances
-    // of the same address added, we are in deep problems anyway.
-    size_t size = addrs_.size();
-    addrs_.erase(remove(addrs_.begin(), addrs_.end(), addr), addrs_.end());
-    return (addrs_.size() < size);
+    for (AddressCollection::iterator a = addrs_.begin();
+         a!=addrs_.end(); ++a) {
+        if (*a==addr) {
+            addrs_.erase(a);
+            return (true);
+        }
+    }
+    return (false);
 }
 
 bool IfaceMgr::Iface::delSocket(uint16_t sockfd) {
@@ -148,10 +149,10 @@ void IfaceMgr::closeSockets() {
 }
 
 IfaceMgr::~IfaceMgr() {
-    closeSockets();
-
     // control_buf_ is deleted automatically (scoped_ptr)
     control_buf_len_ = 0;
+
+    closeSockets();
 }
 
 void
@@ -200,42 +201,93 @@ void IfaceMgr::detectIfaces() {
 }
 #endif
 
-void IfaceMgr::openSockets6(uint16_t port) {
-    int sock1, sock2;
+bool IfaceMgr::openSockets4(uint16_t port) {
+    int sock;
+    int count = 0;
 
-    for (IfaceCollection::iterator iface = ifaces_.begin();
-         iface != ifaces_.end(); ++iface) {
+    for (IfaceCollection::iterator iface=ifaces_.begin();
+         iface!=ifaces_.end();
+         ++iface) {
+
+        cout << "Trying interface " << iface->getFullName() << endl;
+
+        if (iface->flag_loopback_ ||
+            !iface->flag_up_ ||
+            !iface->flag_running_) {
+            continue;
+        }
 
         AddressCollection addrs = iface->getAddresses();
 
-        for (AddressCollection::iterator addr = addrs.begin();
+        for (AddressCollection::iterator addr= addrs.begin();
              addr != addrs.end();
              ++addr) {
 
-            sock1 = openSocket(iface->getName(), *addr, port);
-            if (sock1 < 0) {
-                isc_throw(Unexpected, "Failed to open unicast socket on "
-                          << " interface " << iface->getFullName());
+            // skip IPv4 addresses
+            if (addr->getFamily() != AF_INET) {
+                continue;
+            }
+
+            sock = openSocket(iface->getName(), *addr, port);
+            if (sock<0) {
+                cout << "Failed to open unicast socket." << endl;
+                return (false);
+            }
+
+            count++;
+        }
+    }
+    return (count > 0);
+
+}
+
+bool IfaceMgr::openSockets6(uint16_t port) {
+    int sock;
+    int count = 0;
+
+    for (IfaceCollection::iterator iface=ifaces_.begin();
+         iface!=ifaces_.end();
+         ++iface) {
+
+        AddressCollection addrs = iface->getAddresses();
+
+        for (AddressCollection::iterator addr= addrs.begin();
+             addr != addrs.end();
+             ++addr) {
+
+            // skip IPv4 addresses
+            if (addr->getFamily() != AF_INET6) {
+                continue;
+            }
+
+            sock = openSocket(iface->getName(), *addr, port);
+            if (sock<0) {
+                cout << "Failed to open unicast socket." << endl;
+                return (false);
             }
 
-            if ( !joinMcast(sock1, iface->getName(),
+            if ( !joinMcast(sock, iface->getName(),
                              string(ALL_DHCP_RELAY_AGENTS_AND_SERVERS) ) ) {
-                close(sock1);
+                close(sock);
                 isc_throw(Unexpected, "Failed to join " << ALL_DHCP_RELAY_AGENTS_AND_SERVERS
                           << " multicast group.");
             }
 
+            count++;
+#if 0
             // this doesn't work too well on NetBSD
             sock2 = openSocket(iface->getName(),
                                IOAddress(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
-                               port);
-            if (sock2 < 0) {
+                               DHCP6_SERVER_PORT);
+            if (sock2<0) {
                 isc_throw(Unexpected, "Failed to open multicast socket on "
                           << " interface " << iface->getFullName());
                 iface->delSocket(sock1); // delete previously opened socket
             }
+#endif
         }
     }
+    return (count > 0);
 }
 
 void
@@ -268,11 +320,11 @@ IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) {
 
 IfaceMgr::Iface*
 IfaceMgr::getIface(int ifindex) {
-    for (IfaceCollection::iterator iface = ifaces_.begin();
-         iface != ifaces_.end(); ++iface) {
-        if (iface->getIndex() == ifindex) {
+    for (IfaceCollection::iterator iface=ifaces_.begin();
+         iface!=ifaces_.end();
+         ++iface) {
+        if (iface->getIndex() == ifindex)
             return (&(*iface));
-        }
     }
 
     return (NULL); // not found
@@ -280,18 +332,18 @@ IfaceMgr::getIface(int ifindex) {
 
 IfaceMgr::Iface*
 IfaceMgr::getIface(const std::string& ifname) {
-    for (IfaceCollection::iterator iface = ifaces_.begin();
-         iface != ifaces_.end(); ++iface) {
-        if (iface->getName() == ifname) {
+    for (IfaceCollection::iterator iface=ifaces_.begin();
+         iface!=ifaces_.end();
+         ++iface) {
+        if (iface->getName() == ifname)
             return (&(*iface));
-        }
     }
 
     return (NULL); // not found
 }
 
-int
-IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr,
+int IfaceMgr::openSocket(const std::string& ifname,
+                     const IOAddress& addr,
                      int port) {
     Iface* iface = getIface(ifname);
     if (!iface) {
@@ -308,8 +360,7 @@ IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr,
     }
 }
 
-int
-IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
+int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
 
     cout << "Creating UDP4 socket on " << iface.getFullName()
          << " " << addr.toText() << "/port=" << port << endl;
@@ -318,8 +369,10 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     memset(&addr4, 0, sizeof(sockaddr));
     addr4.sin_family = AF_INET;
     addr4.sin_port = htons(port);
-    memcpy(&addr4.sin_addr, addr.getAddress().to_v4().to_bytes().data(),
-           sizeof(addr4.sin_addr));
+
+    addr4.sin_addr.s_addr = htonl(addr);
+    //addr4.sin_addr.s_addr = 0; // anyaddr: this will receive 0.0.0.0 => 255.255.255.255 traffic
+    // addr4.sin_addr.s_addr = 0xffffffffu; // broadcast address. This will receive 0.0.0.0 => 255.255.255.255 as well
 
     int sock = socket(AF_INET, SOCK_DGRAM, 0);
     if (sock < 0) {
@@ -332,8 +385,8 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
                   << "/port=" << port);
     }
 
-    // If there is no support for IP_PKTINFO, we are really out of luck.
-    // It will be difficult to understand, where this packet came from.
+    // if there is no support for IP_PKTINFO, we are really out of luck
+    // it will be difficult to undersand, where this packet came from
 #if defined(IP_PKTINFO)
     int flag = 1;
     if (setsockopt(sock, IPPROTO_IP, IP_PKTINFO, &flag, sizeof(flag)) != 0) {
@@ -345,13 +398,13 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     cout << "Created socket " << sock << " on " << iface.getName() << "/" <<
         addr.toText() << "/port=" << port << endl;
 
-    iface.addSocket(SocketInfo(sock, addr, port));
+    SocketInfo info(sock, addr, port);
+    iface.addSocket(info);
 
     return (sock);
 }
 
-int
-IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
+int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
 
     cout << "Creating UDP6 socket on " << iface.getFullName()
          << " " << addr.toText() << "/port=" << port << endl;
@@ -378,8 +431,8 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
         isc_throw(Unexpected, "Failed to create UDP6 socket.");
     }
 
-    // Set the REUSEADDR option so that we don't fail to start if
-    // we're being restarted.
+    /* Set the REUSEADDR option so that we don't fail to start if
+       we're being restarted. */
     int flag = 1;
     if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
                    (char *)&flag, sizeof(flag)) < 0) {
@@ -393,14 +446,14 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
                   << "/port=" << port);
     }
 #ifdef IPV6_RECVPKTINFO
-    // RFC3542 - a new way
+    /* RFC3542 - a new way */
     if (setsockopt(sock, IPPROTO_IPV6, IPV6_RECVPKTINFO,
                    &flag, sizeof(flag)) != 0) {
         close(sock);
         isc_throw(Unexpected, "setsockopt: IPV6_RECVPKTINFO failed.");
     }
 #else
-    // RFC2292 - an old way
+    /* RFC2292 - an old way */
     if (setsockopt(sock, IPPROTO_IPV6, IPV6_PKTINFO,
                    &flag, sizeof(flag)) != 0) {
         close(sock);
@@ -425,7 +478,8 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
     cout << "Created socket " << sock << " on " << iface.getName() << "/" <<
         addr.toText() << "/port=" << port << endl;
 
-    iface.addSocket(SocketInfo(sock, addr, port));
+    SocketInfo info(sock, addr, port);
+    iface.addSocket(info);
 
     return (sock);
 }
@@ -526,19 +580,232 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
 }
 
 bool
-IfaceMgr::send(boost::shared_ptr<Pkt4>& )
+IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
 {
-    /// TODO: Implement this (ticket #1240)
-    isc_throw(NotImplemented, "Pkt4 send not implemented yet.");
+    struct msghdr m;
+    struct iovec v;
+    int result;
+
+    Iface* iface = getIface(pkt->getIface());
+    if (!iface) {
+        isc_throw(BadValue, "Unable to send Pkt4. Invalid interface ("
+                  << pkt->getIface() << ") specified.");
+    }
+
+    memset(&control_buf_[0], 0, control_buf_len_);
+
+    // Initialize our message header structure.
+    memset(&m, 0, sizeof(m));
+
+    // Set the target address we're sending to.
+    sockaddr_in to;
+    memset(&to, 0, sizeof(to));
+    to.sin_family = AF_INET;
+    to.sin_port = htons(pkt->getRemotePort());
+    to.sin_addr.s_addr = htonl(pkt->getRemoteAddr());
+
+    m.msg_name = &to;
+    m.msg_namelen = sizeof(to);
+
+    // 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_len = pkt->getBuffer().getLength();
+    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.
+    //
+    // We have to create a "control message", and set that to
+    // 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);
+    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));
+    pktinfo->ipi_ifindex = pkt->getIndex();
+    m.msg_controllen = cmsg->cmsg_len;
+#endif
+
+    cout << "Trying to send " << pkt->getBuffer().getLength() << " bytes to "
+         << pkt->getRemoteAddr().toText() << ":" << pkt->getRemotePort()
+         << " over socket " << getSocket(*pkt) << " on interface "
+         << getIface(pkt->getIface())->getFullName() << endl;
+
+    result = sendmsg(getSocket(*pkt), &m, 0);
+    if (result < 0) {
+        isc_throw(Unexpected, "Pkt4 send failed.");
+    }
+
+    cout << "Sent " << pkt->getBuffer().getLength() << " bytes over socket " << getSocket(*pkt)
+         << " on " << iface->getFullName() << " interface: "
+         << " dst=" << pkt->getRemoteAddr().toText() << ":" << pkt->getRemotePort()
+         << ", src=" << pkt->getLocalAddr().toText() << ":" << pkt->getLocalPort()
+         << endl;
+
+    return (result);
 }
 
 
 boost::shared_ptr<Pkt4>
 IfaceMgr::receive4() {
-    isc_throw(NotImplemented, "Pkt4 reception not implemented yet.");
 
-    // TODO: To be implemented (ticket #1239)
-    return (boost::shared_ptr<Pkt4>()); // NULL
+    const SocketInfo* candidate = 0;
+    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()) {
+
+            // We don't want IPv6 addresses here.
+            if (s->addr_.getFamily() != AF_INET) {
+                ++s;
+                continue;
+            }
+
+            // This address looks good.
+            if (!candidate) {
+                candidate = &(*s);
+                break;
+            }
+
+            ++s;
+        }
+
+        if (candidate) {
+            break;
+        }
+    }
+
+    if (!candidate) {
+        isc_throw(Unexpected, "Failed to find any suitable sockets on all interfaces.");
+    }
+
+    cout << "Trying to receive over UDP4 socket " << candidate->sockfd_ << " bound to "
+         << candidate->addr_.toText() << "/port=" << candidate->port_ << " on "
+         << 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;
+    uint8_t* buf = (uint8_t*) malloc(RCVBUFSIZE);
+
+    memset(&control_buf_[0], 0, control_buf_len_);
+    memset(&from_addr, 0, sizeof(from_addr));
+    memset(&to_addr, 0, sizeof(to_addr));
+
+    // Initialize our message header structure.
+    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;
+    v.iov_len = RCVBUFSIZE;
+    m.msg_iov = &v;
+    m.msg_iovlen = 1;
+
+    // Getting the interface is a bit more involved.
+    //
+    // We set up some space for a "control message". We have
+    // previously asked the kernel to give us packet
+    // information (when we initialized the interface), so we
+    // should get the destination address from that.
+    m.msg_control = &control_buf_[0];
+    m.msg_controllen = control_buf_len_;
+
+    result = recvmsg(candidate->sockfd_, &m, 0);
+
+    if (result < 0) {
+        cout << "Failed to receive UDP4 data." << endl;
+        delete buf;
+        return (boost::shared_ptr<Pkt4>()); // NULL
+    }
+
+// OS_LINUX defines are part of ticket #1237
+#if defined(OS_LINUX)
+    struct cmsghdr* cmsg;
+    struct in_pktinfo* pktinfo;
+    unsigned int ifindex = 0;
+
+    int found_pktinfo = 0;
+    cmsg = CMSG_FIRSTHDR(&m);
+    while (cmsg != NULL) {
+        if ((cmsg->cmsg_level == IPPROTO_IP) &&
+            (cmsg->cmsg_type == IP_PKTINFO)) {
+            pktinfo = (struct in_pktinfo*)CMSG_DATA(cmsg);
+
+            ifindex = pktinfo->ipi_ifindex;
+            to_addr = pktinfo->ipi_addr;
+
+            // This field is useful, when we are bound to unicast
+            // address e.g. 192.0.2.1 and the packet was sent to
+            // broadcast. This will return broadcast address, not
+            // the address we are bound to.
+
+            // IOAddress tmp(htonl(pktinfo->ipi_spec_dst.s_addr));
+            // cout << "The other addr is: " << tmp.toText() << endl;
+
+            // Perhaps we should uncomment this:
+            // to_addr = pktinfo->ipi_spec_dst;
+            found_pktinfo = 1;
+        }
+        cmsg = CMSG_NXTHDR(&m, cmsg);
+    }
+    if (!found_pktinfo) {
+        cout << "Unable to find pktinfo" << endl;
+        delete buf;
+        return (boost::shared_ptr<Pkt4>()); // NULL
+    }
+#endif
+
+    IOAddress to(htonl(to_addr.s_addr));
+    IOAddress from(htonl(from_addr.sin_addr.s_addr));
+    uint16_t from_port = htons(from_addr.sin_port);
+
+    cout << "Received " << result << " bytes from " << from.toText()
+         << "/port=" << from_port
+         << " sent to " << to.toText() << " over interface "
+         << iface->getFullName() << endl;
+
+    // we have all data let's create Pkt4 object
+    pkt = boost::shared_ptr<Pkt4>(new Pkt4(buf, result));
+
+    pkt->setIface(iface->getName());
+    pkt->setIndex(iface->getIndex());
+    pkt->setLocalAddr(to);
+    pkt->setRemoteAddr(from);
+    pkt->setRemotePort(from_port);
+    pkt->setLocalPort(candidate->port_);
+
+    return (pkt);
 }
 
 boost::shared_ptr<Pkt6>
@@ -572,27 +839,35 @@ IfaceMgr::receive6() {
     memset(&from, 0, sizeof(from));
     memset(&to_addr, 0, sizeof(to_addr));
 
-    // Initialize our message header structure.
+    /*
+     * Initialize our message header structure.
+     */
     memset(&m, 0, sizeof(m));
 
-    // Point so we can get the from address.
+    /*
+     * Point so we can get the from address.
+     */
     m.msg_name = &from;
     m.msg_namelen = sizeof(from);
 
-    // Set the data buffer we're receiving. (Using this wacky
-    // "scatter-gather" stuff... but we that doesn't really make
-    // sense for us, so we use a single vector entry.)
+    /*
+     * Set the data buffer we're receiving. (Using this wacky
+     * "scatter-gather" stuff... but we that doesn't really make
+     * sense for us, so we use a single vector entry.)
+     */
     v.iov_base = (void*)&pkt->data_[0];
     v.iov_len = pkt->data_len_;
     m.msg_iov = &v;
     m.msg_iovlen = 1;
 
-    // Getting the interface is a bit more involved.
-    //
-    // We set up some space for a "control message". We have
-    // previously asked the kernel to give us packet
-    // information (when we initialized the interface), so we
-    // should get the destination address from that.
+    /*
+     * Getting the interface is a bit more involved.
+     *
+     * We set up some space for a "control message". We have
+     * previously asked the kernel to give us packet
+     * information (when we initialized the interface), so we
+     * should get the destination address from that.
+     */
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
 
@@ -606,12 +881,16 @@ IfaceMgr::receive6() {
     SocketCollection::const_iterator s = iface->sockets_.begin();
     const SocketInfo* candidate = 0;
     while (s != iface->sockets_.end()) {
+        if (s->addr_.getFamily() != AF_INET6) {
+            ++s;
+            continue;
+        }
         if (s->addr_.getAddress().to_v6().is_multicast()) {
             candidate = &(*s);
             break;
         }
         if (!candidate) {
-            candidate = &(*s); // it's not multicast, but it's better than none
+            candidate = &(*s); // it's not multicast, but it's better than nothing
         }
         ++s;
     }
@@ -620,18 +899,20 @@ IfaceMgr::receive6() {
                   << " does not have any sockets open.");
     }
 
-    cout << "Trying to receive over socket " << candidate->sockfd_ << " bound to "
+    cout << "Trying to receive over UDP6 socket " << candidate->sockfd_ << " bound to "
          << candidate->addr_.toText() << "/port=" << candidate->port_ << " on "
          << iface->getFullName() << endl;
     result = recvmsg(candidate->sockfd_, &m, 0);
 
     if (result >= 0) {
-        // If we did read successfully, then we need to loop
-        // through the control messages we received and
-        // find the one with our destination address.
-        //
-        // We also keep a flag to see if we found it. If we
-        // didn't, then we consider this to be an error.
+        /*
+         * If we did read successfully, then we need to loop
+         * through the control messages we received and
+         * find the one with our destination address.
+         *
+         * We also keep a flag to see if we found it. If we
+         * didn't, then we consider this to be an error.
+         */
         int found_pktinfo = 0;
         cmsg = CMSG_FIRSTHDR(&m);
         while (cmsg != NULL) {

src/lib/dhcp/iface_mgr.h

@@ -20,6 +20,8 @@
 #include <boost/scoped_array.hpp>
 #include <boost/noncopyable.hpp>
 #include <asiolink/io_address.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/dhcp4.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/pkt6.h>
 
@@ -325,13 +327,10 @@ public:
     int openSocket(const std::string& ifname,
                    const isc::asiolink::IOAddress& addr, int port);
 
-    /// Opens IPv4 sockets on detected interfaces.
-    ///
-    /// Will throw exception if socket creation fails.
+    /// @param port specifies port number (usually DHCP6_SERVER_PORT)
     ///
-    /// @param port specifies port number (usually DHCP4_SERVER_PORT)
-    void openSockets4(uint16_t port);
-
+    /// @return true if any sockets were open
+    bool openSockets6(uint16_t port = DHCP6_SERVER_PORT);
 
     /// @brief Closes all open sockets.
     /// Is used in destructor, but also from Dhcpv4_srv and Dhcpv6_srv classes.
@@ -342,6 +341,14 @@ public:
     /// @return number of detected interfaces
     uint16_t countIfaces() { return ifaces_.size(); }
 
+    /// Opens IPv4 sockets on detected interfaces.
+    /// Will throw exception if socket creation fails.
+    ///
+    /// @param port specifies port number (usually DHCP4_SERVER_PORT)
+    ///
+    /// @return true if any sockets were open
+    bool openSockets4(uint16_t port = DHCP4_SERVER_PORT);
+
     // don't use private, we need derived classes in tests
 protected:
 
@@ -351,7 +358,7 @@ protected:
     /// anyone to create instances of IfaceMgr. Use instance() method instead.
     IfaceMgr();
 
-    ~IfaceMgr();
+    virtual ~IfaceMgr();
 
     /// @brief Opens IPv4 socket.
     ///
@@ -429,10 +436,6 @@ protected:
     boost::scoped_array<char> control_buf_;
 
 private:
-    /// Opens IPv6 sockets on detected interfaces.
-    ///
-    /// @param port specifies port on which sockets will be open
-    void openSockets6(uint16_t port);
 
     /// creates a single instance of this class (a singleton implementation)
     static void

src/lib/dhcp/libdhcp++.cc

@@ -93,13 +93,13 @@ LibDHCP::unpackOptions4(const std::vector<uint8_t>& buf,
     // 2 - header of DHCPv4 option
     while (offset + 1 <= buf.size()) {
         uint8_t opt_type = buf[offset++];
-        if (offset + 1 == buf.size()) {
-            if (opt_type == DHO_END)
-                return; // just return. Don't need to add DHO_END option
-            else {
-                isc_throw(OutOfRange, "Attempt to parse truncated option "
-                          << opt_type);
-            }
+
+        if (opt_type == DHO_END)
+          return; // just return. Don't need to add DHO_END option
+
+        if (offset + 1 >= buf.size()) {
+          isc_throw(OutOfRange, "Attempt to parse truncated option "
+                    << opt_type);
         }
 
         uint8_t opt_len =  buf[offset++];

src/lib/dhcp/option.cc

@@ -301,6 +301,31 @@ Option::addOption(boost::shared_ptr<Option> opt) {
     options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
 }
 
+uint8_t Option::getUint8() {
+    if (data_.size() < sizeof(uint8_t) ) {
+        isc_throw(OutOfRange, "Attempt to read uint8 from option " << type_
+                  << " that has size " << data_.size());
+    }
+    return (data_[0]);
+}
+
+uint16_t Option::getUint16() {
+    if (data_.size() < sizeof(uint16_t) ) {
+        isc_throw(OutOfRange, "Attempt to read uint16 from option " << type_
+                  << " that has size " << data_.size());
+    }
+
+    return ( readUint16(&data_[0]) );
+}
+
+uint32_t Option::getUint32() {
+    if (data_.size() < sizeof(uint32_t) ) {
+        isc_throw(OutOfRange, "Attempt to read uint32 from option " << type_
+                  << " that has size " << data_.size());
+    }
+    return ( readUint32(&data_[0]) );
+}
+
 Option::~Option() {
 
 }

src/lib/dhcp/option.h

@@ -236,9 +236,29 @@ public:
     bool
     delOption(unsigned short type);
 
+    /// @brief Returns content of first byte.
+    ///
+    /// @exception OutOfRange Thrown if the option has a length of 0.
+    ///
+    /// @return value of the first byte
+    uint8_t getUint8();
+
+    /// @brief Returns content of first word.
+    ///
+    /// @exception OutOfRange Thrown if the option has a length less than 2.
+    ///
+    /// @return uint16_t value stored on first two bytes
+    uint16_t getUint16();
+
+    /// @brief Returns content of first double word.
+    ///
+    /// @exception OutOfRange Thrown if the option has a length less than 4.
+    ///
+    /// @return uint32_t value stored on first four bytes
+    uint32_t getUint32();
+
     /// just to force that every option has virtual dtor
-    virtual
-    ~Option();
+    virtual ~Option();
 
 protected:
     /// Builds raw (over-wire) buffer of this option, including all

src/lib/dhcp/pkt4.cc

@@ -75,8 +75,8 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
 {
     if (len < DHCPV4_PKT_HDR_LEN) {
         isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
-                  << " received, at least " << DHCPV4_PKT_HDR_LEN
-                  << "is expected");
+                  << ") received, at least " << DHCPV4_PKT_HDR_LEN
+                  << " is expected.");
     }
 
     data_.resize(len);
@@ -114,6 +114,9 @@ Pkt4::pack() {
     bufferOut_.writeData(sname_, MAX_SNAME_LEN);
     bufferOut_.writeData(file_, MAX_FILE_LEN);
 
+    // write DHCP magic cookie
+    bufferOut_.writeUint32(DHCP_OPTIONS_COOKIE);
+
     LibDHCP::packOptions(bufferOut_, options_);
 
     // add END option that indicates end of options
@@ -128,7 +131,7 @@ Pkt4::unpack() {
     // input buffer (used during message reception)
     isc::util::InputBuffer bufferIn(&data_[0], data_.size());
 
-    if (bufferIn.getLength()<DHCPV4_PKT_HDR_LEN) {
+    if (bufferIn.getLength() < DHCPV4_PKT_HDR_LEN) {
         isc_throw(OutOfRange, "Received truncated DHCPv4 packet (len="
                   << bufferIn.getLength() << " received, at least "
                   << DHCPV4_PKT_HDR_LEN << "is expected");
@@ -149,8 +152,26 @@ Pkt4::unpack() {
     bufferIn.readData(sname_, MAX_SNAME_LEN);
     bufferIn.readData(file_, MAX_FILE_LEN);
 
+    if (bufferIn.getLength() == bufferIn.getPosition()) {
+        // this is *NOT* DHCP packet. It does not have any DHCPv4 options. In
+        // particular, it does not have magic cookie, a 4 byte sequence that
+        // differentiates between DHCP and BOOTP packets.
+        return (true);
+    }
+
+    if (bufferIn.getLength() - bufferIn.getPosition() < 4) {
+      // there is not enough data to hold magic DHCP cookie
+      isc_throw(Unexpected, "Truncated or no DHCP packet.");
+    }
+
+    uint32_t magic = bufferIn.readUint32();
+    if (magic != DHCP_OPTIONS_COOKIE) {
+      isc_throw(Unexpected, "Invalid or missing DHCP magic cookie");
+    }
+
     size_t opts_len = bufferIn.getLength() - bufferIn.getPosition();
     vector<uint8_t> optsBuffer;
+
     // fist use of readVector
     bufferIn.readVector(optsBuffer, opts_len);
     LibDHCP::unpackOptions4(optsBuffer, options_);
@@ -158,15 +179,40 @@ Pkt4::unpack() {
     return (true);
 }
 
+void Pkt4::check() {
+    boost::shared_ptr<Option> typeOpt = getOption(DHO_DHCP_MESSAGE_TYPE);
+    if (typeOpt) {
+        uint8_t msg_type = typeOpt->getUint8();
+        if (msg_type>DHCPLEASEACTIVE) {
+            isc_throw(BadValue, "Invalid DHCP message type received:" << msg_type);
+        }
+        msg_type_ = msg_type;
+
+    } else {
+        isc_throw(Unexpected, "Missing DHCP Message Type option");
+    }
+}
+
+void Pkt4::repack() {
+    cout << "Convering RX packet to TX packet: " << data_.size() << " bytes." << endl;
+
+    bufferOut_.writeData(&data_[0], data_.size());
+}
+
 std::string
 Pkt4::toText() {
     stringstream tmp;
-    tmp << "localAddr=[" << local_addr_.toText() << "]:" << local_port_
-        << " remoteAddr=[" << remote_addr_.toText()
-        << "]:" << remote_port_ << endl;
-    tmp << "msgtype=" << msg_type_
-        << ", transid=0x" << hex << transid_ << dec
-        << endl;
+    tmp << "localAddr=" << local_addr_.toText() << ":" << local_port_
+        << " remoteAddr=" << remote_addr_.toText()
+        << ":" << remote_port_ << ", msgtype=" << int(msg_type_)
+        << ", transid=0x" << hex << transid_ << dec << endl;
+
+    for (isc::dhcp::Option::OptionCollection::iterator opt=options_.begin();
+         opt != options_.end();
+         ++opt) {
+        tmp << "  " << opt->second->toText() << std::endl;
+    }
+
 
     return tmp.str();
 }
@@ -256,7 +302,6 @@ Pkt4::getOption(uint8_t type) {
     return boost::shared_ptr<isc::dhcp::Option>(); // NULL
 }
 
-
 } // end of namespace isc::dhcp
 
 } // end of namespace isc

src/lib/dhcp/pkt4.h

@@ -78,6 +78,25 @@ public:
     bool
     unpack();
 
+    /// @brief performs sanity check on a packet.
+    ///
+    /// This is usually performed after unpack(). It checks if packet is sane:
+    /// required options are present, fields have sane content etc.
+    /// For example verifies that DHCP_MESSAGE_TYPE is present and have
+    /// reasonable value. This method is expected to grow significantly.
+    /// It makes sense to separate unpack() and check() for testing purposes.
+    ///
+    /// Method will throw exception if anomaly is found.
+    void check();
+
+    /// @brief Copies content of input buffer to output buffer.
+    ///
+    /// This is mostly a diagnostic function. It is being used for sending
+    /// received packet. Received packet is stored in bufferIn_, but
+    /// transmitted data is stored in bufferOut_. If we want to send packet
+    /// that we just received, a copy between those two buffers is necessary.
+    void repack();
+
     /// @brief Returns text representation of the packet.
     ///
     /// This function is useful mainly for debugging.

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -56,6 +56,10 @@ public:
         fakeifaces << LOOPBACK << " ::1";
         fakeifaces.close();
     }
+
+    ~IfaceMgrTest() {
+            unlink(INTERFACE_FILE);
+    }
 };
 
 // We need some known interface to work reliably. Loopback interface
@@ -148,6 +152,8 @@ TEST_F(IfaceMgrTest, dhcp6Sniffer) {
 TEST_F(IfaceMgrTest, basic) {
     // checks that IfaceManager can be instantiated
 
+    createLoInterfacesTxt();
+
     IfaceMgr & ifacemgr = IfaceMgr::instance();
     ASSERT_TRUE(&ifacemgr != 0);
 }
@@ -160,13 +166,14 @@ TEST_F(IfaceMgrTest, ifaceClass) {
     EXPECT_STREQ("eth5/7", iface->getFullName().c_str());
 
     delete iface;
-
 }
 
 // TODO: Implement getPlainMac() test as soon as interface detection
 // is implemented.
 TEST_F(IfaceMgrTest, getIface) {
 
+    createLoInterfacesTxt();
+
     cout << "Interface checks. Please ignore socket binding errors." << endl;
     NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
 
@@ -210,6 +217,7 @@ TEST_F(IfaceMgrTest, getIface) {
     EXPECT_EQ(static_cast<void*>(NULL), ifacemgr->getIface("wifi0") );
 
     delete ifacemgr;
+
 }
 
 #if !defined(OS_LINUX)
@@ -365,6 +373,90 @@ TEST_F(IfaceMgrTest, sendReceive6) {
     unlink(INTERFACE_FILE);
 }
 
+TEST_F(IfaceMgrTest, sendReceive4) {
+
+    // testing socket operation in a portable way is tricky
+    // without interface detection implemented
+    createLoInterfacesTxt();
+
+    NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
+
+    // let's assume that every supported OS have lo interface
+    IOAddress loAddr("127.0.0.1");
+    int socket1 = 0, socket2 = 0;
+    EXPECT_NO_THROW(
+        socket1 = ifacemgr->openSocket(LOOPBACK, loAddr, DHCP4_SERVER_PORT + 10000);
+        socket2 = ifacemgr->openSocket(LOOPBACK, loAddr, DHCP4_SERVER_PORT + 10000 + 1);
+    );
+
+    boost::shared_ptr<Pkt4> sendPkt(new Pkt4(DHCPDISCOVER, 1234) );
+
+    sendPkt->setLocalAddr(IOAddress("127.0.0.1"));
+
+    sendPkt->setLocalPort(DHCP4_SERVER_PORT + 10000 + 1);
+    sendPkt->setRemotePort(DHCP4_SERVER_PORT + 10000);
+    sendPkt->setRemoteAddr(IOAddress("127.0.0.1"));
+    sendPkt->setIndex(1);
+    sendPkt->setIface(string(LOOPBACK));
+    sendPkt->setHops(6);
+    sendPkt->setSecs(42);
+    sendPkt->setCiaddr(IOAddress("192.0.2.1"));
+    sendPkt->setSiaddr(IOAddress("192.0.2.2"));
+    sendPkt->setYiaddr(IOAddress("192.0.2.3"));
+    sendPkt->setGiaddr(IOAddress("192.0.2.4"));
+
+    uint8_t sname[] = "That's just a string that will act as SNAME";
+    sendPkt->setSname(sname, strlen((const char*)sname));
+    uint8_t file[] = "/another/string/that/acts/as/a/file_name.txt";
+    sendPkt->setFile(file, strlen((const char*)file));
+
+    ASSERT_NO_THROW(
+        sendPkt->pack();
+    );
+
+    boost::shared_ptr<Pkt4> rcvPkt;
+
+    EXPECT_EQ(true, ifacemgr->send(sendPkt));
+
+    rcvPkt = ifacemgr->receive4();
+
+    ASSERT_TRUE( rcvPkt ); // received our own packet
+
+    ASSERT_NO_THROW(
+        rcvPkt->unpack();
+    );
+
+    // let's check that we received what was sent
+    EXPECT_EQ(sendPkt->len(), rcvPkt->len());
+
+    EXPECT_EQ("127.0.0.1", rcvPkt->getRemoteAddr().toText());
+    EXPECT_EQ(sendPkt->getRemotePort(), rcvPkt->getLocalPort());
+
+    // now let's check content
+    EXPECT_EQ(sendPkt->getHops(), rcvPkt->getHops());
+    EXPECT_EQ(sendPkt->getOp(),   rcvPkt->getOp());
+    EXPECT_EQ(sendPkt->getSecs(), rcvPkt->getSecs());
+    EXPECT_EQ(sendPkt->getFlags(), rcvPkt->getFlags());
+    EXPECT_EQ(sendPkt->getCiaddr(), rcvPkt->getCiaddr());
+    EXPECT_EQ(sendPkt->getSiaddr(), rcvPkt->getSiaddr());
+    EXPECT_EQ(sendPkt->getYiaddr(), rcvPkt->getYiaddr());
+    EXPECT_EQ(sendPkt->getGiaddr(), rcvPkt->getGiaddr());
+    EXPECT_EQ(sendPkt->getTransid(), rcvPkt->getTransid());
+    EXPECT_EQ(sendPkt->getType(), rcvPkt->getType());
+    EXPECT_TRUE(sendPkt->getSname() == rcvPkt->getSname());
+    EXPECT_TRUE(sendPkt->getFile() == rcvPkt->getFile());
+    EXPECT_EQ(sendPkt->getHtype(), rcvPkt->getHtype());
+    EXPECT_EQ(sendPkt->getHlen(), rcvPkt->getHlen());
+
+    // since we opened 2 sockets on the same interface and none of them is multicast,
+    // none is preferred over the other for sending data, so we really should not
+    // assume the one or the other will always be choosen for sending data. We should
+    // skip checking source port of sent address.
+
+    delete ifacemgr;
+}
+
+
 TEST_F(IfaceMgrTest, socket4) {
 
     createLoInterfacesTxt();

src/lib/dhcp/tests/option_unittest.cc

@@ -419,3 +419,41 @@ TEST_F(OptionTest, v6_toText) {
 
     EXPECT_EQ("type=258, len=3: 00:0f:ff", opt->toText());
 }
+
+TEST_F(OptionTest, getUintX) {
+    boost::shared_array<uint8_t> buf(new uint8_t[5]);
+    buf[0] = 0x5;
+    buf[1] = 0x4;
+    buf[2] = 0x3;
+    buf[3] = 0x2;
+    buf[4] = 0x1;
+
+    // five options with varying lengths
+    boost::shared_ptr<Option> opt1(new Option(Option::V6, 258, buf, 0, 1));
+    boost::shared_ptr<Option> opt2(new Option(Option::V6, 258, buf, 0, 2));
+    boost::shared_ptr<Option> opt3(new Option(Option::V6, 258, buf, 0, 3));
+    boost::shared_ptr<Option> opt4(new Option(Option::V6, 258, buf, 0, 4));
+    boost::shared_ptr<Option> opt5(new Option(Option::V6, 258, buf, 0, 5));
+
+    EXPECT_EQ(5, opt1->getUint8());
+    EXPECT_THROW(opt1->getUint16(), OutOfRange);
+    EXPECT_THROW(opt1->getUint32(), OutOfRange);
+
+    EXPECT_EQ(5, opt2->getUint8());
+    EXPECT_EQ(0x0504, opt2->getUint16());
+    EXPECT_THROW(opt2->getUint32(), OutOfRange);
+
+    EXPECT_EQ(5, opt3->getUint8());
+    EXPECT_EQ(0x0504, opt3->getUint16());
+    EXPECT_THROW(opt3->getUint32(), OutOfRange);
+
+    EXPECT_EQ(5, opt4->getUint8());
+    EXPECT_EQ(0x0504, opt4->getUint16());
+    EXPECT_EQ(0x05040302, opt4->getUint32());
+
+    // the same as for 4-byte long, just get first 1,2 or 4 bytes
+    EXPECT_EQ(5, opt5->getUint8());
+    EXPECT_EQ(0x0504, opt5->getUint16());
+    EXPECT_EQ(0x05040302, opt5->getUint32());
+
+}

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -486,14 +486,14 @@ TEST(Pkt4Test, options) {
     );
 
     const OutputBuffer& buf = pkt->getBuffer();
-    // check that all options are stored, they should take sizeof(v4Opts)
-    // there also should be OPTION_END added (just one byte)
-    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + sizeof(v4Opts) + 1,
-              buf.getLength());
+    // check that all options are stored, they should take sizeof(v4Opts),
+    // DHCP magic cookie (4 bytes), and OPTION_END added (just one byte)
+    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + sizeof(DHCP_OPTIONS_COOKIE)
+              + sizeof(v4Opts) + 1, buf.getLength());
 
     // that that this extra data actually contain our options
     const uint8_t* ptr = static_cast<const uint8_t*>(buf.getData());
-    ptr += Pkt4::DHCPV4_PKT_HDR_LEN; // rewind to end of fixed part
+    ptr += Pkt4::DHCPV4_PKT_HDR_LEN + sizeof(DHCP_OPTIONS_COOKIE); // rewind to end of fixed part
     EXPECT_EQ(0, memcmp(ptr, v4Opts, sizeof(v4Opts)));
     EXPECT_EQ(DHO_END, static_cast<uint8_t>(*(ptr + sizeof(v4Opts))));
 
@@ -506,6 +506,11 @@ TEST(Pkt4Test, unpackOptions) {
 
     vector<uint8_t> expectedFormat = generateTestPacket2();
 
+    expectedFormat.push_back(0x63);
+    expectedFormat.push_back(0x82);
+    expectedFormat.push_back(0x53);
+    expectedFormat.push_back(0x63);
+
     for (int i = 0; i < sizeof(v4Opts); i++) {
         expectedFormat.push_back(v4Opts[i]);
     }
@@ -575,7 +580,6 @@ TEST(Pkt4Test, metaFields) {
     EXPECT_EQ(42, pkt->getIndex());
     EXPECT_EQ("1.2.3.4", pkt->getRemoteAddr().toText());
     EXPECT_EQ("4.3.2.1", pkt->getLocalAddr().toText());
-
 }
 
 } // end of anonymous namespace