[#3590] Finished required precedence update

changelog_unreleased/3590-required-precence

@@ -0,0 +1,6 @@
+[func]*		fdupont
+	Required classes are now evaluated in the same 	order as
+	for option data, i.e. (pd-)pool, subnet and shared network.
+	Before the order was reversed but this feature was not
+	used.
+	(Gitlab #3590)

doc/sphinx/arm/classify.rst

@@ -138,8 +138,8 @@ The classification process is conducted in several steps:
     callouts are called here.
 
 12. Classes marked as "required" are evaluated in the order in which
-    they are listed: first the shared network, then the subnet, and
+    they are listed: first pools, then the subnet, and finally
-    finally the pools that assigned resources belong to.
+    the shared network that assigned resources belong to.
 
 13. Options are assigned, again possibly based on the class information
     in the order that classes were associated with the incoming packet.
@@ -910,15 +910,16 @@ subnet, shared network, or pools are known but output-option processing has not
 yet been done. For this purpose, the ``only-if-required`` flag, which is
 ``false`` by default, allows the evaluation of the ``test`` expression or the
 ``template-test`` expression only when it is required, i.e. in a
-``require-client-classes`` list of the selected subnet, shared network, or pool.
+``require-client-classes`` list of the selected pool, subnet, or shared network.
 
-The ``require-client-classes`` list, which is valid for shared-network, subnet,
+The ``require-client-classes`` list, which is valid for pool, subnet,
-and pool scope, specifies the classes which are evaluated in the second pass
+and shared-network scope, specifies the classes which are evaluated in
-before output-option processing. The list is built in reverse-precedence
+the second pass before output-option processing. The list is built in
-order of the option data, i.e. an option data item in a subnet takes precedence over
+same precedence order of the option data, i.e. an option data item in
-one in a shared network, but a required class in a subnet is added after one in a
+a subnet takes precedence over one in a shared network, and also a
-shared network. The mechanism is related to the ``only-if-required`` flag but it
+required class in a subnet is added before one in a shared
-is not mandatory that the flag be set to ``true``.
+network. The mechanism is related to the ``only-if-required`` flag but
+it is not mandatory that the flag be set to ``true``.
 
 .. note ::
 

doc/sphinx/arm/dhcp4-srv.rst

@@ -3322,8 +3322,8 @@ DNS servers set to 192.0.2.1 and 192.0.2.2.
 Required Classification
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-In some cases it is useful to limit the scope of a class to a
+In some cases it is useful to limit the scope of a class to a pool,
-shared network, subnet, or pool. There are two parameters which are used
+subnet, or shared network. There are two parameters which are used
 to limit the scope of the class by instructing the server to evaluate test
 expressions when required.
 
@@ -3333,9 +3333,9 @@ is not evaluated at the reception of the incoming packet but later, and
 only if the class evaluation is required.
 
 The second is ``require-client-classes``, which takes a list of class
-names and is valid in shared-network, subnet, and pool scope. Classes in
+names and is valid in pool, subnet, and shared network scope. Classes in
 these lists are marked as required and evaluated after selection of this
-specific shared network/subnet/pool and before output-option processing.
+specific pool/subnet/shared network and before output-option processing.
 
 In this example, a class is assigned to the incoming packet when the
 specified subnet is used:
@@ -3370,9 +3370,9 @@ over ``option-data`` in a class. If ``option-data`` is moved to a
 required class and required in the subnet, a class evaluated earlier
 may take precedence.
 
-Required evaluation is also available at the shared-network and pool levels.
+Required evaluation is also available at the shared network and pool levels.
-The order in which required classes are considered is: shared-network,
+The order in which required classes are considered is: pool, subnet,
-subnet, and pool, i.e. in the reverse order from the way in which
+and shared network, i.e. in the same order from the way in which
 ``option-data`` is processed.
 
 .. note::

doc/sphinx/arm/dhcp6-srv.rst

@@ -3096,8 +3096,8 @@ eRouter1.0 client class are allowed to use that pool.
 Required Classification
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-In some cases it is useful to limit the scope of a class to a
+In some cases it is useful to limit the scope of a class to a pool,
-shared network, subnet, or pool. There are two parameters which are used
+subnet, or shared network. There are two parameters which are used
 to limit the scope of the class by instructing the server to evaluate test
 expressions when required.
 
@@ -3107,9 +3107,9 @@ is not evaluated at the reception of the incoming packet but later, and
 only if the class evaluation is required.
 
 The second is ``require-client-classes``, which takes a list of class
-names and is valid in shared-network, subnet, and pool scope. Classes in
+names and is valid in pool, subnet, and shared network scope. Classes in
 these lists are marked as required and evaluated after selection of this
-specific shared network/subnet/pool and before output-option processing.
+specific pool/subnet/shared network and before output-option processing.
 
 In this example, a class is assigned to the incoming packet when the
 specified subnet is used:
@@ -3148,9 +3148,9 @@ over ``option-data`` in a class. If ``option-data`` is moved to a
 required class and required in the subnet, a class evaluated earlier
 may take precedence.
 
-Required evaluation is also available at shared-network and pool/pd-pool
+Required evaluation is also available at shared network and pool/pd-pool
 levels. The order in which required classes are considered is:
-shared-network, subnet, and (pd-)pool, i.e. in the reverse order from the
+(pd-)pool, subnet, and shared network, i.e. in the same order from the
 way in which ``option-data`` is processed.
 
 .. _dhcp6-ddns-config:

src/bin/dhcp6/tests/classify_unittest.cc

@@ -2030,6 +2030,87 @@ TEST_F(ClassifyTest, precedencePool) {
     EXPECT_EQ("2001:db8:1::1", addrs[0].toText());
 }
 
+// This test checks the precedence order in required evaluation.
+// This order is: pools > subnet > shared-network
+TEST_F(ClassifyTest, precedencePdPool) {
+    std::string config =
+        "{"
+        "\"interfaces-config\": {"
+        "   \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"client-classes\": ["
+        "    {"
+        "       \"name\": \"for-pool\","
+        "       \"test\": \"member('ALL')\","
+        "       \"only-if-required\": true,"
+        "       \"option-data\": [ {"
+        "           \"name\": \"dns-servers\","
+        "           \"data\": \"2001:db8:1::1\""
+        "       } ]"
+        "    },"
+        "    {"
+        "       \"name\": \"for-subnet\","
+        "       \"test\": \"member('ALL')\","
+        "       \"only-if-required\": true,"
+        "       \"option-data\": [ {"
+        "           \"name\": \"dns-servers\","
+        "           \"data\": \"2001:db8:1::2\""
+        "       } ]"
+        "    },"
+        "    {"
+        "       \"name\": \"for-network\","
+        "       \"test\": \"member('ALL')\","
+        "       \"only-if-required\": true,"
+        "       \"option-data\": [ {"
+        "           \"name\": \"dns-servers\","
+        "           \"data\": \"2001:db8:1::3\""
+        "       } ]"
+        "    }"
+        "],"
+        "\"shared-networks\": [ {"
+        "    \"name\": \"frog\","
+        "    \"interface\": \"eth1\","
+        "    \"require-client-classes\": [ \"for-network\" ],"
+        "    \"subnet6\": [ { "
+        "        \"subnet\": \"2001:db8:1::/64\","
+        "        \"id\": 1,"
+        "        \"require-client-classes\": [ \"for-subnet\" ],"
+        "        \"pd-pools\": [ { "
+        "            \"prefix\": \"2001:db8:1::\","
+        "            \"prefix-len\": 48, \"delegated-len\": 64,"
+        "            \"require-client-classes\": [ \"for-pool\" ]"
+        "        } ]"
+        "    } ]"
+        "} ],"
+        "\"valid-lifetime\": 600"
+        "}";
+
+    // Create a client requesting dns-servers option
+    Dhcp6Client client;
+    client.setInterface("eth1");
+    client.requestPrefix(0xabca);
+    client.requestOption(D6O_NAME_SERVERS);
+
+    // Load the config and perform a SARR
+    configure(config, *client.getServer());
+    ASSERT_NO_THROW(client.doSARR());
+
+    // Check response
+    EXPECT_EQ(1, client.getLeaseNum());
+    Pkt6Ptr resp = client.getContext().response_;
+    ASSERT_TRUE(resp);
+
+    // Check dns-servers option
+    OptionPtr opt = resp->getOption(D6O_NAME_SERVERS);
+    ASSERT_TRUE(opt);
+    Option6AddrLstPtr servers =
+        boost::dynamic_pointer_cast<Option6AddrLst>(opt);
+    ASSERT_TRUE(servers);
+    auto addrs = servers->getAddresses();
+    ASSERT_EQ(1, addrs.size());
+    EXPECT_EQ("2001:db8:1::1", addrs[0].toText());
+}
+
 // This test checks the precedence order in required evaluation.
 // This order is: pools > subnet > shared-network
 TEST_F(ClassifyTest, precedenceSubnet) {