]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#1387] Addressed comments
authorFrancis Dupont <fdupont@isc.org>
Tue, 3 Sep 2024 14:15:49 +0000 (16:15 +0200)
committerFrancis Dupont <fdupont@isc.org>
Wed, 4 Sep 2024 13:10:46 +0000 (15:10 +0200)
doc/sphinx/arm/dhcp6-srv.rst
src/bin/dhcp6/dhcp6_srv.h
src/bin/dhcp6/tests/sarr_unittest.cc
src/lib/dhcpsrv/parsers/host_reservation_parser.cc
src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc
src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc

index b561028c364030c4357bc64abcb564f9db806c2f..61b124f364cbe46f95e1905495f7de8a34f88a31 100644 (file)
@@ -4273,17 +4273,16 @@ another.
 
 .. note::
 
-   Since Kea 2.7.1, a reserved (so delegated) prefix can be associated
-   with a single prefix to exclude as for prefix delegation pools
-   :ref:`pd-exclude-option`. The host reservation syntax is extended
-   by a new entry ``excluded-prefixes`` which when is present must have
-   the same size as the ``prefixes`` entry: both contains strings
-   representing IPv6 prefixes (e.g. ``2001:db8::/48``). Each element of
-   the ``excluded-prefixes`` must be either the empty string or match
-   the prefix at the same position in the ``prefixes`` list, e.g.
-   ``2001:db8:0:1::/64`` matches / can be associated with ``2001:db8::/48``.
-   An empty ``excluded-prefixes`` list or a list with only empty strings
-   can be omitted (and will be omitted when produced by Kea).
+   Beginning with Kea 2.7.3, the host reservation syntax supports
+   a new entry, ``excluded-prefixes``. It can be used to specify
+   prefixes the client should exclude from delegated prefixes.
+   When present it must have the same number of elements as the
+   ``prefixes`` entry. Both entries contain strings representing IPv6
+   prefixes. Each element of the ``excluded-prefixes`` must be either
+   an empty string or match the prefix at the same position in
+   ``prefixes``. An empty ``excluded-prefixes`` list or a list with
+   only empty strings can be omitted. An example which excludes
+   ``2001:db8:0:1::/64`` from ``2001:db8::/48`` is shown below:
 
 ::
 
@@ -4303,10 +4302,11 @@ another.
 
 .. note::
 
-   Host reservations have precedence over prefix pools so when a reserved
-   prefix without an excluded prefix is assigned no pd-exclude option
-   is added to the prefix option even the prefix is in a configured
-   prefix pool with an excluded prefix (different from previous behavior).
+   Since host reservations have precedence over prefix pools, a reserved
+   prefix without an excluded prefix will not add a pd-exclude option
+   to the prefix option even if the delegated prefix is in a configured
+   prefix pool that does specify an excluded prefix (different from
+   previous behavior).
 
 .. _reservation6-conflict:
 
index 3ef6e9cff4c3621a3c23801814842eca50dd90b0..c0da468e411d5289dc12d35a1f28a9b9e75e48c0 100644 (file)
@@ -1064,6 +1064,7 @@ protected:
     ///
     /// @param ctx client context (contains subnet and hosts).
     /// @param lease lease (contains address/prefix and prefix length).
+    /// @param the prefix exclude option or null.
     OptionPtr getPDExclude(const AllocEngine::ClientContext6& ctx,
                            const Lease6Ptr& lease);
 
index 074b54857d89adce95fb183df5950ad642640848..f5e81e41ebbb03eab97cdb53e83651447e019551 100644 (file)
@@ -419,12 +419,16 @@ public:
     /// @brief This test verifies that it is possible to specify an excluded
     /// prefix (RFC 6603) and send it back to the client requesting prefix
     /// delegation using a pool.
-    void directClientExcludedPrefixPool();
+    ///
+    /// @param request_pdx request pd exclude option.
+    void directClientExcludedPrefixPool(bool request_pdx);
 
     /// @brief This test verifies that it is possible to specify an excluded
     /// prefix (RFC 6603) and send it back to the client requesting prefix
     /// delegation using a reservation.
-    void directClientExcludedPrefixHost();
+    ///
+    /// @param request_pdx request pd exclude option.
+    void directClientExcludedPrefixHost(bool request_pdx);
 
     /// @brief Check that when the client includes the Rapid Commit option in
     /// its Solicit, the server responds with Reply and commits the lease.
@@ -682,11 +686,14 @@ TEST_F(SARRTest, optionsInheritanceMultiThreading) {
 }
 
 void
-SARRTest::directClientExcludedPrefixPool() {
+SARRTest::directClientExcludedPrefixPool(bool request_pdx) {
     Dhcp6Client client;
     // Configure client to request IA_PD.
     client.requestPrefix();
-    client.requestOption(D6O_PD_EXCLUDE);
+    // Request pd exclude option when wanted.
+    if (request_pdx) {
+        client.requestOption(D6O_PD_EXCLUDE);
+    }
     configure(CONFIGS[3], *client.getServer());
     // Make sure we ended-up having expected number of subnets configured.
     const Subnet6Collection* subnets = CfgMgr::instance().getCurrentCfg()->
@@ -713,6 +720,10 @@ SARRTest::directClientExcludedPrefixPool() {
     Option6IAPrefixPtr pd_option = boost::dynamic_pointer_cast<Option6IAPrefix>(option);
     ASSERT_TRUE(pd_option);
     option = pd_option->getOption(D6O_PD_EXCLUDE);
+    if (!request_pdx) {
+        EXPECT_FALSE(option);
+        return;
+    }
     ASSERT_TRUE(option);
     Option6PDExcludePtr pd_exclude = boost::dynamic_pointer_cast<Option6PDExclude>(option);
     ASSERT_TRUE(pd_exclude);
@@ -723,22 +734,35 @@ SARRTest::directClientExcludedPrefixPool() {
 
 TEST_F(SARRTest, directClientExcludedPrefixPool) {
     Dhcpv6SrvMTTestGuard guard(*this, false);
-    directClientExcludedPrefixPool();
+    directClientExcludedPrefixPool(true);
 }
 
 TEST_F(SARRTest, directClientExcludedPrefixPoolMultiThreading) {
     Dhcpv6SrvMTTestGuard guard(*this, true);
-    directClientExcludedPrefixPool();
+    directClientExcludedPrefixPool(true);
+}
+
+TEST_F(SARRTest, directClientExcludedPrefixPoolNoOro) {
+    Dhcpv6SrvMTTestGuard guard(*this, false);
+    directClientExcludedPrefixPool(false);
+}
+
+TEST_F(SARRTest, directClientExcludedPrefixPoolNoOroMultiThreading) {
+    Dhcpv6SrvMTTestGuard guard(*this, true);
+    directClientExcludedPrefixPool(false);
 }
 
 void
-SARRTest::directClientExcludedPrefixHost() {
+SARRTest::directClientExcludedPrefixHost(bool request_pdx) {
     Dhcp6Client client;
     // Set DUID matching the one used to create host reservations.
     client.setDUID("01:02:03:05");
     // Configure client to request IA_PD.
     client.requestPrefix();
-    client.requestOption(D6O_PD_EXCLUDE);
+    // Request pd exclude option when wanted.
+    if (request_pdx) {
+        client.requestOption(D6O_PD_EXCLUDE);
+    }
     configure(CONFIGS[8], *client.getServer());
     // Make sure we ended-up having expected number of subnets configured.
     const Subnet6Collection* subnets = CfgMgr::instance().getCurrentCfg()->
@@ -765,6 +789,10 @@ SARRTest::directClientExcludedPrefixHost() {
     Option6IAPrefixPtr pd_option = boost::dynamic_pointer_cast<Option6IAPrefix>(option);
     ASSERT_TRUE(pd_option);
     option = pd_option->getOption(D6O_PD_EXCLUDE);
+    if (!request_pdx) {
+        EXPECT_FALSE(option);
+        return;
+    }
     ASSERT_TRUE(option);
     Option6PDExcludePtr pd_exclude = boost::dynamic_pointer_cast<Option6PDExclude>(option);
     ASSERT_TRUE(pd_exclude);
@@ -775,12 +803,22 @@ SARRTest::directClientExcludedPrefixHost() {
 
 TEST_F(SARRTest, directClientExcludedPrefixHost) {
     Dhcpv6SrvMTTestGuard guard(*this, false);
-    directClientExcludedPrefixHost();
+    directClientExcludedPrefixHost(true);
 }
 
 TEST_F(SARRTest, directClientExcludedPrefixHostMultiThreading) {
     Dhcpv6SrvMTTestGuard guard(*this, true);
-    directClientExcludedPrefixHost();
+    directClientExcludedPrefixHost(true);
+}
+
+TEST_F(SARRTest, directClientExcludedPrefixHostNoOro) {
+    Dhcpv6SrvMTTestGuard guard(*this, false);
+    directClientExcludedPrefixHost(false);
+}
+
+TEST_F(SARRTest, directClientExcludedPrefixHostNoOroMultiThreading) {
+    Dhcpv6SrvMTTestGuard guard(*this, true);
+    directClientExcludedPrefixHost(false);
 }
 
 void
index bf2185e41ae2a09b17bb0fc2aaf52a0ac61071ad..c7bb9ff6c2bf198f8f0f36733607f446df0e6be8 100644 (file)
@@ -381,6 +381,8 @@ HostReservationParser6::parseInternal(const SubnetID& subnet_id,
                     uint8_t excluded_prefix_len(0);
                     parsePrefix(exclude, excluded_prefix, excluded_prefix_len,
                                 "exclude prefix");
+                    // setPDExclude calls the Option6PDExclude constructor
+                    // which throws on invalid prefix.
                     res.setPDExclude(excluded_prefix, excluded_prefix_len);
                 }
                 host->addReservation(res);
index 115d323b868c454b02f4983fc4f0d335afac822a..0815ed82d2a60c11670c82710fc4970d4045efe5 100644 (file)
@@ -968,6 +968,24 @@ TEST_F(HostReservationParserTest, dhcp6ExcludedTooLong) {
     testInvalidConfig<HostReservationParser6>(config);
 }
 
+// This test verifies that the configuration parser throws an exception
+// when the excluded prefix is invalid (prefixes do not match).
+TEST_F(HostReservationParserTest, dhcp6ExcludedPrefixNotMatch) {
+    std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
+        "\"prefixes\": [ \"2001:db8::/48\" ],"
+        "\"excluded-prefixes\": [ \"2001:db9:0:1::/64\" ] }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
+// This test verifies that the configuration parser throws an exception
+// when the excluded prefix is invalid (bad length).
+TEST_F(HostReservationParserTest, dhcp6ExcludedPrefixBadLength) {
+    std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
+        "\"prefixes\": [ \"2001:db8::/48\" ],"
+        "\"excluded-prefixes\": [ \"2001:db8::/48\" ] }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
 // This test verifies that the configuration parser throws an exception
 // when invalid prefix length type is specified.
 TEST_F(HostReservationParserTest, dhcp6InvalidPrefixLengthType) {
index 361b3fb9f3f037f77f250219551a352ebfe15ecc..0e99bc8ce2c8b88232b077d67789209745482ece 100644 (file)
@@ -1531,19 +1531,8 @@ GenericHostDataSourceTest::testPrefixExclude(std::string prefix,
     ASSERT_TRUE(from_hds);
 
     HostDataSourceUtils::compareHosts(host, from_hds);
-
-#if 0
-    // Verify the test is meaningful.
-    HostPtr host2 = HostDataSourceUtils::initializeHost6(prefix,
-                                                         "2001:db8:0:2::",
-                                                         Host::IDENT_DUID,
-                                                         false);
-    host2->setIPv4SubnetID(host->getIPv4SubnetID());
-    host2->setIPv6SubnetID(host->getIPv6SubnetID());
-    ASSERT_TRUE(host2);
-    HostDataSourceUtils::compareHosts(host, host2);
-#endif
 }
+
 void
 GenericHostDataSourceTest::testMultipleSubnets(int subnets,
                                                const Host::IdentifierType& id) {