From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 4 Jun 2018 11:38:25 +0000 (+0200)
Subject: [5638] Fixed invalid callouts status for the v4 case.
X-Git-Tag: trac5117_base~1^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=34c51378739f71083d01e17a1542ecfa9b83eb02;p=thirdparty%2Fkea.git

[5638] Fixed invalid callouts status for the v4 case.
---

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 2be4a8fd17..476a893dcd 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -3544,6 +3544,13 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) {
             max_attempts = 0;
         }
 
+        // Set the default status code in case the lease4_select callouts
+        // do not exist and the callout handle has a status returned by
+        // any of the callouts already invoked for this packet.
+        if (ctx.callout_handle_) {
+            ctx.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
+        }
+
         for (uint64_t i = 0; i < max_attempts; ++i) {
             IOAddress candidate = allocator->pickAddress(subnet,
                                                          ctx.query_->getClasses(),
@@ -3551,12 +3558,14 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) {
                                                          ctx.requested_address_);
             // If address is not reserved for another client, try to allocate it.
             if (!addressReserved(candidate, ctx)) {
+
                 // The call below will return the non-NULL pointer if we
                 // successfully allocate this lease. This means that the
                 // address is not in use by another client.
                 new_lease = allocateOrReuseLease4(candidate, ctx);
                 if (new_lease) {
                     return (new_lease);
+
                 } else if (ctx.callout_handle_ &&
                            (ctx.callout_handle_->getStatus() !=
                             CalloutHandle::NEXT_STEP_CONTINUE)) {
diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index 64b7b842bd..f46ca30c05 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -9,6 +9,8 @@
 #include <dhcpsrv/shared_network.h>
 #include <dhcpsrv/tests/alloc_engine_utils.h>
 #include <dhcpsrv/tests/test_utils.h>
+#include <hooks/hooks_manager.h>
+#include <hooks/callout_handle.h>
 #include <stats/stats_mgr.h>
 
 using namespace std;
@@ -78,6 +80,32 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
 
     // Assigned addresses should have incremented.
     EXPECT_TRUE(testStatistics("assigned-addresses", 1, subnet_->getID()));
+
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
+    AllocEngine::ClientContext4 ctx2(subnet_, ClientIdPtr(), hwaddr2, IOAddress("0.0.0.0"),
+                                     false, true, "anotherhost.example.com.",
+                                     false);
+    ctx2.query_.reset(new Pkt4(DHCPREQUEST, 1234));
+
+    ctx2.callout_handle_ = HooksManager::createCalloutHandle();
+    ctx2.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_SKIP);
+
+    pool_->resetLastAllocated();
+
+    lease = engine->allocateLease4(ctx2);
+    // The new lease has been allocated, so the old lease should not exist.
+    EXPECT_FALSE(ctx2.old_lease_);
+
+    // Check that we got a lease
+    ASSERT_TRUE(lease);
+
+    // Check that the lease is indeed in LeaseMgr
+    from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+
+    // Now check that the lease in LeaseMgr has the same parameters
+    detailCompareLease(lease, from_mgr);
 }
 
 // This test checks if the fake allocation (for DHCPDISCOVER) can succeed