From: Francis Dupont Date: Mon, 23 Nov 2015 14:34:53 +0000 (+0100) Subject: [4097a] Optimized the no configured options cases X-Git-Tag: trac4204fd_base~2^2~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f397db49d4ea48e34b0d089dc0d15a3df6b0560e;p=thirdparty%2Fkea.git [4097a] Optimized the no configured options cases --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index ba520b55d4..04f6a01963 100644 --- 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 vendor_req = boost::dynamic_pointer_cast< OptionVendor>(ex.getQuery()->getOption(DHO_VIVSO_SUBOPTIONS)); @@ -944,7 +961,6 @@ Dhcpv4Srv::appendRequestedVendorOptions(Dhcpv4Exchange& ex) { for (std::vector::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 2fb2439a9d..3a3027aeb8 100644 --- 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 vendor_req = boost::dynamic_pointer_cast(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 6258db0e50..a6ecfbae59 100644 --- 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 2fea357acc..2872924a2a 100644 --- 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 68556b80cc..98285f9920 100644 --- 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 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 3f941c6a25..4ad0a31902 100644 --- 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;