[4097a] Optimized the no configured options cases

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index ba520b55d49ea04bc13a2050bdc6fb4ca07cf585..04f6a019639747125c5daa6b8a6a16e0744cdd11 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -827,7 +827,7 @@ Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) {
 
     // First subnet configured options
     Subnet4Ptr subnet = ex.getContext()->subnet_;
-    if (subnet) {
+    if (subnet && !subnet->getCfgOption()->empty()) {
         co_list.push_back(subnet->getCfgOption());
     }
 
@@ -845,11 +845,17 @@ Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) {
                 .arg(*cclass);
             continue;
         }
+        if (ccdef->getCfgOption()->empty()) {
+            // Skip classes which don't configure options
+            continue;
+        }
         co_list.push_back(ccdef->getCfgOption());
     }
 
     // Last global options
-    co_list.push_back(CfgMgr::instance().getCurrentCfg()->getCfgOption());
+    if (!CfgMgr::instance().getCurrentCfg()->getCfgOption()->empty()) {
+        co_list.push_back(CfgMgr::instance().getCurrentCfg()->getCfgOption());
+    }
 }
 
 void
@@ -865,6 +871,12 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
         return;
     }
 
+    // Unlikely short cut
+    const CfgOptionList& co_list = ex.getCfgOptionList();
+    if (co_list.empty()) {
+        return;
+    }
+
     Pkt4Ptr query = ex.getQuery();
 
     // try to get the 'Parameter Request List' option which holds the
@@ -888,7 +900,6 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
         // Add nothing when it is already there
         if (!resp->getOption(*opt)) {
             // Iterate on the configured option list
-            const CfgOptionList& co_list = ex.getCfgOptionList();
             for (CfgOptionList::const_iterator copts = co_list.begin();
                  copts != co_list.end(); ++copts) {
                 OptionDescriptor desc = (*copts)->get("dhcp4", *opt);
@@ -915,6 +926,12 @@ Dhcpv4Srv::appendRequestedVendorOptions(Dhcpv4Exchange& ex) {
         return;
     }
 
+    // Unlikely short cut
+    const CfgOptionList& co_list = ex.getCfgOptionList();
+    if (co_list.empty()) {
+        return;
+    }
+
     // Try to get the vendor option
     boost::shared_ptr<OptionVendor> vendor_req = boost::dynamic_pointer_cast<
         OptionVendor>(ex.getQuery()->getOption(DHO_VIVSO_SUBOPTIONS));
@@ -944,7 +961,6 @@ Dhcpv4Srv::appendRequestedVendorOptions(Dhcpv4Exchange& ex) {
     for (std::vector<uint8_t>::const_iterator code = requested_opts.begin();
          code != requested_opts.end(); ++code) {
         if  (!vendor_rsp->getOption(*code)) {
-            const CfgOptionList& co_list = ex.getCfgOptionList();
             for (CfgOptionList::const_iterator copts = co_list.begin();
                  copts != co_list.end(); ++copts) {
                 OptionDescriptor desc = (*copts)->get(vendor_id, *code);
@@ -981,6 +997,12 @@ Dhcpv4Srv::appendBasicOptions(Dhcpv4Exchange& ex) {
         return;
     }
 
+    // Unlikely short cut
+    const CfgOptionList& co_list = ex.getCfgOptionList();
+    if (co_list.empty()) {
+        return;
+    }
+
     Pkt4Ptr resp = ex.getResponse();
 
     // Try to find all 'required' options in the outgoing
@@ -989,7 +1011,6 @@ Dhcpv4Srv::appendBasicOptions(Dhcpv4Exchange& ex) {
         OptionPtr opt = resp->getOption(required_options[i]);
         if (!opt) {
             // Check whether option has been configured.
-            const CfgOptionList& co_list = ex.getCfgOptionList();
             for (CfgOptionList::const_iterator copts = co_list.begin();
                  copts != co_list.end(); ++copts) {
                 OptionDescriptor desc = (*copts)->get("dhcp4", required_options[i]);

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 2fb2439a9d5d0e5f8a6af867d9c3fd912a04cd7d..3a3027aeb8a12704ff15b2a927e92ece1c3687d4 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -895,7 +895,7 @@ Dhcpv6Srv::buildCfgOptionList(const Pkt6Ptr& question,
                               AllocEngine::ClientContext6& ctx,
                               CfgOptionList& co_list) {
     // First subnet configured options
-    if (ctx.subnet_) {
+    if (ctx.subnet_ && !ctx.subnet_->getCfgOption()->empty()) {
         co_list.push_back(ctx.subnet_->getCfgOption());
     }
 
@@ -912,11 +912,17 @@ Dhcpv6Srv::buildCfgOptionList(const Pkt6Ptr& question,
                 .arg(*cclass);
             continue;
         }
+        if (ccdef->getCfgOption()->empty()) {
+            // Skip classes which don't configure options
+            continue;
+        }
         co_list.push_back(ccdef->getCfgOption());
     }
 
     // Last global options
-    co_list.push_back(CfgMgr::instance().getCurrentCfg()->getCfgOption());
+    if (!CfgMgr::instance().getCurrentCfg()->getCfgOption()->empty()) {
+        co_list.push_back(CfgMgr::instance().getCurrentCfg()->getCfgOption());
+    }
 }
 
 void
@@ -930,7 +936,7 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer,
         (question->getOption(D6O_ORO));
 
     // Option ORO not found? We're done here then.
-    if (!option_oro) {
+    if (!option_oro || co_list.empty()) {
         return;
     }
 
@@ -968,7 +974,7 @@ Dhcpv6Srv::appendRequestedVendorOptions(const Pkt6Ptr& question,
     // Try to get the vendor option
     boost::shared_ptr<OptionVendor> vendor_req =
         boost::dynamic_pointer_cast<OptionVendor>(question->getOption(D6O_VENDOR_OPTS));
-    if (!vendor_req) {
+    if (!vendor_req || co_list.empty()) {
         return;
     }
 

diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc
index 6258db0e508ac4901f5191e321fb41ea8b7cee7b..a6ecfbae598ea9c43b6cb6caf0f2a8200f337908 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option.cc
+++ b/src/lib/dhcpsrv/cfg_option.cc
@@ -31,6 +31,11 @@ OptionDescriptor::equals(const OptionDescriptor& other) const {
 CfgOption::CfgOption() {
 }
 
+bool
+CfgOption::empty() const {
+    return (options_.empty() && vendor_options_.empty());
+}
+
 bool
 CfgOption::equals(const CfgOption& other) const {
     return (options_.equals(other.options_) &&

diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h
index 2fea357acc8a9f3d803f684633909245254b626e..2872924a2aeebd0cf3e807aea41b3680e3b1bb05 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option.h
+++ b/src/lib/dhcpsrv/cfg_option.h
@@ -202,6 +202,11 @@ public:
     /// @brief default constructor
     CfgOption();
 
+    /// @brief Indicates the object is empty
+    ///
+    /// @return true when the object is empty
+    bool empty() const;
+
     /// @name Methods and operators used for comparing objects.
     ///
     //@{

diff --git a/src/lib/dhcpsrv/option_space_container.h b/src/lib/dhcpsrv/option_space_container.h
index 68556b80ccfbfce3d65749e7d036e2afafb334e2..98285f9920a946b680dc5d1ba88944b8571d8bad 100644 (file)
--- a/src/lib/dhcpsrv/option_space_container.h
+++ b/src/lib/dhcpsrv/option_space_container.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -39,6 +39,13 @@ public:
     /// Pointer to the container.
     typedef boost::shared_ptr<ContainerType> ItemsContainerPtr;
 
+    /// @brief Indicates the container is empty
+    ///
+    /// @return true when the confainer is empty
+    bool empty() const {
+        return (option_space_map_.empty());
+    }
+
     /// @brief Adds a new item to the option_space.
     ///
     /// @param item reference to the item being added.

diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc
index 3f941c6a255470b0fdd080dffc34bce8efcb63f6..4ad0a31902d1f11a9c16560a9bd368dd3726000d 100644 (file)
--- a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc
@@ -28,6 +28,27 @@ using namespace isc::dhcp;
 
 namespace {
 
+// This test verifies the empty predicate.
+TEST(CfgOptionTest, empty) {
+    CfgOption cfg1;
+    CfgOption cfg2;
+
+    // Initially the option configurations should be empty.
+    ASSERT_TRUE(cfg1.empty());
+    ASSERT_TRUE(cfg2.empty());
+
+    // Add an option to each configuration
+    OptionPtr option(new Option(Option::V6, 1));
+    ASSERT_NO_THROW(cfg1.add(option, false, "dhcp6"));
+    ASSERT_NO_THROW(cfg2.add(option, true, "isc"));
+
+    // The first option configuration has an option
+    ASSERT_FALSE(cfg1.empty());
+
+    // The second option configuration has a vendor option
+    ASSERT_FALSE(cfg2.empty());
+}
+
 // This test verifies that the option configurations can be compared.
 TEST(CfgOptionTest, equals) {
     CfgOption cfg1;