Parcourir la source

[2315] Group options held under Subnet object by option space name.

Marcin Siodelski il y a 12 ans
Parent
commit
5530bff261

+ 6 - 4
src/bin/dhcp4/config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -894,7 +894,7 @@ public:
             subnet->addPool4(*it);
         }
 
-        const Subnet::OptionContainer& options = subnet->getOptions();
+        const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
         const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
 
         // Add subnet specific options.
@@ -904,7 +904,7 @@ public:
                 LOG_WARN(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE)
                     .arg(desc.option->getType()).arg(addr.toText());
             }
-            subnet->addOption(desc.option);
+            subnet->addOption(desc.option, false, "dhcp4");
         }
 
         // Check all global options and add them to the subnet object if
@@ -914,6 +914,8 @@ public:
         BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
             // Get all options specified locally in the subnet and having
             // code equal to global option's code.
+            const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
+            const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
             Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             // @todo: In the future we will be searching for options using either
             // an option code or namespace. Currently we have only the option
@@ -924,7 +926,7 @@ public:
             // want to issue a warning about dropping the configuration of
             // a global option if one already exsists.
             if (std::distance(range.first, range.second) == 0) {
-                subnet->addOption(desc.option);
+                subnet->addOption(desc.option, false, "dhcp4");
             }
         }
 

+ 6 - 6
src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -461,7 +461,7 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
 
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
     ASSERT_EQ(2, options.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -529,7 +529,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
 
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.24"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
     ASSERT_EQ(2, options.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -594,7 +594,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
 
     Subnet4Ptr subnet1 = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.100"));
     ASSERT_TRUE(subnet1);
-    const Subnet::OptionContainer& options1 = subnet1->getOptions();
+    const Subnet::OptionContainer& options1 = subnet1->getOptions("dhcp4");
     ASSERT_EQ(1, options1.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -618,7 +618,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
     // Test another subnet in the same way.
     Subnet4Ptr subnet2 = CfgMgr::instance().getSubnet4(IOAddress("192.0.3.102"));
     ASSERT_TRUE(subnet2);
-    const Subnet::OptionContainer& options2 = subnet2->getOptions();
+    const Subnet::OptionContainer& options2 = subnet2->getOptions("dhcp4");
     ASSERT_EQ(1, options2.size());
 
     const Subnet::OptionContainerTypeIndex& idx2 = options2.get<1>();
@@ -703,7 +703,7 @@ TEST_F(Dhcp4ParserTest, optionDataLowerCase) {
 
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp4");
     ASSERT_EQ(1, options.size());
 
     // Get the search index. Index #1 is to search using option code.

+ 6 - 4
src/bin/dhcp6/config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -925,7 +925,7 @@ public:
             subnet->addPool6(*it);
         }
 
-        const Subnet::OptionContainer& options = subnet->getOptions();
+        const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
         const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
 
         // Add subnet specific options.
@@ -935,7 +935,7 @@ public:
                 LOG_WARN(dhcp6_logger, DHCP6_CONFIG_OPTION_DUPLICATE)
                     .arg(desc.option->getType()).arg(addr.toText());
             }
-            subnet->addOption(desc.option);
+            subnet->addOption(desc.option, false, "dhcp6");
         }
 
         // Check all global options and add them to the subnet object if
@@ -945,6 +945,8 @@ public:
         BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
             // Get all options specified locally in the subnet and having
             // code equal to global option's code.
+            const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
+            const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
             Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             // @todo: In the future we will be searching for options using either
             // an option code or namespace. Currently we have only the option
@@ -955,7 +957,7 @@ public:
             // want to issue a warning about dropping the configuration of
             // a global option if one already exsists.
             if (std::distance(range.first, range.second) == 0) {
-                subnet->addOption(desc.option);
+                subnet->addOption(desc.option, false, "dhcp6");
             }
         }
 

+ 2 - 2
src/bin/dhcp6/dhcp6_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 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
@@ -340,7 +340,7 @@ void Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer)
     // Get the list of options that client requested.
     const std::vector<uint16_t>& requested_opts = option_oro->getValues();
     // Get the list of options configured for a subnet.
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
     const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
     // Try to match requested options with those configured for a subnet.
     // If match is found, append configured option to the answer message.

+ 7 - 7
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -449,7 +449,7 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) {
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
     ASSERT_EQ(2, options.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -524,7 +524,7 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) {
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
     ASSERT_EQ(2, options.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -590,7 +590,7 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
 
     Subnet6Ptr subnet1 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet1);
-    const Subnet::OptionContainer& options1 = subnet1->getOptions();
+    const Subnet::OptionContainer& options1 = subnet1->getOptions("dhcp6");
     ASSERT_EQ(1, options1.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -614,7 +614,7 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
     // Test another subnet in the same way.
     Subnet6Ptr subnet2 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:2::4"));
     ASSERT_TRUE(subnet2);
-    const Subnet::OptionContainer& options2 = subnet2->getOptions();
+    const Subnet::OptionContainer& options2 = subnet2->getOptions("dhcp6");
     ASSERT_EQ(1, options2.size());
 
     const Subnet::OptionContainerTypeIndex& idx2 = options2.get<1>();
@@ -708,7 +708,7 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) {
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
     ASSERT_EQ(1, options.size());
 
     // Get the search index. Index #1 is to search using option code.
@@ -749,7 +749,7 @@ TEST_F(Dhcp6ParserTest, stdOptionData) {
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
-    const Subnet::OptionContainer& options = subnet->getOptions();
+    const Subnet::OptionContainer& options = subnet->getOptions("dhcp6");
     ASSERT_EQ(1, options.size());
 
     // Get the search index. Index #1 is to search using option code.

+ 35 - 4
src/lib/dhcpsrv/subnet.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -44,14 +44,45 @@ bool Subnet::inRange(const isc::asiolink::IOAddress& addr) const {
 }
 
 void
-Subnet::addOption(OptionPtr& option, bool persistent /* = false */) {
+Subnet::addOption(OptionPtr& option, bool persistent,
+                  const std::string& option_space) {
+    // @todo Once the #2313 is merged we need to use the OptionSpace object to
+    // validate the option space name here. For now, let's check that the name
+    // is not empty as the empty namespace has a special meaning here - it is
+    // returned when desired namespace is not found when getOptions is called.
+    if (option_space.empty()) {
+        isc_throw(isc::BadValue, "option space name must not be empty");
+    }
     validateOption(option);
-    options_.push_back(OptionDescriptor(option, persistent));
+    option_spaces_[option_space].push_back(OptionDescriptor(option, persistent));
 }
 
 void
 Subnet::delOptions() {
-    options_.clear();
+    option_spaces_.clear();
+}
+
+const Subnet::OptionContainer&
+Subnet::emptyOptionContainer() {
+    static OptionContainer container;
+    return (container);
+}
+
+const Subnet::OptionContainer&
+Subnet::getOptions(const std::string& option_space) const {
+    // Search the map to get the options container for the particular
+    // option space.
+    const std::map<std::string, OptionContainer>::const_iterator& options =
+        option_spaces_.find(option_space);
+    // If the option space has not been found it means that no option
+    // has been configured for this option space yet. Thus we have to
+    // return an empty container to the caller.
+    if (options == option_spaces_.end()) {
+        return (emptyOptionContainer());
+    }
+    // We found some option container for the option space specified.
+    // Let's return a const reference to it.
+    return (options->second);
 }
 
 std::string Subnet::toText() const {

+ 20 - 7
src/lib/dhcpsrv/subnet.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -218,11 +218,23 @@ public:
     /// requested it or not.
     ///
     /// @throw isc::BadValue if invalid option provided.
-    void addOption(OptionPtr& option, bool persistent = false);
+    void addOption(OptionPtr& option, bool persistent,
+                   const std::string& option_space);
 
     /// @brief Delete all options configured for the subnet.
     void delOptions();
 
+    /// @brief Return a reference to an empty option container.
+    ///
+    /// The empty option container is returned when no other
+    /// suitable container can be found. For example, this is
+    /// the case when trying to get a set of options for the
+    /// particular option space that has no options configured
+    /// for the subnet.
+    ///
+    /// @return reference to the empty option container.
+    static const OptionContainer& emptyOptionContainer();
+
     /// @brief checks if the specified address is in pools
     ///
     /// Note the difference between inSubnet() and inPool(). For a given
@@ -254,12 +266,12 @@ public:
 
     /// @brief Return a collection of options.
     ///
+    /// @param option_space name of the option space
+    ///
     /// @return reference to collection of options configured for a subnet.
     /// The returned reference is valid as long as the Subnet object which
     /// returned it still exists.
-    const OptionContainer& getOptions() const {
-        return (options_);
-    }
+    const OptionContainer& getOptions(const std::string& option_space) const;
 
     /// @brief returns the last address that was tried from this pool
     ///
@@ -353,8 +365,9 @@ protected:
     /// @brief a tripet (min/default/max) holding allowed valid lifetime values
     Triplet<uint32_t> valid_;
 
-    /// @brief a collection of DHCP options configured for a subnet.
-    OptionContainer options_;
+    /// @brief a collection of DHCP option spaces holding options
+    /// configured for a subnet.
+    std::map<std::string, OptionContainer> option_spaces_;
 
     /// @brief last allocated address
     ///

+ 45 - 15
src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -115,13 +115,15 @@ TEST(Subnet4Test, addInvalidOption) {
     // Create option with invalid universe (V6 instead of V4).
     // Attempt to add this option should result in exception.
     OptionPtr option1(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
-    EXPECT_THROW(subnet->addOption(option1), isc::BadValue);
+    EXPECT_THROW(subnet->addOption(option1, false, "dhcp4"),
+                 isc::BadValue);
 
     // Create NULL pointer option. Attempt to add NULL option
     // should result in exception.
     OptionPtr option2;
     ASSERT_FALSE(option2);
-    EXPECT_THROW(subnet->addOption(option2), isc::BadValue);
+    EXPECT_THROW(subnet->addOption(option2, false, "dhcp4"),
+                 isc::BadValue);
 }
 
 // This test verifies that inRange() and inPool() methods work properly.
@@ -261,14 +263,21 @@ TEST(Subnet6Test, addOptions) {
     // Differentiate options by their codes (100-109)
     for (uint16_t code = 100; code < 110; ++code) {
         OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
-        ASSERT_NO_THROW(subnet->addOption(option));
+        ASSERT_NO_THROW(subnet->addOption(option, false, "dhcp6"));
+    }
+
+    // Add 7 options to another option space. The option codes partially overlap
+    // with option codes that we have added to dhcp6 option space.
+    for (uint16_t code = 105; code < 112; ++code) {
+        OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
+        ASSERT_NO_THROW(subnet->addOption(option, false, "isc"));
     }
 
     // Get options from the Subnet and check if all 10 are there.
-    Subnet::OptionContainer options = subnet->getOptions();
+    Subnet::OptionContainer options = subnet->getOptions("dhcp6");
     ASSERT_EQ(10, options.size());
 
-    // Validate codes of added options.
+    // Validate codes of options added to dhcp6 option space.
     uint16_t expected_code = 100;
     for (Subnet::OptionContainer::const_iterator option_desc = options.begin();
          option_desc != options.end(); ++option_desc) {
@@ -277,9 +286,30 @@ TEST(Subnet6Test, addOptions) {
         ++expected_code;
     }
 
+    options = subnet->getOptions("isc");
+    ASSERT_EQ(7, options.size());
+
+    // Validate codes of options added to isc option space.
+    expected_code = 105;
+    for (Subnet::OptionContainer::const_iterator option_desc = options.begin();
+         option_desc != options.end(); ++option_desc) {
+        ASSERT_TRUE(option_desc->option);
+        EXPECT_EQ(expected_code, option_desc->option->getType());
+        ++expected_code;
+    }
+
+    // Try to get options from a non-existing option space.
+    options = subnet->getOptions("abcd");
+    EXPECT_TRUE(options.empty());
+
+    // Delete options from all spaces.
     subnet->delOptions();
 
-    options = subnet->getOptions();
+    // Make sure that all options have been removed.
+    options = subnet->getOptions("dhcp6");
+    EXPECT_EQ(0, options.size());
+
+    options = subnet->getOptions("isc");
     EXPECT_EQ(0, options.size());
 }
 
@@ -292,12 +322,12 @@ TEST(Subnet6Test, addNonUniqueOptions) {
         // In the inner loop we create options with unique codes (100-109).
         for (uint16_t code = 100; code < 110; ++code) {
             OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
-            ASSERT_NO_THROW(subnet->addOption(option));
+            ASSERT_NO_THROW(subnet->addOption(option, false, "dhcp6"));
         }
     }
 
     // Sanity check that all options are there.
-    Subnet::OptionContainer options = subnet->getOptions();
+    Subnet::OptionContainer options = subnet->getOptions("dhcp6");
     ASSERT_EQ(20, options.size());
 
     // Use container index #1 to get the options by their codes.
@@ -329,7 +359,7 @@ TEST(Subnet6Test, addNonUniqueOptions) {
 
     subnet->delOptions();
 
-    options = subnet->getOptions();
+    options = subnet->getOptions("dhcp6");
     EXPECT_EQ(0, options.size());
 }
 
@@ -342,13 +372,13 @@ TEST(Subnet6Test, addInvalidOption) {
     // Create option with invalid universe (V4 instead of V6).
     // Attempt to add this option should result in exception.
     OptionPtr option1(new Option(Option::V4, code, OptionBuffer(10, 0xFF)));
-    EXPECT_THROW(subnet->addOption(option1), isc::BadValue);
+    EXPECT_THROW(subnet->addOption(option1, false, "dhcp6"), isc::BadValue);
 
     // Create NULL pointer option. Attempt to add NULL option
     // should result in exception.
     OptionPtr option2;
     ASSERT_FALSE(option2);
-    EXPECT_THROW(subnet->addOption(option2), isc::BadValue);
+    EXPECT_THROW(subnet->addOption(option2, false, "dhcp6"), isc::BadValue);
 }
 
 TEST(Subnet6Test, addPersistentOption) {
@@ -367,11 +397,11 @@ TEST(Subnet6Test, addPersistentOption) {
         // and options with these codes will be flagged non-persistent.
         // Options with other codes will be flagged persistent.
         bool persistent = (code % 3) ? true : false;
-        ASSERT_NO_THROW(subnet->addOption(option, persistent));
+        ASSERT_NO_THROW(subnet->addOption(option, persistent, "dhcp6"));
     }
 
     // Get added options from the subnet.
-    Subnet::OptionContainer options = subnet->getOptions();
+    Subnet::OptionContainer options = subnet->getOptions("dhcp6");
 
     // options.get<2> returns reference to container index #2. This
     // index is used to access options by the 'persistent' flag.
@@ -393,7 +423,7 @@ TEST(Subnet6Test, addPersistentOption) {
 
     subnet->delOptions();
 
-    options = subnet->getOptions();
+    options = subnet->getOptions("dhcp6");
     EXPECT_EQ(0, options.size());
 }