[2320] Allocation Engine hooked up to DHCPv4 server

 - DORA support added
 - REQUEST/Renewing support added
 - RELEASE support added
 - AllocationEngine now supports lease4 renewals
 - many other smaller code additions
 - many tests written

src/bin/dhcp4/dhcp4_messages.mes

@@ -26,12 +26,6 @@ to establish a session with the BIND 10 control channel.
 A debug message listing the command (and possible arguments) received
 from the BIND 10 control system by the IPv4 DHCP server.
 
-% DHCP4_CONFIG_COMPLETE DHCPv4 server has completed configuration: %1
-This is an informational message announcing the successful processing of a
-new configuration. it is output during server startup, and when an updated
-configuration is committed by the administrator.  Additional information
-may be provided.
-
 % DHCP4_CONFIG_LOAD_FAIL failed to load configuration: %1
 This critical error message indicates that the initial DHCPv4
 configuration has failed. The server will start, but nothing will be
@@ -41,19 +35,51 @@ served until the configuration has been corrected.
 This is an informational message reporting that the configuration has
 been extended to include the specified IPv4 subnet.
 
+% DHCP4_CONFIG_OPTION_DUPLICATE multiple options with the code: %1 added to the subnet: %2
+This warning message is issued on an attempt to configure multiple options with the
+same option code for the particular subnet. Adding multiple options is uncommon
+for DHCPv4, but it is not prohibited.
+
+% DHCP4_CONFIG_UPDATE updated configuration received: %1
+A debug message indicating that the IPv4 DHCP server has received an
+updated configuration from the BIND 10 configuration system.
+
 % DHCP4_CONFIG_START DHCPv4 server is processing the following configuration: %1
 This is a debug message that is issued every time the server receives a
 configuration. That happens at start up and also when a server configuration
 change is committed by the administrator.
 
-% DHCP4_CONFIG_UPDATE updated configuration received: %1
-A debug message indicating that the IPv4 DHCP server has received an
-updated configuration from the BIND 10 configuration system.
+% DHCP4_CONFIG_COMPLETE DHCPv4 server has completed configuration: %1
+This is an informational message announcing the successful processing of a
+new configuration. it is output during server startup, and when an updated
+configuration is committed by the administrator.  Additional information
+may be provided.
 
-% DHCP4_CONFIG_OPTION_DUPLICATE multiple options with the code: %1 added to the subnet: %2
-This warning message is issued on an attempt to configure multiple options with the
-same option code for the particular subnet. Adding multiple options is uncommon
-for DHCPv4, but it is not prohibited.
+% DHCP4_DB_BACKEND_STARTED lease database started (type: %1, name: %2)
+This informational message is printed every time DHCPv4 server is started.
+It indicates what database backend type is being to store lease and
+other information.
+
+% DHCP4_LEASE_ADVERT lease %1 advertised (client client-id=%2, iaid=%3)
+This debug message indicates that the server successfully advertised
+a lease. It is up to the client to choose one server out of othe advertised
+and continue allocation with that server. This is a normal behavior and
+indicates successful operation.
+
+% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id=%1, iaid=%2
+This message indicates that the server failed to offer (in response to
+received DISCOVER) a lease for a given client. There may be many reasons for
+such failure. Each specific failure is logged in a separate log entry.
+
+% DHCP4_LEASE_ALLOC lease %1 has been allocated (client duid=%2, iaid=%3)
+This debug message indicates that the server successfully granted (in
+response to client's REQUEST message) a lease. This is a normal behavior
+and incicates successful operation.
+
+% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client duid=%1, iaid=%2
+This message indicates that the server failed to grant (in response to
+received REQUEST) a lease for a given client. There may be many reasons for
+such failure. Each specific failure is logged in a separate log entry.
 
 % DHCP4_NOT_RUNNING IPv4 DHCP server is not running
 A warning message is issued when an attempt is made to shut down the
@@ -91,6 +117,36 @@ to be a programming error: please raise a bug report.
 % DHCP4_QUERY_DATA received packet type %1, data is <%2>
 A debug message listing the data received from the client.
 
+% DHCP4_RELEASE address %1 belonging to client-id=%2, hwaddr=%3 was released properly.
+This debug message indicates that an address was released properly. It
+is a normal operation during client shutdown.
+
+% DHCP4_RELEASE_FAIL failed to remove lease for address %1 for duid=%2, hwaddr=%3
+This error message indicates that the software failed to remove a
+lease from the lease database. It is probably due to an error during a
+database operation: resolution will most likely require administrator
+intervention (e.g. check if DHCP process has sufficient privileges to
+update the database). It may also be triggered if a lease was manually
+removed from the database during RELEASE message processing.
+
+% DHCP4_RELEASE_FAIL_NO_LEASE client (client-id=%2) tried to release address %1, but there is no lease for such address.
+This warning message is printed when client attempts to release a lease,
+but no such lease is known by the server.
+
+% DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID client (client-id=%2) tried to release address %1, but it belongs to client (client-id=%3)
+This warning message indicates that client tried to release an address
+that belongs to a different client. This should not happen in normal
+circumstances and may indicate a misconfiguration of the client.  However,
+since the client releasing the address will stop using it anyway, there
+is a good chance that the situation will correct itself.
+
+% DHCP4_RELEASE_FAIL_WRONG_HWADDR client (client-id=%2) tried to release address %1, but sent from a wrong hardware address (%3)
+This warning message indicates that client tried to release an address
+that does belong to it, but the lease information was associated with a different
+hardware address. One possible reason for using different hardware address is
+that a cloned virtual machine was not updated and both clones use the same
+client-id.
+
 % DHCP4_RESPONSE_DATA responding with packet type %1, data is <%2>
 A debug message listing the data returned to the client.
 
@@ -131,3 +187,13 @@ processed any command-line switches and is starting.
 This is a debug message issued during the IPv4 DHCP server startup.
 It lists some information about the parameters with which the server
 is running.
+
+% DHCP4_SUBNET_SELECTED the %1 subnet was selected for client assignment
+This is a debug message informing that a given subnet was selected. It will
+be used for address and option assignment. This is one of the early steps
+in the processing of incoming client message.
+
+% DHCP4_SUBNET_SELECTION_FAILED failed to select a subnet for incoming packet, src=%1 type=%2
+This warning message is output when a packet was received from a subnet for
+which the DHCPv4 server has not been configured. The cause is most likely due
+to a misconfiguration of the server.

src/bin/dhcp4/dhcp4_srv.cc

@@ -16,9 +16,19 @@
 #include <dhcp/dhcp4.h>
 #include <dhcp/iface_mgr.h>
 #include <dhcp/option4_addrlst.h>
+#include <dhcp/option_int.h>
 #include <dhcp/pkt4.h>
+#include <dhcp/duid.h>
+#include <dhcp/hwaddr.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcp4/dhcp4_srv.h>
+#include <dhcpsrv/utils.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/lease_mgr.h>
+#include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/subnet.h>
+#include <dhcpsrv/utils.h>
+#include <dhcpsrv/addr_utilities.h>
 
 using namespace isc;
 using namespace isc::asiolink;
@@ -28,26 +38,35 @@ using namespace std;
 
 // These are hardcoded parameters. Currently this is a skeleton server that only
 // grants those options and a single, fixed, hardcoded lease.
-const std::string HARDCODED_LEASE = "192.0.2.222"; // assigned lease
-const std::string HARDCODED_NETMASK = "255.255.255.0";
-const uint32_t    HARDCODED_LEASE_TIME = 60; // in seconds
 const std::string HARDCODED_GATEWAY = "192.0.2.1";
 const std::string HARDCODED_DNS_SERVER = "192.0.2.2";
 const std::string HARDCODED_DOMAIN_NAME = "isc.example.com";
 const std::string HARDCODED_SERVER_ID = "192.0.2.1";
 
-Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
+Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
         // First call to instance() will create IfaceMgr (it's a singleton)
         // it may throw something if things go wrong
         IfaceMgr::instance();
 
-        /// @todo: instantiate LeaseMgr here once it is imlpemented.
-        IfaceMgr::instance().openSockets4(port);
+        if (port) {
+            // open sockets only if port is non-zero. Port 0 is used
+            // for non-socket related testing.
+            IfaceMgr::instance().openSockets4(port);
+        }
 
         setServerID();
 
+        // Instantiate LeaseMgr
+        LeaseMgrFactory::create(dbconfig);
+        LOG_INFO(dhcp4_logger, DHCP4_DB_BACKEND_STARTED)
+            .arg(LeaseMgrFactory::instance().getType())
+            .arg(LeaseMgrFactory::instance().getName());
+
+        // Instantiate allocation engine
+        alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
+
     } catch (const std::exception &e) {
         LOG_ERROR(dhcp4_logger, DHCP4_SRV_CONSTRUCT_ERROR).arg(e.what());
         shutdown_ = true;
@@ -167,18 +186,11 @@ Dhcpv4Srv::run() {
 
 void
 Dhcpv4Srv::setServerID() {
-    /// TODO implement this for real once interface detection (ticket 1237)
-    /// is done. Use hardcoded server-id for now.
-
-#if 0
-    // uncomment this once ticket 1350 is merged.
-    IOAddress srvId("127.0.0.1");
-    serverid_ = OptionPtr(
-      new Option4AddrLst(Option::V4, DHO_DHCP_SERVER_IDENTIFIER, srvId));
-#endif
+    /// @todo: implement this for real (see ticket #2588)
+    serverid_ = OptionPtr(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
+                                             IOAddress(HARDCODED_SERVER_ID)));
 }
 
-
 void Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     answer->setIface(question->getIface());
     answer->setIndex(question->getIndex());
@@ -201,6 +213,10 @@ void Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         answer->setRemoteAddr(question->getRemoteAddr());
     }
 
+    OptionPtr client_id = question->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    if (client_id) {
+        answer->addOption(client_id);
+    }
 }
 
 void Dhcpv4Srv::appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type) {
@@ -213,9 +229,7 @@ void Dhcpv4Srv::appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type) {
     msg->addOption(opt);
 
     // DHCP Server Identifier (type 54)
-    opt = OptionPtr
-        (new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER, IOAddress(HARDCODED_SERVER_ID)));
-    msg->addOption(opt);
+    msg->addOption(getServerID());
 
     // more options will be added here later
 }
@@ -235,25 +249,125 @@ void Dhcpv4Srv::appendRequestedOptions(Pkt4Ptr& msg) {
     msg->addOption(opt);
 }
 
-void Dhcpv4Srv::tryAssignLease(Pkt4Ptr& msg) {
-    OptionPtr opt;
+void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
+
+    // We need to select a subnet the client is connected in.
+    Subnet4Ptr subnet = selectSubnet(question);
+    if (!subnet) {
+        // This particular client is out of luck today. We do not have
+        // information about the subnet he is connected to. This likely means
+        // misconfiguration of the server (or some relays). We will continue to
+        // process this message, but our response will be almost useless: no
+        // addresses or prefixes, no subnet specific configuration etc. The only
+        // thing this client can get is some global information (like DNS
+        // servers).
+
+        // perhaps this should be logged on some higher level? This is most likely
+        // configuration bug.
+        LOG_ERROR(dhcp4_logger, DHCP4_SUBNET_SELECTION_FAILED)
+            .arg(question->getRemoteAddr().toText())
+            .arg(serverReceivedPacketName(question->getType()));
+        setMsgType(answer, DHCPNAK);
+        answer->setYiaddr(IOAddress("0.0.0.0"));
+        return;
+    } else {
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED)
+            .arg(subnet->toText());
+    }
 
-    // TODO: Implement actual lease assignment here
-    msg->setYiaddr(IOAddress(HARDCODED_LEASE));
+    // Get client-id option
+    ClientIdPtr client_id;
+    OptionPtr opt = question->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    if (opt) {
+        client_id = ClientIdPtr(new ClientId(opt->getData()));
+    }
+    // client-id is not mandatory in DHCPv4
 
-    // IP Address Lease time (type 51)
-    opt = OptionPtr(new Option(Option::V4, DHO_DHCP_LEASE_TIME));
-    opt->setUint32(HARDCODED_LEASE_TIME);
-    msg->addOption(opt);
-    // TODO: create Option_IntArray that holds list of integers, similar to Option4_AddrLst
+    IOAddress hint = question->getYiaddr();
 
-    // Subnet mask (type 1)
-    opt = OptionPtr(new Option4AddrLst(DHO_SUBNET_MASK, IOAddress(HARDCODED_NETMASK)));
-    msg->addOption(opt);
+    HWAddrPtr hwaddr = question->getHWAddr();
 
-    // Router (type 3)
-    opt = OptionPtr(new Option4AddrLst(DHO_ROUTERS, IOAddress(HARDCODED_GATEWAY)));
-    msg->addOption(opt);
+    // "Fake" allocation is processing of DISCOVER message. We pretend to do an
+    // allocation, but we do not put the lease in the database. That is ok,
+    // because we do not guarantee that the user will get that exact lease. If
+    // the user selects this server to do actual allocation (i.e. sends REQUEST)
+    // it should include this hint. That will help us during the actual lease
+    // allocation.
+    bool fake_allocation = false;
+    if (question->getType() == DHCPDISCOVER) {
+        fake_allocation = true;
+    }
+
+    // Use allocation engine to pick a lease for this client. Allocation engine
+    // will try to honour the hint, but it is just a hint - some other address
+    // may be used instead. If fake_allocation is set to false, the lease will
+    // be inserted into the LeaseMgr as well.
+    Lease4Ptr lease = alloc_engine_->allocateAddress4(subnet, client_id, hwaddr,
+                                                      hint, fake_allocation);
+
+    if (lease) {
+        // We have a lease! Let's wrap its content into IA_NA option
+        // with IAADDR suboption.
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, fake_allocation?
+                  DHCP4_LEASE_ADVERT:DHCP4_LEASE_ALLOC)
+            .arg(client_id?client_id->toText():"(no client-id)")
+            .arg(hwaddr?hwaddr->toText():"(no hwaddr info)")
+            .arg(hint.toText());
+
+        answer->setYiaddr(lease->addr_);
+
+        // IP Address Lease time (type 51)
+        opt = OptionPtr(new Option(Option::V4, DHO_DHCP_LEASE_TIME));
+        opt->setUint32(lease->valid_lft_);
+        answer->addOption(opt);
+
+        // @todo: include real router information here
+        // Router (type 3)
+        opt = OptionPtr(new Option4AddrLst(DHO_ROUTERS, IOAddress(HARDCODED_GATEWAY)));
+        answer->addOption(opt);
+
+        // Subnet mask (type 1)
+        answer->addOption(getNetmaskOption(subnet));
+
+        // @todo: send renew timer option (T1, option 58)
+        // @todo: send rebind timer option (T2, option 59)
+
+    } else {
+        // Allocation engine did not allocate a lease. The engine logged
+        // cause of that failure. The only thing left is to insert
+        // status code to pass the sad news to the client.
+
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, fake_allocation?
+                  DHCP4_LEASE_ADVERT_FAIL:DHCP4_LEASE_ALLOC_FAIL)
+            .arg(client_id?client_id->toText():"(no client-id)")
+            .arg(hwaddr?hwaddr->toText():"(no hwaddr info)")
+            .arg(hint.toText());
+
+        setMsgType(answer, DHCPNAK);
+        answer->setYiaddr(IOAddress("0.0.0.0"));
+    }
+}
+
+OptionPtr Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
+    uint32_t netmask = getNetmask4(subnet->get().second);
+
+    OptionPtr opt(new OptionInt<uint32_t>(Option::V4,
+                  DHO_SUBNET_MASK, netmask));
+
+    return (opt);
+}
+
+void Dhcpv4Srv::setMsgType(Pkt4Ptr& pkt, uint8_t dhcp_type) {
+    OptionPtr opt = pkt->getOption(DHO_DHCP_MESSAGE_TYPE);
+    if (opt) {
+        // There is message type option already, update it
+        opt->setUint8(dhcp_type);
+    } else {
+        // There is no message type option yet, add it
+        std::vector<uint8_t> tmp(1, dhcp_type);
+        opt = OptionPtr(new Option(Option::V4, DHO_DHCP_MESSAGE_TYPE, tmp));
+        pkt->addOption(opt);
+    }
 }
 
 Pkt4Ptr Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
@@ -264,7 +378,7 @@ Pkt4Ptr Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
     appendDefaultOptions(offer, DHCPOFFER);
     appendRequestedOptions(offer);
 
-    tryAssignLease(offer);
+    assignLease(discover, offer);
 
     return (offer);
 }
@@ -277,13 +391,56 @@ Pkt4Ptr Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
     appendDefaultOptions(ack, DHCPACK);
     appendRequestedOptions(ack);
 
-    tryAssignLease(ack);
+    assignLease(request, ack);
 
     return (ack);
 }
 
 void Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
-    /// TODO: Implement this.
+    ClientIdPtr client_id;
+    OptionPtr opt = release->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    if (opt) {
+        client_id = ClientIdPtr(new ClientId(opt->getData()));
+    }
+
+    Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(release->getYiaddr());
+
+    if (!lease) {
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_NO_LEASE)
+            .arg(release->getYiaddr().toText())
+            .arg(release->getHWAddr()->toText())
+            .arg(client_id ? client_id->toText() : "(no client-id)");
+        return;
+    }
+
+    if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) {
+        // @todo: Print hwaddr from lease as part of ticket #2589
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
+            .arg(release->getYiaddr().toText())
+            .arg(client_id ? client_id->toText() : "(no client-id)")
+            .arg(release->getHWAddr()->toText());
+        return;
+    }
+
+    if (lease->client_id_ && client_id && *lease->client_id_ != *client_id) {
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID)
+            .arg(release->getYiaddr().toText())
+            .arg(client_id->toText())
+            .arg(lease->client_id_->toText());
+        return;
+    }
+
+    if (LeaseMgrFactory::instance().deleteLease(lease->addr_)) {
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE)
+            .arg(lease->addr_.toText())
+            .arg(client_id ? client_id->toText() : "(no client-id)")
+            .arg(release->getHWAddr()->toText());
+    } else {
+        LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
+            .arg(lease->addr_.toText())
+            .arg(client_id ? client_id->toText() : "(no client-id)")
+            .arg(release->getHWAddr()->toText());
+    }
 }
 
 void Dhcpv4Srv::processDecline(Pkt4Ptr& decline) {
@@ -325,3 +482,33 @@ Dhcpv4Srv::serverReceivedPacketName(uint8_t type) {
     }
     return (UNKNOWN);
 }
+
+Subnet4Ptr Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(question->getRemoteAddr());
+
+    return (subnet);
+}
+
+void Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) {
+    OptionPtr server_id = pkt->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+    switch (serverid) {
+    case FORBIDDEN:
+        if (server_id) {
+            isc_throw(RFCViolation, "Server-id option was not expected, but "
+                      << "received in " << serverReceivedPacketName(pkt->getType()));
+        }
+        break;
+
+    case MANDATORY:
+        if (!server_id) {
+            isc_throw(RFCViolation, "Server-id option was expected, but not "
+                      " received in message "
+                      << serverReceivedPacketName(pkt->getType()));
+        }
+        break;
+
+    case OPTIONAL:
+        // do nothing here
+        ;
+    }
+}

src/bin/dhcp4/dhcp4_srv.h

@@ -18,14 +18,16 @@
 #include <dhcp/dhcp4.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/option.h>
+#include <dhcpsrv/subnet.h>
+#include <dhcpsrv/alloc_engine.h>
 
 #include <boost/noncopyable.hpp>
 
 #include <iostream>
 
 namespace isc {
-
 namespace dhcp {
+
 /// @brief DHCPv4 server service.
 ///
 /// This singleton class represents DHCPv4 server. It contains all
@@ -44,6 +46,14 @@ namespace dhcp {
 class Dhcpv4Srv : public boost::noncopyable {
 
     public:
+
+    /// @brief defines if certain option may, must or must not appear
+    typedef enum {
+        FORBIDDEN,
+        MANDATORY,
+        OPTIONAL
+    } RequirementLevel;
+
     /// @brief Default constructor.
     ///
     /// Instantiates necessary services, required to run DHCPv4 server.
@@ -54,7 +64,10 @@ class Dhcpv4Srv : public boost::noncopyable {
     /// for testing purposes.
     ///
     /// @param port specifies port number to listen on
-    Dhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT);
+    /// @param dbconfig Lease manager configuration string.  The default
+    ///        of the "memfile" manager is used for testing.
+    Dhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT,
+              const char* dbconfig = "type=memfile");
 
     /// @brief Destructor. Used during DHCPv4 service shutdown.
     ~Dhcpv4Srv();
@@ -82,6 +95,8 @@ class Dhcpv4Srv : public boost::noncopyable {
     /// As the operation of the method does not depend on any server state, it
     /// is declared static.
     ///
+    /// @todo: This should be named static Pkt4::getName()
+    ///
     /// @param type DHCPv4 packet type
     ///
     /// @return Pointer to "const" string containing the packet name.
@@ -90,6 +105,17 @@ class Dhcpv4Srv : public boost::noncopyable {
     static const char* serverReceivedPacketName(uint8_t type);
 
 protected:
+
+    /// @brief verifies if specified packet meets RFC requirements
+    ///
+    /// Checks if mandatory option is really there, that forbidden option
+    /// is not there, and that client-id or server-id appears only once.
+    ///
+    /// @param pkt packet to be checked
+    /// @param serverid expectation regarding server-id option
+    /// @throw RFCViolation if any issues are detected
+    void sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid);
+
     /// @brief Processes incoming DISCOVER and returns response.
     ///
     /// Processes received DISCOVER message and verifies that its sender
@@ -156,11 +182,19 @@ protected:
     /// client and assigning it. Options corresponding to the lease
     /// are added to specific message.
     ///
-    /// Note: Lease manager is not implemented yet, so this method
-    /// used fixed, hardcoded lease.
+    /// @param question DISCOVER or REQUEST message from client
+    /// @param answer OFFER or ACK/NAK message (lease options will be added here)
+    void assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer);
+
+    /// @brief Attempts to renew received addresses
+    ///
+    /// Attempts to renew existing lease. This typically includes finding a lease that
+    /// corresponds to the received address. If no such lease is found, a status code
+    /// response is generated.
     ///
-    /// @param msg OFFER or ACK message (lease options will be added here)
-    void tryAssignLease(Pkt4Ptr& msg);
+    /// @param renew client's message asking for renew
+    /// @param reply server's response (ACK or NAK)
+    void renewLease(const Pkt4Ptr& renew, Pkt4Ptr& reply);
 
     /// @brief Appends default options to a message
     ///
@@ -173,6 +207,14 @@ protected:
     /// @return server-id option
     OptionPtr getServerID() { return serverid_; }
 
+
+    /// @brief Sets DHCPv4 message type
+    ///
+    /// It tries to find existing DHCP TYPE (53) option and update
+    /// it to specified value. If there is no such option, it is
+    /// added.
+    static void setMsgType(Pkt4Ptr& pkt, uint8_t dhcp_type);
+
     /// @brief Sets server-identifier.
     ///
     /// This method attempts to set server-identifier DUID. It tries to
@@ -184,12 +226,32 @@ protected:
     //          previously stored configuration and no network interfaces available)
     void setServerID();
 
+    /// @brief Selects a subnet for a given client's packet.
+    ///
+    /// @param question client's message
+    /// @return selected subnet (or NULL if no suitable subnet was found)
+    isc::dhcp::Subnet4Ptr selectSubnet(const Pkt4Ptr& question);
+
     /// server DUID (to be sent in server-identifier option)
     OptionPtr serverid_;
 
     /// indicates if shutdown is in progress. Setting it to true will
     /// initiate server shutdown procedure.
     volatile bool shutdown_;
+
+    private:
+
+    /// @brief Constructs netmask option based on subnet4
+    ///
+    /// @return Option that contains netmask information
+    static OptionPtr getNetmaskOption(const Subnet4Ptr& subnet);
+
+    /// @brief Allocation Engine.
+    /// Pointer to the allocation engine that we are currently using
+    /// It must be a pointer, because we will support changing engines
+    /// during normal operation (e.g. to use different allocators)
+    boost::shared_ptr<AllocEngine> alloc_engine_;
+
 };
 
 }; // namespace isc::dhcp

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -19,7 +19,10 @@
 #include <dhcp/dhcp4.h>
 #include <dhcp/option.h>
 #include <dhcp4/dhcp4_srv.h>
-
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/lease_mgr.h>
+#include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/utils.h>
 #include <gtest/gtest.h>
 
 #include <fstream>
@@ -38,36 +41,27 @@ const char* const INTERFACE_FILE = "interfaces.txt";
 class NakedDhcpv4Srv: public Dhcpv4Srv {
     // "naked" DHCPv4 server, exposes internal fields
 public:
-    NakedDhcpv4Srv():Dhcpv4Srv(DHCP4_SERVER_PORT + 10000) { }
-
-    boost::shared_ptr<Pkt4> processDiscover(boost::shared_ptr<Pkt4>& discover) {
-        return Dhcpv4Srv::processDiscover(discover);
-    }
-    boost::shared_ptr<Pkt4> processRequest(boost::shared_ptr<Pkt4>& request) {
-        return Dhcpv4Srv::processRequest(request);
-    }
-    void processRelease(boost::shared_ptr<Pkt4>& release) {
-        return Dhcpv4Srv::processRelease(release);
-    }
-    void processDecline(boost::shared_ptr<Pkt4>& decline) {
-        Dhcpv4Srv::processDecline(decline);
-    }
-    boost::shared_ptr<Pkt4> processInform(boost::shared_ptr<Pkt4>& inform) {
-        return Dhcpv4Srv::processInform(inform);
-    }
+    NakedDhcpv4Srv(uint16_t port = 0):Dhcpv4Srv(port) { }
+
+    using Dhcpv4Srv::processDiscover;
+    using Dhcpv4Srv::processRequest;
+    using Dhcpv4Srv::processRelease;
+    using Dhcpv4Srv::processDecline;
+    using Dhcpv4Srv::processInform;
+    using Dhcpv4Srv::getServerID;
+    using Dhcpv4Srv::sanityCheck;
 };
 
 class Dhcpv4SrvTest : public ::testing::Test {
 public:
     Dhcpv4SrvTest() {
-        unlink(INTERFACE_FILE);
-        fstream fakeifaces(INTERFACE_FILE, ios::out | ios::trunc);
-        if (if_nametoindex("lo") > 0) {
-            fakeifaces << "lo 127.0.0.1";
-        } else if (if_nametoindex("lo0") > 0) {
-            fakeifaces << "lo0 127.0.0.1";
-        }
-        fakeifaces.close();
+        subnet_ = Subnet4Ptr(new Subnet4(IOAddress("192.0.2.0"), 24, 1000,
+                                         2000, 3000));
+        pool_ = Pool4Ptr(new Pool4(IOAddress("192.0.2.100"), IOAddress("192.0.2.110")));
+        subnet_->addPool(pool_);
+
+        CfgMgr::instance().deleteSubnets4();
+        CfgMgr::instance().addSubnet4(subnet_);
     }
 
     void MessageCheck(const boost::shared_ptr<Pkt4>& q,
@@ -94,9 +88,126 @@ public:
         EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
     }
 
+    // Generate client-id option
+    OptionPtr generateClientId(size_t size = 4) {
+
+        OptionBuffer clnt_id(size);
+        for (int i = 0; i < size; i++) {
+            clnt_id[i] = 100 + i;
+        }
+
+        client_id_ = ClientIdPtr(new ClientId(clnt_id));
+
+        return (OptionPtr(new Option(Option::V4, DHO_DHCP_CLIENT_IDENTIFIER,
+                                     clnt_id.begin(),
+                                     clnt_id.begin() + size)));
+    }
+
+    // Check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    void checkAddressParams(const Pkt4Ptr& rsp, const SubnetPtr subnet,
+                            bool t1_mandatory = false, bool t2_mandatory = false) {
+
+        // Technically inPool implies inRange, but let's be on the safe
+        // side and check both.
+        EXPECT_TRUE(subnet->inRange(rsp->getYiaddr()));
+        EXPECT_TRUE(subnet->inPool(rsp->getYiaddr()));
+
+        // Check lease time
+        OptionPtr tmp = rsp->getOption(DHO_DHCP_LEASE_TIME);
+        if (!tmp) {
+            ADD_FAILURE() << "Lease time option missing in response";
+        } else {
+            EXPECT_EQ(tmp->getUint32(), subnet->getValid());
+        }
+
+        // Check T1 timer
+        tmp = rsp->getOption(DHO_DHCP_RENEWAL_TIME);
+        if (tmp) {
+            EXPECT_EQ(tmp->getUint32(), subnet->getT1());
+        } else {
+            if (t1_mandatory) {
+                ADD_FAILURE() << "Required T1 option missing";
+            }
+        }
+
+        // Check T2 timer
+        tmp = rsp->getOption(DHO_DHCP_REBINDING_TIME);
+        if (tmp) {
+            EXPECT_EQ(tmp->getUint32(), subnet->getT2());
+        } else {
+            if (t1_mandatory) {
+                ADD_FAILURE() << "Required T2 option missing";
+            }
+        }
+    }
+
+    // Basic checks for generated response (message type and transaction-id).
+    void checkResponse(const Pkt4Ptr& rsp, uint8_t expected_message_type,
+                       uint32_t expected_transid) {
+        ASSERT_TRUE(rsp);
+        EXPECT_EQ(expected_message_type, rsp->getType());
+        EXPECT_EQ(expected_transid, rsp->getTransid());
+    }
+
+    // Checks if the lease sent to client is present in the database
+    Lease4Ptr checkLease(const Pkt4Ptr& rsp, const OptionPtr& client_id,
+                         const HWAddrPtr& hwaddr, const IOAddress& expected_addr) {
+
+        ClientIdPtr id;
+        if (client_id) {
+            OptionBuffer data = client_id->getData();
+            id.reset(new ClientId(data));
+        }
+
+        Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(expected_addr);
+        if (!lease) {
+            cout << "Lease for " << expected_addr.toText()
+                 << " not found in the database backend.";
+            return (Lease4Ptr());
+        }
+
+        EXPECT_EQ(expected_addr.toText(), lease->addr_.toText());
+        if (client_id) {
+            EXPECT_TRUE(*lease->client_id_ == *id);
+        }
+        EXPECT_EQ(subnet_->getID(), lease->subnet_id_);
+
+        return (lease);
+    }
+
+    // Checks if server response (OFFER, ACK, NAK) includes proper server-id.
+    void checkServerId(const Pkt4Ptr& rsp, const OptionPtr& expected_srvid) {
+        // check that server included its server-id
+        OptionPtr tmp = rsp->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+        ASSERT_TRUE(tmp);
+        EXPECT_EQ(tmp->getType(), expected_srvid->getType() );
+        EXPECT_EQ(tmp->len(), expected_srvid->len() );
+        EXPECT_TRUE(tmp->getData() == expected_srvid->getData());
+    }
+
+    // Checks if server response (OFFER, ACK, NAK) includes proper client-id.
+    void checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_clientid) {
+        // check that server included our own client-id
+        OptionPtr tmp = rsp->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+        ASSERT_TRUE(tmp);
+        EXPECT_EQ(expected_clientid->getType(), tmp->getType());
+        EXPECT_EQ(expected_clientid->len(), tmp->len());
+        EXPECT_TRUE(expected_clientid->getData() == tmp->getData());
+    }
+
     ~Dhcpv4SrvTest() {
-        unlink(INTERFACE_FILE);
+        CfgMgr::instance().deleteSubnets4();
     };
+
+    // A subnet used in most tests
+    Subnet4Ptr subnet_;
+
+    // A pool used in most tests
+    Pool4Ptr pool_;
+
+    // A client-id used in most tests
+    ClientIdPtr client_id_;
 };
 
 TEST_F(Dhcpv4SrvTest, basic) {
@@ -107,12 +218,27 @@ TEST_F(Dhcpv4SrvTest, basic) {
     ASSERT_NO_THROW({
         srv = new Dhcpv4Srv(DHCP4_SERVER_PORT + 10000);
     });
+    delete srv;
+
+    NakedDhcpv4Srv* naked_srv = NULL;
+    ASSERT_NO_THROW({
+        naked_srv = new NakedDhcpv4Srv(DHCP4_SERVER_PORT + 10000);
+    });
+    EXPECT_TRUE(naked_srv->getServerID());
+
+    ASSERT_NO_THROW({
+        naked_srv = new NakedDhcpv4Srv(0);
+    });
+    EXPECT_TRUE(naked_srv->getServerID());
 
     delete srv;
+
+
+
 }
 
 TEST_F(Dhcpv4SrvTest, processDiscover) {
-    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv(0);
     vector<uint8_t> mac(6);
     for (int i = 0; i < 6; i++) {
         mac[i] = 255 - i;
@@ -170,7 +296,7 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
 }
 
 TEST_F(Dhcpv4SrvTest, processRequest) {
-    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv(0);
     vector<uint8_t> mac(6);
     for (int i = 0; i < 6; i++) {
         mac[i] = i*10;
@@ -305,4 +431,384 @@ TEST_F(Dhcpv4SrvTest, serverReceivedPacketName) {
     }
 }
 
+// This test verifies that incoming DISCOVER can be handled properly, that an
+// OFFER is generated, that the response has an address and that address
+// really belongs to the configured pool.
+//
+// constructed very simple DISCOVER message with:
+// - client-id option
+//
+// expected returned OFFER message:
+// - copy of client-id
+// - server-id
+// - offered address
+TEST_F(Dhcpv4SrvTest, DiscoverBasic) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+
+    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    dis->setRemoteAddr(IOAddress("192.0.2.1"));
+    OptionPtr clientid = generateClientId();
+    dis->addOption(clientid);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr offer = srv->processDiscover(dis);
+
+    // check if we get response at all
+    checkResponse(offer, DHCPOFFER, 1234);
+
+    // check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    checkAddressParams(offer, subnet_);
+
+    // check identifiers
+    checkServerId(offer, srv->getServerID());
+    checkClientId(offer, clientid);
+}
+
+
+// This test verifies that incoming DISCOVER can be handled properly, that an
+// OFFER is generated, that the response has an address and that address
+// really belongs to the configured pool.
+//
+// constructed very simple DISCOVER message with:
+// - client-id option
+// - address set to specific value as hint
+//
+// expected returned OFFER message:
+// - copy of client-id
+// - server-id
+// - offered address
+TEST_F(Dhcpv4SrvTest, DiscoverHint) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+    IOAddress hint("192.0.2.107");
+
+    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    dis->setRemoteAddr(IOAddress("192.0.2.1"));
+    OptionPtr clientid = generateClientId();
+    dis->addOption(clientid);
+    dis->setYiaddr(hint);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr offer = srv->processDiscover(dis);
+
+    // check if we get response at all
+    checkResponse(offer, DHCPOFFER, 1234);
+
+    // check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    checkAddressParams(offer, subnet_);
+
+    EXPECT_EQ(offer->getYiaddr().toText(), hint.toText());
+
+    // check identifiers
+    checkServerId(offer, srv->getServerID());
+    checkClientId(offer, clientid);
+}
+
+
+// This test verifies that incoming DISCOVER can be handled properly, that an
+// OFFER is generated, that the response has an address and that address
+// really belongs to the configured pool.
+//
+// constructed very simple DISCOVER message with:
+// - address set to specific value as hint
+//
+// expected returned OFFER message:
+// - copy of client-id
+// - server-id
+// - offered address
+TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+    IOAddress hint("192.0.2.107");
+
+    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    dis->setRemoteAddr(IOAddress("192.0.2.1"));
+    dis->setYiaddr(hint);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr offer = srv->processDiscover(dis);
+
+    // check if we get response at all
+    checkResponse(offer, DHCPOFFER, 1234);
+
+    // check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    checkAddressParams(offer, subnet_);
+
+    EXPECT_EQ(offer->getYiaddr().toText(), hint.toText());
+
+    // check identifiers
+    checkServerId(offer, srv->getServerID());
+}
+
+// This test verifies that incoming DISCOVER can be handled properly, that an
+// OFFER is generated, that the response has an address and that address
+// really belongs to the configured pool.
+//
+// constructed very simple DISCOVER message with:
+// - client-id option
+// - address set to specific value as hint, but that hint is invalid
+//
+// expected returned OFFER message:
+// - copy of client-id
+// - server-id
+// - offered address (!= hint)
+TEST_F(Dhcpv4SrvTest, DiscoverInvalidHint) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+    IOAddress hint("10.1.2.3");
+
+    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    dis->setRemoteAddr(IOAddress("192.0.2.107"));
+    OptionPtr clientid = generateClientId();
+    dis->addOption(clientid);
+    dis->setYiaddr(hint);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr offer = srv->processDiscover(dis);
+
+    // check if we get response at all
+    checkResponse(offer, DHCPOFFER, 1234);
+
+    // check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    checkAddressParams(offer, subnet_);
+
+    EXPECT_NE(offer->getYiaddr().toText(), hint.toText());
+
+    // check identifiers
+    checkServerId(offer, srv->getServerID());
+    checkClientId(offer, clientid);
+}
+
+/// @todo: Add a test that client sends hint that is in pool, but currently
+/// being used by a different client.
+
+// This test checks that the server is offering different addresses to different
+// clients in ADVERTISEs. Please note that ADVERTISE is not a guarantee that such
+// and address will be assigned. Had the pool was very small and contained only
+// 2 addresses, the third client would get the same advertise as the first one
+// and this is a correct behavior. It is REQUEST that will fail for the third
+// client. ADVERTISE is basically saying "if you send me a request, you will
+// probably get an address like this" (there are no guarantees).
+TEST_F(Dhcpv4SrvTest, ManyDiscovers) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+
+    Pkt4Ptr dis1 = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    Pkt4Ptr dis2 = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 2345));
+    Pkt4Ptr dis3 = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 3456));
+
+    dis1->setRemoteAddr(IOAddress("192.0.2.1"));
+    dis2->setRemoteAddr(IOAddress("192.0.2.2"));
+    dis3->setRemoteAddr(IOAddress("192.0.2.3"));
+
+    // different client-id sizes
+    OptionPtr clientid1 = generateClientId(4); // length 4
+    OptionPtr clientid2 = generateClientId(5); // length 5
+    OptionPtr clientid3 = generateClientId(6); // length 6
+
+    dis1->addOption(clientid1);
+    dis2->addOption(clientid2);
+    dis3->addOption(clientid3);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr reply1 = srv->processDiscover(dis1);
+    Pkt4Ptr reply2 = srv->processDiscover(dis2);
+    Pkt4Ptr reply3 = srv->processDiscover(dis3);
+
+    // check if we get response at all
+    checkResponse(reply1, DHCPOFFER, 1234);
+    checkResponse(reply2, DHCPOFFER, 2345);
+    checkResponse(reply3, DHCPOFFER, 3456);
+
+    IOAddress addr1 = reply1->getYiaddr();
+    IOAddress addr2 = reply2->getYiaddr();
+    IOAddress addr3 = reply3->getYiaddr();
+
+    // Check that the assigned address is indeed from the configured pool
+    checkAddressParams(reply1, subnet_);
+    checkAddressParams(reply2, subnet_);
+    checkAddressParams(reply3, subnet_);
+
+    // check DUIDs
+    checkServerId(reply1, srv->getServerID());
+    checkServerId(reply2, srv->getServerID());
+    checkServerId(reply3, srv->getServerID());
+    checkClientId(reply1, clientid1);
+    checkClientId(reply2, clientid2);
+    checkClientId(reply3, clientid3);
+
+    // Finally check that the addresses offered are different
+    EXPECT_NE(addr1.toText(), addr2.toText());
+    EXPECT_NE(addr2.toText(), addr3.toText());
+    EXPECT_NE(addr3.toText(), addr1.toText());
+    cout << "Offered address to client1=" << addr1.toText() << endl;
+    cout << "Offered address to client2=" << addr2.toText() << endl;
+    cout << "Offered address to client3=" << addr3.toText() << endl;
+}
+
+// This test verifies that incoming DISCOVER can be handled properly, that an
+// OFFER is generated, that the response has an address and that address
+// really belongs to the configured pool.
+//
+// constructed very simple DISCOVER message with:
+// - client-id option
+//
+// expected returned OFFER message:
+// - copy of client-id
+// - server-id
+// - offered address
+TEST_F(Dhcpv4SrvTest, RequestBasic) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+
+    IOAddress hint("192.0.2.107");
+    Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
+    req->setRemoteAddr(IOAddress("192.0.2.1"));
+    OptionPtr clientid = generateClientId();
+    req->addOption(clientid);
+    req->setYiaddr(hint);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr ack = srv->processRequest(req);
+
+    // check if we get response at all
+    checkResponse(ack, DHCPACK, 1234);
+    EXPECT_EQ(hint.toText(), ack->getYiaddr().toText());
+
+    // check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    checkAddressParams(ack, subnet_);
+
+    // check identifiers
+    checkServerId(ack, srv->getServerID());
+    checkClientId(ack, clientid);
+
+    // check that the lease is really in the database
+    Lease4Ptr l = checkLease(ack, clientid, req->getHWAddr(), hint);
+
+    ASSERT_TRUE(l);
+    LeaseMgrFactory::instance().deleteLease(l->addr_);
+}
+
+// This test verifies that incoming (positive) RENEW can be handled properly, that a
+// REPLY is generated, that the response has an address and that address
+// really belongs to the configured pool and that lease is actually renewed.
+//
+// expected:
+// - returned REPLY message has copy of client-id
+// - returned REPLY message has server-id
+// - returned REPLY message has IA that includes IAADDR
+// - lease is actually renewed in LeaseMgr
+TEST_F(Dhcpv4SrvTest, RenewBasic) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+
+    const IOAddress addr("192.0.2.106");
+    uint32_t temp_t1 = 50;
+    uint32_t temp_t2 = 75;
+    uint32_t temp_valid = 100;
+    time_t temp_timestamp = time(NULL) - 10;
+
+    // Generate client-id also duid_
+    OptionPtr clientid = generateClientId();
+
+    // Check that the address we are about to use is indeed in pool
+    ASSERT_TRUE(subnet_->inPool(addr));
+
+    // let's create a lease and put it in the LeaseMgr
+    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
+                              temp_valid, temp_t1, temp_t2, temp_timestamp,
+                              subnet_->getID()));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Check that the lease is really in the database
+    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
+    ASSERT_TRUE(l);
+
+    // Check that T1, T2, preferred, valid and cltt really set.
+    // Constructed lease looks as if it was assigned 10 seconds ago
+    // EXPECT_EQ(l->t1_, temp_t1);
+    // EXPECT_EQ(l->t2_, temp_t2);
+    EXPECT_EQ(l->valid_lft_, temp_valid);
+    EXPECT_EQ(l->cltt_, temp_timestamp);
+
+    // Let's create a RENEW
+    Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
+    req->setRemoteAddr(IOAddress(addr));
+    req->setYiaddr(addr);
+
+    req->addOption(clientid);
+    req->addOption(srv->getServerID());
+
+    // Pass it to the server and hope for a REPLY
+    Pkt4Ptr ack = srv->processRequest(req);
+
+    // Check if we get response at all
+    checkResponse(ack, DHCPACK, 1234);
+    EXPECT_EQ(addr.toText(), ack->getYiaddr().toText());
+
+    // check that address was returned from proper range, that its lease
+    // lifetime is correct, that T1 and T2 are returned properly
+    checkAddressParams(ack, subnet_);
+
+    // Check identifiers
+    checkServerId(ack, srv->getServerID());
+    checkClientId(ack, clientid);
+
+    // Check that the lease is really in the database
+    l = checkLease(ack, clientid, req->getHWAddr(), addr);
+    ASSERT_TRUE(l);
+
+    // Check that T1, T2, preferred, valid and cltt were really updated
+    EXPECT_EQ(l->t1_, subnet_->getT1());
+    EXPECT_EQ(l->t2_, subnet_->getT2());
+    EXPECT_EQ(l->valid_lft_, subnet_->getValid());
+
+    // Checking for CLTT is a bit tricky if we want to avoid off by 1 errors
+    int32_t cltt = static_cast<int32_t>(l->cltt_);
+    int32_t expected = static_cast<int32_t>(time(NULL));
+    // equality or difference by 1 between cltt and expected is ok.
+    EXPECT_GE(1, abs(cltt - expected));
+
+    EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr));
+}
+
+// @todo: Implement tests for rejecting renewals
+
+// This test verifies if the sanityCheck() really checks options presence.
+TEST_F(Dhcpv4SrvTest, sanityCheck) {
+    boost::scoped_ptr<NakedDhcpv4Srv> srv;
+    ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+
+    Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+
+    // check that the packets originating from local addresses can be
+    pkt->setRemoteAddr(IOAddress("192.0.2.1"));
+
+    // client-id is optional for information-request, so
+    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::OPTIONAL));
+
+    // empty packet, no server-id
+    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY), RFCViolation);
+
+    pkt->addOption(srv->getServerID());
+
+    // server-id is mandatory and present = no exception
+    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY));
+
+    // server-id is forbidden, but present => exception
+    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::FORBIDDEN),
+                 RFCViolation);
+}
+
+// @todo: write tests for RELEASE
+
 } // end of anonymous namespace

src/bin/dhcp6/dhcp6_messages.mes

@@ -61,7 +61,7 @@ A debug message indicating that the IPv6 DHCP server has received an
 updated configuration from the BIND 10 configuration system.
 
 % DHCP6_DB_BACKEND_STARTED lease database started (type: %1, name: %2)
-This informational message is printed every time DHCPv6 is started.
+This informational message is printed every time DHCPv6 server is started.
 It indicates what database backend type is being to store lease and
 other information.
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -32,6 +32,7 @@
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/subnet.h>
+#include <dhcpsrv/utils.h>
 #include <exceptions/exceptions.h>
 #include <util/io_utilities.h>
 #include <util/range_utilities.h>
@@ -405,7 +406,7 @@ void Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid,
     switch (serverid) {
     case FORBIDDEN:
         if (server_ids.size() > 0) {
-            isc_throw(RFCViolation, "Exactly 1 server-id option expected, but "
+            isc_throw(RFCViolation, "Server-id option was not expected, but "
                       << server_ids.size() << " received in " << pkt->getName());
         }
         break;
@@ -450,7 +451,10 @@ void Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer) {
 
         // perhaps this should be logged on some higher level? This is most likely
         // configuration bug.
-        LOG_ERROR(dhcp6_logger, DHCP6_SUBNET_SELECTION_FAILED);
+        LOG_ERROR(dhcp6_logger, DHCP6_SUBNET_SELECTION_FAILED)
+            .arg(question->getRemoteAddr().toText())
+            .arg(question->getName());
+
     } else {
         LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL_DATA, DHCP6_SUBNET_SELECTED)
             .arg(subnet->toText());

src/bin/dhcp6/dhcp6_srv.h

@@ -31,22 +31,6 @@
 namespace isc {
 namespace dhcp {
 
-/// An exception that is thrown if a DHCPv6 protocol violation occurs while
-/// processing a message (e.g. a mandatory option is missing)
-class RFCViolation : public isc::Exception {
-public:
-
-/// @brief constructor
-///
-/// @param file name of the file, where exception occurred
-/// @param line line of the file, where exception occurred
-/// @param what text description of the issue that caused exception
-RFCViolation(const char* file, size_t line, const char* what) :
-    isc::Exception(file, line, what) {}
-};
-
-
-
 /// @brief DHCPv6 server service.
 ///
 /// This class represents DHCPv6 server. It contains all
@@ -83,7 +67,7 @@ public:
     /// @param dbconfig Lease manager configuration string.  The default
     ///        of the "memfile" manager is used for testing.
     Dhcpv6Srv(uint16_t port = DHCP6_SERVER_PORT,
-            const char* dbconfig = "type=memfile");
+              const char* dbconfig = "type=memfile");
 
     /// @brief Destructor. Used during DHCPv6 service shutdown.
     virtual ~Dhcpv6Srv();
@@ -291,7 +275,7 @@ private:
     boost::shared_ptr<AllocEngine> alloc_engine_;
 
     /// Server DUID (to be sent in server-identifier option)
-    boost::shared_ptr<isc::dhcp::Option> serverid_;
+    OptionPtr serverid_;
 
     /// Indicates if shutdown is in progress. Setting it to true will
     /// initiate server shutdown procedure.

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -29,6 +29,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/utils.h>
 #include <util/buffer.h>
 #include <util/range_utilities.h>
 
@@ -73,6 +74,7 @@ public:
         pool_ = Pool6Ptr(new Pool6(Pool6::TYPE_IA, IOAddress("2001:db8:1:1::"), 64));
         subnet_->addPool(pool_);
 
+        CfgMgr::instance().deleteSubnets6();
         CfgMgr::instance().addSubnet6(subnet_);
     }
 

src/lib/dhcp/hwaddr.cc

@@ -14,6 +14,9 @@
 
 #include <dhcp/hwaddr.h>
 #include <dhcp/dhcp4.h>
+#include <iomanip>
+#include <sstream>
+#include <vector>
 
 namespace isc {
 namespace dhcp {
@@ -30,6 +33,22 @@ HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype)
     :hwaddr_(hwaddr), htype_(htype) {
 }
 
+std::string HWAddr::toText() const {
+    std::stringstream tmp;
+    tmp << "type=" << htype_ << " ";
+    tmp << std::hex;
+    bool delim = false;
+    for (std::vector<uint8_t>::const_iterator it = hwaddr_.begin();
+         it != hwaddr_.end(); ++it) {
+        if (delim) {
+            tmp << ":";
+        }
+        tmp << std::setw(2) << std::setfill('0') << static_cast<unsigned int>(*it);
+        delim = true;
+    }
+    return (tmp.str());
+}
+
 bool HWAddr::operator==(const HWAddr& other) const {
     return ((this->htype_  == other.htype_) && 
             (this->hwaddr_ == other.hwaddr_));

src/lib/dhcp/hwaddr.h

@@ -35,6 +35,9 @@ public:
     // Hardware type
     uint8_t htype_;
 
+    /// @brief Returns textual representation of a client-id (e.g. 00:01:02:03)
+    std::string toText() const;
+
     /// @brief Compares two hardware addresses for equality
     bool operator==(const HWAddr& other) const;
 

src/lib/dhcp/pkt4.h

@@ -526,7 +526,7 @@ protected:
     ///
     /// @warning This protected member is accessed by derived
     /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
+    /// @ref perfdhcp::PerfPkt4. The impact on derived classes'
     /// behavior must be taken into consideration before making
     /// changes to this member such as access scope restriction or
     /// data format change etc.

src/lib/dhcpsrv/addr_utilities.cc

@@ -195,5 +195,15 @@ isc::asiolink::IOAddress lastAddrInPrefix(const isc::asiolink::IOAddress& prefix
     }
 }
 
+isc::asiolink::IOAddress getNetmask4(uint8_t len) {
+    if (len > 32) {
+        isc_throw(BadValue, "Invalid netmask size " << len << ", allowed range "
+                  "is 0..32");
+    }
+    uint32_t x = ~bitMask4[len];
+
+    return (IOAddress(x));
+}
+
 };
 };

src/lib/dhcpsrv/addr_utilities.h

@@ -52,6 +52,11 @@ isc::asiolink::IOAddress firstAddrInPrefix(const isc::asiolink::IOAddress& prefi
 isc::asiolink::IOAddress lastAddrInPrefix(const isc::asiolink::IOAddress& prefix,
                                            uint8_t len);
 
+/// @brief generates an IPv4 netmask of specified length
+/// @throw BadValue if len is greater than 32
+/// @return netmask
+isc::asiolink::IOAddress getNetmask4(uint8_t len);
+
 };
 };
 

src/lib/dhcpsrv/alloc_engine.cc

@@ -16,7 +16,7 @@
 #include <dhcpsrv/lease_mgr_factory.h>
 
 #include <cstring>
-
+#include <vector>
 #include <string.h>
 
 using namespace isc::asiolink;
@@ -278,17 +278,31 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
     if (existing) {
         // we have a lease already. This is a returning client, probably after
         // his reboot.
-        return (existing);
+
+        existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
+
+        if (existing) {
+            return (existing);
+        }
+
+        // If renewal failed (e.g. the lease no longer matches current configuration)
+        // let's continue allocation process
     }
 
-    existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
-    if (existing) {
-        // we have a lease already. This is a returning client, probably after
-        // his reboot.
+    if (clientid) {
+        existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
+        if (existing) {
+            // we have a lease already. This is a returning client, probably after
+            // his reboot.
 
-        // @todo: produce a warning. We haven't found him using MAC address, but
-        // we found him using client-id
-        return (existing);
+            existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
+
+            // @todo: produce a warning. We haven't found him using MAC address, but
+            // we found him using client-id
+            if (existing) {
+                return (existing);
+            }
+        }
     }
 
     // check if the hint is in pool and is available
@@ -368,6 +382,28 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
               << " tries");
 }
 
+Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
+                                   const ClientIdPtr& clientid,
+                                   const HWAddrPtr& hwaddr,
+                                   const Lease4Ptr& lease,
+                                   bool fake_allocation /* = false */) {
+
+    lease->subnet_id_ = subnet->getID();
+    lease->hwaddr_ = hwaddr->hwaddr_;
+    lease->client_id_ = clientid;
+    lease->cltt_ = time(NULL);
+    lease->t1_ = subnet->getT1();
+    lease->t2_ = subnet->getT2();
+    lease->valid_lft_ = subnet->getValid();
+
+    if (!fake_allocation) {
+        // for REQUEST we do update the lease
+        LeaseMgrFactory::instance().updateLease4(lease);
+    }
+
+    return (lease);
+}
+
 Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
                                          const Subnet6Ptr& subnet,
                                          const DuidPtr& duid,
@@ -494,10 +530,17 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
         isc_throw(BadValue, "Can't create a lease with NULL HW address");
     }
     time_t now = time(NULL);
+
+    // @todo: remove this kludge after ticket #2590 is implemented
+    std::vector<uint8_t> local_copy;
+    if (clientid) {
+        local_copy = clientid->getDuid();
+    }
+
     Lease4Ptr lease(new Lease4(addr, &hwaddr->hwaddr_[0], hwaddr->hwaddr_.size(),
-                               &clientid->getDuid()[0], clientid->getDuid().size(),
-                               subnet->getValid(), subnet->getT1(), subnet->getT2(),
-                               now, subnet->getID()));
+                               &local_copy[0], local_copy.size(), subnet->getValid(),
+                               subnet->getT1(), subnet->getT2(), now,
+                               subnet->getID()));
 
     if (!fake_allocation) {
         // That is a real (REQUEST) allocation

src/lib/dhcpsrv/alloc_engine.h

@@ -195,6 +195,28 @@ protected:
                      const isc::asiolink::IOAddress& hint,
                      bool fake_allocation);
 
+    /// @brief Renews a IPv4 lease
+    ///
+    /// Since both request and renew are implemented in DHCPv4 as sending
+    /// REQUEST packet, it is difficult to easily distinguish between those
+    /// cases. Therefore renew for DHCPv4 is done in allocation engine.
+    /// This method is also used when client crashed/rebooted and tries
+    /// to get a new lease. It thinks that it gets a new lease, but in fact
+    /// we are only renewing still valid lease for that client.
+    ///
+    /// @param subnet subnet the client is attached to
+    /// @param clientid client identifier
+    /// @param hwaddr client's hardware address
+    /// @param lease lease to be renewed
+    /// @param renewed lease (typically the same passed as lease parameter)
+    ///        or NULL if the lease cannot be renewed
+    Lease4Ptr
+    renewLease4(const SubnetPtr& subnet,
+                const ClientIdPtr& clientid,
+                const HWAddrPtr& hwaddr,
+                const Lease4Ptr& lease,
+                bool fake_allocation /* = false */);
+
     /// @brief Allocates an IPv6 lease
     ///
     /// This method uses currently selected allocator to pick an address from

src/lib/dhcpsrv/utils.h

@@ -0,0 +1,40 @@
+// Copyright (C) 2012 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DHCPSRV_UTILS_H
+#define DHCPSRV_UTILS_H
+
+#include <exceptions/exceptions.h>
+
+namespace isc {
+namespace dhcp {
+
+/// An exception that is thrown if a DHCPv6 protocol violation occurs while
+/// processing a message (e.g. a mandatory option is missing)
+class RFCViolation : public isc::Exception {
+public:
+
+/// @brief constructor
+///
+/// @param file name of the file, where exception occurred
+/// @param line line of the file, where exception occurred
+/// @param what text description of the issue that caused exception
+RFCViolation(const char* file, size_t line, const char* what) :
+    isc::Exception(file, line, what) {}
+};
+
+}; // namespace isc::dhcp
+}; // namespace isc
+
+#endif // DHCPSRV_UTILS_H