BIND 10 trac2325, updated. a40fac0707fac08e85b06974092bbdc15720f37f [2325] Unknown renew is now logged in dhcp6

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Dec 11 15:16:46 UTC 2012


The branch, trac2325 has been updated
       via  a40fac0707fac08e85b06974092bbdc15720f37f (commit)
       via  196c4d239ece786aa29b86e6b48bef337ca330fc (commit)
       via  1f943baf9448dcec53f5bcde3e9d21fae2eeb121 (commit)
      from  d6fb00a3198fe327cdbf4ebfe14f47eb23086325 (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 a40fac0707fac08e85b06974092bbdc15720f37f
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 11 16:11:27 2012 +0100

    [2325] Unknown renew is now logged in dhcp6

commit 196c4d239ece786aa29b86e6b48bef337ca330fc
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 11 15:57:45 2012 +0100

    [2325] Changes after review.

commit 1f943baf9448dcec53f5bcde3e9d21fae2eeb121
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 11 15:13:58 2012 +0100

    [2325] Option7 IA unittest improvement.

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

Summary of changes:
 src/bin/dhcp6/dhcp6_messages.mes          |   26 ++++++++++++++---
 src/bin/dhcp6/dhcp6_srv.cc                |   11 +++++--
 src/bin/dhcp6/dhcp6_srv.h                 |    5 ++--
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc |   45 ++++++++++++++++-------------
 src/lib/dhcp/tests/option6_ia_unittest.cc |    8 ++++-
 5 files changed, 66 insertions(+), 29 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index be50591..d38acb9 100644
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/src/bin/dhcp6/dhcp6_messages.mes
@@ -22,6 +22,11 @@ successfully established a session with the BIND 10 control channel.
 This debug message is issued just before the IPv6 DHCP server attempts
 to establish a session with the BIND 10 control channel.
 
+% DHCP6_CLIENTID_MISSING mandatory client-id option is missing, message from %1 dropped
+This error message indicates that received message does not include
+mandatory client-id option that is necessary for address assignment,
+so the message is being dropped. This likely indicates a buggy client.
+
 % DHCP6_COMMAND_RECEIVED received command %1, arguments: %2
 A debug message listing the command (and possible arguments) received
 from the BIND 10 control system by the IPv6 DHCP server.
@@ -82,10 +87,11 @@ received REQUEST) a lease for a given client. There may be many reasons for
 such failure. Each specific failure is logged in a separate log entry.
 
 % DHCP6_REQUIRED_OPTIONS_CHECK_FAIL %1 message received from %2 failed the following check: %3
-This message indicates that received message has invalid number of options:
-mandatory client-id option is missing, server-id forbidden in that particular
-type of message is present, there is more than one instance of client-id
-or server-id etc. Exact reason for rejecting the packed is printed.
+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,
+the server-id forbidden in that particular type of message is present,
+there is more than one instance of client-id or server-id present,
+etc. The exact reason for rejecting the packet is included in the message.
 
 % DHCP6_NOT_RUNNING IPv6 DHCP server is not running
 A warning message is issued when an attempt is made to shut down the
@@ -195,3 +201,15 @@ which the DHCPv6 server has not been configured. The cause is most likely due
 to a misconfiguration of the server. The packet processing will continue, but
 the response will only contain generic configuration parameters and no
 addresses or prefixes.
+
+% DHCP6_UNKNOWN_RENEW received RENEW from client (duid=%1, iaid=%2) in subnet %3
+This warning message is printed when client attempts to renew a lease, but no
+such lease is known by the server. This typically means that client attempts to
+use its lease past its lifetime, e.g. due to time adjustment or poor support
+for sleep/recovery. Properly implemented client will recover from such case
+(it should restart lease allocation process after receiving a negative reply
+from the server). Alternatively, it may mean that the server lost its
+database recently and does not recognize its well behaving clients. This
+is likely the case if you see many such messages. Clients will recover from
+this, but they will likely get another IP addresses and experience brief
+service interruption.
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 5edb6ea..da8aa52 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -433,7 +433,7 @@ void Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer) {
 
         // perhaps this should be logged on some higher level? This is most likely
         // configuration bug.
-        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_SUBNET_SELECTION_FAILED);
+        LOG_ERROR(dhcp6_logger, DHCP6_SUBNET_SELECTION_FAILED);
     } else {
         LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL_DATA, DHCP6_SUBNET_SELECTED)
             .arg(subnet->toText());
@@ -451,6 +451,7 @@ void Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer) {
     if (opt_duid) {
         duid = DuidPtr(new DUID(opt_duid->getData()));
     } else {
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_CLIENTID_MISSING);
         // Let's drop the message. This client is not sane.
         isc_throw(RFCViolation, "Mandatory client-id is missing in received message");
     }
@@ -582,6 +583,12 @@ OptionPtr Dhcpv6Srv::renewIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         // Insert status code NoAddrsAvail.
         ia_rsp->addOption(createStatusCode(STATUS_NoAddrsAvail,
                           "Sorry, no known leases for this duid/iaid."));
+
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_UNKNOWN_RENEW)
+            .arg(duid->toText())
+            .arg(ia->getIAID())
+            .arg(subnet->toText());
+
         return (ia_rsp);
     }
 
@@ -624,7 +631,7 @@ void Dhcpv6Srv::renewLeases(const Pkt6Ptr& renew, Pkt6Ptr& reply) {
 
         // perhaps this should be logged on some higher level? This is most likely
         // configuration bug.
-        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_SUBNET_SELECTION_FAILED);
+        LOG_ERROR(dhcp6_logger, DHCP6_SUBNET_SELECTION_FAILED);
     } else {
         LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL_DATA, DHCP6_SUBNET_SELECTED)
             .arg(subnet->toText());
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index 8490d9b..b6b0306 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -213,8 +213,8 @@ protected:
     /// @brief Renews specific IA_NA option
     ///
     /// Generates response to IA_NA. This typically includes finding a lease that
-    /// corresponds to the received address. If no such lease is found, IA_NA
-    /// response is generates with appropriate status code.
+    /// corresponds to the received address. If no such lease is found, an IA_NA
+    /// response is generated with an appropriate status code.
     ///
     /// @param subnet subnet the sender belongs to
     /// @param duid client's duid
@@ -255,6 +255,7 @@ protected:
     ///
     /// It supports addresses (IA_NA) only. It does NOT support temporary
     /// addresses (IA_TA) nor prefixes (IA_PD).
+    /// @todo: Extend this method once TA and PD becomes supported
     ///
     /// @param question client's message (with requested IA_NA)
     /// @param answer server's message (IA_NA options will be added here)
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index b02bee4..62f32cd 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -796,12 +796,15 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
     boost::scoped_ptr<NakedDhcpv6Srv> srv;
     ASSERT_NO_THROW( srv.reset(new NakedDhcpv6Srv(0)) );
 
-    IOAddress addr("2001:db8:1:1::cafe:babe");
+    const IOAddress addr("2001:db8:1:1::cafe:babe");
     const uint32_t iaid = 234;
 
-    // generate client-id also duid_
+    // Generate client-id also duid_
     OptionPtr clientid = generateClientId();
 
+    // Check that the address we are about to use is indeed in pool
+    ASSERT_TRUE(subnet_->inPool(addr));
+
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(Lease6::LEASE_IA_NA, addr, duid_, iaid,
@@ -809,11 +812,12 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
-    // check that the lease is really in the database
+    // Check that the lease is really in the database
     Lease6Ptr l = LeaseMgrFactory::instance().getLease6(addr);
     ASSERT_TRUE(l);
 
-    // Check that T1, T2, preferred, valid and cltt really
+    // Check that T1, T2, preferred, valid and cltt really set and not using
+    // previous (500, 501, etc.) values
     EXPECT_NE(l->t1_, subnet_->getT1());
     EXPECT_NE(l->t2_, subnet_->getT2());
     EXPECT_NE(l->preferred_lft_, subnet_->getPreferred());
@@ -825,38 +829,37 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
     req->setRemoteAddr(IOAddress("fe80::abcd"));
     boost::shared_ptr<Option6IA> ia = generateIA(iaid, 1500, 3000);
 
-    ASSERT_TRUE(subnet_->inPool(addr));
     OptionPtr renewed_addr_opt(new Option6IAAddr(D6O_IAADDR, addr, 300, 500));
     ia->addOption(renewed_addr_opt);
     req->addOption(ia);
     req->addOption(clientid);
 
-    // server-id is mandatory in RENEW
+    // Server-id is mandatory in RENEW
     req->addOption(srv->getServerID());
 
     // Pass it to the server and hope for a REPLY
     Pkt6Ptr reply = srv->processRenew(req);
 
-    // check if we get response at all
+    // Check if we get response at all
     checkResponse(reply, DHCPV6_REPLY, 1234);
 
     OptionPtr tmp = reply->getOption(D6O_IA_NA);
     ASSERT_TRUE(tmp);
 
-    // check that IA_NA was returned and that there's an address included
+    // Check that IA_NA was returned and that there's an address included
     boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(reply, 234, subnet_->getT1(),
                                                            subnet_->getT2());
 
-    // check that we've got the address we requested
+    // Check that we've got the address we requested
     checkIAAddr(addr_opt, addr, subnet_->getPreferred(), subnet_->getValid());
 
-    // check DUIDs
+    // Check DUIDs
     checkServerId(reply, srv->getServerID());
     checkClientId(reply, clientid);
 
-    // check that the lease is really in the database
+    // Check that the lease is really in the database
     l = checkLease(duid_, reply->getOption(D6O_IA_NA), addr_opt);
-    EXPECT_TRUE(l);
+    ASSERT_TRUE(l);
 
     // Check that T1, T2, preferred, valid and cltt were really updated
     EXPECT_EQ(l->t1_, subnet_->getT1());
@@ -864,10 +867,10 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
     EXPECT_EQ(l->preferred_lft_, subnet_->getPreferred());
     EXPECT_EQ(l->valid_lft_, subnet_->getValid());
 
-    // checking for CLTT is a bit tricky if we want to avoid off by 1 errors
+    // Checking for CLTT is a bit tricky if we want to avoid off by 1 errors
     int32_t cltt = static_cast<int32_t>(l->cltt_);
     int32_t expected = static_cast<int32_t>(time(NULL));
-    // 1 >= difference between cltt and expected
+    // equality or difference by 1 between cltt and expected is ok.
     EXPECT_GE(1, abs(cltt - expected));
 
     EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease6(addr_opt->getAddress()));
@@ -886,18 +889,22 @@ 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)) );
 
-    IOAddress addr("2001:db8:1:1::dead");
+    const IOAddress addr("2001:db8:1:1::dead");
     const uint32_t transid = 1234;
     const uint32_t valid_iaid = 234;
     const uint32_t bogus_iaid = 456;
 
-    // generate client-id also duid_
+    // Quick sanity check that the address we're about to use is ok
+    ASSERT_TRUE(subnet_->inPool(addr));
+
+    // GenerateClientId() also sets duid_
     OptionPtr clientid = generateClientId();
 
-    // check that the lease is really in the database
+    // Check that the lease is NOT in the database
     Lease6Ptr l = LeaseMgrFactory::instance().getLease6(addr);
     ASSERT_FALSE(l);
 
@@ -906,13 +913,12 @@ TEST_F(Dhcpv6SrvTest, RenewReject) {
     req->setRemoteAddr(IOAddress("fe80::abcd"));
     boost::shared_ptr<Option6IA> ia = generateIA(bogus_iaid, 1500, 3000);
 
-    ASSERT_TRUE(subnet_->inPool(addr));
     OptionPtr renewed_addr_opt(new Option6IAAddr(D6O_IAADDR, addr, 300, 500));
     ia->addOption(renewed_addr_opt);
     req->addOption(ia);
     req->addOption(clientid);
 
-    // server-id is mandatory in RENEW
+    // Server-id is mandatory in RENEW
     req->addOption(srv->getServerID());
 
     // Case 1: No lease known to server
@@ -972,7 +978,6 @@ TEST_F(Dhcpv6SrvTest, RenewReject) {
     ASSERT_TRUE(ia);
     checkRejectedIA_NA(ia, STATUS_NoAddrsAvail);
 
-
     lease = LeaseMgrFactory::instance().getLease6(addr);
     ASSERT_TRUE(lease);
     // Verify that the lease was not updated.
diff --git a/src/lib/dhcp/tests/option6_ia_unittest.cc b/src/lib/dhcp/tests/option6_ia_unittest.cc
index 4e0bda1..24f101d 100644
--- a/src/lib/dhcp/tests/option6_ia_unittest.cc
+++ b/src/lib/dhcp/tests/option6_ia_unittest.cc
@@ -109,7 +109,13 @@ TEST_F(Option6IATest, basic) {
 }
 
 TEST_F(Option6IATest, simple) {
-    Option6IA * ia = new Option6IA(D6O_IA_NA, 1234);
+    Option6IA* ia = new Option6IA(D6O_IA_NA, 1234);
+
+    // Check that the values are really different than what we are about
+    // to set them to.
+    EXPECT_NE(2345, ia->getT1());
+    EXPECT_NE(3456, ia->getT2());
+
     ia->setT1(2345);
     ia->setT2(3456);
 



More information about the bind10-changes mailing list