[3695] Use broadcast only with raw sockets, and unicast with udp.

Log a warning if multiple addresses on the same interface are in use,
when raw sockets are in use.

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 983123506a7394b0a850dd1a3597acd50300b44c..1a6228056c0c597a8978f3d55ace6fe116a80b98 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -282,6 +282,18 @@ Iface::setActive(const bool active) {
     }
 }
 
+uint16_t
+Iface::countActive() const {
+    uint16_t count = 0;
+    for (AddressCollection::const_iterator addr_it = addrs_.begin();
+         addr_it != addrs_.end(); ++addr_it) {
+        if (addr_it->get().isV4() && addr_it->isSpecified()) {
+            ++count;
+        }
+    }
+    return (count);
+}
+
 void IfaceMgr::closeSockets() {
     for (IfaceCollection::iterator iface = ifaces_.begin();
          iface != ifaces_.end(); ++iface) {

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index e66a2f040e781a89db265ba21007259079293250..ccb3acfce6392f083ad4ae689cb23b300b8bf112 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -340,6 +340,9 @@ public:
     /// should be active (if true) or inactive (if false).
     void setActive(const bool active);
 
+    /// @brief Returns a number of activated IPv4 addresses on the interface.
+    uint16_t countActive() const;
+
     /// @brief Deletes an address from an interface.
     ///
     /// This only deletes address from collection, it does not physically

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 4bcab923d66b15fae05dfc56abe6d3855632afcf..a0564f37f2991cd7a63766d21819a17c9ee50136 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -90,6 +90,28 @@ TEST(IfaceTest, readBuffer) {
     }
 }
 
+// Check that counting the number of active addresses on the interface
+// works as expected.
+TEST(IfaceTest, countActive) {
+    Iface iface("eth0", 0);
+    ASSERT_EQ(0, iface.countActive());
+
+    iface.addAddress(IOAddress("192.168.0.2"));
+    ASSERT_EQ(1, iface.countActive());
+
+    iface.addAddress(IOAddress("2001:db8:1::1"));
+    ASSERT_EQ(1, iface.countActive());
+
+    iface.addAddress(IOAddress("192.168.0.3"));
+    ASSERT_EQ(2, iface.countActive());
+
+    ASSERT_NO_THROW(iface.setActive(IOAddress("192.168.0.2"), false));
+    ASSERT_EQ(1, iface.countActive());
+
+    ASSERT_NO_THROW(iface.setActive(IOAddress("192.168.0.3"), false));
+    ASSERT_EQ(0, iface.countActive());
+}
+
 /// Mock object implementing PktFilter class.  It is used by
 /// IfaceMgrTest::setPacketFilter to verify that IfaceMgr::setPacketFilter
 /// sets this object as a handler for opening sockets. This dummy

diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc
index 43ceb5f46782982f8fe8283cddd55996a570710b..0e043443afb4caf94a8b94342b277b6ab8a2bbf9 100644 (file)
--- a/src/lib/dhcpsrv/cfg_iface.cc
+++ b/src/lib/dhcpsrv/cfg_iface.cc
@@ -43,6 +43,18 @@ CfgIface::equals(const CfgIface& other) const {
             socket_type_ == other.socket_type_);
 }
 
+bool
+CfgIface::multipleAddressesPerInterfaceActive() const {
+    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
+         iface != ifaces.end(); ++iface) {
+        if (iface->countActive() > 1) {
+            return (true);
+        }
+    }
+    return (false);
+}
+
 void
 CfgIface::openSockets(const uint16_t family, const uint16_t port,
                       const bool use_bcast) const {
@@ -124,10 +136,18 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port,
         boost::bind(&CfgIface::socketOpenErrorHandler, _1);
     bool sopen;
     if (family == AF_INET) {
-        // 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);
+        // Use broadcast only if we're using raw sockets. For the UDP sockets,
+        // we only handle the relayed (unicast) traffic.
+        const bool can_use_bcast = use_bcast && (socket_type_ == SOCKET_RAW);
+        // Opening multiple raw sockets handling brodcast traffic on the single
+        // interface may lead to processing the same message multiple times.
+        // We don't prohibit such configuration because raw sockets can as well
+        // handle the relayed traffic. We have to issue a warning, however, to
+        // draw administrator's attention.
+        if (can_use_bcast && multipleAddressesPerInterfaceActive()) {
+            LOG_WARN(dhcpsrv_logger, DHCPSRV_MULTIPLE_RAW_SOCKETS_PER_IFACE);
+        }
+        sopen = IfaceMgr::instance().openSockets4(port, can_use_bcast, error_callback);
     } else {
         // use_bcast is ignored for V6.
         sopen = IfaceMgr::instance().openSockets6(port, error_callback);

diff --git a/src/lib/dhcpsrv/cfg_iface.h b/src/lib/dhcpsrv/cfg_iface.h
index cf55d5739d005ab52f8cc3f5801e10ec3528be0a..4edad85f06fb8011c90a7b82b7204b36edffb72a 100644 (file)
--- a/src/lib/dhcpsrv/cfg_iface.h
+++ b/src/lib/dhcpsrv/cfg_iface.h
@@ -258,6 +258,20 @@ public:
 
 private:
 
+    /// @brief Checks if multiple IPv4 addresses has been activate on any
+    /// interface.
+    ///
+    /// This method is useful to check if the current configuration uses
+    /// multiple IPv4 addresses on any interface. This is important when
+    /// using raw sockets to recieve messages from the clients because
+    /// each packet may be received multiple times when it is sent from
+    /// a directly connected client. If this is the case, a warning must
+    /// be logged.
+    ///
+    /// @return true if multiple addresses are activated on any interface,
+    /// false otherwise.
+    bool multipleAddressesPerInterfaceActive() const;
+
     /// @brief Selects or deselects interfaces.
     ///
     /// This function selects all interfaces to receive DHCP traffic or

diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index e3ba5595d5bf4fd45d41644ccc7c64942e7ad920..08afdf4cc0d78e06157ab4502ff032e42aff8c7c 100644 (file)
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -79,6 +79,13 @@ of active interfaces. This doesn't prevent the server from listening to
 the DHCP traffic through open sockets, but will rather be used by Interface
 Manager to select active interfaces when sockets are re-opened.
 
+% DHCPSRV_MULTIPLE_RAW_SOCKETS_PER_IFACE current configuration will result in opening multiple brodcast capable sockets on some interfaces and some DHCP messages may be duplicated
+A warning message issued when the current configuration indicates that multiple
+sockets, capable of receiving brodcast traffic, will be opened on some of the
+interfaces. It must be noted that this may lead to receiving and processing
+the same DHCP message multiple times, as it will be received by each socket
+individually.
+
 % DHCPSRV_CFGMGR_NO_SUBNET4 no suitable subnet is defined for address hint %1
 This debug message is output when the DHCP configuration manager has received
 a request for an IPv4 subnet for the specified address, but no such