Browse Source

[master] Merge branch 'trac3695'

Marcin Siodelski 10 years ago
parent
commit
3116243706

+ 33 - 6
doc/guide/dhcp4-srv.xml

@@ -377,11 +377,9 @@ It is anticipated that this form of usage will only be used when it is desired t
 temporarily override a list of interface names and listen on all interfaces.
   </para>
   <para>Some deployments of the DHCP servers require that the servers listen
-  on the interfaces with multiple IPv4 addresses configured. In some cases,
-  multiple instances of the DHCP servers are running concurrently and each
-  instance should be bound to a different address on the particular interface.
-  In these situations, the address to use can be selected by
-  appending an IPv4 address to the interface name in the following manner:
+  on the interfaces with multiple IPv4 addresses configured. In these situations,
+  the address to use can be selected by appending an IPv4 address to the interface
+  name in the following manner:
   <screen>
 "Dhcp4": {
     "interfaces-config": {
@@ -390,9 +388,24 @@ temporarily override a list of interface names and listen on all interfaces.
     ...
 }
   </screen>
-  Note that only one address can be specified on each interface.
   </para>
 
+  <para>If it is desired that the server listens on multiple IPv4 addresses assigned
+  to the same interface, multiple addresses can be specified for this interface
+  as in the example below:
+  <screen>
+"Dhcp4": {
+    "interfaces-config": {
+        "interfaces": [ <userinput>"eth1/10.0.0.1", "eth1/10.0.0.2"</userinput> ]
+    },
+    ...
+}
+  </screen>
+  </para>
+
+  <para>Alternatively, if the server should listen on all addresses for the particular
+  interface, an interface name without any address should be specified.</para>
+
   <para>Kea supports responding to directly connected clients which don't have
   an address configured on the interface yet. This requires that the server
   injects the hardware address of the destination into the data link layer
@@ -428,6 +441,20 @@ temporarily override a list of interface names and listen on all interfaces.
   default value <userinput>raw</userinput> is used.
   </para>
 
+  <para>Using UDP sockets automatically disables the reception of brodcast
+  packets from directly connected clients. This effectively means that the
+  UDP sockets can be used for relayed traffic only. When using the raw sockets,
+  both the traffic from the directly connected clients and the relayed traffic
+  will be handled. Caution should be taken when configuring the server to open
+  multiple raw sockets on the interface with several IPv4 addresses assigned.
+  If the directly connected client sends the message to the brodcast address
+  all sockets on this link will receive this message and multiple responses
+  will be sent to the client. Hence, the configuration with multiple IPv4
+  addresses assigned to the interface should not be used when the directly
+  connected clients are operating on that link. To use a single address on
+  such interface, the "interface-name/address" notation should be used.
+  </para>
+
   <note>
     <para>Specifying the value <userinput>raw</userinput> as the socket type,
     doesn't guarantee that the raw sockets will be used! The use of raw sockets

+ 41 - 32
src/bin/dhcp4/dhcp4_srv.cc

@@ -522,10 +522,10 @@ Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     answer->setIface(question->getIface());
     answer->setIndex(question->getIndex());
 
-    answer->setSiaddr(IOAddress("0.0.0.0")); // explicitly set this to 0
+    answer->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS()); // explicitly set this to 0
     // ciaddr is always 0, except for the Renew/Rebind state when it may
     // be set to the ciaddr sent by the client.
-    answer->setCiaddr(IOAddress("0.0.0.0"));
+    answer->setCiaddr(IOAddress::IPV4_ZERO_ADDRESS());
     answer->setHops(question->getHops());
 
     // copy MAC address
@@ -949,7 +949,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
             .arg(question->getRemoteAddr().toText())
             .arg(serverReceivedPacketName(question->getType()));
         answer->setType(DHCPNAK);
-        answer->setYiaddr(IOAddress("0.0.0.0"));
+        answer->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
         return;
     }
 
@@ -981,11 +981,11 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     // ciaddr.
     OptionCustomPtr opt_requested_address = boost::dynamic_pointer_cast<
         OptionCustom>(question->getOption(DHO_DHCP_REQUESTED_ADDRESS));
-    IOAddress hint("0.0.0.0");
+    IOAddress hint(0);
     if (opt_requested_address) {
         hint = opt_requested_address->readAddress();
 
-    } else if (question->getCiaddr() != IOAddress("0.0.0.0")) {
+    } else if (question->getCiaddr() != IOAddress::IPV4_ZERO_ADDRESS()) {
         hint = question->getCiaddr();
 
     }
@@ -1023,7 +1023,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
                 .arg(hwaddr ? hwaddr->toText():"(no hwaddr info)");
 
             answer->setType(DHCPNAK);
-            answer->setYiaddr(IOAddress("0.0.0.0"));
+            answer->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
             return;
         }
         // Now check the second error case: unknown client.
@@ -1191,7 +1191,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
             .arg(hint.toText());
 
         answer->setType(DHCPNAK);
-        answer->setYiaddr(IOAddress("0.0.0.0"));
+        answer->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
 
         answer->delOption(DHO_FQDN);
         answer->delOption(DHO_HOST_NAME);
@@ -1211,6 +1211,27 @@ Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) {
     response->setRemotePort(query->isRelayed() ? DHCP4_SERVER_PORT :
                             DHCP4_CLIENT_PORT);
 
+
+    IOAddress local_addr = query->getLocalAddr();
+
+    // In many cases the query is sent to a broadcast address. This address
+    // apears as a local address in the query message. We can't simply copy
+    // this address to a response message and use it as a source address.
+    // Instead we will need to use the address assigned to the interface
+    // on which the query has been received. In other cases, we will just
+    // use this address as a source address for the response.
+    if (local_addr == IOAddress::IPV4_BCAST_ADDRESS()) {
+        SocketInfo sock_info = IfaceMgr::instance().getSocket(*query);
+        local_addr = sock_info.addr_;
+    }
+    // We assume that there is an appropriate socket bound to this address
+    // and that the address is correct. This is safe assumption because
+    // the local address of the query is set when the query is received.
+    // The query sent to an incorrect address wouldn't have been received.
+    // However, if socket is closed for this address between the reception
+    // of the query and sending a response, the IfaceMgr should detect it
+    // and return an error.
+    response->setLocalAddr(local_addr);
     // In many cases the query is sent to a broadcast address. This address
     // appears as a local address in the query message. Therefore we can't
     // simply copy local address from the query and use it as a source
@@ -1219,9 +1240,6 @@ Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) {
     // may throw if for some reason the socket is closed.
     /// @todo Consider an optimization that we use local address from
     /// the query if this address is not broadcast.
-    SocketInfo sock_info = IfaceMgr::instance().getSocket(*query);
-    // Set local address, port and interface.
-    response->setLocalAddr(sock_info.addr_);
     response->setLocalPort(DHCP4_SERVER_PORT);
     response->setIface(query->getIface());
     response->setIndex(query->getIndex());
@@ -1229,13 +1247,6 @@ Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) {
 
 void
 Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) {
-    // Let's create static objects representing zeroed and broadcast
-    // addresses. We will use them further in this function to test
-    // other addresses against them. Since they are static, they will
-    // be created only once.
-    static const IOAddress zero_addr("0.0.0.0");
-    static const IOAddress bcast_addr("255.255.255.255");
-
     // The DHCPINFORM is slightly different than other messages in a sense
     // that the server should always unicast the response to the ciaddr.
     // It appears however that some clients don't set the ciaddr. We still
@@ -1244,7 +1255,7 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) {
     if (question->getType() == DHCPINFORM) {
         // If client adheres to RFC2131 it will set the ciaddr and in this
         // case we always unicast our response to this address.
-        if (question->getCiaddr() != zero_addr) {
+        if (question->getCiaddr() != IOAddress::IPV4_ZERO_ADDRESS()) {
             response->setRemoteAddr(question->getCiaddr());
 
         // If we received DHCPINFORM via relay and the ciaddr is not set we
@@ -1275,24 +1286,24 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) {
         // the message is relayed. Therefore, we set the BROADCAST flag so
         // as the relay can broadcast the packet.
         if ((question->getType() == DHCPINFORM) &&
-            (question->getCiaddr() == zero_addr)) {
+            (question->getCiaddr() == IOAddress::IPV4_ZERO_ADDRESS())) {
             response->setFlags(BOOTP_BROADCAST);
         }
         response->setRemoteAddr(question->getGiaddr());
 
     // If giaddr is 0 but client set ciaddr, server should unicast the
     // response to ciaddr.
-    } else if (question->getCiaddr() != zero_addr) {
+    } else if (question->getCiaddr() != IOAddress::IPV4_ZERO_ADDRESS()) {
         response->setRemoteAddr(question->getCiaddr());
 
     // We can't unicast the response to the client when sending NAK,
     // because we haven't allocated address for him. Therefore,
     // NAK is broadcast.
     } else if (response->getType() == DHCPNAK) {
-        response->setRemoteAddr(bcast_addr);
+        response->setRemoteAddr(IOAddress::IPV4_BCAST_ADDRESS());
 
     // If yiaddr is set it means that we have created a lease for a client.
-    } else if (response->getYiaddr() != zero_addr) {
+    } else if (response->getYiaddr() != IOAddress::IPV4_ZERO_ADDRESS()) {
         // If the broadcast bit is set in the flags field, we have to
         // send the response to broadcast address. Client may have requested it
         // because it doesn't support reception of messages on the interface
@@ -1301,7 +1312,7 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) {
         // directly to a client without address assigned.
         const bool bcast_flag = ((question->getFlags() & Pkt4::FLAG_BROADCAST_MASK) != 0);
         if (!IfaceMgr::instance().isDirectResponseSupported() || bcast_flag) {
-            response->setRemoteAddr(bcast_addr);
+            response->setRemoteAddr(IOAddress::IPV4_BCAST_ADDRESS());
 
         // Client cleared the broadcast bit and we support direct responses
         // so we should unicast the response to a newly allocated address -
@@ -1356,7 +1367,7 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
     }
 
     // Adding any other options makes sense only when we got the lease.
-    if (offer->getYiaddr() != IOAddress("0.0.0.0")) {
+    if (offer->getYiaddr() != IOAddress::IPV4_ZERO_ADDRESS()) {
         appendRequestedOptions(discover, offer);
         appendRequestedVendorOptions(discover, offer);
         // There are a few basic options that we always want to
@@ -1408,7 +1419,7 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
     }
 
     // Adding any other options makes sense only when we got the lease.
-    if (ack->getYiaddr() != IOAddress("0.0.0.0")) {
+    if (ack->getYiaddr() != IOAddress::IPV4_ZERO_ADDRESS()) {
         appendRequestedOptions(request, ack);
         appendRequestedVendorOptions(request, ack);
         // There are a few basic options that we always want to
@@ -1562,7 +1573,7 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
     // Also Relay Agent Options should be removed if present.
     if (ack->getRemoteAddr() != inform->getGiaddr()) {
         ack->setHops(0);
-        ack->setGiaddr(IOAddress("0.0.0.0"));
+        ack->setGiaddr(IOAddress::IPV4_ZERO_ADDRESS());
         ack->delOption(DHO_DHCP_AGENT_OPTIONS);
     }
 
@@ -1703,11 +1714,10 @@ Dhcpv4Srv::acceptDirectRequest(const Pkt4Ptr& pkt) const {
     // The source address must not be zero for the DHCPINFORM message from
     // the directly connected client because the server will not know where
     // to respond if the ciaddr was not present.
-    static const IOAddress zero_addr("0.0.0.0");
     try {
         if (pkt->getType() == DHCPINFORM) {
-            if ((pkt->getRemoteAddr() == zero_addr) &&
-                (pkt->getCiaddr() == zero_addr)) {
+            if ((pkt->getRemoteAddr() == IOAddress::IPV4_ZERO_ADDRESS()) &&
+                (pkt->getCiaddr() == IOAddress::IPV4_ZERO_ADDRESS())) {
                 return (false);
             }
         }
@@ -1717,8 +1727,7 @@ Dhcpv4Srv::acceptDirectRequest(const Pkt4Ptr& pkt) const {
         // we validate the message type prior to calling this function.
         return (false);
     }
-    static const IOAddress bcast("255.255.255.255");
-    return ((pkt->getLocalAddr() != bcast || selectSubnet(pkt)));
+    return ((pkt->getLocalAddr() != IOAddress::IPV4_BCAST_ADDRESS() || selectSubnet(pkt)));
 }
 
 bool
@@ -2034,7 +2043,7 @@ bool Dhcpv4Srv::classSpecificProcessing(const Pkt4Ptr& query, const Pkt4Ptr& rsp
     if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER)) {
 
         // Do not set TFTP server address for eRouter devices.
-        rsp->setSiaddr(IOAddress("0.0.0.0"));
+        rsp->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS());
     }
 
     return (true);

+ 3 - 3
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -81,7 +81,7 @@ TEST_F(Dhcpv4SrvTest, adjustIfaceDataRelay) {
     req->setFlags(0x0000);
 
     // Set local address, port and interface.
-    req->setLocalAddr(IOAddress("192.0.2.1"));
+    req->setLocalAddr(IOAddress("192.0.2.5"));
     req->setLocalPort(1001);
     req->setIface("eth1");
     req->setIndex(1);
@@ -104,7 +104,7 @@ TEST_F(Dhcpv4SrvTest, adjustIfaceDataRelay) {
     // The query has been relayed, so the response must be sent to the port 67.
     EXPECT_EQ(DHCP4_SERVER_PORT, resp->getRemotePort());
     // Local address should be the address assigned to interface eth1.
-    EXPECT_EQ("192.0.2.3", resp->getLocalAddr().toText());
+    EXPECT_EQ("192.0.2.5", resp->getLocalAddr().toText());
     // The local port is always DHCPv4 server port 67.
     EXPECT_EQ(DHCP4_SERVER_PORT, resp->getLocalPort());
     // We will send response over the same interface which was used to receive
@@ -184,7 +184,7 @@ TEST_F(Dhcpv4SrvTest, adjustIfaceDataRenew) {
     EXPECT_EQ(DHCP4_CLIENT_PORT, resp->getRemotePort());
     // The response should be sent from the unicast address on which the
     // query has been received.
-    EXPECT_EQ("192.0.2.3", resp->getLocalAddr().toText());
+    EXPECT_EQ("192.0.2.1", resp->getLocalAddr().toText());
     // The response should be sent from the DHCPv4 server port.
     EXPECT_EQ(DHCP4_SERVER_PORT, resp->getLocalPort());
     // The interface data should match the data in the query.

+ 2 - 0
src/bin/dhcp4/tests/dora_unittest.cc

@@ -656,6 +656,8 @@ TEST_F(DORATest, reservationsWithConflicts) {
 
     // Client A performs 4-way exchange.
     client.setState(Dhcp4Client::SELECTING);
+    // Revert to the broadcast address for the selcting client.
+    client.setDestAddress(IOAddress::IPV4_BCAST_ADDRESS());
     // Obtain a lease from the server using the 4-way exchange.
     ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
                                   IOAddress>(new IOAddress("0.0.0.0"))));

+ 24 - 1
src/lib/asiolink/io_address.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2010,2015 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
@@ -230,6 +230,29 @@ public:
     ///         network byte order
     operator uint32_t () const;
 
+    /// @name Methods returning @c IOAddress objects encapsulating typical addresses.
+    ///
+    //@{
+    /// @brief Returns an address set to all zeros.
+    static const IOAddress& IPV4_ZERO_ADDRESS() {
+        static IOAddress address(0);
+        return (address);
+    }
+
+    /// @brief Returns a "255.255.255.255" broadcast address.
+    static const IOAddress& IPV4_BCAST_ADDRESS() {
+        static IOAddress address(0xFFFFFFFF);
+        return (address);
+    }
+
+    /// @brief Returns an IPv6 zero address.
+    static const IOAddress& IPV6_ZERO_ADDRESS() {
+        static IOAddress address("::");
+        return (address);
+    }
+
+    //@}
+
 private:
     asio::ip::address asio_address_;
 };

+ 7 - 1
src/lib/asiolink/tests/io_address_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011, 2015 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
@@ -217,3 +217,9 @@ TEST(IOAddressTest, accessClassificationMethods) {
     EXPECT_FALSE(addr5.isV6LinkLocal());
     EXPECT_TRUE (addr5.isV6Multicast());
 }
+
+TEST(IOAddressTest, staticAddresses) {
+    EXPECT_EQ(IOAddress("0.0.0.0"), IOAddress::IPV4_ZERO_ADDRESS());
+    EXPECT_EQ(IOAddress("255.255.255.255"), IOAddress::IPV4_BCAST_ADDRESS());
+    EXPECT_EQ(IOAddress("::"), IOAddress::IPV6_ZERO_ADDRESS());
+}

+ 32 - 9
src/lib/dhcp/iface_mgr.cc

@@ -257,6 +257,18 @@ Iface::setActive(const bool active) {
     }
 }
 
+unsigned int
+Iface::countActive4() 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) {
@@ -1140,7 +1152,7 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
 uint16_t IfaceMgr::getSocket(const isc::dhcp::Pkt6& pkt) {
     Iface* iface = getIface(pkt.getIface());
     if (iface == NULL) {
-        isc_throw(BadValue, "Tried to find socket for non-existent interface");
+        isc_throw(IfaceNotFound, "Tried to find socket for non-existent interface");
     }
 
 
@@ -1189,7 +1201,7 @@ uint16_t IfaceMgr::getSocket(const isc::dhcp::Pkt6& pkt) {
         return (candidate->sockfd_);
     }
 
-    isc_throw(Unexpected, "Interface " << iface->getFullName()
+    isc_throw(SocketNotFound, "Interface " << iface->getFullName()
               << " does not have any suitable IPv6 sockets open.");
 }
 
@@ -1197,22 +1209,33 @@ SocketInfo
 IfaceMgr::getSocket(isc::dhcp::Pkt4 const& pkt) {
     Iface* iface = getIface(pkt.getIface());
     if (iface == NULL) {
-        isc_throw(BadValue, "Tried to find socket for non-existent interface");
+        isc_throw(IfaceNotFound, "Tried to find socket for non-existent interface");
     }
 
     const Iface::SocketCollection& socket_collection = iface->getSockets();
+    // A candidate being an end of the iterator marks that it is a begining of
+    // the socket search and that the candidate needs to be set to the first
+    // socket found.
+    Iface::SocketCollection::const_iterator candidate = socket_collection.end();
     Iface::SocketCollection::const_iterator s;
     for (s = socket_collection.begin(); s != socket_collection.end(); ++s) {
         if (s->family_ == AF_INET) {
-            return (*s);
+            if (s->addr_ == pkt.getLocalAddr()) {
+                return (*s);
+            }
+
+            if (candidate == socket_collection.end()) {
+                candidate = s;
+            }
         }
-        /// TODO: Add more checks here later. If remote address is
-        /// not link-local, we can't use link local bound socket
-        /// to send data.
     }
 
-    isc_throw(Unexpected, "Interface " << iface->getFullName()
-              << " does not have any suitable IPv4 sockets open.");
+    if (candidate == socket_collection.end()) {
+        isc_throw(SocketNotFound, "Interface " << iface->getFullName()
+                  << " does not have any suitable IPv4 sockets open.");
+    }
+
+    return (*candidate);
 }
 
 } // end of namespace isc::dhcp

+ 30 - 9
src/lib/dhcp/iface_mgr.h

@@ -82,6 +82,20 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief IfaceMgr exception thrown when there is no suitable interface.
+class IfaceNotFound : public Exception {
+public:
+    IfaceNotFound(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+/// @brief IfaceMgr exception thrown when there is no suitable socket found.
+class SocketNotFound : public Exception {
+public:
+    SocketNotFound(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 
 /// Holds information about socket.
 struct SocketInfo {
@@ -325,6 +339,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.
+    unsigned int countActive4() const;
+
     /// @brief Deletes an address from an interface.
     ///
     /// This only deletes address from collection, it does not physically
@@ -614,26 +631,30 @@ public:
     ///
     /// This method takes Pkt6 (see overloaded implementation that takes
     /// Pkt4) and chooses appropriate socket to send it. This method
-    /// may throw BadValue if specified packet does not have outbound
-    /// interface specified, no such interface exists, or specified
-    /// interface does not have any appropriate sockets open.
+    /// may throw if specified packet does not have outbound interface specified,
+    /// no such interface exists, or specified interface does not have any
+    /// appropriate sockets open.
     ///
     /// @param pkt a packet to be transmitted
     ///
     /// @return a socket descriptor
+    /// @throw SocketNotFound If no suitable socket found.
+    /// @throw IfaceNotFound If interface is not set for the packet.
     uint16_t getSocket(const isc::dhcp::Pkt6& pkt);
 
     /// @brief Return most suitable socket for transmitting specified IPv4 packet.
     ///
-    /// This method takes Pkt4 (see overloaded implementation that takes
-    /// Pkt6) and chooses appropriate socket to send it. This method
-    /// may throw BadValue if specified packet does not have outbound
-    /// interface specified, no such interface exists, or specified
-    /// interface does not have any appropriate sockets open.
+    /// This method uses the local address assigned to the packet and tries
+    /// to match it with addresses to which sockets are bound for the particular
+    /// interface. If the match is not found, the method returns the first IPv4
+    /// socket found for the particular interface. In case, there are no IPv4
+    /// sockets assigned to the interface the exception is thrown.
     ///
-    /// @param pkt a packet to be transmitted
+    /// @param pkt A packet to be transmitted. It must hold a local address and
+    /// a valid pointer to the interface.
     ///
     /// @return A structure describing a socket.
+    /// @throw SocketNotFound if no suitable socket found.
     SocketInfo getSocket(const isc::dhcp::Pkt4& pkt);
 
     /// Debugging method that prints out all available interfaces.

+ 39 - 7
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, countActive4) {
+    Iface iface("eth0", 0);
+    ASSERT_EQ(0, iface.countActive4());
+
+    iface.addAddress(IOAddress("192.168.0.2"));
+    ASSERT_EQ(1, iface.countActive4());
+
+    iface.addAddress(IOAddress("2001:db8:1::1"));
+    ASSERT_EQ(1, iface.countActive4());
+
+    iface.addAddress(IOAddress("192.168.0.3"));
+    ASSERT_EQ(2, iface.countActive4());
+
+    ASSERT_NO_THROW(iface.setActive(IOAddress("192.168.0.2"), false));
+    ASSERT_EQ(1, iface.countActive4());
+
+    ASSERT_NO_THROW(iface.setActive(IOAddress("192.168.0.3"), false));
+    ASSERT_EQ(0, iface.countActive4());
+}
+
 /// 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
@@ -2158,17 +2180,17 @@ TEST_F(IfaceMgrTest, socketInfo) {
 
     Pkt6 pkt6(DHCPV6_REPLY, 123456);
 
-    // pkt6 dos not have interface set yet
+    // pkt6 does not have interface set yet
     EXPECT_THROW(
         ifacemgr->getSocket(pkt6),
-        BadValue
+        IfaceNotFound
     );
 
     // Try to send over non-existing interface
     pkt6.setIface("nosuchinterface45");
     EXPECT_THROW(
         ifacemgr->getSocket(pkt6),
-        BadValue
+        IfaceNotFound
     );
 
     // This will work
@@ -2184,7 +2206,7 @@ TEST_F(IfaceMgrTest, socketInfo) {
     // It should throw again, there's no usable socket anymore
     EXPECT_THROW(
         ifacemgr->getSocket(pkt6),
-        Unexpected
+        SocketNotFound
     );
 
     // Repeat for pkt4
@@ -2193,20 +2215,30 @@ TEST_F(IfaceMgrTest, socketInfo) {
     // pkt4 does not have interface set yet.
     EXPECT_THROW(
         ifacemgr->getSocket(pkt4),
-        BadValue
+        IfaceNotFound
     );
 
     // Try to send over non-existing interface.
     pkt4.setIface("nosuchinterface45");
     EXPECT_THROW(
         ifacemgr->getSocket(pkt4),
-        BadValue
+        IfaceNotFound
     );
 
     // Socket info is set, packet has well defined interface. It should work.
     pkt4.setIface(LOOPBACK);
     EXPECT_EQ(7, ifacemgr->getSocket(pkt4).sockfd_);
 
+    // Set the local address to check if the socket for this address will
+    // be returned.
+    pkt4.setLocalAddr(IOAddress("192.0.2.56"));
+    EXPECT_EQ(7, ifacemgr->getSocket(pkt4).sockfd_);
+
+    // Modify the local address and expect that the other socket will be
+    // returned.
+    pkt4.setLocalAddr(IOAddress("192.0.2.53"));
+    EXPECT_EQ(8, ifacemgr->getSocket(pkt4).sockfd_);
+
     EXPECT_NO_THROW(
         ifacemgr->getIface(LOOPBACK)->delSocket(7);
         ifacemgr->getIface(LOOPBACK)->delSocket(8);
@@ -2215,7 +2247,7 @@ TEST_F(IfaceMgrTest, socketInfo) {
     // It should throw again, there's no usable socket anymore.
     EXPECT_THROW(
         ifacemgr->getSocket(pkt4),
-        Unexpected
+        SocketNotFound
     );
 }
 

+ 2 - 2
src/lib/dhcp/tests/pkt_captures4.cc

@@ -62,9 +62,9 @@ Pkt4Ptr PktCaptures::packetFromCapture(const std::string& hex_string) {
 
 void PktCaptures::captureSetDefaultFields(const Pkt4Ptr& pkt) {
     pkt->setRemotePort(546);
-    pkt->setRemoteAddr(IOAddress("fe80::1"));
+    pkt->setRemoteAddr(IOAddress("10.0.0.2"));
     pkt->setLocalPort(0);
-    pkt->setLocalAddr(IOAddress("ff02::1:2"));
+    pkt->setLocalAddr(IOAddress("10.0.0.1"));
     pkt->setIndex(2);
     pkt->setIface("eth0");
 }

+ 80 - 50
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;
 
@@ -42,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->countActive4() > 1) {
+            return (true);
+        }
+    }
+    return (false);
+}
+
 void
 CfgIface::openSockets(const uint16_t family, const uint16_t port,
                       const bool use_bcast) const {
@@ -85,20 +98,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;
@@ -106,19 +106,24 @@ 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");
-            }
+    // Select unicast sockets for DHCPv6 or activate specific IPv4 addresses
+    // for DHCPv4.
+    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,8 +137,18 @@ 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,
-                                                  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);
@@ -170,14 +185,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 +239,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 +255,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 +274,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 +344,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 +372,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");
         }

+ 59 - 6
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
@@ -232,6 +258,20 @@ public:
 
 private:
 
+    /// @brief Checks if multiple IPv4 addresses has been activated 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
@@ -246,6 +286,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 active,
+                            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 +317,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.

+ 7 - 0
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

+ 19 - 2
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) {