[#1139] Extended commentary

The commentary explaning why we do another client classification evaluation
pass has been extended per review comments.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 028a5d882489a788b199cec0712791fac511f70f..b6041748aeb8d6c01211198e991466aae3aa116d 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -201,10 +201,19 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine,
     if ((global_host && !global_host->getClientClasses4().empty()) ||
         (!sn && current_host && !current_host->getClientClasses4().empty())) {
         // We have already evaluated client classes and some of them may
-        // be in conflict with the reserved classes. Therefore, we need to
-        // remove those that were assigned as a result of evaluation.
-        // That preserves built-in classes and the classes set explicitly
-        // by the hooks libraries.
+        // be in conflict with the reserved classes. Suppose there are
+        // two classes defined in the server configuration: first_class
+        // and second_class and the test for the second_class it looks
+        // like this: "not member('first_class')". If the first_class
+        // initially evaluates to false, the second_class evaluates to
+        // true. If the first_class is now set within the hosts reservations
+        // and we don't remove the previously evaluated second_class we'd
+        // end up with both first_class and second_class evaluated to
+        // true. In order to avoid that, we have to remove the classes
+        // evaluated in the first pass and evaluate them again. As
+        // a result, the first_class set via the host reservation will
+        // replace the second_class because the second_class will this
+        // time evaluate to false as desired.
         const ClientClassDictionaryPtr& dict =
             CfgMgr::instance().getCurrentCfg()->getClientClassDictionary();
         const ClientClassDefListPtr& defs_ptr = dict->getClasses();

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 97844184a66361e1a6efb30e62cf8bc9b4d01481..abcfe4b25c8e470fe5e0b14d72c3db376d520a5a 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -439,10 +439,19 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt,
     if ((global_host && !global_host->getClientClasses6().empty()) ||
         (!sn && current_host && !current_host->getClientClasses6().empty())) {
         // We have already evaluated client classes and some of them may
-        // be in conflict with the reserved classes. Therefore, we need to
-        // remove those that were assigned as a result of evaluation.
-        // That preserves built-in classes and the classes set explicitly
-        // by the hooks libraries.
+        // be in conflict with the reserved classes. Suppose there are
+        // two classes defined in the server configuration: first_class
+        // and second_class and the test for the second_class it looks
+        // like this: "not member('first_class')". If the first_class
+        // initially evaluates to false, the second_class evaluates to
+        // true. If the first_class is now set within the hosts reservations
+        // and we don't remove the previously evaluated second_class we'd
+        // end up with both first_class and second_class evaluated to
+        // true. In order to avoid that, we have to remove the classes
+        // evaluated in the first pass and evaluate them again. As
+        // a result, the first_class set via the host reservation will
+        // replace the second_class because the second_class will this
+        // time evaluate to false as desired.
         const ClientClassDictionaryPtr& dict =
             CfgMgr::instance().getCurrentCfg()->getClientClassDictionary();
         const ClientClassDefListPtr& defs_ptr = dict->getClasses();