Merge branch 'trac992' into trac1237

ChangeLog

@@ -1,3 +1,9 @@
+3XX.	[func]		tomek
+	dhcp4: Dummy DHCPv4 component implemented. Currently it does
+	nothing useful, except providing skeleton implementation that can
+	be expanded in the future.
+	(Trac #992, git TBD)
+
 328.	[func]		jelte
 	b10-auth now passes IXFR requests on to b10-xfrout, and no longer
 	responds to them with NOTIMPL.

src/bin/dhcp4/Makefile.am

@@ -35,10 +35,6 @@ b10_dhcp4_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_dhcp4_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_dhcp4_LDADD += $(top_builddir)/src/lib/log/liblog.la
 
-# TODO: This is ugly hack. iface_mgr should be moved to src/lib/dhcp
-b10_dhcp4_LDADD += $(top_builddir)/src/bin/dhcp6/iface_mgr.o
-
-
 # TODO: config.h.in is wrong because doesn't honor pkgdatadir
 # and can't use @datadir@ because doesn't expand default ${prefix}
 b10_dhcp4dir = $(pkgdatadir)

src/bin/dhcp4/dhcp4_srv.cc

@@ -23,8 +23,8 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
-Dhcpv4Srv::Dhcpv4Srv() {
-    cout << "Initialization" << endl;
+Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
+    cout << "Initialization: opening sockets on port " << port << endl;
 
     // first call to instance() will create IfaceMgr (it's a singleton)
     // it may throw something if things go wrong
@@ -34,7 +34,7 @@ Dhcpv4Srv::Dhcpv4Srv() {
 
     setServerID();
 
-    shutdown = false;
+    shutdown_ = false;
 }
 
 Dhcpv4Srv::~Dhcpv4Srv() {
@@ -43,11 +43,12 @@ Dhcpv4Srv::~Dhcpv4Srv() {
 
 bool
 Dhcpv4Srv::run() {
-    while (!shutdown) {
+    while (!shutdown_) {
         boost::shared_ptr<Pkt4> query; // client's message
         boost::shared_ptr<Pkt4> rsp;   // server's response
 
 #if 0
+        // uncomment this once ticket 1239 is merged.
         query = IfaceMgr::instance().receive4();
 #endif
 
@@ -80,25 +81,26 @@ Dhcpv4Srv::run() {
             cout << "Received " << query->len() << " bytes packet type="
                  << query->getType() << endl;
 
-            /// DEBUG
+            // TODO: print out received packets only if verbose (or debug)
+            // mode is enabled
             cout << query->toText();
 
             if (rsp) {
-#if 0
-                rsp->remote_addr_ = query->remote_addr_;
-                rsp->local_addr_ = query->local_addr_;
-                rsp->remote_port_ = DHCP6_CLIENT_PORT;
-                rsp->local_port_ = DHCP6_SERVER_PORT;
-                rsp->ifindex_ = query->ifindex_;
-                rsp->iface_ = query->iface_;
-#endif
+                rsp->setRemoteAddr(query->getRemoteAddr());
+                rsp->setLocalAddr(query->getLocalAddr());
+                rsp->setRemotePort(DHCP4_CLIENT_PORT);
+                rsp->setLocalPort(DHCP4_SERVER_PORT);
+                rsp->setIface(query->getIface());
+                rsp->setIndex(query->getIndex());
+
                 cout << "Replying with:" << rsp->getType() << endl;
                 cout << rsp->toText();
                 cout << "----" << endl;
                 if (rsp->pack()) {
-                    cout << "#### pack successful." << endl;
+                    cout << "Packet assembled correctly." << endl;
                 }
 #if 0
+                // uncomment this once ticket 1240 is merged.
                 IfaceMgr::instance().send4(rsp);
 #endif
             }
@@ -113,9 +115,11 @@ Dhcpv4Srv::run() {
 
 void
 Dhcpv4Srv::setServerID() {
-    /// TODO implement this for real once interface detection is done.
-    /// Use hardcoded server-id for now
+    /// 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_ = boost::shared_ptr<Option>(
       new Option4AddrLst(Option::V4, DHO_DHCP_SERVER_IDENTIFIER, srvId));
@@ -123,29 +127,28 @@ Dhcpv4Srv::setServerID() {
 }
 
 boost::shared_ptr<Pkt4>
-Dhcpv4Srv::processDiscover(boost::shared_ptr<Pkt4> discover) {
-    /// TODO: Echo mode. Implement this for real
+Dhcpv4Srv::processDiscover(boost::shared_ptr<Pkt4>& discover) {
+    /// TODO: Currently implemented echo mode. Implement this for real
     return (discover);
 }
 
 boost::shared_ptr<Pkt4>
-Dhcpv4Srv::processRequest(boost::shared_ptr<Pkt4> request) {
-    /// TODO: Echo mode. Implement this for real
+Dhcpv4Srv::processRequest(boost::shared_ptr<Pkt4>& request) {
+    /// TODO: Currently implemented echo mode. Implement this for real
     return (request);
 }
 
-void Dhcpv4Srv::processRelease(boost::shared_ptr<Pkt4> release) {
+void Dhcpv4Srv::processRelease(boost::shared_ptr<Pkt4>& release) {
     /// TODO: Implement this.
     cout << "Received RELEASE on " << release->getIface() << " interface." << endl;
 }
- 
-void Dhcpv4Srv::processDecline(boost::shared_ptr<Pkt4> decline) {
+
+void Dhcpv4Srv::processDecline(boost::shared_ptr<Pkt4>& decline) {
     /// TODO: Implement this.
     cout << "Received DECLINE on " << decline->getIface() << " interface." << endl;
 }
 
-boost::shared_ptr<Pkt4> processInform(boost::shared_ptr<Pkt4> inform) {
-    /// TODO: Echo mode. Implement this for real
+boost::shared_ptr<Pkt4> Dhcpv4Srv::processInform(boost::shared_ptr<Pkt4>& inform) {
+    /// TODO: Currently implemented echo mode. Implement this for real
     return (inform);
 }
-

src/bin/dhcp4/dhcp4_srv.h

@@ -17,6 +17,7 @@
 
 #include <boost/shared_ptr.hpp>
 #include <boost/noncopyable.hpp>
+#include <dhcp/dhcp4.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/option.h>
 #include <iostream>
@@ -34,14 +35,18 @@ namespace dhcp {
 /// appropriate responses.
 class Dhcpv4Srv : public boost::noncopyable {
 
-public:
+    public:
     /// @brief Default constructor.
     ///
     /// Instantiates necessary services, required to run DHCPv6 server.
     /// In particular, creates IfaceMgr that will be responsible for
     /// network interaction. Will instantiate lease manager, and load
-    /// old or create new DUID.
-    Dhcpv4Srv();
+    /// old or create new DUID. It is possible to specify alternate
+    /// port on which DHCPv4 server will listen on. That is mostly useful
+    /// for testing purposes.
+    ///
+    /// @param port specifies port number to listen on
+    Dhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT);
 
     /// @brief Destructor. Used during DHCPv6 service shutdown.
     ~Dhcpv4Srv();
@@ -67,13 +72,13 @@ protected:
     ///
     /// @return OFFER message or NULL
     boost::shared_ptr<Pkt4>
-    processDiscover(boost::shared_ptr<Pkt4> discover);
+    processDiscover(boost::shared_ptr<Pkt4>& discover);
 
     /// @brief Processes incoming REQUEST and returns REPLY response.
     ///
     /// Processes incoming REQUEST message and verifies that its sender
     /// should be served. In particular, verifies that requested lease
-    /// is valid, not expired, not reserved, not used by other client and 
+    /// is valid, not expired, not reserved, not used by other client and
     /// that requesting client is allowed to use it.
     ///
     /// Returns ACK message, NACK message, or NULL
@@ -81,7 +86,7 @@ protected:
     /// @param request a message received from client
     ///
     /// @return ACK or NACK message
-    boost::shared_ptr<Pkt4> processRequest(boost::shared_ptr<Pkt4> request);
+    boost::shared_ptr<Pkt4> processRequest(boost::shared_ptr<Pkt4>& request);
 
     /// @brief Stub function that will handle incoming RELEASE messages.
     ///
@@ -89,17 +94,17 @@ protected:
     /// this function does not return anything.
     ///
     /// @param release message received from client
-    void processRelease(boost::shared_ptr<Pkt4> release);
+    void processRelease(boost::shared_ptr<Pkt4>& release);
 
     /// @brief Stub function that will handle incoming DHCPDECLINE messages.
     ///
     /// @param decline message received from client
-    void processDecline(boost::shared_ptr<Pkt4> decline);
+    void processDecline(boost::shared_ptr<Pkt4>& decline);
 
     /// @brief Stub function that will handle incoming INFORM messages.
     ///
     /// @param infRequest message received from client
-    boost::shared_ptr<Pkt4> processInform(boost::shared_ptr<Pkt4> inform);
+    boost::shared_ptr<Pkt4> processInform(boost::shared_ptr<Pkt4>& inform);
 
     /// @brief Returns server-intentifier option
     ///
@@ -123,7 +128,7 @@ protected:
 
     /// indicates if shutdown is in progress. Setting it to true will
     /// initiate server shutdown procedure.
-    volatile bool shutdown;
+    volatile bool shutdown_;
 };
 
 }; // namespace isc::dhcp

src/bin/dhcp4/main.cc

@@ -81,8 +81,8 @@ main(int argc, char* argv[]) {
 
     int ret = 0;
 
-    // TODO remainder of auth to dhcp6 code copy. We need to enable this in
-    //      dhcp6 eventually
+    // TODO remainder of auth to dhcp4 code copy. We need to enable this in
+    //      dhcp4 eventually
 #if 0
     Session* cc_session = NULL;
     Session* statistics_session = NULL;
@@ -104,7 +104,7 @@ main(int argc, char* argv[]) {
         srv->run();
 
     } catch (const std::exception& ex) {
-        cerr << "[b10-dhcp6] Server failed: " << ex.what() << endl;
+        cerr << "[b10-dhcp4] Server failed: " << ex.what() << endl;
         ret = 1;
     }
 

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -30,37 +30,132 @@ using namespace isc::dhcp;
 namespace {
 
 class NakedDhcpv4Srv: public Dhcpv4Srv {
-    // "naked" Interface Manager, exposes internal fields
+    // "naked" DHCPv4 server, exposes internal fields
 public:
     NakedDhcpv4Srv() { }
 
-    boost::shared_ptr<Pkt4>
-    processDiscover(boost::shared_ptr<Pkt4>& discover) {
+    boost::shared_ptr<Pkt4> processDiscover(boost::shared_ptr<Pkt4>& discover) {
         return Dhcpv4Srv::processDiscover(discover);
     }
-    boost::shared_ptr<Pkt4>
-    processRequest(boost::shared_ptr<Pkt4>& request) {
+    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);
+    }
 };
 
 class Dhcpv4SrvTest : public ::testing::Test {
 public:
     Dhcpv4SrvTest() {
     }
+
+    ~Dhcpv4SrvTest() {
+    };
 };
 
 TEST_F(Dhcpv4SrvTest, basic) {
-    // there's almost no code now. What's there provides echo capability
-    // that is just a proof of concept and will be removed soon
-    // No need to thoroughly test it
+    // nothing to test. DHCPv4_srv instance is created
+    // in test fixture. It is destroyed in destructor
+
+    Dhcpv4Srv* srv = 0;
+    ASSERT_NO_THROW({
+        srv = new Dhcpv4Srv();
+    });
+
+    delete srv;
+}
+
+TEST_F(Dhcpv4SrvTest, processDiscover) {
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+
+    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPDISCOVER, 1234));
+
+    // should not throw
+    EXPECT_NO_THROW(
+        srv->processDiscover(pkt);
+    );
+
+    // should return something
+    EXPECT_TRUE(srv->processDiscover(pkt));
+
+    // TODO: Implement more reasonable tests before starting
+    // work on processSomething() method.
+    delete srv;
+}
+
+TEST_F(Dhcpv4SrvTest, processRequest) {
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+
+    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPREQUEST, 1234));
+
+    // should not throw
+    EXPECT_NO_THROW(
+        srv->processRequest(pkt);
+    );
+
+    // should return something
+    EXPECT_TRUE(srv->processRequest(pkt));
+
+    // TODO: Implement more reasonable tests before starting
+    // work on processSomething() method.
+    delete srv;
+}
+
+TEST_F(Dhcpv4SrvTest, processRelease) {
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+
+    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPRELEASE, 1234));
+
+    // should not throw
+    EXPECT_NO_THROW(
+        srv->processRelease(pkt);
+    );
+
+    // TODO: Implement more reasonable tests before starting
+    // work on processSomething() method.
+
+    delete srv;
+}
+
+TEST_F(Dhcpv4SrvTest, processDecline) {
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+
+    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPDECLINE, 1234));
+
+    // should not throw
+    EXPECT_NO_THROW(
+        srv->processDecline(pkt);
+    );
+
+    // TODO: Implement more reasonable tests before starting
+    // work on processSomething() method.
+    delete srv;
+}
+
+TEST_F(Dhcpv4SrvTest, processInform) {
+    NakedDhcpv4Srv* srv = new NakedDhcpv4Srv();
+
+    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPINFORM, 1234));
+
+    // should not throw
+    EXPECT_NO_THROW(
+        srv->processInform(pkt);
+    );
 
-    EXPECT_NO_THROW( {
-        Dhcpv4Srv * srv = new Dhcpv4Srv();
+    // should return something
+    EXPECT_TRUE(srv->processInform(pkt));
 
-        delete srv;
-        });
+    // TODO: Implement more reasonable tests before starting
+    // work on processSomething() method.
 
+    delete srv;
 }
 
 } // end of anonymous namespace

src/bin/dhcp4/tests/dhcp4_unittests.cc

@@ -24,5 +24,5 @@ main(int argc, char* argv[]) {
 
     int result = RUN_ALL_TESTS();
 
-    return result;
+    return (result);
 }

src/lib/dhcp/pkt4.h

@@ -311,6 +311,73 @@ public:
     /// @return interface name
     std::string getIface() { return iface_; };
 
+    /// @brief Sets interface name.
+    ///
+    /// Sets interface name over which packet was received or is
+    /// going to be transmitted.
+    ///
+    /// @return interface name
+    void setIface(const std::string& iface ) { iface_ = iface; };
+
+    /// @brief Sets interface index.
+    ///
+    /// @param ifindex specifies interface index.
+    void setIndex(uint32_t ifindex) { ifindex_ = ifindex; };
+
+    /// @brief Returns interface index.
+    ///
+    /// @return interface index
+    uint32_t getIndex() { return (ifindex_); };
+
+    /// @brief Sets remote address.
+    ///
+    /// @params remote specifies remote address
+    void setRemoteAddr(const isc::asiolink::IOAddress& remote) {
+        remote_addr_ = remote;
+    }
+
+    /// @brief Returns remote address
+    ///
+    /// @return remote address
+    const isc::asiolink::IOAddress& getRemoteAddr() {
+        return (remote_addr_);
+    }
+
+    /// @brief Sets local address.
+    ///
+    /// @params local specifies local address
+    void setLocalAddr(const isc::asiolink::IOAddress& local) {
+        local_addr_ = local;
+    }
+
+    /// @brief Returns local address.
+    ///
+    /// @return local address
+    const isc::asiolink::IOAddress& getLocalAddr() {
+        return (local_addr_);
+    }
+
+    /// @brief Sets local port.
+    ///
+    /// @params local specifies local port
+    void setLocalPort(uint16_t local) { local_port_ = local; }
+
+    /// @brief Returns local port.
+    ///
+    /// @return local port
+    uint16_t getLocalPort() { return (local_port_); }
+
+    /// @brief Sets remote port.
+    ///
+    /// @params remote specifies remote port
+    void setRemotePort(uint16_t remote) { remote_port_ = remote; }
+
+    /// @brief Returns remote port.
+    ///
+    /// @return remote port
+    uint16_t getRemotePort() { return (remote_port_); }
+
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
@@ -335,13 +402,13 @@ protected:
     /// Each network interface has assigned unique ifindex. It is functional
     /// equvalent of name, but sometimes more useful, e.g. when using crazy
     /// systems that allow spaces in interface names e.g. MS Windows)
-    int ifindex_;
+    uint32_t ifindex_;
 
     /// local UDP port
-    int local_port_;
+    uint16_t local_port_;
 
     /// remote UDP port
-    int remote_port_;
+    uint16_t remote_port_;
 
     /// @brief message operation code
     ///

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -504,7 +504,7 @@ TEST(Pkt4Test, unpackOptions) {
 
     vector<uint8_t> expectedFormat = generateTestPacket2();
 
-    for (int i=0; i < sizeof(v4Opts); i++) {
+    for (int i = 0; i < sizeof(v4Opts); i++) {
         expectedFormat.push_back(v4Opts[i]);
     }
 
@@ -559,4 +559,19 @@ TEST(Pkt4Test, unpackOptions) {
     EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+22, 3)); // data len=3
 }
 
+TEST(Pkt4Test, metaFields) {
+
+    Pkt4* pkt = new Pkt4(DHCPOFFER, 1234);
+    pkt->setIface("loooopback");
+    pkt->setIndex(42);
+    pkt->setRemoteAddr(IOAddress("1.2.3.4"));
+    pkt->setLocalAddr(IOAddress("4.3.2.1"));
+
+    EXPECT_EQ("loooopback", pkt->getIface());
+    EXPECT_EQ(42, pkt->getIndex());
+    EXPECT_EQ("1.2.3.4", pkt->getRemoteAddr().toText());
+    EXPECT_EQ("4.3.2.1", pkt->getLocalAddr().toText());
+
+}
+
 } // end of anonymous namespace