[#2866] Changes after review

- ChangeLog edits
- ARM edits in sections about subnet ids
- Renamed warning message
- Added subnet id to default configurations

ChangeLog

@@ -1,9 +1,8 @@
 2157.	[func]*		fdupont
-	As the first step to remove the unnumbered / auto-generate
+	Added a warning issued when the configuration contains subnets
-	subnet identifier feature added a warning issued when a
+	without explicit subnet identifiers (without "id" entry). The
-	subnet is configured with a zero or without an identifer
+	auto-generated subnet identifiers are now deprecated.
-	("id" entry).
+	(Gitlab #2866)
-	(Gitlab 2866)
 
 2156.	[func]		razvan
 	Added an optional 'pool-id' parameter that can be used to enable

doc/sphinx/arm/dhcp4-srv.rst

@@ -1175,10 +1175,11 @@ IPv4 Subnet Identifier
 
 The subnet identifier (subnet ID) is a unique number associated with a particular
 subnet. In principle, it is used to associate clients' leases with their
-respective subnets. When a subnet identifier is not specified for a
+respective subnets. The server configuration should contain unique and stable
-subnet being configured, it is automatically assigned by the
+identifiers for all subnets. When a subnet identifier is not specified for a
-configuration mechanism. The identifiers are assigned starting at 1 and are
+subnet, it is automatically assigned by the configuration mechanism. The identifiers
-monotonically increased for each subsequent subnet: 1, 2, 3, ....
+are assigned starting at 1 and are monotonically increased for each subsequent
+subnet: 1, 2, 3, ....
 
 If there are multiple subnets configured with auto-generated identifiers
 and one of them is removed, the subnet identifiers may be renumbered.
@@ -1186,8 +1187,14 @@ For example: if there are four subnets and the third is removed, the
 last subnet will be assigned the identifier that the third subnet had
 before removal. As a result, the leases stored in the lease database for
 subnet 3 are now associated with subnet 4, something that may have
-unexpected consequences. The only remedy for this issue at present is to
+unexpected consequences. It is one of the reasons why auto-generated subnet
-manually specify a unique identifier for each subnet.
+identifiers are deprecated starting from Kea version 2.4.0.
+
+.. note::
+
+   The auto-generation of the subnet identifiers will be removed in a future
+   release. Starting from Kea 2.4.0, a subnet without an ``id`` entry
+   or with the zero value raises a warning at the configuration time.
 
 .. note::
 
@@ -1212,12 +1219,6 @@ This identifier will not change for this subnet unless the ``id``
 parameter is removed or set to 0. The value of 0 forces auto-generation
 of the subnet identifier.
 
-.. note::
-
-   The auto-generation of subnet identifier will be removed in a future
-   release. From 2.4.0 a subnet without an ``id`` entry or with the zero
-   value raises a warning at configuration time.
-
 .. _ipv4-subnet-prefix:
 
 IPv4 Subnet Prefix

doc/sphinx/arm/dhcp6-srv.rst

@@ -956,10 +956,11 @@ IPv6 Subnet Identifier
 
 The subnet identifier (subnet ID) is a unique number associated with a particular
 subnet. In principle, it is used to associate clients' leases with their
-respective subnets. When a subnet identifier is not specified for a
+respective subnets. The server configuration should contain unique and stable
-subnet being configured, it is automatically assigned by the
+identifiers for all subnets. When a subnet identifier is not specified for a
-configuration mechanism. The identifiers are assigned starting at 1 and are
+subnet, it is automatically assigned by the configuration mechanism. The identifiers
-monotonically increased for each subsequent subnet: 1, 2, 3, ....
+are assigned starting at 1 and are monotonically increased for each subsequent
+subnet: 1, 2, 3, ....
 
 If there are multiple subnets configured with auto-generated identifiers
 and one of them is removed, the subnet identifiers may be renumbered.
@@ -967,8 +968,14 @@ For example: if there are four subnets and the third is removed, the
 last subnet will be assigned the identifier that the third subnet had
 before removal. As a result, the leases stored in the lease database for
 subnet 3 are now associated with subnet 4, something that may have
-unexpected consequences. The only remedy for this issue at present is to
+unexpected consequences. It is one of the reasons why auto-generated subnet
-manually specify a unique identifier for each subnet.
+identifiers are deprecated starting from Kea version 2.4.0.
+
+.. note::
+
+   The auto-generation of the subnet identifiers will be removed in a future
+   release. Starting from Kea 2.4.0, a subnet without an ``id`` entry
+   or with the zero value raises a warning at the configuration time.
 
 .. note::
 
@@ -993,12 +1000,6 @@ This identifier will not change for this subnet unless the ``id``
 parameter is removed or set to 0. The value of 0 forces auto-generation
 of the subnet identifier.
 
-.. note::
-
-   The auto-generation of subnet identifier will be removed in a future
-   release. From 2.4.0 a subnet without an ``id`` entry or with the zero
-   value raises a warning at configuration time.
-
 .. _ipv6-subnet-prefix:
 
 IPv6 Subnet Prefix

src/bin/keactrl/kea-dhcp4.conf.pre

@@ -288,8 +288,12 @@
         {
             // This defines the whole subnet. Kea will use this information to
             // determine where the clients are connected. This is the whole
-            // subnet in your network. This is mandatory parameter for each
+            // subnet in your network.
-            // subnet.
+
+            // Subnet identifier should be unique for each subnet.
+            "id": 1,
+
+            // This is mandatory parameter for each subnet.
             "subnet": "192.0.2.0/24",
 
             // Pools define the actual part of your subnet that is governed

src/bin/keactrl/kea-dhcp6.conf.pre

@@ -234,8 +234,12 @@
         {
             // This defines the whole subnet. Kea will use this information to
             // determine where the clients are connected. This is the whole
-            // subnet in your network. This is mandatory parameter for each
+            // subnet in your network.
-            // subnet.
+
+            // Subnet identifier should be unique for each subnet.
+            "id": 1,
+
+            // This is mandatory parameter for each subnet.
             "subnet": "2001:db8:1::/64",
 
             // Pools define the actual part of your subnet that is governed

src/lib/dhcpsrv/dhcpsrv_messages.cc

@@ -54,6 +54,7 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS = "DHCPSRV_CFGMGR_US
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR = "DHCPSRV_CFGMGR_USE_ALLOCATOR";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST = "DHCPSRV_CFGMGR_USE_UNICAST";
 extern const isc::log::MessageID DHCPSRV_CLOSE_DB = "DHCPSRV_CLOSE_DB";
+extern const isc::log::MessageID DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID = "DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID";
 extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL = "DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL";
 extern const isc::log::MessageID DHCPSRV_DEPRECATED = "DHCPSRV_DEPRECATED";
 extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET = "DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET";
@@ -279,7 +280,6 @@ extern const isc::log::MessageID DHCPSRV_TIMERMGR_UNREGISTER_ALL_TIMERS = "DHCPS
 extern const isc::log::MessageID DHCPSRV_TIMERMGR_UNREGISTER_TIMER = "DHCPSRV_TIMERMGR_UNREGISTER_TIMER";
 extern const isc::log::MessageID DHCPSRV_UNEXPECTED_NAME = "DHCPSRV_UNEXPECTED_NAME";
 extern const isc::log::MessageID DHCPSRV_UNKNOWN_DB = "DHCPSRV_UNKNOWN_DB";
-extern const isc::log::MessageID DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET = "DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET";
 
 } // namespace dhcp
 } // namespace isc
@@ -334,6 +334,7 @@ const char* values[] = {
     "DHCPSRV_CFGMGR_USE_ALLOCATOR", "using the %1 allocator for %2 leases in subnet %3",
     "DHCPSRV_CFGMGR_USE_UNICAST", "listening on unicast address %1, on interface %2",
     "DHCPSRV_CLOSE_DB", "closing currently open %1 database",
+    "DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID", "a subnet was configured without an id: %1",
     "DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL", "ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it",
     "DHCPSRV_DEPRECATED", "This configuration is using a deprecated feature: %1",
     "DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET", "received bad DHCPv4o6 packet: %1",
@@ -559,7 +560,6 @@ const char* values[] = {
     "DHCPSRV_TIMERMGR_UNREGISTER_TIMER", "unregistering timer: %1",
     "DHCPSRV_UNEXPECTED_NAME", "database access parameters passed through '%1', expected 'lease-database'",
     "DHCPSRV_UNKNOWN_DB", "unknown database type: %1",
-    "DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET", "a subnet was configured unnumbered: %1",
     NULL
 };
 

src/lib/dhcpsrv/dhcpsrv_messages.h

@@ -55,6 +55,7 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST;
 extern const isc::log::MessageID DHCPSRV_CLOSE_DB;
+extern const isc::log::MessageID DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID;
 extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL;
 extern const isc::log::MessageID DHCPSRV_DEPRECATED;
 extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET;
@@ -280,7 +281,6 @@ extern const isc::log::MessageID DHCPSRV_TIMERMGR_UNREGISTER_ALL_TIMERS;
 extern const isc::log::MessageID DHCPSRV_TIMERMGR_UNREGISTER_TIMER;
 extern const isc::log::MessageID DHCPSRV_UNEXPECTED_NAME;
 extern const isc::log::MessageID DHCPSRV_UNKNOWN_DB;
-extern const isc::log::MessageID DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET;
 
 } // namespace dhcp
 } // namespace isc

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -280,6 +280,13 @@ the database access parameters are changed: in the latter case, the
 server closes the currently open database, and opens a database using
 the new parameters.
 
+% DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID a subnet was configured without an id: %1
+A warning message issued when a subnet was configured with a zero or without
+an id, causing the server to auto-generate it. Using auto-generated subnet
+ids is now deprecated. Each configured subnet should have an explicit subnet id
+specified with the "id" entry. The sole argument of this warning message contains
+a subnet prefix.
+
 % DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it
 A debug message issued when the DDNS TTL value calculated using the
 ddns-ttl-percent is zero.  Kea will ignore the value and calculate
@@ -1393,6 +1400,3 @@ indicate an error in the source code, please submit a bug report.
 The database access string specified a database type (given in the
 message) that is unknown to the software. This is a configuration error.
 
-% DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET a subnet was configured unnumbered: %1
-A warning message issued when a subnet was configured with a zero or without
-an id. The prefix of the subnet is displayed.

src/lib/dhcpsrv/subnet.cc

@@ -75,7 +75,7 @@ Subnet::Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len,
       shared_network_name_() {
     if ((id == 0) && (id_ == 1)) {
         // Emit a warning on the first auto-numbered subnet.
-        LOG_WARN(dhcpsrv_logger, DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET)
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID)
             .arg(toText());
     }
     if ((prefix.isV6() && len > 128) ||

src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -2035,8 +2035,8 @@ TEST_F(SubnetIdTest, unnumbered) {
     EXPECT_EQ("2001:db8:1::/64", subnet3->toText());
 
     // Subnet 1 and 3 are logged.
-    std::string msg = "DHCPSRV_UNNUMBERED_CONFIGURED_SUBNET ";
+    std::string msg = "DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID ";
-    msg += "a subnet was configured unnumbered: ";
+    msg += "a subnet was configured without an id: ";
     addString(msg + subnet1->toText());
     addString(msg + subnet3->toText());
     EXPECT_TRUE(checkFile());