[#1139] Extended commentary

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

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
+        // be in conflict with the reserved classes. Suppose there are
-        // remove those that were assigned as a result of evaluation.
+        // two classes defined in the server configuration: first_class
-        // That preserves built-in classes and the classes set explicitly
+        // and second_class and the test for the second_class it looks
-        // by the hooks libraries.
+        // 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();

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
+        // be in conflict with the reserved classes. Suppose there are
-        // remove those that were assigned as a result of evaluation.
+        // two classes defined in the server configuration: first_class
-        // That preserves built-in classes and the classes set explicitly
+        // and second_class and the test for the second_class it looks
-        // by the hooks libraries.
+        // 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();