From: Marcin Siodelski <marcin@isc.org>
Date: Wed, 12 Oct 2016 16:27:04 +0000 (+0200)
Subject: [5022] Addressed review comments.
X-Git-Tag: trac4631a_base~3^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=145c33f79f9b626b41f8233ac40532e1bc742660;p=thirdparty%2Fkea.git

[5022] Addressed review comments.

Removed a todo, updated example with multiple options and added
item descriptions in the DHCPv6 spec file.
---

diff --git a/doc/examples/kea6/multiple-options.json b/doc/examples/kea6/multiple-options.json
index bf7984c511..a35cb85f64 100644
--- a/doc/examples/kea6/multiple-options.json
+++ b/doc/examples/kea6/multiple-options.json
@@ -31,11 +31,16 @@
 # Defining a subnet. There are 2 DHCP options returned to the
 # clients connected to this subnet. The first option is identified
 # by the name. The second option is identified by the code.
+# There are two address pools defined within this subnet. Pool
+# specific value for option 12 is defined for the pool:
+# 2001:db8:1::1 - 2001:db8:1::100. Clients obtaining an address
+# from this pool will be assigned option 12 with a value of
+# 3001:cafe::21. Clients belonging to this subnet but obtaining
+# addresses from the other pool, or the clients obtaining
+# stateless configuration will be assigned subnet specific value
+# of option 12, i.e. 2001:db8:1:0:ff00::1.
   "subnet6": [
     {
-      "pools": [ { "pool": "2001:db8:1::/80" } ],
-      "subnet": "2001:db8:1::/64",
-      "interface": "ethX",
       "option-data": [
         {
             "name": "dns-servers",
@@ -45,7 +50,23 @@
             "code": 12,
             "data": "2001:db8:1:0:ff00::1"
         },
-      ]
+      ],
+      "pools": [
+        {
+            "pool": "2001:db8:1::1 - 2001:db8:1::100",
+            "option-data": [
+              {
+                  "code": 12,
+                  "data": "3001:cafe::21"
+              }
+            ]
+        },
+        {
+            "pool": "2001:db8:1::500 - 2001:db8:2::1000"
+        }
+      ],
+      "subnet": "2001:db8:1::/64",
+      "interface": "ethX",
     }
   ]
 },
diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec
index 0c3a74df72..6800fc25c4 100644
--- a/src/bin/dhcp6/dhcp6.spec
+++ b/src/bin/dhcp6/dhcp6.spec
@@ -462,39 +462,46 @@
                         "item_type": "list",
                         "item_optional": false,
                         "item_default": [],
+                        "item_description": "Holds a list of pool specific DHCP options.",
                         "list_item_spec":
                         {
                             "item_name": "single-option-data",
                             "item_type": "map",
                             "item_optional": false,
                             "item_default": {},
+                            "item_description": "One of the pool specific DHCP options.",
                             "map_item_spec": [
                                 {
                                     "item_name": "name",
                                     "item_type": "string",
                                     "item_optional": false,
-                                    "item_default": ""
+                                    "item_default": "",
+                                    "item_description": "Option name."
                                 },
 
                                 { "item_name": "code",
                                   "item_type": "integer",
                                   "item_optional": false,
-                                  "item_default": 0
+                                  "item_default": 0,
+                                  "item_description": "Option code."
                                 },
                                 { "item_name": "data",
                                   "item_type": "string",
                                   "item_optional": false,
-                                  "item_default": ""
+                                  "item_default": "",
+                                  "item_description": "Option value."
                                 },
                                 { "item_name": "csv-format",
                                   "item_type": "boolean",
                                   "item_optional": false,
-                                  "item_default": false
+                                  "item_default": false,
+                                  "item_description": "Indicates if option value is specified as comma separated values."
                                 },
                                 { "item_name": "space",
                                   "item_type": "string",
                                   "item_optional": false,
-                                  "item_default": "dhcp6"
+                                  "item_default": "dhcp6",
+                                  "item_description": "Option space."
                                 } ]
                         }
                       } ]
@@ -558,39 +565,47 @@
                            "item_type": "list",
                            "item_optional": false,
                            "item_default": [],
+                           "item_description": "Holds a list of the pd-pool specific DHCP options.",
                            "list_item_spec":
                            {
                                "item_name": "single-option-data",
                                "item_type": "map",
                                "item_optional": false,
                                "item_default": {},
+                               "item_description": "One of the pd-pool specific DHCP options.",
                                "map_item_spec": [
                                 {
                                     "item_name": "name",
                                     "item_type": "string",
                                     "item_optional": false,
-                                    "item_default": ""
+                                    "item_default": "",
+                                    "item_description": "Option name."
                                 },
 
                                 { "item_name": "code",
                                   "item_type": "integer",
                                   "item_optional": false,
-                                  "item_default": 0
+                                  "item_default": 0,
+                                  "item_description": "Option code."
+
                                 },
                                 { "item_name": "data",
                                   "item_type": "string",
                                   "item_optional": false,
-                                  "item_default": ""
+                                  "item_default": "",
+                                  "item_description": "Option value."
                                 },
                                 { "item_name": "csv-format",
                                   "item_type": "boolean",
                                   "item_optional": false,
-                                  "item_default": false
+                                  "item_default": false,
+                                  "item_description": "Indicates if option value is specified as comma separated values."
                                 },
                                 { "item_name": "space",
                                   "item_type": "string",
                                   "item_optional": false,
-                                  "item_default": "dhcp6"
+                                  "item_default": "dhcp6",
+                                  "item_description": "Option space."
                                 } ]
                            }
                        } ]
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 85205c0b77..31cdcfab2b 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -825,10 +825,6 @@ Dhcpv6Srv::buildCfgOptionList(const Pkt6Ptr& question,
     if (ctx.subnet_) {
         BOOST_FOREACH(const AllocEngine::ResourceType& resource,
                       ctx.allocated_resources_) {
-            /// @todo This is has significant performance implications. We
-            /// are performing full scan of pools within this subnet to
-            /// find the one we're interested in. We need to implement the
-            /// Patricia trie based storage for pools.
             PoolPtr pool = ctx.subnet_->getPool(resource.second == 128 ?
                                                 Lease::TYPE_NA : Lease::TYPE_PD,
                                                 resource.first, false);