From ff8a8d4ce42b5597e5d11982d8c830d57592f5aa Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 13 Feb 2015 20:19:04 +0100 Subject: [PATCH] [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. --- src/lib/dhcp/iface_mgr.cc | 12 ++++++++++ src/lib/dhcp/iface_mgr.h | 3 +++ src/lib/dhcp/tests/iface_mgr_unittest.cc | 22 +++++++++++++++++++ src/lib/dhcpsrv/cfg_iface.cc | 28 ++++++++++++++++++++---- src/lib/dhcpsrv/cfg_iface.h | 14 ++++++++++++ src/lib/dhcpsrv/dhcpsrv_messages.mes | 7 ++++++ 6 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 983123506a..1a6228056c 100644 --- 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 e66a2f040e..ccb3acfce6 100644 --- 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 4bcab923d6..a0564f37f2 100644 --- 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 43ceb5f467..0e043443af 100644 --- 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 cf55d5739d..4edad85f06 100644 --- 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 e3ba5595d5..08afdf4cc0 100644 --- 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 -- 2.47.3