From: Thomas Markwalder <tmark@isc.org>
Date: Tue, 19 Jul 2022 20:15:58 +0000 (-0400)
Subject: [#1781] Additional review comments
X-Git-Tag: Kea-2.2.0~38
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7937f91440f91074ff61cf644cdb24762e55d2cc;p=thirdparty%2Fkea.git

[#1781] Additional review comments

src/hooks/dhcp/high_availability/query_filter.h
    Added commentary

src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc
    Added commentary
---

diff --git a/src/hooks/dhcp/high_availability/query_filter.h b/src/hooks/dhcp/high_availability/query_filter.h
index 0094d247b8..003bc89121 100644
--- a/src/hooks/dhcp/high_availability/query_filter.h
+++ b/src/hooks/dhcp/high_availability/query_filter.h
@@ -138,9 +138,17 @@ public:
 
     /// @brief Checks if this server should process the DHCPv4 query.
     ///
-    /// This method takes into account enabled scopes for this server and
-    /// HA mode to determine whether this query should be processed. It
-    /// triggers load balancing when load balancing mode is enabled.
+    /// This method uses a table of DHCPv4 message types to determine whether or
+    /// not a query needs further analysis to determine which HA server
+    /// should handle it or if it should simply be processed by this server.
+    ///
+    /// For message types that qualify (e.g. DHCPDISCOVER, DHCPREQUEST, etc),
+    /// it then takes into account enabled scopes for this server and HA mode
+    /// to determine whether this query should be processed. It triggers load
+    /// balancing when load balancing mode is enabled.
+    ///
+    /// Non-qualifying message types (e.g. DHCPLEASEQUERY) are passed
+    /// through for handling by this server.
     ///
     /// @param query4 pointer to the DHCPv4 query instance.
     /// @param [out] scope_class name of the class which corresponds to the
@@ -154,9 +162,17 @@ public:
 
     /// @brief Checks if this server should process the DHCPv6 query.
     ///
-    /// This method takes into account enabled scopes for this server and
-    /// HA mode to determine whether this query should be processed. It
-    /// triggers load balancing when load balancing mode is enabled.
+    /// This method uses a table of DHCPv6 message types to determine whether or
+    /// not a query needs further analysis to determine which HA server
+    /// should handle it or if it should simply be processed by this server.
+    ///
+    /// For message types that qualify (e.g. DHCPV6_SOLICIT, DHCPV6_REQUEST, etc),
+    /// it then takes into account enabled scopes for this server and HA mode
+    /// to determine whether this query should be processed. It triggers load
+    /// balancing when load balancing mode is enabled.
+    ///
+    /// Non-qualifying message types (e.g. DHCPV6_LEASEQUERY) are passed through
+    /// for handling by this server.
     ///
     /// @param query6 pointer to the DHCPv6 query instance.
     /// @param [out] scope_class name of the class which corresponds to the
diff --git a/src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc b/src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc
index a9c4a69f51..4f7f2c4f27 100644
--- a/src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc
+++ b/src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc
@@ -768,7 +768,9 @@ QueryFilterTest::loadBalancingHaTypes4() {
     ASSERT_TRUE(server1_pkt && server2_pkt) << "do not have both scopes in "
                 << max_scope_tries << ", load balance broken?";
 
-    // We exceed DHCP_TYPES_EOF just to be sure.
+    // Iterate over message types.  While setting message type to zero is
+    // semantically invalid, it is useful for testing here. Similarly we exceed
+    // DHCP_TYPES_EOF just to be sure.
     for (uint8_t msg_type = 0; msg_type < DHCP_TYPES_EOF + 2; ++msg_type) {
         // All message types should be in scope for server1.
         server1_pkt->setType(msg_type);
@@ -844,7 +846,9 @@ QueryFilterTest::loadBalancingHaTypes6() {
     ASSERT_TRUE(server1_pkt && server2_pkt) << "do not have both scopes in "
                 << max_scope_tries << ", load balance broken?";
 
-    // We exceed DHCPV6_TYPES_EOF just to be sure.
+    // Iterate over message types.  While setting message type to zero is
+    // semantically invalid, it is useful for testing here. Similarly we exceed
+    // DHCPV6_TYPES_EOF just to be sure.
     for (uint8_t msg_type = 0; msg_type < DHCPV6_TYPES_EOF + 2; ++msg_type) {
         // All message types should be in scope for server1.
         server1_pkt->setType(msg_type);