Parcourir la source

[3988] Changes after review:

 - several v4 => v6
 - simpler remove_tmp logic
 - ExpirationAllocEngine4Test constructor now cleans up hook buffers
Tomek Mrugalski il y a 9 ans
Parent
commit
1195d77278

+ 2 - 2
src/bin/dhcp6/dhcp6_hooks.dox

@@ -288,7 +288,7 @@ packet processing. Hook points that are not specific to packet processing
   skip flag is only set when the lease has been actually reclaimed in the
   database by the callout.
 
-@subsection dhcpv6HooksLease4Recover lease6_recover
+@subsection dhcpv6HooksLease6Recover lease6_recover
 
 - @b Arguments:
   - name: @b lease6, type: isc::dhcp::Lease6Ptr, direction: <b>in</b>
@@ -299,7 +299,7 @@ packet processing. Hook points that are not specific to packet processing
   from any client identifying information when it was put into declined
   state. In principle the callouts can modify the lease in this hook,
   but it makes little sense. There's no useful data in the lease, except
-  the IPv4 address (which must not be modified).
+  the IPv6 address (which must not be modified).
 
 - <b>Next step status</b>: if the callout sets the next step action to SKIP,
   the server will skip the lease recovery. In other words, it will keep

+ 3 - 5
src/lib/dhcpsrv/alloc_engine.cc

@@ -1396,14 +1396,13 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
                     // There's no point in keeping declined lease after its
                     // reclaimation. Declined lease doesn't have any client
                     // identifying information anymore.
-                    remove_tmp = true;
 
                     // Do extra steps required for declined lease reclaimation:
                     // - bump decline-related stats
                     // - log separate message
                     // - call lease6_recover hooks
                     // (hooks can override the removal decision and keep the lease)
-                    remove_tmp = reclaimDeclined(lease) && remove_lease;
+                    remove_tmp = reclaimDeclined(lease);
                 }
 
                 // Reclaim the lease - depending on the configuration, set the
@@ -1621,12 +1620,11 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
                     // There's no point in keeping declined lease after its
                     // reclaimation. Declined lease doesn't have any client
                     // identifying information anymore.
-                    remove_tmp = true;
 
                     // Do extra steps required for declined lease reclaimation:
                     // - bump decline-related stats
                     // - log separate message
-                    remove_tmp = reclaimDeclined(lease) && remove_tmp;
+                    remove_tmp = reclaimDeclined(lease);
                 }
 
                 // Reclaim the lease - depending on the configuration, set the
@@ -1745,7 +1743,7 @@ AllocEngine::reclaimDeclined(const Lease4Ptr& lease) {
     if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease4_recover_)) {
 
         // Let's use a static callout handle. It will be initialized the first
-        // time lease6_recover is called and will keep to that value.
+        // time lease4_recover is called and will keep to that value.
         static CalloutHandlePtr callout_handle;
         if (!callout_handle) {
             callout_handle = HooksManager::createCalloutHandle();

+ 6 - 2
src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc

@@ -1644,6 +1644,10 @@ ExpirationAllocEngine4Test::ExpirationAllocEngine4Test()
     : ExpirationAllocEngineTest<Lease4Ptr>("type=memfile universe=4 persist=false") {
     createLeases();
     callout_argument_name = "lease4";
+
+    // Let's clear any garbage previous test may have left in static fields.
+    callout_name_ = string("");
+    callout_lease_.reset();
 }
 
 void
@@ -1987,13 +1991,13 @@ TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedStats) {
     testReclaimDeclinedStats("assigned-addresses");
 }
 
-// This test verifies if the hooks installed on lease6_recover are called
+// This test verifies if the hooks installed on lease4_recover are called
 // when the lease expires.
 TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedHook1) {
     testReclaimDeclinedHook(false); // false = don't use skip callout
 }
 
-// This test verifies if the hooks installed on lease6_recover are called
+// This test verifies if the hooks installed on lease4_recover are called
 // when the lease expires and that the next step status set to SKIP
 // causes the recovery to not be conducted.
 TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedHook2) {