[#1781] HA now load balances only specific message types

Added a ChangeLog

src/hooks/dhcp/high_availability/query_filter.*
    QueryFilter::isHaType(const dhcp::Pkt4Ptr& query4)
    QueryFilter::isHaType(const dhcp::Pkt6Ptr& query6)
    - new functions

    QueryFilter::inScope() - uses new functions to balance
    and scope messages of specific types

src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc
    TEST_F(QueryFilterTest, loadBalancingHaTypes4)
    TEST_F(QueryFilterTest, loadBalancingHaTypes4MultiThreading)
    TEST_F(QueryFilterTest, loadBalancingHaTypes6)
    TEST_F(QueryFilterTest, loadBalancingHaTypes6MultiThreading)
    - new tests

src/lib/dhcp/dhcp6.h
    Uncommented v6 message types.

ChangeLog

@@ -1,3 +1,12 @@
+2041.	[bug]		tmark
+	HA now applies load balancing and scoping only to inbound
+	client packet types that apply to client lease fulfillment,
+	e.g. DHCPDISCOVER, DHCPREQUEST, DHCPV6_SOLICIT, DHCPV6_REQUEST,
+	etc. Prior to this it was indiscriminatly balancing and
+	scoping all inbound packets including those related to lease
+	query.
+	(Gitlab #1781)
+
 2040.	[func]		djt
 	Added support for Alpine 3.16 in hammer.py.
 	(Gitlab #2491)

src/hooks/dhcp/high_availability/query_filter.cc

@@ -47,6 +47,70 @@ std::array<uint8_t, 256> loadb_mx_tbl = { {
     149, 80, 170, 68, 6, 169, 234, 151 }
 };
 
+/// @brief Table indicating which DHCPv4 message types are of interest to HA.
+std::array<bool, DHCP_TYPES_EOF> v4_ha_types = {
+    false,  // DHCP_NOTYPE          = 0
+    true,   // DHCPDISCOVER         = 1
+    false,  // DHCPOFFER            = 2
+    true,   // DHCPREQUEST          = 3
+    true,   // DHCPDECLINE          = 4
+    false,  // DHCPACK              = 5
+    false,  // DHCPNAK              = 6
+    true,   // DHCPRELEASE          = 7
+    true,   // DHCPINFORM           = 8
+    false,  // DHCPFORCERENEW       = 9
+    false,  // DHCPLEASEQUERY       = 10
+    false,  // DHCPLEASEUNASSIGNED  = 11
+    false,  // DHCPLEASEUNKNOWN     = 12
+    false,  // DHCPLEASEACTIVE      = 13
+    false,  // DHCPBULKLEASEQUERY   = 14
+    false,  // DHCPLEASEQUERYDONE   = 15
+    false,  // DHCPACTIVELEASEQUERY = 16
+    false,  // DHCPLEASEQUERYSTATUS = 17
+    false   // DHCPTLS              = 18
+};
+
+/// @brief Table indicating which DHCPv6 message types are of interest to HA.
+std::array<bool, DHCPV6_TYPES_EOF> v6_ha_types = {
+    false,  // DHCPV6_NOTYPE               = 0
+    true,   // DHCPV6_SOLICIT              = 1
+    false,  // DHCPV6_ADVERTISE            = 2
+    true,   // DHCPV6_REQUEST              = 3
+    true,   // DHCPV6_CONFIRM              = 4
+    true,   // DHCPV6_RENEW                = 5
+    true,   // DHCPV6_REBIND               = 6
+    false,  // DHCPV6_REPLY                = 7
+    true,   // DHCPV6_RELEASE              = 8
+    true,   // DHCPV6_DECLINE              = 9
+    false,  // DHCPV6_RECONFIGURE          = 10
+    false,  // DHCPV6_INFORMATION_REQUEST  = 11
+    false,  // DHCPV6_RELAY_FORW           = 12
+    false,  // DHCPV6_RELAY_REPL           = 13
+    false,  // DHCPV6_LEASEQUERY           = 14
+    false,  // DHCPV6_LEASEQUERY_REPLY     = 15
+    false,  // DHCPV6_LEASEQUERY_DONE      = 16
+    false,  // DHCPV6_LEASEQUERY_DATA      = 17
+    false,  // DHCPV6_RECONFIGURE_REQUEST  = 18
+    false,  // DHCPV6_RECONFIGURE_REPLY    = 19
+    false,  // DHCPV6_DHCPV4_QUERY         = 20
+    false,  // DHCPV6_DHCPV4_RESPONSE      = 21
+    false,  // DHCPV6_ACTIVELEASEQUERY     = 22
+    false,  // DHCPV6_STARTTLS             = 23
+    false,  // DHCPV6_BNDUPD               = 24
+    false,  // DHCPV6_BNDREPLY             = 25
+    false,  // DHCPV6_POOLREQ              = 26
+    false,  // DHCPV6_POOLRESP             = 27
+    false,  // DHCPV6_UPDREQ               = 28
+    false,  // DHCPV6_UPDREQALL            = 29
+    false,  // DHCPV6_UPDDONE              = 30
+    false,  // DHCPV6_CONNECT              = 31
+    false,  // DHCPV6_CONNECTREPLY         = 32
+    false,  // DHCPV6_DISCONNECT           = 33
+    false,  // DHCPV6_STATE                = 34
+    false   // DHCPV6_CONTACT              = 35
+};
+
+
 } // end of anonymous namespace
 
 namespace isc {
@@ -280,6 +344,19 @@ QueryFilter::getServedScopesInternal() const {
     return (scope_set);
 }
 
+bool
+QueryFilter::isHaType(const dhcp::Pkt4Ptr& query4) {
+    auto msg_type = query4->getType();
+    return (msg_type < v4_ha_types.size() && v4_ha_types[msg_type]);
+}
+
+bool
+QueryFilter::isHaType(const dhcp::Pkt6Ptr& query) {
+    auto msg_type = query->getType();
+    return (msg_type < v6_ha_types.size() && v6_ha_types[msg_type]);
+}
+
+
 bool
 QueryFilter::inScope(const dhcp::Pkt4Ptr& query4, std::string& scope_class) const {
     if (MultiThreadingMgr::instance().getMode()) {
@@ -308,6 +385,14 @@ QueryFilter::inScopeInternal(const QueryPtrType& query,
         isc_throw(BadValue, "query must not be null");
     }
 
+
+    // If it's not a type HA cares about, it's in scope for this peer.
+    if (!isHaType(query)) {
+        auto scope = peers_[0]->getName();
+        scope_class = makeScopeClass(scope);
+        return (true);
+    }
+
     int candidate_server = 0;
 
     // If we're doing load balancing we have to check if this query

src/hooks/dhcp/high_availability/query_filter.h

@@ -168,6 +168,20 @@ public:
     /// server, false otherwise.
     bool inScope(const dhcp::Pkt6Ptr& query6, std::string& scope_class) const;
 
+    /// @brief Determines if a DHCPv4 query is a message type HA should process.
+    ///
+    /// @param query4 DHCPv4 packet to test. Must not be null.
+    ///
+    /// @return
+    static bool isHaType(const dhcp::Pkt4Ptr& query4);
+
+    /// @brief Determines if a DHCPv6 query is a message type HA should process.
+    ///
+    /// @param query6 DHCPv6 packet to test. Must not be null.
+    ///
+    /// @return
+    static bool isHaType(const dhcp::Pkt6Ptr& query6);
+
 private:
     /// @brief Enable scope.
     ///

src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc

@@ -101,6 +101,14 @@ public:
     /// @brief This test verifies that it is possible to explicitly enable and
     /// disable certain scopes.
     void explicitlyServeScopes();
+
+    /// @brief This test verifies that load balancing only affects the socpe of
+    /// DHCPv4 message types that HA cares about.
+    void loadBalancingHaTypes4();
+
+    /// @brief This test verifies that load balancing only affects the socpe of
+    /// DHCPv6 message types that HA cares about.
+    void loadBalancingHaTypes6();
 };
 
 void
@@ -721,6 +729,153 @@ QueryFilterTest::explicitlyServeScopes() {
     EXPECT_THROW(filter.serveScopes({ "server1", "unsupported" }), BadValue);
 }
 
+void
+QueryFilterTest::loadBalancingHaTypes4() {
+    HAConfigPtr config = createValidConfiguration();
+
+    QueryFilter filter(config);
+
+    // By default the server1 should serve its own scope only. The
+    // server2 should serve its scope.
+    EXPECT_TRUE(filter.amServingScope("server1"));
+    EXPECT_FALSE(filter.amServingScope("server2"));
+    EXPECT_FALSE(filter.amServingScope("server3"));
+
+    // Use DHCPDISCOVER to find MAC addresses in scope for server1 and server2.
+    Pkt4Ptr server1_pkt;
+    Pkt4Ptr server2_pkt;
+    const unsigned max_scope_tries = 100;
+    for (unsigned i = 0; i < max_scope_tries; ++i) {
+        // Create query with random HW address.
+        std::string scope_class;
+        Pkt4Ptr query4 = createQuery4(randomKey(HWAddr::ETHERNET_HWADDR_LEN));
+        // If the query is in scope then we're done.
+        if (filter.inScope(query4, scope_class)) {
+            ASSERT_EQ("HA_server1", scope_class);
+            server1_pkt = query4;
+            if (server2_pkt) {
+                break;
+            }
+        } else {
+            ASSERT_EQ("HA_server2", scope_class);
+            server2_pkt = query4;
+            if (server1_pkt) {
+                break;
+            }
+        }
+    }
+
+    ASSERT_TRUE(server1_pkt && server2_pkt) << "do not have both scopes in "
+                << max_scope_tries << ", load balance broken?";
+
+    for (uint8_t msg_type = DHCP_NOTYPE; msg_type < DHCP_TYPES_EOF; ++msg_type) {
+        // All message types should be in scope for server1.
+        server1_pkt->setType(msg_type);
+
+        std::string scope_class;
+        bool is_in_scope = filter.inScope(server1_pkt, scope_class);
+        ASSERT_EQ("HA_server1", scope_class);
+        EXPECT_TRUE(is_in_scope);
+
+        server2_pkt->setType(msg_type);
+        scope_class = "";
+        is_in_scope = filter.inScope(server2_pkt, scope_class);
+        switch (msg_type) {
+            case DHCPDISCOVER:
+            case DHCPREQUEST:
+            case DHCPDECLINE:
+            case DHCPRELEASE:
+            case DHCPINFORM:
+                // HA message types should be in scope for server2.
+                ASSERT_EQ("HA_server2", scope_class);
+                EXPECT_FALSE(is_in_scope);
+                break;
+            default:
+                // Non HA message types should be in scope for server1.
+                ASSERT_EQ("HA_server1", scope_class);
+                EXPECT_TRUE(is_in_scope);
+                break;
+        }
+    }
+}
+
+void
+QueryFilterTest::loadBalancingHaTypes6() {
+    HAConfigPtr config = createValidConfiguration();
+
+    QueryFilter filter(config);
+
+    // By default the server1 should serve its own scope only. The
+    // server2 should serve its scope.
+    EXPECT_TRUE(filter.amServingScope("server1"));
+    EXPECT_FALSE(filter.amServingScope("server2"));
+    EXPECT_FALSE(filter.amServingScope("server3"));
+
+    // Use DHCPV6_SOLICIT to find MAC addresses in scope for server1 and server2.
+    Pkt6Ptr server1_pkt;
+    Pkt6Ptr server2_pkt;
+    const unsigned max_scope_tries = 100;
+    for (unsigned i = 0; i < max_scope_tries; ++i) {
+        // Create query with random HW address.
+        std::string scope_class;
+
+        // Create query with random DUID.
+        Pkt6Ptr query6 = createQuery6(randomKey(10));
+        if (filter.inScope(query6, scope_class)) {
+            ASSERT_EQ("HA_server1", scope_class);
+            // In scope for server1, save it.
+            server1_pkt = query6;
+            if (server2_pkt) {
+                // Have both, we're done.
+                break;
+            }
+        } else {
+            ASSERT_EQ("HA_server2", scope_class);
+            // In scope for server2, save it.
+            server2_pkt = query6;
+            if (server1_pkt) {
+                // Have both, we're done.
+                break;
+            }
+        }
+    }
+
+    ASSERT_TRUE(server1_pkt && server2_pkt) << "do not have both scopes in "
+                << max_scope_tries << ", load balance broken?";
+
+    for (uint8_t msg_type = DHCPV6_NOTYPE; msg_type < DHCPV6_TYPES_EOF; ++msg_type) {
+        // All message types should be in scope for server1.
+        server1_pkt->setType(msg_type);
+
+        std::string scope_class;
+        bool is_in_scope = filter.inScope(server1_pkt, scope_class);
+        ASSERT_EQ("HA_server1", scope_class);
+        EXPECT_TRUE(is_in_scope);
+
+        server2_pkt->setType(msg_type);
+        scope_class = "";
+        is_in_scope = filter.inScope(server2_pkt, scope_class);
+        switch (msg_type) {
+            case DHCPV6_SOLICIT:
+            case DHCPV6_REQUEST:
+            case DHCPV6_CONFIRM:
+            case DHCPV6_RENEW:
+            case DHCPV6_REBIND:
+            case DHCPV6_RELEASE:
+            case DHCPV6_DECLINE:
+                // HA message types should be in scope for server2.
+                ASSERT_EQ("HA_server2", scope_class);
+                EXPECT_FALSE(is_in_scope);
+                break;
+            default:
+                // Non HA message types should be in scope for server1.
+                ASSERT_EQ("HA_server1", scope_class);
+                EXPECT_TRUE(is_in_scope);
+                break;
+        }
+    }
+}
+
 TEST_F(QueryFilterTest, loadBalancingClientIdThisPrimary) {
     loadBalancingClientIdThisPrimary();
 }
@@ -847,4 +1002,22 @@ TEST_F(QueryFilterTest, explicitlyServeScopesMultiThreading) {
     explicitlyServeScopes();
 }
 
+TEST_F(QueryFilterTest, loadBalancingHaTypes4) {
+    loadBalancingHaTypes4();
+}
+
+TEST_F(QueryFilterTest, loadBalancingHaTypes4MultiThreading) {
+    MultiThreadingMgr::instance().setMode(true);
+    loadBalancingHaTypes4();
+}
+
+TEST_F(QueryFilterTest, loadBalancingHaTypes6) {
+    loadBalancingHaTypes6();
+}
+
+TEST_F(QueryFilterTest, loadBalancingHaTypes6MultiThreading) {
+    MultiThreadingMgr::instance().setMode(true);
+    loadBalancingHaTypes6();
+}
+
 }

src/lib/dhcp/dhcp6.h

@@ -201,6 +201,7 @@ enum DHCPv6StatusCode {
  * DHCPv6 message types, defined in section 7.3 of RFC 8415
  */
 enum DHCPv6MessageType {
+   DHCPV6_NOTYPE               = 0,
    DHCPV6_SOLICIT              = 1,
    DHCPV6_ADVERTISE            = 2,
    DHCPV6_REQUEST              = 3,
@@ -218,30 +219,31 @@ enum DHCPv6MessageType {
    DHCPV6_LEASEQUERY           = 14,
    DHCPV6_LEASEQUERY_REPLY     = 15,
    /* RFC 5460 */
-// DHCPV6_LEASEQUERY_DONE      = 16,
+   DHCPV6_LEASEQUERY_DONE      = 16,
-// DHCPV6_LEASEQUERY_DATA      = 17,
+   DHCPV6_LEASEQUERY_DATA      = 17,
    /* RFC 6977 */
-// DHCPV6_RECONFIGURE_REQUEST  = 18,
+   DHCPV6_RECONFIGURE_REQUEST  = 18,
-// DHCPV6_RECONFIGURE_REPLY    = 19,
+    DHCPV6_RECONFIGURE_REPLY   = 19,
    /* RFC 7341 */
    DHCPV6_DHCPV4_QUERY         = 20,
-   DHCPV6_DHCPV4_RESPONSE      = 21
+   DHCPV6_DHCPV4_RESPONSE      = 21,
    /* RFC 7653 */
-// DHCPV6_ACTIVELEASEQUERY     = 22,
+   DHCPV6_ACTIVELEASEQUERY     = 22,
-// DHCPV6_STARTTLS             = 23,
+   HCPV6_STARTTLS              = 23,
    /* RFC 8156 */
-// DHCPV6_BNDUPD               = 24,
+   DHCPV6_BNDUPD               = 24,
-// DHCPV6_BNDREPLY             = 25,
+   DHCPV6_BNDREPLY             = 25,
-// DHCPV6_POOLREQ              = 26,
+   DHCPV6_POOLREQ              = 26,
-// DHCPV6_POOLRESP             = 27,
+   DHCPV6_POOLRESP             = 27,
-// DHCPV6_UPDREQ               = 28,
+   DHCPV6_UPDREQ               = 28,
-// DHCPV6_UPDREQALL            = 29,
+   DHCPV6_UPDREQALL            = 29,
-// DHCPV6_UPDDONE              = 30,
+   DHCPV6_UPDDONE              = 30,
-// DHCPV6_CONNECT              = 31,
+   DHCPV6_CONNECT              = 31,
-// DHCPV6_CONNECTREPLY         = 32,
+   DHCPV6_CONNECTREPLY         = 32,
-// DHCPV6_DISCONNECT           = 33,
+   DHCPV6_DISCONNECT           = 33,
-// DHCPV6_STATE                = 34,
+   DHCPV6_STATE                = 34,
-// DHCPV6_CONTACT              = 35
+   DHCPV6_CONTACT              = 35,
+   DHCPV6_TYPES_EOF
 };
 
 extern const char *dhcpv6_type_names[];