[1230] Implemented tests for DISCOVER and OFFER packet handling

- added tests for processDiscover(), processOffer() in dhcpv4_srv
- added test for setUintX in OptionTest
- updates Pkt4Test (now unpack checks if mandatory Message Type option is present)

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -23,10 +23,12 @@
 #include <dhcp/dhcp4.h>
 #include <dhcp4/dhcp4_srv.h>
 #include <dhcp/option.h>
+#include <asiolink/io_address.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::asiolink;
 
 namespace {
 const char* const INTERFACE_FILE = "interfaces.txt";
@@ -66,6 +68,30 @@ public:
         fakeifaces.close();
     }
 
+    void MessageCheck(const boost::shared_ptr<Pkt4>& q,
+                      const boost::shared_ptr<Pkt4>& a) {
+        ASSERT_TRUE(q);
+        ASSERT_TRUE(a);
+
+        EXPECT_EQ(q->getHops(),   a->getHops());
+        EXPECT_EQ(q->getIface(),  a->getIface());
+        EXPECT_EQ(q->getIndex(),  a->getIndex());
+        EXPECT_EQ(q->getGiaddr(), a->getGiaddr());
+
+        // check that bare minimum of required options are there
+        EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
+        EXPECT_TRUE(a->getOption(DHO_ROUTERS));
+        EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
+        EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
+        EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
+        EXPECT_TRUE(a->getOption(DHO_ROUTERS));
+        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
+        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
+
+        // check that something is offered
+        EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
+    }
+
     ~Dhcpv4SrvTest() {
         unlink(INTERFACE_FILE);
     };
@@ -85,37 +111,115 @@ TEST_F(Dhcpv4SrvTest, basic) {
 
 TEST_F(Dhcpv4SrvTest, processDiscover) {
     NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+    vector<uint8_t> mac;
+    mac.resize(6);
+    for (int i = 0; i < 6; i++) {
+        mac[i] = 255 - i;
+    }
 
     boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPDISCOVER, 1234));
+    boost::shared_ptr<Pkt4> offer;
+
+    pkt->setIface("eth0");
+    pkt->setIndex(17);
+    pkt->setHWAddr(1, 6, mac);
+    pkt->setRemoteAddr(IOAddress("192.0.2.56"));
+    pkt->setGiaddr(IOAddress("192.0.2.67"));
+
+    // let's make it a relayed message
+    pkt->setHops(3);
+    pkt->setRemotePort(DHCP4_SERVER_PORT);
 
     // should not throw
     EXPECT_NO_THROW(
-        srv->processDiscover(pkt);
+        offer = srv->processDiscover(pkt);
     );
 
     // should return something
-    EXPECT_TRUE(srv->processDiscover(pkt));
+    ASSERT_TRUE(offer);
+
+    EXPECT_EQ(DHCPOFFER, offer->getType());
+
+    // this is relayed message. It should be sent back to relay address.
+    EXPECT_EQ(pkt->getGiaddr(), offer->getRemoteAddr());
+
+    MessageCheck(pkt, offer);
+
+    // now repeat the test for directly sent message
+    pkt->setHops(0);
+    pkt->setGiaddr(IOAddress("0.0.0.0"));
+    pkt->setRemotePort(DHCP4_CLIENT_PORT);
+
+    EXPECT_NO_THROW(
+        offer = srv->processDiscover(pkt);
+    );
+
+    // should return something
+    ASSERT_TRUE(offer);
+
+    EXPECT_EQ(DHCPOFFER, offer->getType());
+
+    // this is direct message. It should be sent back to origin, not
+    // to relay.
+    EXPECT_EQ(pkt->getRemoteAddr(), offer->getRemoteAddr());
+
+    MessageCheck(pkt, offer);
 
-    // TODO: Implement more reasonable tests before starting
-    // work on processSomething() method.
     delete srv;
 }
 
 TEST_F(Dhcpv4SrvTest, processRequest) {
     NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+    vector<uint8_t> mac;
+    mac.resize(6);
+    for (int i = 0; i < 6; i++) {
+        mac[i] = i*10;
+    }
 
-    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPREQUEST, 1234));
+    boost::shared_ptr<Pkt4> req(new Pkt4(DHCPREQUEST, 1234));
+    boost::shared_ptr<Pkt4> ack;
+
+    req->setIface("eth0");
+    req->setIndex(17);
+    req->setHWAddr(1, 6, mac);
+    req->setRemoteAddr(IOAddress("192.0.2.56"));
+    req->setGiaddr(IOAddress("192.0.2.67"));
 
     // should not throw
+    ASSERT_NO_THROW(
+        ack = srv->processRequest(req);
+    );
+
+    // should return something
+    ASSERT_TRUE(ack);
+
+    EXPECT_EQ(DHCPACK, ack->getType());
+
+    // this is relayed message. It should be sent back to relay address.
+    EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
+
+    MessageCheck(req, ack);
+
+    // now repeat the test for directly sent message
+    req->setHops(0);
+    req->setGiaddr(IOAddress("0.0.0.0"));
+    req->setRemotePort(DHCP4_CLIENT_PORT);
+
     EXPECT_NO_THROW(
-        srv->processRequest(pkt);
+        ack = srv->processDiscover(req);
     );
 
     // should return something
-    EXPECT_TRUE(srv->processRequest(pkt));
+    ASSERT_TRUE(ack);
+
+    EXPECT_EQ(DHCPOFFER, ack->getType());
+
+    // this is direct message. It should be sent back to origin, not
+    // to relay.
+    EXPECT_EQ(ack->getRemoteAddr(), req->getRemoteAddr());
+
+    MessageCheck(req, ack);
 
-    // TODO: Implement more reasonable tests before starting
-    // work on processSomething() method.
     delete srv;
 }
 

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -405,6 +405,12 @@ TEST_F(IfaceMgrTest, sendReceive4) {
     sendPkt->setYiaddr(IOAddress("192.0.2.3"));
     sendPkt->setGiaddr(IOAddress("192.0.2.4"));
 
+    // unpack() now checks if mandatory DHCP_MESSAGE_TYPE is present
+    boost::shared_ptr<Option> msgType(new Option(Option::V4,
+           static_cast<uint16_t>(DHO_DHCP_MESSAGE_TYPE)));
+    msgType->setUint8(static_cast<uint8_t>(DHCPDISCOVER));
+    sendPkt->addOption(msgType);
+
     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";
@@ -762,7 +768,7 @@ void parse_ifconfig(const std::string textFile, IfaceMgr::IfaceCollection& iface
 // (check that each interface is reported, i.e. no missing or extra interfaces) and
 // address completeness is verified.
 //
-// Things that are not tested: 
+// Things that are not tested:
 // - ifindex (ifconfig does not print it out)
 // - address scopes and lifetimes (we don't need it, so it is not implemented in IfaceMgr)
 TEST_F(IfaceMgrTest, detectIfaces_linux) {

src/lib/dhcp/tests/option_unittest.cc

@@ -33,8 +33,14 @@ using namespace isc::util;
 namespace {
 class OptionTest : public ::testing::Test {
 public:
-    OptionTest() {
+    OptionTest(): outBuffer_(255) {
+        buf_ = boost::shared_array<uint8_t>(new uint8_t[255]);
+        for (int i = 0; i < 255; i++) {
+            buf_[i] = 255 - i;
+        }
     }
+    boost::shared_array<uint8_t> buf_;
+    OutputBuffer outBuffer_;
 };
 
 // v4 is not really implemented yet. A simple test will do for now
@@ -421,6 +427,8 @@ TEST_F(OptionTest, v6_toText) {
 }
 
 TEST_F(OptionTest, getUintX) {
+
+    // TODO: Update this test to use buf_ instead of buf
     boost::shared_array<uint8_t> buf(new uint8_t[5]);
     buf[0] = 0x5;
     buf[1] = 0x4;
@@ -457,3 +465,38 @@ TEST_F(OptionTest, getUintX) {
     EXPECT_EQ(0x05040302, opt5->getUint32());
 
 }
+
+TEST_F(OptionTest, setUintX) {
+    boost::shared_ptr<Option> opt1(new Option(Option::V4, 125));
+    boost::shared_ptr<Option> opt2(new Option(Option::V4, 125));
+    boost::shared_ptr<Option> opt4(new Option(Option::V4, 125));
+
+    // verify setUint8
+    opt1->setUint8(255);
+    EXPECT_EQ(255, opt1->getUint8());
+    opt1->pack4(outBuffer_);
+    EXPECT_EQ(3, opt1->len());
+    EXPECT_EQ(3, outBuffer_.getLength());
+    uint8_t exp1[] = {125, 1, 255};
+    EXPECT_TRUE(0 == memcmp(exp1, outBuffer_.getData(), 3));
+
+    // verify getUint16
+    outBuffer_.clear();
+    opt2->setUint16(12345);
+    opt2->pack4(outBuffer_);
+    EXPECT_EQ(12345, opt2->getUint16());
+    EXPECT_EQ(4, opt2->len());
+    EXPECT_EQ(4, outBuffer_.getLength());
+    uint8_t exp2[] = {125, 2, 12345/256, 12345%256};
+    EXPECT_TRUE(0 == memcmp(exp2, outBuffer_.getData(), 4));
+
+    // verity getUint32
+    outBuffer_.clear();
+    opt4->setUint32(0x12345678);
+    opt4->pack4(outBuffer_);
+    EXPECT_EQ(0x12345678, opt4->getUint32());
+    EXPECT_EQ(6, opt4->len());
+    EXPECT_EQ(6, outBuffer_.getLength());
+    uint8_t exp4[] = {125, 4, 0x12, 0x34, 0x56, 0x78};
+    EXPECT_TRUE(0 == memcmp(exp4, outBuffer_.getData(), 6));
+}

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -441,8 +441,9 @@ static uint8_t v4Opts[] = {
     12,  3, 0,   1,  2,
     13,  3, 10, 11, 12,
     14,  3, 20, 21, 22,
+    53, 1, 1, // DHCP_MESSAGE_TYPE (required to not throw exception during unpack)
     128, 3, 30, 31, 32,
-    254, 3, 40, 41, 42
+    254, 3, 40, 41, 42,
 };
 
 TEST(Pkt4Test, options) {
@@ -458,14 +459,17 @@ TEST(Pkt4Test, options) {
     boost::shared_ptr<Option> opt1(new Option(Option::V4, 12, payload[0]));
     boost::shared_ptr<Option> opt2(new Option(Option::V4, 13, payload[1]));
     boost::shared_ptr<Option> opt3(new Option(Option::V4, 14, payload[2]));
+    boost::shared_ptr<Option> optMsgType(new Option(Option::V4, DHO_DHCP_MESSAGE_TYPE));
     boost::shared_ptr<Option> opt5(new Option(Option::V4,128, payload[3]));
     boost::shared_ptr<Option> opt4(new Option(Option::V4,254, payload[4]));
+    optMsgType->setUint8(static_cast<uint8_t>(DHCPDISCOVER));
 
     pkt->addOption(opt1);
     pkt->addOption(opt2);
     pkt->addOption(opt3);
     pkt->addOption(opt4);
     pkt->addOption(opt5);
+    pkt->addOption(optMsgType);
 
     EXPECT_TRUE(pkt->getOption(12));
     EXPECT_TRUE(pkt->getOption(13));
@@ -556,14 +560,14 @@ TEST(Pkt4Test, unpackOptions) {
     EXPECT_EQ(128, x->getType());  // this should be option 254
     ASSERT_EQ(3, x->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+17, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+20, 3)); // data len=3
 
     x = pkt->getOption(254);
     ASSERT_TRUE(x); // option 3 should exist
     EXPECT_EQ(254, x->getType());  // this should be option 254
     ASSERT_EQ(3, x->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+22, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+25, 3)); // data len=3
 }
 
 // This test verifies methods that are used for manipulating meta fields