BIND 10 trac2326, updated. 72beaf8964363734d4f5e7b4b11ae7b8b4b5557c [2326] Changes after review - tests use automatic variables now for Dhcpv6Srv - comments clarified - extra log messages added

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Dec 18 18:22:17 UTC 2012


The branch, trac2326 has been updated
       via  72beaf8964363734d4f5e7b4b11ae7b8b4b5557c (commit)
      from  34cb2bf92262addf0fb401365c23773dee3f3e89 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 72beaf8964363734d4f5e7b4b11ae7b8b4b5557c
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 18 19:22:07 2012 +0100

    [2326] Changes after review
    - tests use automatic variables now for Dhcpv6Srv
    - comments clarified
    - extra log messages added

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp6/dhcp6_messages.mes          |   14 ++-
 src/bin/dhcp6/dhcp6_srv.cc                |   17 ++-
 src/bin/dhcp6/dhcp6_srv.h                 |    8 +-
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc |  170 ++++++++++++++---------------
 4 files changed, 106 insertions(+), 103 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index ba86800..da889ba 100644
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/src/bin/dhcp6/dhcp6_messages.mes
@@ -65,7 +65,7 @@ This informational message is printed every time DHCPv6 is started.
 It indicates what database backend type is being to store lease and
 other information.
 
-% DHCP6_DB_ERROR_LEASE6_WITHOUT_DUID lease for address %1 does not have a DUID
+% DHCP6_LEASE_WITHOUT_DUID lease for address %1 does not have a DUID
 This error message indicates a database consistency failure. The lease
 database has an entry indicating that the given address is in use,
 but the lease does not contain any client identification. This is most
@@ -99,9 +99,11 @@ is a normal operation during client shutdown.
 
 % DHCP6_RELEASE_FAIL failed to remove lease for address %1 for duid=%2, iaid=%3
 This error message indicates that the software failed to remove a
-lease from the lease database.  It indicates a database operation error:
+lease from the lease database.  It indicates a likely database operation error:
 resolution will most likely require administrator intervention (e.g. check
-if DHCP process has sufficient privileges to update the database).
+if DHCP process has sufficient privileges to update the database). It may
+also be triggered in a lease was manually removed from the database during
+RELEASE message processing.
 
 % DHCP6_RELEASE_FAIL_WRONG_DUID client (duid=%2) tried to release address %2, but it belongs to client (duid=%3)
 This warning message indicates that client tried to release an address
@@ -116,6 +118,12 @@ that does belong to it, but the address was expected to be in a different
 IA (identity association) container. This probably means that the client's
 support for multiple addresses is flawed.
 
+% DHCP6_RELEASE_MISSING_CLIENTID client (address=%1) sent RELEASE message without mandatory client-id
+This warning message indicates that client sent RELEASE message without
+mandatory client-id option. This is most likely caused by a buggy client
+(or a relay that malformed forwarded message). This request will not be
+processed and a response with error status code will be sent back.
+
 % DHCP6_REQUIRED_OPTIONS_CHECK_FAIL %1 message received from %2 failed the following check: %3
 This message indicates that received DHCPv6 packet is invalid.  This may be due
 to a number of reasons, e.g. the mandatory client-id option is missing,
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index ccee08f..2d9534c 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -713,6 +713,10 @@ void Dhcpv6Srv::releaseLeases(const Pkt6Ptr& release, Pkt6Ptr& reply) {
     OptionPtr opt_duid = release->getOption(D6O_CLIENTID);
     if (!opt_duid) {
         // This should not happen. We have checked this before.
+        // see sanityCheck() called from processRelease()
+        LOG_WARN(dhcp6_logger, DHCP6_RELEASE_MISSING_CLIENTID)
+            .arg(release->getRemoteAddr().toText());
+
         reply->addOption(createStatusCode(STATUS_UnspecFail,
                          "You did not include mandatory client-id"));
         return;
@@ -731,8 +735,11 @@ void Dhcpv6Srv::releaseLeases(const Pkt6Ptr& release, Pkt6Ptr& reply) {
             }
             break;
         }
+        // @todo: add support for IA_PD
+        // @todo: add support for IA_TA
         default:
-            break;
+            // remaining options are stateless and thus ignored in this context
+            ;
         }
     }
 
@@ -777,7 +784,7 @@ OptionPtr Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, Pkt6Ptr question,
                           "Sorry, no known leases for this duid/iaid, can't release."));
         general_status = STATUS_NoBinding;
 
-        LOG_WARN(dhcp6_logger, DHCP6_UNKNOWN_RELEASE)
+        LOG_INFO(dhcp6_logger, DHCP6_UNKNOWN_RELEASE)
             .arg(duid->toText())
             .arg(ia->getIAID());
 
@@ -789,7 +796,7 @@ OptionPtr Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, Pkt6Ptr question,
         // have mandatory DUID information attached. Someone was messing with our
         // database.
 
-        LOG_ERROR(dhcp6_logger, DHCP6_DB_ERROR_LEASE6_WITHOUT_DUID)
+        LOG_ERROR(dhcp6_logger, DHCP6_LEASE_WITHOUT_DUID)
             .arg(release_addr->getAddress().toText());
 
         general_status = STATUS_UnspecFail;
@@ -825,8 +832,8 @@ OptionPtr Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, Pkt6Ptr question,
         return (ia_rsp);
     }
 
-    // It is not necessary if the address matches as we used getLease6(addr)
-    // method that is supposed to return a proper lease.
+    // It is not necessary to check if the address matches as we used
+    // getLease6(addr) method that is supposed to return a proper lease.
 
     // Ok, we've passed all checks. Let's release this address.
 
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index b3f8c4c..0f3e641 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -231,10 +231,10 @@ protected:
     /// lease is found, an IA_NA response is generated with an appropriate
     /// status code.
     ///
-    /// As RFC3315 requires to also send global (one for the whole message),
-    /// this method may update passed general_status. It is set to SUCCESS
-    /// when message processing begins, but may be update to some error code
-    /// if release process fails.
+    /// As RFC 3315 requires that a single status code be sent for the whole message,
+    /// this method may update the passed general_status: it is set to SUCCESS when
+    /// message processing begins, but may be updated to some error code if the
+    /// release process fails.
     ///
     /// @param duid client's duid
     /// @param question client's message
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index da14655..2f65597 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -383,10 +383,9 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
 
     ElementPtr json = Element::fromJSON(config);
 
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW(srv.reset(new NakedDhcpv6Srv(0)));
+    NakedDhcpv6Srv srv(0);
 
-    EXPECT_NO_THROW(x = configureDhcp6Server(*srv, json));
+    EXPECT_NO_THROW(x = configureDhcp6Server(srv, json));
     ASSERT_TRUE(x);
     comment_ = parseAnswer(rcode_, x);
 
@@ -399,7 +398,7 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
     sol->addOption(clientid);
 
     // Pass it to the server and get an advertise
-    boost::shared_ptr<Pkt6> adv = srv->processSolicit(sol);
+    boost::shared_ptr<Pkt6> adv = srv.processSolicit(sol);
 
     // check if we get response at all
     ASSERT_TRUE(adv);
@@ -423,7 +422,7 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
     sol->addOption(option_oro);
 
     // Need to process SOLICIT again after requesting new option.
-    adv = srv->processSolicit(sol);
+    adv = srv.processSolicit(sol);
     ASSERT_TRUE(adv);
 
     OptionPtr tmp = adv->getOption(D6O_NAME_SERVERS);
@@ -474,8 +473,7 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
 // - server-id
 // - IA that includes IAADDR
 TEST_F(Dhcpv6SrvTest, SolicitBasic) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
     sol->setRemoteAddr(IOAddress("fe80::abcd"));
@@ -484,7 +482,7 @@ TEST_F(Dhcpv6SrvTest, SolicitBasic) {
     sol->addOption(clientid);
 
     // Pass it to the server and get an advertise
-    Pkt6Ptr reply = srv->processSolicit(sol);
+    Pkt6Ptr reply = srv.processSolicit(sol);
 
     // check if we get response at all
     checkResponse(reply, DHCPV6_ADVERTISE, 1234);
@@ -497,7 +495,7 @@ TEST_F(Dhcpv6SrvTest, SolicitBasic) {
     checkIAAddr(addr, addr->getAddress(), subnet_->getPreferred(), subnet_->getValid());
 
     // check DUIDs
-    checkServerId(reply, srv->getServerID());
+    checkServerId(reply, srv.getServerID());
     checkClientId(reply, clientid);
 }
 
@@ -517,8 +515,7 @@ TEST_F(Dhcpv6SrvTest, SolicitBasic) {
 // - server-id
 // - IA that includes IAADDR
 TEST_F(Dhcpv6SrvTest, SolicitHint) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     // Let's create a SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
@@ -535,7 +532,7 @@ TEST_F(Dhcpv6SrvTest, SolicitHint) {
     sol->addOption(clientid);
 
     // Pass it to the server and get an advertise
-    Pkt6Ptr reply = srv->processSolicit(sol);
+    Pkt6Ptr reply = srv.processSolicit(sol);
 
     // check if we get response at all
     checkResponse(reply, DHCPV6_ADVERTISE, 1234);
@@ -551,7 +548,7 @@ TEST_F(Dhcpv6SrvTest, SolicitHint) {
     checkIAAddr(addr, hint, subnet_->getPreferred(), subnet_->getValid());
 
     // check DUIDs
-    checkServerId(reply, srv->getServerID());
+    checkServerId(reply, srv.getServerID());
     checkClientId(reply, clientid);
 }
 
@@ -571,8 +568,7 @@ TEST_F(Dhcpv6SrvTest, SolicitHint) {
 // - server-id
 // - IA that includes IAADDR
 TEST_F(Dhcpv6SrvTest, SolicitInvalidHint) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     // Let's create a SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
@@ -587,7 +583,7 @@ TEST_F(Dhcpv6SrvTest, SolicitInvalidHint) {
     sol->addOption(clientid);
 
     // Pass it to the server and get an advertise
-    Pkt6Ptr reply = srv->processSolicit(sol);
+    Pkt6Ptr reply = srv.processSolicit(sol);
 
     // check if we get response at all
     checkResponse(reply, DHCPV6_ADVERTISE, 1234);
@@ -601,7 +597,7 @@ TEST_F(Dhcpv6SrvTest, SolicitInvalidHint) {
     EXPECT_TRUE(subnet_->inPool(addr->getAddress()));
 
     // check DUIDs
-    checkServerId(reply, srv->getServerID());
+    checkServerId(reply, srv.getServerID());
     checkClientId(reply, clientid);
 }
 
@@ -613,8 +609,7 @@ TEST_F(Dhcpv6SrvTest, SolicitInvalidHint) {
 // client. ADVERTISE is basically saying "if you send me a request, you will
 // probably get an address like this" (there are no guarantees).
 TEST_F(Dhcpv6SrvTest, ManySolicits) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     Pkt6Ptr sol1 = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
     Pkt6Ptr sol2 = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 2345));
@@ -638,9 +633,9 @@ TEST_F(Dhcpv6SrvTest, ManySolicits) {
     sol3->addOption(clientid3);
 
     // Pass it to the server and get an advertise
-    Pkt6Ptr reply1 = srv->processSolicit(sol1);
-    Pkt6Ptr reply2 = srv->processSolicit(sol2);
-    Pkt6Ptr reply3 = srv->processSolicit(sol3);
+    Pkt6Ptr reply1 = srv.processSolicit(sol1);
+    Pkt6Ptr reply2 = srv.processSolicit(sol2);
+    Pkt6Ptr reply3 = srv.processSolicit(sol3);
 
     // check if we get response at all
     checkResponse(reply1, DHCPV6_ADVERTISE, 1234);
@@ -661,9 +656,9 @@ TEST_F(Dhcpv6SrvTest, ManySolicits) {
     checkIAAddr(addr3, addr3->getAddress(), subnet_->getPreferred(), subnet_->getValid());
 
     // check DUIDs
-    checkServerId(reply1, srv->getServerID());
-    checkServerId(reply2, srv->getServerID());
-    checkServerId(reply3, srv->getServerID());
+    checkServerId(reply1, srv.getServerID());
+    checkServerId(reply2, srv.getServerID());
+    checkServerId(reply3, srv.getServerID());
     checkClientId(reply1, clientid1);
     checkClientId(reply2, clientid2);
     checkClientId(reply3, clientid3);
@@ -693,8 +688,7 @@ TEST_F(Dhcpv6SrvTest, ManySolicits) {
 // - server-id
 // - IA that includes IAADDR
 TEST_F(Dhcpv6SrvTest, RequestBasic) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     // Let's create a REQUEST
     Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234));
@@ -711,10 +705,10 @@ TEST_F(Dhcpv6SrvTest, RequestBasic) {
     req->addOption(clientid);
 
     // server-id is mandatory in REQUEST
-    req->addOption(srv->getServerID());
+    req->addOption(srv.getServerID());
 
     // Pass it to the server and hope for a REPLY
-    Pkt6Ptr reply = srv->processRequest(req);
+    Pkt6Ptr reply = srv.processRequest(req);
 
     // check if we get response at all
     checkResponse(reply, DHCPV6_REPLY, 1234);
@@ -730,7 +724,7 @@ TEST_F(Dhcpv6SrvTest, RequestBasic) {
     checkIAAddr(addr, hint, subnet_->getPreferred(), subnet_->getValid());
 
     // check DUIDs
-    checkServerId(reply, srv->getServerID());
+    checkServerId(reply, srv.getServerID());
     checkClientId(reply, clientid);
 
     // check that the lease is really in the database
@@ -747,8 +741,7 @@ TEST_F(Dhcpv6SrvTest, RequestBasic) {
 // client. ADVERTISE is basically saying "if you send me a request, you will
 // probably get an address like this" (there are no guarantees).
 TEST_F(Dhcpv6SrvTest, ManyRequests) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     Pkt6Ptr req1 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234));
     Pkt6Ptr req2 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 2345));
@@ -772,14 +765,14 @@ TEST_F(Dhcpv6SrvTest, ManyRequests) {
     req3->addOption(clientid3);
 
     // server-id is mandatory in REQUEST
-    req1->addOption(srv->getServerID());
-    req2->addOption(srv->getServerID());
-    req3->addOption(srv->getServerID());
+    req1->addOption(srv.getServerID());
+    req2->addOption(srv.getServerID());
+    req3->addOption(srv.getServerID());
 
     // Pass it to the server and get an advertise
-    Pkt6Ptr reply1 = srv->processRequest(req1);
-    Pkt6Ptr reply2 = srv->processRequest(req2);
-    Pkt6Ptr reply3 = srv->processRequest(req3);
+    Pkt6Ptr reply1 = srv.processRequest(req1);
+    Pkt6Ptr reply2 = srv.processRequest(req2);
+    Pkt6Ptr reply3 = srv.processRequest(req3);
 
     // check if we get response at all
     checkResponse(reply1, DHCPV6_REPLY, 1234);
@@ -800,9 +793,9 @@ TEST_F(Dhcpv6SrvTest, ManyRequests) {
     checkIAAddr(addr3, addr3->getAddress(), subnet_->getPreferred(), subnet_->getValid());
 
     // check DUIDs
-    checkServerId(reply1, srv->getServerID());
-    checkServerId(reply2, srv->getServerID());
-    checkServerId(reply3, srv->getServerID());
+    checkServerId(reply1, srv.getServerID());
+    checkServerId(reply2, srv.getServerID());
+    checkServerId(reply3, srv.getServerID());
     checkClientId(reply1, clientid1);
     checkClientId(reply2, clientid2);
     checkClientId(reply3, clientid3);
@@ -826,8 +819,7 @@ TEST_F(Dhcpv6SrvTest, ManyRequests) {
 // - returned REPLY message has IA that includes IAADDR
 // - lease is actually renewed in LeaseMgr
 TEST_F(Dhcpv6SrvTest, RenewBasic) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     const IOAddress addr("2001:db8:1:1::cafe:babe");
     const uint32_t iaid = 234;
@@ -868,10 +860,10 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
     req->addOption(clientid);
 
     // Server-id is mandatory in RENEW
-    req->addOption(srv->getServerID());
+    req->addOption(srv.getServerID());
 
     // Pass it to the server and hope for a REPLY
-    Pkt6Ptr reply = srv->processRenew(req);
+    Pkt6Ptr reply = srv.processRenew(req);
 
     // Check if we get response at all
     checkResponse(reply, DHCPV6_REPLY, 1234);
@@ -887,7 +879,7 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
     checkIAAddr(addr_opt, addr, subnet_->getPreferred(), subnet_->getValid());
 
     // Check DUIDs
-    checkServerId(reply, srv->getServerID());
+    checkServerId(reply, srv.getServerID());
     checkClientId(reply, clientid);
 
     // Check that the lease is really in the database
@@ -922,9 +914,7 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
 // - returned REPLY message has IA that includes STATUS-CODE
 // - No lease in LeaseMgr
 TEST_F(Dhcpv6SrvTest, RenewReject) {
-
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     const IOAddress addr("2001:db8:1:1::dead");
     const uint32_t transid = 1234;
@@ -952,12 +942,12 @@ TEST_F(Dhcpv6SrvTest, RenewReject) {
     req->addOption(clientid);
 
     // Server-id is mandatory in RENEW
-    req->addOption(srv->getServerID());
+    req->addOption(srv.getServerID());
 
     // Case 1: No lease known to server
 
     // Pass it to the server and hope for a REPLY
-    Pkt6Ptr reply = srv->processRenew(req);
+    Pkt6Ptr reply = srv.processRenew(req);
 
     // Check if we get response at all
     checkResponse(reply, DHCPV6_REPLY, transid);
@@ -983,7 +973,7 @@ TEST_F(Dhcpv6SrvTest, RenewReject) {
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // Pass it to the server and hope for a REPLY
-    reply = srv->processRenew(req);
+    reply = srv.processRenew(req);
     checkResponse(reply, DHCPV6_REPLY, transid);
     tmp = reply->getOption(D6O_IA_NA);
     ASSERT_TRUE(tmp);
@@ -1002,7 +992,7 @@ TEST_F(Dhcpv6SrvTest, RenewReject) {
     req->addOption(generateClientId(13)); // generate different DUID
                                           // (with length 13)
 
-    reply = srv->processRenew(req);
+    reply = srv.processRenew(req);
     checkResponse(reply, DHCPV6_REPLY, transid);
     tmp = reply->getOption(D6O_IA_NA);
     ASSERT_TRUE(tmp);
@@ -1029,8 +1019,7 @@ TEST_F(Dhcpv6SrvTest, RenewReject) {
 // - returned REPLY message has IA that does not include an IAADDR
 // - lease is actually removed from LeaseMgr
 TEST_F(Dhcpv6SrvTest, ReleaseBasic) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     const IOAddress addr("2001:db8:1:1::cafe:babe");
     const uint32_t iaid = 234;
@@ -1063,10 +1052,10 @@ TEST_F(Dhcpv6SrvTest, ReleaseBasic) {
     req->addOption(clientid);
 
     // Server-id is mandatory in RELEASE
-    req->addOption(srv->getServerID());
+    req->addOption(srv.getServerID());
 
     // Pass it to the server and hope for a REPLY
-    Pkt6Ptr reply = srv->processRelease(req);
+    Pkt6Ptr reply = srv.processRelease(req);
 
     // Check if we get response at all
     checkResponse(reply, DHCPV6_REPLY, 1234);
@@ -1083,7 +1072,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseBasic) {
     EXPECT_FALSE(tmp->getOption(D6O_IAADDR));
 
     // Check DUIDs
-    checkServerId(reply, srv->getServerID());
+    checkServerId(reply, srv.getServerID());
     checkClientId(reply, clientid);
 
     // Check that the lease is really gone in the database
@@ -1110,8 +1099,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseBasic) {
 // - No lease in LeaseMgr
 TEST_F(Dhcpv6SrvTest, ReleaseReject) {
 
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     const IOAddress addr("2001:db8:1:1::dead");
     const uint32_t transid = 1234;
@@ -1128,24 +1116,24 @@ TEST_F(Dhcpv6SrvTest, ReleaseReject) {
     Lease6Ptr l = LeaseMgrFactory::instance().getLease6(addr);
     ASSERT_FALSE(l);
 
-    // Let's create a RENEW
+    // Let's create a RELEASE
     Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_RELEASE, transid));
     req->setRemoteAddr(IOAddress("fe80::abcd"));
     boost::shared_ptr<Option6IA> ia = generateIA(bogus_iaid, 1500, 3000);
 
-    OptionPtr renewed_addr_opt(new Option6IAAddr(D6O_IAADDR, addr, 300, 500));
-    ia->addOption(renewed_addr_opt);
+    OptionPtr released_addr_opt(new Option6IAAddr(D6O_IAADDR, addr, 300, 500));
+    ia->addOption(released_addr_opt);
     req->addOption(ia);
     req->addOption(clientid);
 
     // Server-id is mandatory in RENEW
-    req->addOption(srv->getServerID());
+    req->addOption(srv.getServerID());
 
     // Case 1: No lease known to server
     SCOPED_TRACE("CASE 1: No lease known to server");
 
     // Pass it to the server and hope for a REPLY
-    Pkt6Ptr reply = srv->processRelease(req);
+    Pkt6Ptr reply = srv.processRelease(req);
 
     // Check if we get response at all
     checkResponse(reply, DHCPV6_REPLY, transid);
@@ -1169,7 +1157,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseReject) {
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // Pass it to the server and hope for a REPLY
-    reply = srv->processRelease(req);
+    reply = srv.processRelease(req);
     checkResponse(reply, DHCPV6_REPLY, transid);
     tmp = reply->getOption(D6O_IA_NA);
     ASSERT_TRUE(tmp);
@@ -1192,7 +1180,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseReject) {
     req->addOption(generateClientId(13)); // generate different DUID
                                           // (with length 13)
 
-    reply = srv->processRelease(req);
+    reply = srv.processRelease(req);
     checkResponse(reply, DHCPV6_REPLY, transid);
     tmp = reply->getOption(D6O_IA_NA);
     ASSERT_TRUE(tmp);
@@ -1212,8 +1200,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseReject) {
 
 // This test verifies if the status code option is generated properly.
 TEST_F(Dhcpv6SrvTest, StatusCode) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     // a dummy content for client-id
     uint8_t expected[] = {
@@ -1223,7 +1210,7 @@ TEST_F(Dhcpv6SrvTest, StatusCode) {
         0x41, 0x42, 0x43, 0x44, 0x45 // string value ABCDE
     };
     // Create the option.
-    OptionPtr status = srv->createStatusCode(3, "ABCDE");
+    OptionPtr status = srv.createStatusCode(3, "ABCDE");
     // Allocate an output buffer. We will store the option
     // in wire format here.
     OutputBuffer buf(sizeof(expected));
@@ -1237,34 +1224,34 @@ TEST_F(Dhcpv6SrvTest, StatusCode) {
 
 // This test verifies if the sanityCheck() really checks options presence.
 TEST_F(Dhcpv6SrvTest, sanityCheck) {
-    boost::scoped_ptr<NakedDhcpv6Srv> srv;
-    ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
+    NakedDhcpv6Srv srv(0);
 
     Pkt6Ptr pkt = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
 
-    // check that the packets originating from local addresses can be
+    // Set link-local sender address, so appropriate subnet can be
+    // selected for this packet.
     pkt->setRemoteAddr(IOAddress("fe80::abcd"));
 
     // client-id is optional for information-request, so
-    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::OPTIONAL));
+    EXPECT_NO_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::OPTIONAL));
 
     // empty packet, no client-id, no server-id
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN),
                  RFCViolation);
 
     // This doesn't make much sense, but let's check it for completeness
-    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::FORBIDDEN, Dhcpv6Srv::FORBIDDEN));
+    EXPECT_NO_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::FORBIDDEN, Dhcpv6Srv::FORBIDDEN));
 
     OptionPtr clientid = generateClientId();
     pkt->addOption(clientid);
 
     // client-id is mandatory, server-id is forbidden (as in SOLICIT or REBIND)
-    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN));
+    EXPECT_NO_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN));
 
-    pkt->addOption(srv->getServerID());
+    pkt->addOption(srv.getServerID());
 
     // both client-id and server-id are mandatory (as in REQUEST, RENEW, RELEASE, DECLINE)
-    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY));
+    EXPECT_NO_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY));
 
     // sane section ends here, let's do some negative tests as well
 
@@ -1272,13 +1259,13 @@ TEST_F(Dhcpv6SrvTest, sanityCheck) {
     pkt->addOption(clientid);
 
     // with more than one client-id it should throw, no matter what
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::OPTIONAL),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::OPTIONAL),
                  RFCViolation);
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::OPTIONAL),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::OPTIONAL),
                  RFCViolation);
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::MANDATORY),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::MANDATORY),
                  RFCViolation);
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY),
                  RFCViolation);
 
     pkt->delOption(D6O_CLIENTID);
@@ -1287,20 +1274,21 @@ TEST_F(Dhcpv6SrvTest, sanityCheck) {
     // again we have only one client-id
 
     // let's try different type of insanity - several server-ids
-    pkt->addOption(srv->getServerID());
-    pkt->addOption(srv->getServerID());
+    pkt->addOption(srv.getServerID());
+    pkt->addOption(srv.getServerID());
 
     // with more than one server-id it should throw, no matter what
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::OPTIONAL),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::OPTIONAL),
                  RFCViolation);
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::OPTIONAL),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::OPTIONAL),
                  RFCViolation);
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::MANDATORY),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::OPTIONAL, Dhcpv6Srv::MANDATORY),
                  RFCViolation);
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY),
+    EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY),
                  RFCViolation);
-
-
 }
 
+/// @todo: Add more negative tests for processX(), e.g. extend sanityCheck() test
+/// to call processX() methods.
+
 }   // end of anonymous namespace



More information about the bind10-changes mailing list