From bb784771295f2b68e5d89c6fdad9dce813a38f14 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 2 Oct 2024 15:15:15 +0200 Subject: [PATCH] [#3590] Finished required precedence update --- changelog_unreleased/3590-required-precence | 6 ++ doc/sphinx/arm/classify.rst | 21 +++--- doc/sphinx/arm/dhcp4-srv.rst | 14 ++-- doc/sphinx/arm/dhcp6-srv.rst | 12 +-- src/bin/dhcp6/tests/classify_unittest.cc | 81 +++++++++++++++++++++ 5 files changed, 111 insertions(+), 23 deletions(-) create mode 100644 changelog_unreleased/3590-required-precence diff --git a/changelog_unreleased/3590-required-precence b/changelog_unreleased/3590-required-precence new file mode 100644 index 0000000000..c9c09dbb3d --- /dev/null +++ b/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) diff --git a/doc/sphinx/arm/classify.rst b/doc/sphinx/arm/classify.rst index bf1f816015..3d60f63473 100644 --- a/doc/sphinx/arm/classify.rst +++ b/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 - finally the pools that assigned resources belong to. + they are listed: first pools, then the subnet, and finally + 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, -and pool scope, specifies the classes which are evaluated in the second pass -before output-option processing. The list is built in reverse-precedence -order of the option data, i.e. an option data item in a subnet takes precedence over -one in a shared network, but a required class in a subnet is added after one in a -shared network. The mechanism is related to the ``only-if-required`` flag but it -is not mandatory that the flag be set to ``true``. +The ``require-client-classes`` list, which is valid for pool, subnet, +and shared-network scope, specifies the classes which are evaluated in +the second pass before output-option processing. The list is built in +same precedence order of the option data, i.e. an option data item in +a subnet takes precedence over one in a shared network, and also a +required class in a subnet is added before one in a shared +network. The mechanism is related to the ``only-if-required`` flag but +it is not mandatory that the flag be set to ``true``. .. note :: diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index 87e00e51f8..f7568521e9 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/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 -shared network, subnet, or pool. There are two parameters which are used +In some cases it is useful to limit the scope of a class to a pool, +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. -The order in which required classes are considered is: shared-network, -subnet, and pool, i.e. in the reverse order from the way in which +Required evaluation is also available at the shared network and pool levels. +The order in which required classes are considered is: pool, subnet, +and shared network, i.e. in the same order from the way in which ``option-data`` is processed. .. note:: diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst index 4b21372adc..324ef3509e 100644 --- a/doc/sphinx/arm/dhcp6-srv.rst +++ b/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 -shared network, subnet, or pool. There are two parameters which are used +In some cases it is useful to limit the scope of a class to a pool, +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: diff --git a/src/bin/dhcp6/tests/classify_unittest.cc b/src/bin/dhcp6/tests/classify_unittest.cc index a9b3f2f053..2892076aba 100644 --- a/src/bin/dhcp6/tests/classify_unittest.cc +++ b/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(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) {