[3695] Allow for multiple IPv4 address selected for an interface.

src/lib/dhcpsrv/cfg_iface.cc

@@ -17,6 +17,7 @@
 #include <dhcpsrv/cfg_iface.h>
 #include <util/strutil.h>
 #include <boost/bind.hpp>
+#include <algorithm>
 
 using namespace isc::asiolink;
 
@@ -85,20 +86,7 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port,
 
             } else if (family == AF_INET) {
                 iface->inactive4_ = false;
-                ExplicitAddressMap::const_iterator addr =
-                    address_map_.find(iface->getName());
-                // If user has specified an address to listen on, let's activate
-                // only this address.
-                if (addr != address_map_.end()) {
-                    iface->setActive(addr->second, true);
-
-                // Otherwise, activate first one.
-                } else {
-                    IOAddress address(0);
-                    if (iface->getAddress4(address)) {
-                        iface->setActive(address, true);
-                    }
-                }
+                setIfaceAddrsState(family, true, *iface);
 
             } else {
                 iface->inactive6_ = false;
@@ -107,18 +95,22 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port,
     }
 
     // Select unicast sockets. It works only for V6. Ignore for V4.
-    if (family == AF_INET6) {
-        for (ExplicitAddressMap::const_iterator unicast = address_map_.begin();
-             unicast != address_map_.end(); ++unicast) {
-            Iface* iface = IfaceMgr::instance().getIface(unicast->first);
-            if (iface == NULL) {
-                isc_throw(Unexpected,
-                          "fail to open unicast socket on interface '"
-                          << unicast->first << "' as this interface doesn't"
-                          " exist");
-            }
+    for (ExplicitAddressMap::const_iterator unicast = address_map_.begin();
+         unicast != address_map_.end(); ++unicast) {
+        Iface* iface = IfaceMgr::instance().getIface(unicast->first);
+        if (iface == NULL) {
+            isc_throw(Unexpected,
+                      "fail to open unicast socket on interface '"
+                      << unicast->first << "' as this interface doesn't"
+                      " exist");
+        }
+        if (family == AF_INET6) {
             iface->addUnicast(unicast->second);
             iface->inactive6_ = false;
+
+        } else {
+            iface->setActive(unicast->second, true);
+            iface->inactive4_ = false;
         }
     }
 
@@ -132,7 +124,9 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port,
         boost::bind(&CfgIface::socketOpenErrorHandler, _1);
     bool sopen;
     if (family == AF_INET) {
-        sopen = IfaceMgr::instance().openSockets4(port, use_bcast,
+        // Do not use broadcast if there are multiple addresses selected for any of the
+        // interfaces. In that case, we fallback to unicast only (relayed traffic).
+        sopen = IfaceMgr::instance().openSockets4(port, use_bcast && address_map_.empty(),
                                                   error_callback);
     } else {
         // use_bcast is ignored for V6.
@@ -170,14 +164,20 @@ CfgIface::setState(const uint16_t family, const bool inactive,
         }
 
         // Activate/deactivate all addresses.
-        const Iface::AddressCollection addresses = iface_ptr->getAddresses();
-        for (Iface::AddressCollection::const_iterator addr_it =
+        setIfaceAddrsState(family, !inactive, *iface_ptr);
+    }
+}
+
+void
+CfgIface::setIfaceAddrsState(const uint16_t family, const bool active,
+                             Iface& iface) const {
+    // Activate/deactivate all addresses.
+    const Iface::AddressCollection addresses = iface.getAddresses();
+    for (Iface::AddressCollection::const_iterator addr_it =
                  addresses.begin(); addr_it != addresses.end(); ++addr_it) {
-            if (addr_it->get().getFamily() == family) {
-                iface_ptr->setActive(addr_it->get(), !iface_inactive);
-            }
+        if (addr_it->get().getFamily() == family) {
+            iface.setActive(addr_it->get(), active);
         }
-
     }
 }
 
@@ -218,9 +218,9 @@ CfgIface::textToSocketType(const std::string& socket_type_name) const {
 
 void
 CfgIface::use(const uint16_t family, const std::string& iface_name) {
-    // The interface name specified may have two formats, e.g.:
-    // - eth0
-    // - eth0/2001:db8:1::1.
+    // The interface name specified may have two formats:
+    // - "interface-name", e.g. eth0
+    // - "interface-name/address", e.g. eth0/10.0.0.1 or eth/2001:db8:1::1
     // The latter format is used to open unicast socket on the specified
     // interface. Here we are detecting which format was used and we strip
     // all extraneous spaces.
@@ -234,7 +234,7 @@ CfgIface::use(const uint16_t family, const std::string& iface_name) {
             isc_throw(InvalidIfaceName,
                       "empty interface name used in configuration");
 
-        } if (name != ALL_IFACES_KEYWORD) {
+        } else if (name != ALL_IFACES_KEYWORD) {
             if (IfaceMgr::instance().getIface(name) == NULL) {
                 isc_throw(NoSuchIface, "interface '" << name
                           << "' doesn't exist in the system");
@@ -253,7 +253,7 @@ CfgIface::use(const uint16_t family, const std::string& iface_name) {
 
     } else {
         // The interface name includes the address on which the socket should
-        // be opened, we we need to split interface name and the address to
+        // be opened, and we need to split interface name and the address to
         // two variables.
         name = util::str::trim(iface_name.substr(0, pos));
         addr_str = util::str::trim(iface_name.substr(pos + 1));
@@ -323,13 +323,21 @@ CfgIface::use(const uint16_t family, const std::string& iface_name) {
                       << addr << "' assigned");
         }
 
-        // Insert address and the interface to the collection of unicast
-        // addresses.
-        if (address_map_.find(name) != address_map_.end()) {
-            isc_throw(DuplicateIfaceName, "must not select address '"
+        // For the IPv4, if the interface name was specified (instead of the interface-
+        // address tuple) all addresses are already activated. Adding an explicit address
+        // for the interface should result in error.
+        if ((family == AF_INET) && (iface_set_.find(iface->getName()) != iface_set_.end())) {
+            isc_throw(DuplicateIfaceName, "interface '" << iface->getName()
+                      << "' has already been selected");
+        }
+
+        // Check if the address hasn't been selected already.
+        std::pair<const std::string, IOAddress> iface_address_tuple(name, addr);
+        if (std::find(address_map_.begin(), address_map_.end(),
+                      iface_address_tuple) != address_map_.end()) {
+            isc_throw(DuplicateAddress, "must not select address '"
                       << addr << "' for interface '" << name << "' "
-                      "because another address has already been"
-                      " specified for this interface");
+                      "because this address is already selected");
         }
 
         if (family == AF_INET6) {
@@ -343,12 +351,13 @@ CfgIface::use(const uint16_t family, const std::string& iface_name) {
         address_map_.insert(std::pair<std::string, IOAddress>(name, addr));
     }
 
-    // If interface name was explicitly specified and we're not parsing
-    // a unicast IPv6 address, add the interface to the interface set.
-    if ((name != ALL_IFACES_KEYWORD) &&
-        ((family == AF_INET) || ((family == AF_INET6) && addr_str.empty()))) {
-        // If interface has already been specified.
-        if (iface_set_.find(name) != iface_set_.end()) {
+    // If interface name was explicitly specified without an address, we will
+    // insert the interface name to the set of enabled interfaces.
+    if ((name != ALL_IFACES_KEYWORD) && addr_str.empty()) {
+        // An interface has been selected or an IPv4 address on this interface
+        // has been selected it is not allowed to select the whole interface.
+        if ((iface_set_.find(name) != iface_set_.end()) ||
+            ((family == AF_INET) && address_map_.count(name) > 0)) {
             isc_throw(DuplicateIfaceName, "interface '" << name
                       << "' has already been specified");
         }

src/lib/dhcpsrv/cfg_iface.h

@@ -16,6 +16,7 @@
 #define CFG_IFACE_H
 
 #include <asiolink/io_address.h>
+#include <dhcp/iface_mgr.h>
 #include <boost/shared_ptr.hpp>
 #include <map>
 #include <set>
@@ -45,6 +46,13 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief Exception thrown when duplicated address specified.
+class DuplicateAddress : public Exception {
+public:
+    DuplicateAddress(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 /// @brief Exception thrown when specified unicast address is not assigned
 /// to the interface specified.
 class NoSuchAddress : public Exception {
@@ -65,9 +73,9 @@ public:
 ///
 /// This class manages selection of interfaces on which the DHCP server is
 /// listening to queries. The interfaces are selected in the server
-/// configuration by their names or by the pairs of interface names and unicast
-/// addresses (e.g. eth0/2001:db8:1::1). The latter format is only accepted when
-/// IPv6 configuration is in use.
+/// configuration by their names or by the pairs of interface names and
+/// addresses, e.g. eth0/2001:db8:1::1 (DHCPv6) or e.g. eth0/192.168.8.1
+/// (DHCPv4).
 ///
 /// This class also accepts "wildcard" interface name which, if specified,
 /// instructs the server to listen on all available interfaces.
@@ -146,9 +154,9 @@ public:
     /// passed as the argument of this function may appear in one of the following
     /// formats:
     /// - interface-name, e.g. eth0
-    /// - interface-name/unicast-address, e.g. eth0/2001:db8:1::1 (V6 only)
+    /// - interface-name/address, e.g. eth0/2001:db8:1::1 or eth0/192.168.8.1
     ///
-    /// Extraneous spaces surrounding the interface name and/or unicast address
+    /// Extraneous spaces surrounding the interface name and/or address
     /// are accepted. For example: eth0 / 2001:db8:1::1 will be accepted.
     ///
     /// When only interface name is specified (without an address) it is allowed
@@ -158,6 +166,24 @@ public:
     /// not allowed when specifying a unicast address. For example:
     /// */2001:db8:1::1 is not allowed.
     ///
+    /// The DHCPv6 configuration accepts simultaneous use of the "interface-name"
+    /// and "interface-name/address" tuple for the same interface, e.g.
+    /// "eth0", "eth0/2001:db8:1::1" specifies that the server should open a
+    /// socket and bind to link local address as well as open a socket bound to
+    /// the specified unicast address.
+    ///
+    /// The DHCPv4 configuration doesn't accept the simulatenous use of the
+    /// "interface-name" and the "interface-name/address" tuple for the
+    /// given interface. When the "interface-name" is specified it implies
+    /// that the sockets will be opened on for all addresses configured on
+    /// this interface. If the tuple of "interface-name/address" is specified
+    /// there will be only one socket opened and bound to the specified address.
+    /// This socket will be configured to listen to the broadcast messages
+    /// reaching the interface as well as unicast messages sent to the address
+    /// to which it is bound. It is allowed to select multiple addresses on the
+    /// particular interface explicitly, e.g. "eth0/192.168.8.1",
+    /// "eth0/192.168.8.2".
+    ///
     /// @param family Address family (AF_INET or AF_INET6).
     /// @param iface_name Explicit interface name, a wildcard name (*) of
     /// the interface(s) or the pair of iterface/unicast-address to be used
@@ -246,6 +272,19 @@ private:
     void setState(const uint16_t family, const bool inactive,
                   const bool loopback_inactive) const;
 
+    /// @brief Selects or deselects addresses on the interface.
+    ///
+    /// This function selects all address on the interface to receive DHCP
+    /// traffic or deselects all addresses so as none of them receives the
+    /// DHCP traffic.
+    ///
+    /// @param family Address family (AF_INET or AF_INET6).
+    /// @param active A boolean value which indicates if all addresses should
+    /// be active (if true), or inactive (if false).
+    /// @param iface An interface on which addresses are selected/deselected.
+    void setIfaceAddrsState(const uint16_t family, const bool inactive,
+                            Iface& iface) const;
+
     /// @brief Error handler for executed when opening a socket fail.
     ///
     /// A pointer to this function is passed to the @c IfaceMgr::openSockets4
@@ -264,7 +303,7 @@ private:
 
     /// @brief A map of interfaces and addresses to which the server
     /// should bind sockets.
-    typedef std::map<std::string, asiolink::IOAddress> ExplicitAddressMap;
+    typedef std::multimap<std::string, asiolink::IOAddress> ExplicitAddressMap;
 
     /// @brief A map which holds the pairs of interface names and addresses
     /// for which the sockets should be opened.

src/lib/dhcpsrv/tests/cfg_iface_unittest.cc

@@ -125,7 +125,6 @@ TEST_F(CfgIfaceTest, explicitNamesAndAddressesV4) {
     CfgIface cfg;
     ASSERT_NO_THROW(cfg.use(AF_INET, "eth0/10.0.0.1"));
     ASSERT_NO_THROW(cfg.use(AF_INET, "eth1/192.0.2.3"));
-    ASSERT_THROW(cfg.use(AF_INET, "eth1/192.0.2.5"), DuplicateIfaceName);
 
     // Open sockets on specified interfaces and addresses.
     cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
@@ -146,7 +145,6 @@ TEST_F(CfgIfaceTest, explicitNamesAndAddressesV4) {
     // Now check that the socket can be bound to a different address on
     // eth1.
     ASSERT_NO_THROW(cfg.use(AF_INET, "eth1/192.0.2.5"));
-    ASSERT_THROW(cfg.use(AF_INET, "eth1/192.0.2.3"), DuplicateIfaceName);
 
     // Open sockets according to the new configuration.
     cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
@@ -172,6 +170,25 @@ TEST_F(CfgIfaceTest, explicitNamesAndAddressesInvalidV4) {
     EXPECT_THROW(cfg.use(AF_INET, "eth1/192.0.2.3"), DuplicateIfaceName);
 }
 
+// This test checks that it is possible to explicitly select multiple
+// IPv4 addresses on a single interface.
+TEST_F(CfgIfaceTest, multipleAddressesSameInterfaceV4) {
+    CfgIface cfg;
+    ASSERT_NO_THROW(cfg.use(AF_INET, "eth1/192.0.2.3"));
+    // Cannot add the same address twice.
+    ASSERT_THROW(cfg.use(AF_INET, "eth1/192.0.2.3"), DuplicateAddress);
+    // Can add another address on this interface.
+    ASSERT_NO_THROW(cfg.use(AF_INET, "eth1/192.0.2.5"));
+    // Can't select the whole interface.
+    ASSERT_THROW(cfg.use(AF_INET, "eth1"), DuplicateIfaceName);
+
+    cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
+
+    EXPECT_FALSE(socketOpen("eth0", "10.0.0.1"));
+    EXPECT_TRUE(socketOpen("eth1", "192.0.2.3"));
+    EXPECT_TRUE(socketOpen("eth1", "192.0.2.5"));
+}
+
 // This test checks that the interface names can be explicitly selected
 // by their names and IPv6 sockets are opened on these interfaces.
 TEST_F(CfgIfaceTest, explicitNamesV6) {