From 3a40eb1695b2afde057c9a91b36eefa10f06da96 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 12 Jul 2022 16:40:39 -0400 Subject: [PATCH] [#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 | 9 + .../dhcp/high_availability/query_filter.cc | 85 +++++++++ .../dhcp/high_availability/query_filter.h | 14 ++ .../tests/query_filter_unittest.cc | 173 ++++++++++++++++++ src/lib/dhcp/dhcp6.h | 40 ++-- 5 files changed, 302 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index e04efe9859..da72d6dbb9 100644 --- a/ChangeLog +++ b/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) diff --git a/src/hooks/dhcp/high_availability/query_filter.cc b/src/hooks/dhcp/high_availability/query_filter.cc index 651b73df0b..e5c9b0c19e 100644 --- a/src/hooks/dhcp/high_availability/query_filter.cc +++ b/src/hooks/dhcp/high_availability/query_filter.cc @@ -47,6 +47,70 @@ std::array 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 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 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 diff --git a/src/hooks/dhcp/high_availability/query_filter.h b/src/hooks/dhcp/high_availability/query_filter.h index b50b6780f8..0094d247b8 100644 --- a/src/hooks/dhcp/high_availability/query_filter.h +++ b/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. /// 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 2e162d4f56..8f35f9b2c3 100644 --- a/src/hooks/dhcp/high_availability/tests/query_filter_unittest.cc +++ b/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(); +} + } diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index d971d22fb1..be7ef0aba8 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/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_DATA = 17, + DHCPV6_LEASEQUERY_DONE = 16, + DHCPV6_LEASEQUERY_DATA = 17, /* RFC 6977 */ -// DHCPV6_RECONFIGURE_REQUEST = 18, -// DHCPV6_RECONFIGURE_REPLY = 19, + DHCPV6_RECONFIGURE_REQUEST = 18, + DHCPV6_RECONFIGURE_REPLY = 19, /* RFC 7341 */ DHCPV6_DHCPV4_QUERY = 20, - DHCPV6_DHCPV4_RESPONSE = 21 + DHCPV6_DHCPV4_RESPONSE = 21, /* RFC 7653 */ -// DHCPV6_ACTIVELEASEQUERY = 22, -// DHCPV6_STARTTLS = 23, + DHCPV6_ACTIVELEASEQUERY = 22, + HCPV6_STARTTLS = 23, /* RFC 8156 */ -// DHCPV6_BNDUPD = 24, -// DHCPV6_BNDREPLY = 25, -// DHCPV6_POOLREQ = 26, -// DHCPV6_POOLRESP = 27, -// DHCPV6_UPDREQ = 28, -// DHCPV6_UPDREQALL = 29, -// DHCPV6_UPDDONE = 30, -// DHCPV6_CONNECT = 31, -// DHCPV6_CONNECTREPLY = 32, -// DHCPV6_DISCONNECT = 33, -// DHCPV6_STATE = 34, -// DHCPV6_CONTACT = 35 + DHCPV6_BNDUPD = 24, + DHCPV6_BNDREPLY = 25, + DHCPV6_POOLREQ = 26, + DHCPV6_POOLRESP = 27, + DHCPV6_UPDREQ = 28, + DHCPV6_UPDREQALL = 29, + DHCPV6_UPDDONE = 30, + DHCPV6_CONNECT = 31, + DHCPV6_CONNECTREPLY = 32, + DHCPV6_DISCONNECT = 33, + DHCPV6_STATE = 34, + DHCPV6_CONTACT = 35, + DHCPV6_TYPES_EOF }; extern const char *dhcpv6_type_names[]; -- 2.47.2