[#3141] addressed review comments

- Mostly further corrections in ARM docs and in comments.

diff --git a/doc/examples/kea4/all-options.json b/doc/examples/kea4/all-options.json
index 00cbbdbfdfbd87cc8a8b2fa0d75691a518006d95..f74dfcfad124577a303cb32f2e9e04e1665c5e82 100644 (file)
--- a/doc/examples/kea4/all-options.json
+++ b/doc/examples/kea4/all-options.json
@@ -1641,7 +1641,7 @@
         // For each DNR Instance comma delimited fields must be provided:
         // - service priority (mandatory),
         // - ADN (mandatory),
-        // - IP address/es (optional - if more than one - they must be space-separated)
+        // - IP address(es) (optional - if more than one - they must be space-separated)
         // - SvcParams (optional - if more than one - they must be space-separated;
         //   to provide more than one alpn-id separate them with double-backslash escaped comma like in the
         //   example below).

diff --git a/doc/examples/kea6/all-options.json b/doc/examples/kea6/all-options.json
index 57857dc47cc594c615dfb1f54b339311f4f4c57d..e3058f34e0a08b2f3827174ef89a1dc59cebc573 100644 (file)
--- a/doc/examples/kea6/all-options.json
+++ b/doc/examples/kea6/all-options.json
@@ -1820,7 +1820,7 @@
         // DNR option may be configured using convenient notation. Comma separated fields must be provided:
         // - service priority (mandatory),
         // - ADN (mandatory),
-        // - IP address/es (optional - if more than one - they must be space-separated)
+        // - IP address(es) (optional - if more than one - they must be space-separated)
         // - SvcParams (optional - if more than one - they must be space-separated;
         //   to provide more than one alpn-id separate them with double-backslash escaped comma like in the
         //   example below).

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index 4cbefbff8209c000953d16b1d84f1d54126ca0ff..eea264844b4421326a9d66b0280ba01706526fec 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -4731,7 +4731,7 @@ point.
 DNR (Discovery of Network-designated Resolvers) Options for DHCPv4
 ------------------------------------------------------------------
 
-One of the more recently added option is the Discovery of
+One of the more recently added options is the Discovery of
 Network-designated Resolvers or DNR option,
 introduced in `RFC 9463 <https://tools.ietf.org/html/rfc9463>`__. The goal of that RFC is
 to provide a way to communicate location of DNS resolvers available over means other than
@@ -4749,11 +4749,11 @@ For detailed example how to configure DNR option, see :ref:`dnr6-options`.
 The only difference for DNR DHCPv4 options configuration is that it allows
 to configure more than one DNR instance and the DNR instances are separated
 with the "pipe" (``0x7C``) character.
-For each DNR Instance comma delimited fields must be provided:
+For each DNR Instance comma delimited fields must be provided in the following order:
 
 - Service Priority (mandatory),
 - ADN FQDN (mandatory),
-- IP address/es (optional - if more than one - they must be space-separated)
+- IP address(es) (optional - if more than one - they must be space-separated)
 - SvcParams as a set of key=value pairs (optional - if more than one - they must be space-separated;
   to provide more than one alpn-id separate them with double backslash escaped comma like in the
   example below).

diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index 5d10f98ef3c94260db625a589133171c47d0cd63..f2f64de053c125031b6668acbc97d228d02a4f9d 100644 (file)
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -2070,7 +2070,7 @@ Parameters option.
 DNR (Discovery of Network-designated Resolvers) Options for DHCPv6
 ------------------------------------------------------------------
 
-One of the more recently added option is the Discovery of
+One of the more recently added options is the Discovery of
 Network-designated Resolvers or DNR option,
 introduced in `RFC 9463 <https://www.rfc-editor.org/rfc/rfc9463>`__. The goal of that RFC is
 to provide a way to communicate location of DNS resolvers available over means other than
@@ -2078,11 +2078,11 @@ the classic DNS over UDP port 53. At the time of this writing, the supported tec
 are DoT (DNS-over-TLS), DoH (DNS-over-HTTPS), and DoQ (DNS-over-QUIC), but the option was
 designed to be extensible to accommodate other protocols in the future.
 
-DNR option may be configured using convenient notation. Comma delimited fields must be provided:
+DNR option may be configured using convenient notation. Comma delimited fields must be provided in the following order:
 
 - Service Priority (mandatory),
 - ADN FQDN (mandatory),
-- IP address/es (optional - if more than one - they must be space-separated)
+- IP address(es) (optional - if more than one - they must be space-separated)
 - SvcParams as a set of key=value pairs (optional - if more than one - they must be space-separated;
   to provide more than one alpn-id separate them with double backslash escaped comma like in the
   example below).
@@ -2109,20 +2109,19 @@ The above option will be encoded on-wire as follows:
 
 ::
 
-        // 00 64 - service priority (100 in hex as unsigned 16 bit integer)
-        // 00 12 - length of the Authentication Domain Name (name of the resolver) FQDN (18 in hex as unsigned 16 bit integer)
-        // 04 64 6f 74 31 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 - 18 octets of the ADN FQDN
-        // 00 20 - 32 octets is the length of the following two IPv6 addresses
-        // 20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 01 - 2001:db8::1
-        // 20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 02 - 2001:db8::2
-        // Remaining part is to be interpreted as SvcParams field. In particular:
-        // 00 01 - next record is alpn
-        // 00 04 - length of the alpn SvcParamValue field (4 octets)
-        // 03    - length of the following alpn-id coded on one octet
-        // 64 6f 74 - "dot" - value of the alpn
-        // 00 03 - next record is port
-        // 00 02 - length of the SvcParamValue field is 2 octets
-        // 21 52 - the actual is 0x2152 or 8530 in decimal
+        00 64 - service priority (100 in hex as unsigned 16 bit integer)
+        00 12 - length of the Authentication Domain Name (name of the resolver) FQDN (18 in hex as unsigned 16 bit integer)
+        04 64 6f 74 31 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 - 18 octets of the ADN FQDN
+        00 20 - 32 octets is the length of the following two IPv6 addresses
+        20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 01 - 2001:db8::1
+        20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 02 - 2001:db8::2
+        00 01 - SvsParams begin - this is alpn SvcParamKey
+        00 04 - length of the alpn SvcParamValue field (4 octets)
+        03    - length of the following alpn-id coded on one octet
+        64 6f 74 - "dot" - value of the alpn
+        00 03 - this is port SvcParamKey
+        00 02 - length of the SvcParamValue field is 2 octets
+        21 52 - the actual value is 0x2152 or 8530 in decimal
 
 The following example shows how to configure more than one ``ALPN`` protocol in Service Parameters.
 The example specifies a resolver known as ``resolver.example`` that supports:
@@ -2144,26 +2143,25 @@ The above option will be encoded on-wire as follows:
 
 ::
 
-        // 00 96 - service priority (150 in hex as unsigned 16 bit integer)
-        // 00 12 - length of the Authentication Domain Name (name of the resolver) FQDN (18 in hex as unsigned 16 bit integer)
-        // 08 72 65 73 6f 6c 76 65 72 07 65 78 61 6d 70 6c 65 00 - 18 octets of the ADN FQDN
-        // 00 20 - 32 octets is the length of the following two IPv6 addresses
-        // 20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 01 - 2001:db8::1
-        // 20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 02 - 2001:db8::2
-        // Remaining part is to be interpreted as SvcParams field. In particular:
-        // 00 01 - next record is alpn
-        // 00 0e - length of the alpn SvcParamValue field (14 octets)
-        // 03    - length of the following alpn-id coded on one octet
-        // 64 6f 74 - "dot" - value of the alpn
-        // 03    - length of the following alpn-id coded on one octet
-        // 64 6f 71 - "doq" - value of the alpn
-        // 02    - length of the following alpn-id coded on one octet
-        // 68 32 - "h2" - value of the alpn "HTTP/2 over TLS"
-        // 02    - length of the following alpn-id coded on one octet
-        // 68 33 - "h3" - value of the alpn "HTTP/3"
-        // 00 07 - next record is dohpath
-        // 00 08 - length of the SvcParamValue field is 8 octets
-        // 2f 71 7b 3f 64 6e 73 7d - "/q{?dns}" dohpath
+        00 96 - service priority (150 in hex as unsigned 16 bit integer)
+        00 12 - length of the Authentication Domain Name (name of the resolver) FQDN (18 in hex as unsigned 16 bit integer)
+        08 72 65 73 6f 6c 76 65 72 07 65 78 61 6d 70 6c 65 00 - 18 octets of the ADN FQDN
+        00 20 - 32 octets is the length of the following two IPv6 addresses
+        20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 01 - 2001:db8::1
+        20 01 0d b8 00 00 00 00 00 00 00 00 00 00 00 02 - 2001:db8::2
+        00 01 - SvsParams begin - this is alpn SvcParamKey
+        00 0e - length of the alpn SvcParamValue field (14 octets)
+        03    - length of the following alpn-id coded on one octet
+        64 6f 74 - "dot" - value of the alpn
+        03    - length of the following alpn-id coded on one octet
+        64 6f 71 - "doq" - value of the alpn
+        02    - length of the following alpn-id coded on one octet
+        68 32 - "h2" - value of the alpn "HTTP/2 over TLS"
+        02    - length of the following alpn-id coded on one octet
+        68 33 - "h3" - value of the alpn "HTTP/3"
+        00 07 - this is dohpath SvcParamKey
+        00 08 - length of the SvcParamValue field is 8 octets
+        2f 71 7b 3f 64 6e 73 7d - "/q{?dns}" dohpath
 
 
 .. note::

diff --git a/src/lib/dhcp/option4_dnr.cc b/src/lib/dhcp/option4_dnr.cc
index f89b6c6265cf358bea10c06ba167ce4edad8ad62..03bcdcd1f1a4d121bf01572b12902ef251faca66 100644 (file)
--- a/src/lib/dhcp/option4_dnr.cc
+++ b/src/lib/dhcp/option4_dnr.cc
@@ -573,7 +573,7 @@ DnrInstance::parseDnrInstanceConfigData(const std::string& config_txt) {
     if (tokens.size() > 4) {
         isc_throw(BadValue, getLogPrefix() << "Option config supports maximum 4 comma separated "
                                            << "fields: Service Priority, ADN, resolver IP "
-                                           << "address/es and SvcParams");
+                                           << "address(es) and SvcParams");
     }
 
     // parse Service Priority
@@ -606,7 +606,7 @@ DnrInstance::parseDnrInstanceConfigData(const std::string& config_txt) {
     if (tokens.size() > 2) {
         setAdnOnlyMode(false);
 
-        // parse resolver IP address/es
+        // parse resolver IP address(es)
         std::string txt_addresses = str::trim(tokens[2]);
 
         parseIpAddresses(txt_addresses);
@@ -659,9 +659,6 @@ void
 DnrInstance::parseSvcParams(const std::string& txt_svc_params) {
     // SvcParamKey=SvcParamValue pairs are separated with space
     std::vector<std::string> svc_params_pairs = str::tokens(txt_svc_params, std::string(" "));
-    std::vector<std::string> alpn_ids_tokens;
-
-    OutputBuffer out_buf(2);
 
     for (auto const& svc_param_pair : svc_params_pairs) {
         std::vector<std::string> key_val_tokens = str::tokens(str::trim(svc_param_pair), "=");
@@ -760,6 +757,7 @@ DnrInstance::parseSvcParams(const std::string& txt_svc_params) {
     // (...)
     // SvcParamKeys SHALL appear in increasing numeric order.
     // Note that (...) there are no duplicate SvcParamKeys.
+    OutputBuffer out_buf(2);
 
     for (auto const& svc_param_key : SUPPORTED_SVC_PARAMS) {
         auto it = svc_params_map_.find(svc_param_key);

diff --git a/src/lib/dhcp/option4_dnr.h b/src/lib/dhcp/option4_dnr.h
index e8d78024c23769f36de0a83439398cd8389a009f..8acf11340e882fb73f2c25dc6ed2cb5e4574f8aa 100644 (file)
--- a/src/lib/dhcp/option4_dnr.h
+++ b/src/lib/dhcp/option4_dnr.h
@@ -214,7 +214,7 @@ public:
 
     /// @brief Setter of the @c dnr_instance_data_length_ field.
     ///
-    /// Size is calculated basing on set Service Priority, ADN, IP address/es and SvcParams.
+    /// Size is calculated basing on set Service Priority, ADN, IP address(es) and SvcParams.
     /// This should be called after all fields are set.
     /// This is only used for DHCPv4 Encrypted DNS %Option.
     void setDnrInstanceDataLength() {
@@ -433,9 +433,9 @@ private:
     ///                 @c svc_params_map_
     std::string svcParamValAsText(const std::pair<uint16_t, OpaqueDataTuple>& svc_param) const;
 
-    /// @brief Parses DNR resolver IP address/es from a piece of convenient notation option config.
+    /// @brief Parses DNR resolver IP address(es) from a piece of convenient notation option config.
     ///
-    /// @param txt_addresses a piece of convenient notation option config holding IP address/es
+    /// @param txt_addresses a piece of convenient notation option config holding IP address(es)
     ///
     /// @throw BadValue Thrown in case parser found wrong format of received string.
     void parseIpAddresses(const std::string& txt_addresses);