[3587] Addressed review comments.

src/bin/dhcp4/dhcp4_srv.cc

@@ -1550,7 +1550,7 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) const {
     selector.iface_name_ = question->getIface();
 
     CfgMgr& cfgmgr = CfgMgr::instance();
-    subnet = cfgmgr.getCurrentCfg()->getCfgSubnets4()->get(selector);
+    subnet = cfgmgr.getCurrentCfg()->getCfgSubnets4()->selectSubnet(selector);
 
     // Let's execute all callouts registered for subnet4_select
     if (HooksManager::calloutsPresent(hook_index_subnet4_select_)) {

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -237,7 +237,7 @@ public:
                         const uint16_t option_code,
                         const uint16_t expected_options_count = 1) {
         Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-            getCfgSubnets4()->get(subnet_address);
+            getCfgSubnets4()->selectSubnet(subnet_address);
         if (!subnet) {
             /// @todo replace toText() with the use of operator <<.
             ADD_FAILURE() << "A subnet for the specified address "
@@ -535,7 +535,7 @@ TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) {
     checkGlobalUint32("valid-lifetime", 4000);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_TRUE(subnet->getT1().unspecified());
     EXPECT_FALSE(subnet->getT2().unspecified());
@@ -569,7 +569,7 @@ TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) {
     checkGlobalUint32("valid-lifetime", 4000);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_FALSE(subnet->getT1().unspecified());
     EXPECT_EQ(1000, subnet->getT1());
@@ -604,7 +604,7 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
     // Now check if the configuration was indeed handled and we have
     // expected pool configured.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ(1000, subnet->getT1());
     EXPECT_EQ(2000, subnet->getT2());
@@ -917,7 +917,7 @@ TEST_F(Dhcp4ParserTest, nextServerGlobal) {
     // Now check if the configuration was indeed handled and we have
     // expected pool configured.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
 }
@@ -947,7 +947,7 @@ TEST_F(Dhcp4ParserTest, nextServerSubnet) {
     // Now check if the configuration was indeed handled and we have
     // expected pool configured.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
 }
@@ -1038,7 +1038,7 @@ TEST_F(Dhcp4ParserTest, nextServerOverride) {
     // Now check if the configuration was indeed handled and we have
     // expected pool configured.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
 }
@@ -1111,7 +1111,7 @@ TEST_F(Dhcp4ParserTest, subnetLocal) {
     checkResult(status, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ(1, subnet->getT1());
     EXPECT_EQ(2, subnet->getT2());
@@ -1223,7 +1223,7 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) {
     checkResult(status, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ(1000, subnet->getT1());
     EXPECT_EQ(2000, subnet->getT2());
@@ -1813,7 +1813,7 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
     checkResult(x, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     OptionContainerPtr options = subnet->getCfgOption()->getAll("dhcp4");
     ASSERT_EQ(2, options->size());
@@ -1899,7 +1899,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) {
 
     // Options should be now available for the subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     // Try to get the option from the space dhcp4.
     OptionDescriptor desc1 = subnet->getCfgOption()->get("dhcp4", 56);
@@ -2053,7 +2053,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
 
     // Get the subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.5"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
 
     // We should have one option available.
@@ -2120,7 +2120,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
     checkResult(x, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.24"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.24"));
     ASSERT_TRUE(subnet);
     OptionContainerPtr options = subnet->getCfgOption()->getAll("dhcp4");
     ASSERT_EQ(2, options->size());
@@ -2271,7 +2271,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
     checkResult(x, 0);
 
     Subnet4Ptr subnet1 = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.100"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.100"));
     ASSERT_TRUE(subnet1);
     OptionContainerPtr options1 = subnet1->getCfgOption()->getAll("dhcp4");
     ASSERT_EQ(1, options1->size());
@@ -2296,7 +2296,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
 
     // Test another subnet in the same way.
     Subnet4Ptr subnet2 = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.3.102"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.3.102"));
     ASSERT_TRUE(subnet2);
     OptionContainerPtr options2 = subnet2->getCfgOption()->getAll("dhcp4");
     ASSERT_EQ(1, options2->size());
@@ -2374,7 +2374,7 @@ TEST_F(Dhcp4ParserTest, optionDataLowerCase) {
     checkResult(x, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.5"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
     OptionContainerPtr options = subnet->getCfgOption()->getAll("dhcp4");
     ASSERT_EQ(1, options->size());
@@ -2417,7 +2417,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) {
     checkResult(x, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.5"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
     OptionContainerPtr options =
         subnet->getCfgOption()->getAll("dhcp4");
@@ -2625,7 +2625,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
 
     // Get the subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.5"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
 
     // We should have one option available.
@@ -2709,7 +2709,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsHex) {
 
     // Options should be now available for the subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.5"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
 
     // Try to get the option from the vendor space 4491
@@ -2770,7 +2770,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsCsv) {
 
     // Options should be now available for the subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.5"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
 
     // Try to get the option from the vendor space 4491
@@ -3146,7 +3146,7 @@ TEST_F(Dhcp4ParserTest, subnetRelayInfo) {
     checkResult(status, 0);
 
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
-        getCfgSubnets4()->get(IOAddress("192.0.2.200"));
+        getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     EXPECT_EQ("192.0.2.123", subnet->getRelayInfo().addr_.toText());
 }

src/bin/dhcp4/tests/direct_client_unittest.cc

@@ -227,7 +227,7 @@ TEST_F(DirectClientTest,  twoSubnets) {
     ASSERT_EQ(DHCPOFFER, response->getType());
     // Check that the offered address belongs to the suitable subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(response->getYiaddr());
+        getCfgSubnets4()->selectSubnet(response->getYiaddr());
     ASSERT_TRUE(subnet);
     EXPECT_EQ("10.0.0.0", subnet->get().first.toText());
 
@@ -239,7 +239,7 @@ TEST_F(DirectClientTest,  twoSubnets) {
     ASSERT_EQ(DHCPACK, response->getType());
     // Check that the offered address belongs to the suitable subnet.
     subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(response->getYiaddr());
+        getCfgSubnets4()->selectSubnet(response->getYiaddr());
     ASSERT_TRUE(subnet);
     EXPECT_EQ("192.0.2.0", subnet->get().first.toText());
 
@@ -286,7 +286,7 @@ TEST_F(DirectClientTest, oneSubnet) {
     ASSERT_EQ(DHCPOFFER, response->getType());
     // Check that the offered address belongs to the suitable subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(response->getYiaddr());
+        getCfgSubnets4()->selectSubnet(response->getYiaddr());
     ASSERT_TRUE(subnet);
     EXPECT_EQ("10.0.0.0", subnet->get().first.toText());
 
@@ -305,7 +305,7 @@ TEST_F(DirectClientTest, renew) {
     // Make sure that the subnet has been really added. Also, the subnet
     // will be needed to create a lease for a client.
     Subnet4Ptr subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(IOAddress("10.0.0.10"));
+        getCfgSubnets4()->selectSubnet(IOAddress("10.0.0.10"));
     // Create a lease for a client that we will later renewed. By explicitly
     // creating a lease we will get to know the lease parameters, such as
     // leased address etc.
@@ -342,7 +342,7 @@ TEST_F(DirectClientTest, renew) {
     ASSERT_EQ(DHCPACK, response->getType());
     // Check that the offered address belongs to the suitable subnet.
     subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(response->getYiaddr());
+        getCfgSubnets4()->selectSubnet(response->getYiaddr());
     ASSERT_TRUE(subnet);
     EXPECT_EQ("10.0.0.0", subnet->get().first.toText());
 
@@ -364,7 +364,7 @@ TEST_F(DirectClientTest, rebind) {
     // Make sure that the subnet has been really added. Also, the subnet
     // will be needed to create a lease for a client.
     Subnet4Ptr subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(IOAddress("10.0.0.10"));
+        getCfgSubnets4()->selectSubnet(IOAddress("10.0.0.10"));
     // Create a lease, which will be later renewed. By explicitly creating a
     // lease we will know the lease parameters, such as leased address etc.
     const uint8_t hwaddr[] = { 1, 2, 3, 4, 5, 6 };
@@ -408,7 +408,7 @@ TEST_F(DirectClientTest, rebind) {
     EXPECT_EQ(5678, response->getTransid());
     // Check that the offered address belongs to the suitable subnet.
     subnet = CfgMgr::instance().getCurrentCfg()->
-        getCfgSubnets4()->get(response->getYiaddr());
+        getCfgSubnets4()->selectSubnet(response->getYiaddr());
     ASSERT_TRUE(subnet);
     EXPECT_EQ("10.0.0.0", subnet->get().first.toText());
 

src/lib/dhcpsrv/cfg_subnets4.cc

@@ -15,22 +15,17 @@
 #include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfg_subnets4.h>
 #include <dhcpsrv/dhcpsrv_log.h>
+#include <dhcpsrv/subnet_id.h>
 
 using namespace isc::asiolink;
 
 namespace {
 
-/// @brief Returns @c IOAddress object set to "0.0.0.0".
-const IOAddress& ZERO_ADDRESS() {
-    static IOAddress address("0.0.0.0");
-    return (address);
-}
+/// @brief Holds IPv4 address set to "0.0.0.0".
+const IOAddress ZERO_ADDRESS("0.0.0.0");
 
-/// @brief Returns @c IOAddress object holding broadcast address.
-const IOAddress& BCAST_ADDRESS() {
-    static IOAddress address("255.255.255.255");
-    return (address);
-}
+/// @brief Holds IPv4 broadcast address.
+const IOAddress BCAST_ADDRESS("255.255.255.255");
 
 } // end of anonymous namespace
 
@@ -38,8 +33,8 @@ namespace isc {
 namespace dhcp {
 
 CfgSubnets4::Selector::Selector()
-    : ciaddr_(ZERO_ADDRESS()), giaddr_(ZERO_ADDRESS()),
-      local_address_(ZERO_ADDRESS()), remote_address_(ZERO_ADDRESS()),
+    : ciaddr_(ZERO_ADDRESS), giaddr_(ZERO_ADDRESS),
+      local_address_(ZERO_ADDRESS), remote_address_(ZERO_ADDRESS),
       client_classes_(ClientClasses()), iface_name_(std::string()) {
 }
 
@@ -48,7 +43,7 @@ CfgSubnets4::add(const Subnet4Ptr& subnet) {
     /// @todo: Check that this new subnet does not cross boundaries of any
     /// other already defined subnet.
     if (isDuplicate(*subnet)) {
-        isc_throw(isc::dhcp::DuplicateSubnet4ID, "ID of the new IPv4 subnet '"
+        isc_throw(isc::dhcp::DuplicateSubnetID, "ID of the new IPv4 subnet '"
                   << subnet->getID() << "' is already in use");
     }
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_SUBNET4)
@@ -57,22 +52,23 @@ CfgSubnets4::add(const Subnet4Ptr& subnet) {
 }
 
 Subnet4Ptr
-CfgSubnets4::get(const Selector& selector) const {
+CfgSubnets4::selectSubnet(const Selector& selector) const {
     // If relayed message has been received, try to match the giaddr with the
     // relay address specified for a subnet. It is also possible that the relay
     // address will not match with any of the relay addresses accross all
     // subnets, but we need to verify that for all subnets before we can try
     // to use the giaddr to match with the subnet prefix.
-    if (selector.giaddr_ != ZERO_ADDRESS()) {
+    if (selector.giaddr_ != ZERO_ADDRESS) {
         for (Subnet4Collection::const_iterator subnet = subnets_.begin();
              subnet != subnets_.end(); ++subnet) {
-            // Eliminate those subnets that do not meet client class criteria.
-            if (!(*subnet)->clientSupported(selector.client_classes_)) {
+
+            // Check if the giaddr is equal to the one defined for the subnet.
+            if (selector.giaddr_ != (*subnet)->getRelayInfo().addr_) {
                 continue;
             }
 
-            // Check if the giaddr is equal to the one defined for the subnet.
-            if (selector.giaddr_ == (*subnet)->getRelayInfo().addr_) {
+            // Eliminate those subnets that do not meet client class criteria.
+            if ((*subnet)->clientSupported(selector.client_classes_)) {
                 return (*subnet);
             }
         }
@@ -83,19 +79,19 @@ CfgSubnets4::get(const Selector& selector) const {
     // what address from the client's packet to use to match with the
     // subnets' prefixes.
 
-    IOAddress address = ZERO_ADDRESS();
+    IOAddress address = ZERO_ADDRESS;
     // If there is a giaddr, use it for subnet selection.
-    if (selector.giaddr_ != ZERO_ADDRESS()) {
+    if (selector.giaddr_ != ZERO_ADDRESS) {
         address = selector.giaddr_;
 
     // If it is a Renew or Rebind, use the ciaddr.
-    } else if ((selector.ciaddr_ != ZERO_ADDRESS()) &&
-               (selector.local_address_ != BCAST_ADDRESS())) {
+    } else if ((selector.ciaddr_ != ZERO_ADDRESS) &&
+               (selector.local_address_ != BCAST_ADDRESS)) {
         address = selector.ciaddr_;
 
     // If ciaddr is not specified, use the source address.
-    } else if ((selector.remote_address_ != ZERO_ADDRESS()) &&
-               (selector.local_address_ != BCAST_ADDRESS())) {
+    } else if ((selector.remote_address_ != ZERO_ADDRESS) &&
+               (selector.local_address_ != BCAST_ADDRESS)) {
         address = selector.remote_address_;
 
     // If local interface name is known, use the local address on this
@@ -113,28 +109,28 @@ CfgSubnets4::get(const Selector& selector) const {
     }
 
     // Unable to find a suitable address to use for subnet selection.
-    if (address == ZERO_ADDRESS()) {
+    if (address == ZERO_ADDRESS) {
         return (Subnet4Ptr());
     }
 
     // We have identified an address in the client's packet that can be
     // used for subnet selection. Match this packet with the subnets. 
-    return (get(address, selector.client_classes_));
+    return (selectSubnet(address, selector.client_classes_));
 }
 
 Subnet4Ptr
-CfgSubnets4::get(const IOAddress& address,
+CfgSubnets4::selectSubnet(const IOAddress& address,
                  const ClientClasses& client_classes) const {
     for (Subnet4Collection::const_iterator subnet = subnets_.begin();
          subnet != subnets_.end(); ++subnet) {
 
-        // Eliminate those subnets that do not meet client class criteria.
-        if (!(*subnet)->clientSupported(client_classes)) {
+        // Address is in range for the subnet prefix, so return it.
+        if (!(*subnet)->inRange(address)) {
             continue;
         }
 
-        // Address is in range for the subnet prefix, so return it.
-        if ((*subnet)->inRange(address)) {
+        // Eliminate those subnets that do not meet client class criteria.
+        if ((*subnet)->clientSupported(client_classes)) {
             return (*subnet);
         }
     }

src/lib/dhcpsrv/cfg_subnets4.h

@@ -24,14 +24,6 @@
 namespace isc {
 namespace dhcp {
 
-/// @brief Exception thrown upon attempt to add subnet with an ID that belongs
-/// to the subnet that already exists.
-class DuplicateSubnet4ID : public Exception {
-public:
-    DuplicateSubnet4ID(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) { };
-};
-
 /// @brief Holds subnets configured for the DHCPv4 server.
 ///
 /// This class holds a collection of subnets configured for the DHCPv4 server.
@@ -73,7 +65,7 @@ public:
     ///
     /// @param subnet Pointer to the subnet being added.
     ///
-    /// @throw isc::DuplicateSubnet4ID If the subnet id for the new subnet
+    /// @throw isc::DuplicateSubnetID If the subnet id for the new subnet
     /// duplicates id of an existing subnet.
     void add(const Subnet4Ptr& subnet);
 
@@ -94,14 +86,14 @@ public:
     /// parameters extracted from the client's message using the following
     /// logic.
     ///
-    /// If the giaddr value is found it means that the client's message was
-    /// relayed. The subnet configuration allows for setting the relay address
-    /// for each subnet to indicate that the subnet must be assigned when the
-    /// packet was transmitted over the particular relay. This method first
-    /// tries to match the giaddr with the relay addresses specified for
-    /// all subnets. If the relay address for the subnet is equal to the address
-    /// of the relay through which the message was transmitted, the particular
-    /// subnet is returned.
+    /// If the giaddr value is set in the selector it means that the client's
+    /// message was relayed. The subnet configuration allows for setting the
+    /// relay address for each subnet to indicate that the subnet must be
+    /// assigned when the packet was transmitted over the particular relay.
+    /// This method first tries to match the giaddr with the relay addresses
+    /// specified for all subnets. If the relay address for the subnet is equal
+    /// to the address of the relay through which the message was transmitted,
+    /// the particular subnet is returned.
     ///
     /// If the giaddr is not matched with any of the relay addresses in any
     /// subnet or the message was not relayed, the method will need to try to
@@ -124,7 +116,7 @@ public:
     /// @return Pointer to the selected subnet or NULL if no subnet found.
     /// @throw isc::BadValue if the values in the subnet selector are invalid
     /// or they are insufficient to select a subnet.
-    Subnet4Ptr get(const Selector& selector) const;
+    Subnet4Ptr selectSubnet(const Selector& selector) const;
 
     /// @brief Returns pointer to a subnet if provided address is in its range.
     ///
@@ -137,8 +129,9 @@ public:
     /// the client belongs to.
     ///
     /// @return Pointer to the selected subnet or NULL if no subnet found.
-    Subnet4Ptr get(const asiolink::IOAddress& address,
-                   const ClientClasses& client_classes = ClientClasses()) const;
+    Subnet4Ptr selectSubnet(const asiolink::IOAddress& address,
+                            const ClientClasses& client_classes
+                            = ClientClasses()) const;
 
 private:
 

src/lib/dhcpsrv/cfgmgr.cc

@@ -17,6 +17,7 @@
 #include <dhcp/libdhcp++.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
+#include <dhcpsrv/subnet_id.h>
 #include <string>
 
 using namespace isc::asiolink;

src/lib/dhcpsrv/cfgmgr.h

@@ -46,14 +46,6 @@ public:
         isc::Exception(file, line, what) { };
 };
 
-/// @brief Exception thrown upon attempt to add subnet with an ID that belongs
-/// to the subnet that already exists.
-class DuplicateSubnetID : public Exception {
-public:
-    DuplicateSubnetID(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) { };
-};
-
 /// @brief Configuration Manager
 ///
 /// This singleton class holds the whole configuration for DHCPv4 and DHCPv6

src/lib/dhcpsrv/subnet_id.h

@@ -15,6 +15,9 @@
 #ifndef SUBNET_ID_H
 #define SUBNET_ID_H
 
+#include <exceptions/exceptions.h>
+#include <stdint.h>
+
 namespace isc {
 namespace dhcp {
 
@@ -26,6 +29,14 @@ namespace dhcp {
 /// type.
 typedef uint32_t SubnetID;
 
+/// @brief Exception thrown upon attempt to add subnet with an ID that belongs
+/// to the subnet that already exists.
+class DuplicateSubnetID : public Exception {
+public:
+    DuplicateSubnetID(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 }
 }
 

src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc

@@ -17,6 +17,7 @@
 #include <dhcp/tests/iface_mgr_test_config.h>
 #include <dhcpsrv/cfg_subnets4.h>
 #include <dhcpsrv/subnet.h>
+#include <dhcpsrv/subnet_id.h>
 #include <gtest/gtest.h>
 
 using namespace isc;
@@ -41,12 +42,12 @@ TEST(CfgSubnets4Test, getSubnetByCiaddr) {
     selector.ciaddr_ = IOAddress("192.0.2.0");
     // Set some unicast local address to simulate a Renew.
     selector.local_address_ = IOAddress("10.0.0.100");
-    ASSERT_FALSE(cfg.get(selector));
+    ASSERT_FALSE(cfg.selectSubnet(selector));
 
     // Add one subnet and make sure it is returned.
     cfg.add(subnet1);
     selector.ciaddr_ = IOAddress("192.0.2.63");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
 
     // Add all other subnets.
     cfg.add(subnet2);
@@ -54,16 +55,16 @@ TEST(CfgSubnets4Test, getSubnetByCiaddr) {
 
     // Make sure they are returned for the appropriate addresses.
     selector.ciaddr_ = IOAddress("192.0.2.15");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.85");
-    EXPECT_EQ(subnet2, cfg.get(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.191");
-    EXPECT_EQ(subnet3, cfg.get(selector));
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 
     // Also, make sure that the NULL pointer is returned if the subnet
     // cannot be found.
     selector.ciaddr_ = IOAddress("192.0.2.192");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 }
 
 
@@ -87,11 +88,11 @@ TEST(CfgSubnets4Test, getSubnetByClasses) {
     selector.local_address_ = IOAddress("10.0.0.10");
 
     selector.ciaddr_ = IOAddress("192.0.2.5");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.70");
-    EXPECT_EQ(subnet2, cfg.get(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.130");
-    EXPECT_EQ(subnet3, cfg.get(selector));
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 
     ClientClasses client_classes;
     client_classes.insert("bar");
@@ -100,11 +101,11 @@ TEST(CfgSubnets4Test, getSubnetByClasses) {
     // There are no class restrictions defined, so everything should work
     // as before
     selector.ciaddr_ = IOAddress("192.0.2.5");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.70");
-    EXPECT_EQ(subnet2, cfg.get(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.130");
-    EXPECT_EQ(subnet3, cfg.get(selector));
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 
     // Now let's add client class restrictions.
     subnet1->allowClientClass("foo"); // Serve here only clients from foo class
@@ -114,31 +115,31 @@ TEST(CfgSubnets4Test, getSubnetByClasses) {
     // The same check as above should result in client being served only in
     // bar class, i.e. subnet2.
     selector.ciaddr_ = IOAddress("192.0.2.5");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.70");
-    EXPECT_EQ(subnet2, cfg.get(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.130");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 
     // Now let's check that client with wrong class is not supported.
     client_classes.clear();
     client_classes.insert("some_other_class");
     selector.client_classes_ = client_classes;
     selector.ciaddr_ = IOAddress("192.0.2.5");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.70");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.130");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 
     // Finally, let's check that client without any classes is not supported.
     client_classes.clear();
     selector.ciaddr_ = IOAddress("192.0.2.5");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.70");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.ciaddr_ = IOAddress("192.0.2.130");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 }
 
 // This test verifies that the relay information can be used to retrieve the
@@ -160,11 +161,11 @@ TEST(CfgSubnetsTest, getSubnetByRelayAddress) {
 
     // Check that without relay-info specified, subnets are not selected
     selector.giaddr_ = IOAddress("10.0.0.1");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.giaddr_ = IOAddress("10.0.0.2");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.giaddr_ = IOAddress("10.0.0.3");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 
     // Now specify relay info
     subnet1->setRelayInfo(IOAddress("10.0.0.1"));
@@ -173,11 +174,11 @@ TEST(CfgSubnetsTest, getSubnetByRelayAddress) {
 
     // And try again. This time relay-info is there and should match.
     selector.giaddr_ = IOAddress("10.0.0.1");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
     selector.giaddr_ = IOAddress("10.0.0.2");
-    EXPECT_EQ(subnet2, cfg.get(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
     selector.giaddr_ = IOAddress("10.0.0.3");
-    EXPECT_EQ(subnet3, cfg.get(selector));
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 }
 
 // This test verifies that the subnet can be selected for the client
@@ -195,12 +196,12 @@ TEST(CfgSubnetsTest, getSubnetNoCiaddr) {
     selector.remote_address_ = IOAddress("192.0.2.0");
     // Set some unicast local address to simulate a Renew.
     selector.local_address_ = IOAddress("10.0.0.100");
-    ASSERT_FALSE(cfg.get(selector));
+    ASSERT_FALSE(cfg.selectSubnet(selector));
 
     // Add one subnet and make sure it is returned.
     cfg.add(subnet1);
     selector.remote_address_ = IOAddress("192.0.2.63");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
 
     // Add all other subnets.
     cfg.add(subnet2);
@@ -208,21 +209,25 @@ TEST(CfgSubnetsTest, getSubnetNoCiaddr) {
 
     // Make sure they are returned for the appropriate addresses.
     selector.remote_address_ = IOAddress("192.0.2.15");
-    EXPECT_EQ(subnet1, cfg.get(selector));
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
     selector.remote_address_ = IOAddress("192.0.2.85");
-    EXPECT_EQ(subnet2, cfg.get(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
     selector.remote_address_ = IOAddress("192.0.2.191");
-    EXPECT_EQ(subnet3, cfg.get(selector));
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 
     // Also, make sure that the NULL pointer is returned if the subnet
     // cannot be found.
     selector.remote_address_ = IOAddress("192.0.2.192");
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 }
 
 // This test verifies that the subnet can be selected using an address
 // set on the local interface.
 TEST(CfgSubnetsTest, getSubnetInterface) {
+    // The IfaceMgrTestConfig object initializes fake interfaces:
+    // eth0, eth1 and lo on the configuration manager. The CfgSubnets4
+    // object uses addresses assigned to these fake interfaces to
+    // select the appropriate subnet.
     IfaceMgrTestConfig config(true);
 
     CfgSubnets4 cfg;
@@ -231,9 +236,9 @@ TEST(CfgSubnetsTest, getSubnetInterface) {
     // Initially, there are no subnets configured, so none of the IPv4
     // addresses assigned to eth0 and eth1 can match with any subnet.
     selector.iface_name_ = "eth0";
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
     selector.iface_name_ = "eth1";
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 
     // Configure first subnet which address on eth0 corresponds to.
     Subnet4Ptr subnet1(new Subnet4(IOAddress("10.0.0.1"), 24, 1, 2, 3));
@@ -241,12 +246,12 @@ TEST(CfgSubnetsTest, getSubnetInterface) {
 
     // The address on eth0 should match the existing subnet.
     selector.iface_name_ = "eth0";
-    Subnet4Ptr subnet1_ret = cfg.get(selector);
+    Subnet4Ptr subnet1_ret = cfg.selectSubnet(selector);
     ASSERT_TRUE(subnet1_ret);
     EXPECT_EQ(subnet1->get().first, subnet1_ret->get().first);
     // There should still be no match for eth1.
     selector.iface_name_ = "eth1";
-    EXPECT_FALSE(cfg.get(selector));
+    EXPECT_FALSE(cfg.selectSubnet(selector));
 
     // Configure a second subnet.
     Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.1"), 24, 1, 2, 3));
@@ -255,17 +260,17 @@ TEST(CfgSubnetsTest, getSubnetInterface) {
     // There should be match between eth0 and subnet1 and between eth1 and
     // subnet 2.
     selector.iface_name_ = "eth0";
-    subnet1_ret = cfg.get(selector);
+    subnet1_ret = cfg.selectSubnet(selector);
     ASSERT_TRUE(subnet1_ret);
     EXPECT_EQ(subnet1->get().first, subnet1_ret->get().first);
     selector.iface_name_ = "eth1";
-    Subnet4Ptr subnet2_ret = cfg.get(selector);
+    Subnet4Ptr subnet2_ret = cfg.selectSubnet(selector);
     ASSERT_TRUE(subnet2_ret);
     EXPECT_EQ(subnet2->get().first, subnet2_ret->get().first);
 
     // This function throws an exception if the name of the interface is wrong.
     selector.iface_name_ = "bogus-interface";
-    EXPECT_THROW(cfg.get(selector), isc::BadValue);
+    EXPECT_THROW(cfg.selectSubnet(selector), isc::BadValue);
 }
 
 // Checks that detection of duplicated subnet IDs works as expected. It should
@@ -280,7 +285,7 @@ TEST(CfgSubnets4, duplication) {
     ASSERT_NO_THROW(cfg.add(subnet1));
     EXPECT_NO_THROW(cfg.add(subnet2));
     // Subnet 3 has the same ID as subnet 1. It shouldn't be able to add it.
-    EXPECT_THROW(cfg.add(subnet3), isc::dhcp::DuplicateSubnet4ID);
+    EXPECT_THROW(cfg.add(subnet3), isc::dhcp::DuplicateSubnetID);
 }
 
 

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -14,11 +14,12 @@
 
 #include <config.h>
 
-#include <dhcpsrv/cfgmgr.h>
-#include <dhcpsrv/dhcp_parsers.h>
 #include <exceptions/exceptions.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/tests/iface_mgr_test_config.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/dhcp_parsers.h>
+#include <dhcpsrv/subnet_id.h>
 
 #include <gtest/gtest.h>
 

src/lib/util/optional_value.h

@@ -113,7 +113,7 @@ public:
     ///
     /// @return true if the value is unspecified or unequal.
     bool operator!=(const T& value) const {
-        return (!specified_ || (value_ != value));
+        return (!operator==(value));
     }
 
     /// @brief Type cast operator.