[master]Merge branch 'master' of ssh://git.bind10.isc.org//var/bind10/git/bind10

ChangeLog

@@ -62,7 +62,7 @@
 
 488.	[build]		jinmei
 	On configure, changed the search order for Python executable.
-	It first ties more specific file names such as "python3.2" before
+	It first tries more specific file names such as "python3.2" before
 	more generic "python3".  This will prevent configure failure on
 	Mac OS X that installs Python3 via recent versions of Homebrew.
 	(Trac #2339, git 88db890d8d1c64de49be87f03c24a2021bcf63da)

src/bin/auth/auth_messages.mes

@@ -63,17 +63,21 @@ source clients receives a command from the manager.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_FAILED data source builder thread stopped due to an exception: %1
 The separate thread for maintaining data source clients has been
-terminated due to some uncaught exception.  The manager cannot always
-catch this condition in timely fashion, and there is no way to recover
-from this situation except for restarting the entire server.  So this
-message needs to be carefully watched, and should it occur the auth
-server needs to be restarted by hand.
+terminated due to some uncaught exception.  When this happens, the
+thread immediately terminates the entire process because the manager
+cannot always catch this condition in a timely fashion and it would be
+worse to keep running with such a half-broken state.  This is really
+an unexpected event and should generally indicate an internal bug.
+It's advisable to file a bug report when this message is logged (and
+b10-auth subsequently stops).
 
 % AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED data source builder thread stopped due to an unexpected exception
 This is similar to AUTH_DATASRC_CLIENTS_BUILDER_FAILED, but the
-exception type is even more unexpected.  This may rather indicate some
-run time failure than program errors, but in any case the server needs
-to be restarted by hand.
+exception type indicates it's not thrown either within the BIND 10
+implementation or other standard-compliant libraries.  This may rather
+indicate some run time failure than program errors.  As in the other
+failure case, the thread terminates the entire process immediately
+after logging this message.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_CONFIG_ERROR Error in data source configuration: %1
 The thread for maintaining data source clients has received a command to

src/bin/auth/datasrc_clients_mgr.h

@@ -36,6 +36,7 @@
 #include <boost/shared_ptr.hpp>
 #include <boost/noncopyable.hpp>
 
+#include <exception>
 #include <list>
 #include <utility>
 
@@ -406,10 +407,10 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::run() {
         // We explicitly catch exceptions so we can log it as soon as possible.
         LOG_FATAL(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED).
             arg(ex.what());
-        assert(false);
+        std::terminate();
     } catch (...) {
         LOG_FATAL(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED);
-        assert(false);
+        std::terminate();
     }
 }
 

src/lib/dhcp/subnet.cc

@@ -41,6 +41,17 @@ bool Subnet::inRange(const isc::asiolink::IOAddress& addr) const {
     return ((first <= addr) && (addr <= last));
 }
 
+void
+Subnet::addOption(OptionPtr& option, bool persistent /* = false */) {
+    validateOption(option);
+    options_.push_back(OptionDescriptor(option, persistent));
+}
+
+void
+Subnet::delOptions() {
+    options_.clear();
+}
+
 Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t1,
                  const Triplet<uint32_t>& t2,
@@ -85,6 +96,15 @@ Pool4Ptr Subnet4::getPool4(const isc::asiolink::IOAddress& hint /* = IOAddress("
     return (candidate);
 }
 
+void
+Subnet4::validateOption(const OptionPtr& option) const {
+    if (!option) {
+        isc_throw(isc::BadValue, "option configured for subnet must not be NULL");
+    } else if (option->getUniverse() != Option::V4) {
+        isc_throw(isc::BadValue, "expected V4 option to be added to the subnet");
+    }
+}
+
 Subnet6::Subnet6(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t1,
                  const Triplet<uint32_t>& t2,
@@ -131,5 +151,13 @@ Pool6Ptr Subnet6::getPool6(const isc::asiolink::IOAddress& hint /* = IOAddress("
     return (candidate);
 }
 
+void
+Subnet6::validateOption(const OptionPtr& option) const {
+    if (!option) {
+        isc_throw(isc::BadValue, "option configured for subnet must not be NULL");
+    } else if (option->getUniverse() != Option::V6) {
+        isc_throw(isc::BadValue, "expected V6 option to be added to the subnet");
+    }
+}
 } // end of isc::dhcp namespace
 } // end of isc namespace

src/lib/dhcp/subnet.h

@@ -15,10 +15,16 @@
 #ifndef SUBNET_H
 #define SUBNET_H
 
-#include <boost/shared_ptr.hpp>
 #include <asiolink/io_address.h>
 #include <dhcp/pool.h>
 #include <dhcp/triplet.h>
+#include <dhcp/option.h>
+#include <boost/shared_ptr.hpp>
+#include <boost/multi_index_container.hpp>
+#include <boost/multi_index/hashed_index.hpp>
+#include <boost/multi_index/sequenced_index.hpp>
+#include <boost/multi_index/mem_fun.hpp>
+#include <boost/multi_index/member.hpp>
 
 namespace isc {
 namespace dhcp {
@@ -30,14 +36,174 @@ namespace dhcp {
 /// attached to it. In most cases all devices attached to a single link can
 /// share the same parameters. Therefore Subnet holds several values that are
 /// typically shared by all hosts: renew timer (T1), rebind timer (T2) and
-/// leased addresses lifetime (valid-lifetime).
-///
-/// @todo: Implement support for options here
+/// leased addresses lifetime (valid-lifetime). It also holds the set
+/// of DHCP option instances configured for the subnet. These options are
+/// included in DHCP messages being sent to clients which are connected
+/// to the particular subnet.
 class Subnet {
 public:
+
+    /// @brief Option descriptor.
+    ///
+    /// Option descriptor holds information about option configured for
+    /// a particular subnet. This information comprises the actual option
+    /// instance and information whether this option is sent to DHCP client
+    /// only on request (persistent = false) or always (persistent = true).
+    struct OptionDescriptor {
+        /// Option instance.
+        OptionPtr option;
+        /// Persistent flag, if true option is always sent to the client,
+        /// if false option is sent to the client on request.
+        bool persistent;
+
+        /// @brief Constructor.
+        ///
+        /// @param opt option
+        /// @param persist if true option is always sent.
+        OptionDescriptor(OptionPtr& opt, bool persist)
+            : option(opt), persistent(persist) {};
+    };
+
+    /// @brief Extractor class to extract key with another key.
+    ///
+    /// This class solves the problem of accessing index key values
+    /// that are stored in objects nested in other objects.
+    /// Each OptionDescriptor structure contains the OptionPtr object.
+    /// The value retured by one of its accessors (getType) is used
+    /// as an indexing value in the multi_index_container defined below.
+    /// There is no easy way to mark that value returned by Option::getType
+    /// should be an index of this multi_index_container. There are standard
+    /// key extractors such as 'member' or 'mem_fun' but they are not
+    /// sufficient here. The former can be used to mark that member of
+    /// the structure that is held in the container should be used as an
+    /// indexing value. The latter can be used if the indexing value is
+    /// a product of the class being held in the container. In this complex
+    /// scenario when the indexing value is a product of the function that
+    /// is wrapped by the structure, this new extractor template has to be
+    /// defined. The template class provides a 'chain' of two extractors
+    /// to access the value returned by nested object and to use it as
+    /// indexing value.
+    /// For some more examples of complex keys see:
+    /// http://www.cs.brown.edu/~jwicks/boost/libs/multi_index/doc/index.html
+    ///
+    /// @tparam KeyExtractor1 extractor used to access data in
+    /// OptionDescriptor::option
+    /// @tparam KeyExtractor2 extractor used to access
+    /// OptionDescriptor::option member.
+    template<typename KeyExtractor1, typename KeyExtractor2>
+    class KeyFromKey {
+    public:
+        typedef typename KeyExtractor1::result_type result_type;
+
+        /// @brief Constructor.
+        KeyFromKey()
+            : key1_(KeyExtractor1()), key2_(KeyExtractor2()) { };
+
+        /// @brief Extract key with another key.
+        ///
+        /// @param arg the key value.
+        ///
+        /// @tparam key value type.
+        template<typename T>
+        result_type operator() (T& arg) const {
+            return (key1_(key2_(arg)));
+        }
+    private:
+        KeyExtractor1 key1_; ///< key 1.
+        KeyExtractor2 key2_; ///< key 2.
+    };
+
+    /// @brief Multi index container for DHCP option descriptors.
+    ///
+    /// This container comprises three indexes to access option
+    /// descriptors:
+    /// - sequenced index: used to access elements in the order they
+    /// have been added to the container,
+    /// - option type index: used to search option descriptors containing
+    /// options with specific option code (aka option type).
+    /// - persistency flag index: used to search option descriptors with
+    /// 'persistent' flag set to true.
+    ///
+    /// This container is the equivalent of three separate STL containers:
+    /// - std::list of all options,
+    /// - std::multimap of options with option code used as a multimap key,
+    /// - std::multimap of option descriptors with option persistency flag
+    /// used as a multimap key.
+    /// The major advantage of this container over 3 separate STL containers
+    /// is automatic synchronization of all indexes when elements are added,
+    /// removed or modified in the container. With separate containers,
+    /// the synchronization would have to be guaranteed by the Subnet class
+    /// code. This would increase code complexity and presumably it would
+    /// be much harder to add new search criteria (indexes).
+    ///
+    /// @todo we may want to search for options using option spaces when
+    /// they are implemented.
+    ///
+    /// @see http://www.boost.org/doc/libs/1_51_0/libs/multi_index/doc/index.html
+    typedef boost::multi_index_container<
+        // Container comprises elements of OptionDescriptor type.
+        OptionDescriptor,
+        // Here we start enumerating various indexes.
+        boost::multi_index::indexed_by<
+            // Sequenced index allows accessing elements in the same way
+            // as elements in std::list.
+            // Sequenced is an index #0.
+            boost::multi_index::sequenced<>,
+            // Start definition of index #1.
+            boost::multi_index::hashed_non_unique<
+                // KeyFromKey is the index key extractor that allows accessing
+                // option type being held by the OptionPtr through
+                // OptionDescriptor structure.
+                KeyFromKey<
+                    // Use option type as the index key. The type is held
+                    // in OptionPtr object so we have to call Option::getType
+                    // to retrieve this key for each element.
+                    boost::multi_index::mem_fun<
+                        Option,
+                        uint16_t,
+                        &Option::getType
+                    >,
+                    // Indicate that OptionPtr is a member of
+                    // OptionDescriptor structure.
+                    boost::multi_index::member<
+                        OptionDescriptor,
+                        OptionPtr,
+                        &OptionDescriptor::option
+                    >
+                 >
+            >,
+            // Start definition of index #2.
+            // Use 'persistent' struct member as a key.
+            boost::multi_index::hashed_non_unique<
+                boost::multi_index::member<
+                    OptionDescriptor,
+                    bool,
+                    &OptionDescriptor::persistent
+                >
+            >
+        >
+    > OptionContainer;
+
+    /// Type of the index #1 - option type.
+    typedef OptionContainer::nth_index<1>::type OptionContainerTypeIndex;
+    /// Type of the index #2 - option persistency flag.
+    typedef OptionContainer::nth_index<2>::type OptionContainerPersistIndex;
+
     /// @brief checks if specified address is in range
     bool inRange(const isc::asiolink::IOAddress& addr) const;
 
+    /// @brief Add new option instance to the collection.
+    ///
+    /// @param option option instance.
+    /// @param persistent if true, send an option regardless if client
+    /// requested it or not.
+    ///
+    /// @throw isc::BadValue if invalid option provided.
+    void addOption(OptionPtr& option, bool persistent = false);
+
+    /// @brief Delete all options configured for the subnet.
+    void delOptions();
+
     /// @brief return valid-lifetime for addresses in that prefix
     Triplet<uint32_t> getValid() const {
         return (valid_);
@@ -53,6 +219,15 @@ public:
         return (t2_);
     }
 
+    /// @brief Return a collection of options.
+    ///
+    /// @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() {
+        return (options_);
+    }
+
 protected:
     /// @brief protected constructor
     //
@@ -63,6 +238,12 @@ protected:
            const Triplet<uint32_t>& t2,
            const Triplet<uint32_t>& valid_lifetime);
 
+    /// @brief virtual destructor
+    ///
+    /// A virtual destructor is needed because other classes
+    /// derive from this class.
+    virtual ~Subnet() { };
+
     /// @brief returns the next unique Subnet-ID
     ///
     /// @return the next unique Subnet-ID
@@ -71,6 +252,11 @@ protected:
         return (id++);
     }
 
+    /// @brief Check if option is valid and can be added to a subnet.
+    ///
+    /// @param option option to be validated.
+    virtual void validateOption(const OptionPtr& option) const = 0;
+
     /// @brief subnet-id
     ///
     /// Subnet-id is a unique value that can be used to find or identify
@@ -91,6 +277,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 configuration holder for IPv4 subnet.
@@ -133,6 +322,14 @@ public:
     }
 
 protected:
+
+    /// @brief Check if option is valid and can be added to a subnet.
+    ///
+    /// @param option option to be validated.
+    ///
+    /// @throw isc::BadValue if provided option is invalid.
+    virtual void validateOption(const OptionPtr& option) const;
+
     /// @brief collection of pools in that list
     Pool4Collection pools_;
 };
@@ -193,6 +390,14 @@ public:
     }
 
 protected:
+
+    /// @brief Check if option is valid and can be added to a subnet.
+    ///
+    /// @param option option to be validated.
+    ///
+    /// @throw isc::BadValue if provided option is invalid.
+    virtual void validateOption(const OptionPtr& option) const;
+
     /// @brief collection of pools in that list
     Pool6Collection pools_;
 

src/lib/dhcp/tests/Makefile.am

@@ -57,6 +57,7 @@ libdhcpsrv_unittests_LDADD  = $(GTEST_LDADD)
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcpsrv.la
+libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
 
 

src/lib/dhcp/tests/subnet_unittest.cc

@@ -15,6 +15,7 @@
 
 #include <config.h>
 #include <dhcp/subnet.h>
+#include <dhcp/option.h>
 #include <exceptions/exceptions.h>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
@@ -104,6 +105,24 @@ TEST(Subnet4Test, Subnet4_Pool4_checks) {
     EXPECT_THROW(subnet->addPool4(pool3), BadValue);
 }
 
+TEST(Subnet4Test, addInvalidOption) {
+    // Create the V4 subnet.
+    Subnet4Ptr subnet(new Subnet4(IOAddress("192.0.2.0"), 8, 1, 2, 3));
+
+    // Some dummy option code.
+    uint16_t code = 100;
+    // 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);
+
+    // 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);
+}
+
 // Tests for Subnet6
 
 TEST(Subnet6Test, constructor) {
@@ -187,4 +206,146 @@ TEST(Subnet6Test, Subnet6_Pool6_checks) {
     EXPECT_THROW(subnet->addPool6(pool4), BadValue);
 }
 
+TEST(Subnet6Test, addOptions) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // 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));
+    }
+
+    // Get options from the Subnet and check if all 10 are there.
+    Subnet::OptionContainer options = subnet->getOptions();
+    ASSERT_EQ(10, options.size());
+
+    // Validate codes of added options.
+    uint16_t expected_code = 100;
+    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;
+    }
+
+    subnet->delOptions();
+
+    options = subnet->getOptions();
+    EXPECT_EQ(0, options.size());
+}
+
+TEST(Subnet6Test, addNonUniqueOptions) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Create a set of options with non-unique codes.
+    for (int i = 0;  i < 2; ++i) {
+        // 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));
+        }
+    }
+
+    // Sanity check that all options are there.
+    Subnet::OptionContainer options = subnet->getOptions();
+    ASSERT_EQ(20, options.size());
+
+    // Use container index #1 to get the options by their codes.
+    Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    // Look for the codes 100-109.
+    for (uint16_t code = 100; code < 110; ++ code) {
+        // For each code we should get two instances of options.
+        std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+                  Subnet::OptionContainerTypeIndex::const_iterator> range =
+            idx.equal_range(code);
+        // Distance between iterators indicates how many options
+        // have been retured for the particular code.
+        ASSERT_EQ(2, distance(range.first, range.second));
+        // Check that returned options actually have the expected option code.
+        for (Subnet::OptionContainerTypeIndex::const_iterator option_desc = range.first;
+             option_desc != range.second; ++option_desc) {
+            ASSERT_TRUE(option_desc->option);
+            EXPECT_EQ(code, option_desc->option->getType());
+        }
+    }
+
+    // Let's try to find some non-exiting option.
+    const uint16_t non_existing_code = 150;
+    std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+              Subnet::OptionContainerTypeIndex::const_iterator> range =
+        idx.equal_range(non_existing_code);
+    // Empty set is expected.
+    EXPECT_EQ(0, distance(range.first, range.second));
+
+    subnet->delOptions();
+
+    options = subnet->getOptions();
+    EXPECT_EQ(0, options.size());
+}
+
+TEST(Subnet6Test, addInvalidOption) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Some dummy option code.
+    uint16_t code = 100;
+    // 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);
+
+    // 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);
+}
+
+TEST(Subnet6Test, addPersistentOption) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Add 10 options to the subnet with option codes 100 - 109.
+    for (uint16_t code = 100; code < 110; ++code) {
+        OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
+        // We create 10 options and want some of them to be flagged
+        // persistent and some non-persistent. Persistent options are
+        // those that server sends to clients regardless if they ask
+        // for them or not. We pick 3 out of 10 options and mark them
+        // non-persistent and 7 other options persistent.
+        // Code values: 102, 105 and 108 are divisable by 3
+        // 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));
+    }
+
+    // Get added options from the subnet.
+    Subnet::OptionContainer options = subnet->getOptions();
+
+    // options.get<2> returns reference to container index #2. This
+    // index is used to access options by the 'persistent' flag.
+    Subnet::OptionContainerPersistIndex& idx = options.get<2>();
+
+    // Get all persistent options.
+    std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
+              Subnet::OptionContainerPersistIndex::const_iterator> range_persistent =
+        idx.equal_range(true);
+    // 3 out of 10 options have been flagged persistent.
+    ASSERT_EQ(7, distance(range_persistent.first, range_persistent.second));
+
+    // Get all non-persistent options.
+    std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
+              Subnet::OptionContainerPersistIndex::const_iterator> range_non_persistent =
+        idx.equal_range(false);
+    // 7 out of 10 options have been flagged persistent.
+    ASSERT_EQ(3, distance(range_non_persistent.first, range_non_persistent.second));
+
+    subnet->delOptions();
+
+    options = subnet->getOptions();
+    EXPECT_EQ(0, options.size());
+}
 };