BIND 10 experiments/dhcp4, updated. b410e40be54a05e3ab779f51738b942b27743fde Merge remote-tracking branch 'origin/trac1238' into dhcp4

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Dec 7 12:21:03 UTC 2011


The branch, experiments/dhcp4 has been updated
       via  b410e40be54a05e3ab779f51738b942b27743fde (commit)
       via  7f9c9b6b123e8594223cc90f34f3931be8d1b93a (commit)
       via  9d04fff18fb4694e3fc4c096e79e85213b065802 (commit)
       via  d8cd199a66645341270081a7f409c557e596099b (commit)
       via  660cf410c0fb41587b977df992879f5dff934c19 (commit)
       via  b09fdcc6b45d4580b138cc9f59bfc051bd6ad360 (commit)
       via  4d97ef5cdb4833a7a36b6679c16338505b07d4e3 (commit)
       via  424f32864efcd2c647c6e5303125b6a8afb421ea (commit)
       via  cc20ff993da1ddb1c6e8a98370438b45a2be9e0a (commit)
      from  915ee6fd6ea9cd6ba623a8af104caf4dd98036c0 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit b410e40be54a05e3ab779f51738b942b27743fde
Merge: 7f9c9b6b123e8594223cc90f34f3931be8d1b93a 660cf410c0fb41587b977df992879f5dff934c19
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 6 12:00:51 2011 +0100

    Merge remote-tracking branch 'origin/trac1238' into dhcp4
    
    Conflicts:
    	ChangeLog
    	src/bin/dhcp6/dhcp6_srv.cc
    	src/lib/dhcp/iface_mgr.cc
    	src/lib/dhcp/iface_mgr.h

commit 7f9c9b6b123e8594223cc90f34f3931be8d1b93a
Merge: 9d04fff18fb4694e3fc4c096e79e85213b065802 d8cd199a66645341270081a7f409c557e596099b
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 6 11:38:22 2011 +0100

    Merge remote-tracking branch 'origin/trac992' into dhcp4
    
    Conflicts:
    	src/bin/dhcp4/dhcp4_srv.cc

commit 9d04fff18fb4694e3fc4c096e79e85213b065802
Merge: 915ee6fd6ea9cd6ba623a8af104caf4dd98036c0 cc20ff993da1ddb1c6e8a98370438b45a2be9e0a
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Dec 6 11:22:59 2011 +0100

    Merge remote-tracking branch 'origin/trac1350' into dhcp4
    
    Conflicts:
    	ChangeLog

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                 |   11 ++
 src/bin/dhcp4/dhcp4_srv.cc                |    4 +-
 src/bin/dhcp4/dhcp4_srv.h                 |    3 +-
 src/bin/dhcp4/tests/dhcp4_srv_unittest.cc |  122 +++++++++++++++++++--
 src/bin/dhcp6/dhcp6_srv.cc                |   24 ++---
 src/bin/dhcp6/dhcp6_srv.h                 |   11 ++-
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc |   21 ++--
 src/lib/dhcp/iface_mgr.cc                 |  169 ++++++++++++++---------------
 src/lib/dhcp/iface_mgr.h                  |   88 +++++++++++++---
 9 files changed, 312 insertions(+), 141 deletions(-)

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index f033783..a920179 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+3XX.	[func]		tomek
+	libdhcp++: Support for handling both IPv4 and IPv6 added.
+	Also added support for binding IPv4 sockets.
+	(Trac #1238, git TBD)
+
+3XX.	[func]		tomek
+	libdhcp++: Support for DHCPv4 option that can store a single
+	address or a list of IPv4 addresses added. Support for END option
+	added.
+	(Trac #1350, git TBD)
+
 333.    [bug]		dvv
 	Solaris needs "-z now" to force non-lazy binding and prevent g++ static
 	initialization code from deadlocking.
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index fde993b..3c5a247 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -25,8 +25,8 @@ using namespace isc::asiolink;
 
 // #define ECHO_SERVER
 
-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
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 69392ba..c5af8aa 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/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>
@@ -41,7 +42,7 @@ public:
     /// In particular, creates IfaceMgr that will be responsible for
     /// network interaction. Will instantiate lease manager, and load
     /// old or create new DUID.
-    Dhcpv4Srv();
+    Dhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT);
 
     /// @brief Destructor. Used during DHCPv6 service shutdown.
     ~Dhcpv4Srv();
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 1d22db8..897ac6d 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -34,33 +34,135 @@ class NakedDhcpv4Srv: public Dhcpv4Srv {
 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
 
-    EXPECT_NO_THROW( {
-        Dhcpv4Srv * srv = new Dhcpv4Srv();
+    Dhcpv4Srv* srv = 0;
+    ASSERT_NO_THROW({
+        srv = new Dhcpv4Srv();
+    });
 
-        delete srv;
+    if (srv) {
+        ASSERT_NO_THROW({
+            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);
+    );
+
+    // should return something
+    EXPECT_TRUE(srv->processInform(pkt));
+
+    // TODO: Implement more reasonable tests before starting
+    // work on processSomething() method.
 
+    delete srv;
 }
 
 } // end of anonymous namespace
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index e71bf34..0533bab 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -26,28 +26,22 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
-Dhcpv6Srv::Dhcpv6Srv() {
+Dhcpv6Srv::Dhcpv6Srv(uint16_t port) {
+
+//void Dhcpv6Srv::Dhcpv6Srv_impl(uint16_t port) {
     cout << "Initialization" << endl;
 
-    // first call to instance() will create IfaceMgr (it's a singleton)
-    // it may throw something if things go wrong
-    try {
-	IfaceMgr::instance();
-    } catch (const std::exception &e) {
-	cout << "Failed to instantiate InterfaceManager. Aborting." << endl;
-	shutdown = true;
-    }
+    // First call to instance() will create IfaceMgr (it's a singleton).
+    // It may throw something if things go wrong.
+    IfaceMgr::instance();
 
     if (IfaceMgr::instance().countIfaces() == 0) {
 	cout << "Failed to detect any network interfaces. Aborting." << endl;
 	shutdown = true;
     }
 
-    try {
-        IfaceMgr::instance().openSockets6();
-    } catch (const Exception& e) {
-
-    }
+    // Now try to open IPv6 sockets on detected interfaces.
+    IfaceMgr::instance().openSockets(port);
 
     /// @todo: instantiate LeaseMgr here once it is imlpemented.
 
@@ -58,6 +52,8 @@ Dhcpv6Srv::Dhcpv6Srv() {
 
 Dhcpv6Srv::~Dhcpv6Srv() {
     cout << "DHCPv6 Srv shutdown." << endl;
+
+    IfaceMgr::instance().closeSockets();
 }
 
 bool
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index 4daef3a..bcc7818 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -17,8 +17,9 @@
 
 #include <boost/shared_ptr.hpp>
 #include <boost/noncopyable.hpp>
-#include "dhcp/pkt6.h"
-#include "dhcp/option.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp/pkt6.h>
+#include <dhcp/option.h>
 #include <iostream>
 
 namespace isc {
@@ -41,10 +42,12 @@ public:
     /// In particular, creates IfaceMgr that will be responsible for
     /// network interaction. Will instantiate lease manager, and load
     /// old or create new DUID.
-    Dhcpv6Srv();
+    ///
+    /// @param port port on will all sockets will listen
+    Dhcpv6Srv(uint16_t port = DHCP6_SERVER_PORT);
 
     /// @brief Destructor. Used during DHCPv6 service shutdown.
-    ~Dhcpv6Srv();
+    virtual ~Dhcpv6Srv();
 
     /// @brief Returns server-intentifier option
     ///
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 72e48e4..8d6c2f0 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -34,7 +34,7 @@ namespace test {
 class NakedDhcpv6Srv: public Dhcpv6Srv {
     // "naked" Interface Manager, exposes internal fields
 public:
-    NakedDhcpv6Srv() { }
+    NakedDhcpv6Srv():Dhcpv6Srv(DHCP6_SERVER_PORT + 10000) { }
 
     boost::shared_ptr<Pkt6>
     processSolicit(boost::shared_ptr<Pkt6>& request) {
@@ -53,29 +53,28 @@ public:
 };
 
 TEST_F(Dhcpv6SrvTest, 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
-
     // srv has stubbed interface detection. It will read
     // interfaces.txt instead. It will pretend to have detected
     // fe80::1234 link-local address on eth0 interface. Obviously
     // an attempt to bind this socket will fail.
-    EXPECT_NO_THROW( {
-        Dhcpv6Srv * srv = new Dhcpv6Srv();
-
+    Dhcpv6Srv * srv = 0;
+    ASSERT_NO_THROW( {
+        // open an unpriviledged port
+        srv = new Dhcpv6Srv(DHCP6_SERVER_PORT + 10000);
+    });
+    if (srv) {
         delete srv;
-        });
+    }
 
 }
 
 TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     NakedDhcpv6Srv * srv = 0;
-    EXPECT_NO_THROW( srv = new NakedDhcpv6Srv(); );
+    ASSERT_NO_THROW( srv = new NakedDhcpv6Srv(); );
 
     // a dummy content for client-id
     boost::shared_array<uint8_t> clntDuid(new uint8_t[32]);
-    for (int i=0; i<32; i++)
+    for (int i = 0; i < 32; i++)
         clntDuid[i] = 100+i;
 
     boost::shared_ptr<Pkt6> sol =
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index d9ecb59..624f969 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -83,14 +83,18 @@ IfaceMgr::Iface::getPlainMac() const {
 }
 
 bool IfaceMgr::Iface::delAddress(const isc::asiolink::IOAddress& addr) {
-    for (AddressCollection::iterator a = addrs_.begin();
-         a!=addrs_.end(); ++a) {
-        if (*a==addr) {
-            addrs_.erase(a);
-            return (true);
-        }
+
+    // Let's delete all addresses that match. It really shouldn't matter
+    // if we delete first or all, as the OS should allow to add a single
+    // address to an interface only once. If OS allows multiple instances
+    // of the same address added, we are in deep problems anyway.
+    size_t size = addrs_.size();
+    addrs_.erase(remove(addrs_.begin(), addrs_.end(), addr), addrs_.end());
+    if (addrs_.size() < size) {
+        return (true);
+    } else {
+        return (false);
     }
-    return (false);
 }
 
 bool IfaceMgr::Iface::delSocket(uint16_t sockfd) {
@@ -133,7 +137,23 @@ IfaceMgr::IfaceMgr()
     }
 }
 
+void IfaceMgr::closeSockets() {
+    for (IfaceCollection::iterator iface = ifaces_.begin();
+         iface != ifaces_.end(); ++iface) {
+
+        for (SocketCollection::iterator sock = iface->sockets_.begin();
+             sock != iface->sockets_.end(); ++sock) {
+            cout << "Closing socket " << sock->sockfd_ << endl;
+            close(sock->sockfd_);
+        }
+        iface->sockets_.clear();
+    }
+
+}
+
 IfaceMgr::~IfaceMgr() {
+    closeSockets();
+
     // control_buf_ is deleted automatically (scoped_ptr)
     control_buf_len_ = 0;
 }
@@ -184,13 +204,13 @@ void IfaceMgr::detectIfaces() {
 }
 #endif
 
-bool IfaceMgr::openSockets4() {
+bool IfaceMgr::openSockets4(uint16_t port) {
     int sock;
+    int sock1, sock2;
     int count = 0;
 
-    for (IfaceCollection::iterator iface=ifaces_.begin();
-         iface!=ifaces_.end();
-         ++iface) {
+    for (IfaceCollection::iterator iface = ifaces_.begin();
+         iface != ifaces_.end(); ++iface) {
 
         cout << "Trying interface " << iface->getFullName() << endl;
 
@@ -202,22 +222,20 @@ bool IfaceMgr::openSockets4() {
 
         AddressCollection addrs = iface->getAddresses();
 
-        for (AddressCollection::iterator addr= addrs.begin();
+        for (AddressCollection::iterator addr = addrs.begin();
              addr != addrs.end();
              ++addr) {
 
             // skip IPv4 addresses
             if (addr->getFamily() != AF_INET) {
                 continue;
-            }
 
-            sock = openSocket(iface->getName(), *addr,
-                              DHCP4_SERVER_PORT);
-            if (sock<0) {
-                cout << "Failed to open unicast socket." << endl;
-                return (false);
+            sock1 = openSocket(iface->getName(), *addr, port);
+            if (sock1 < 0) {
+                isc_throw(Unexpected, "Failed to open unicast socket on "
+                          << " interface " << iface->getFullName());
             }
-
+            
             count++;
         }
     }
@@ -263,8 +281,8 @@ bool IfaceMgr::openSockets6() {
             // this doesn't work too well on NetBSD
             sock2 = openSocket(iface->getName(),
                                IOAddress(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
-                               DHCP6_SERVER_PORT);
-            if (sock2<0) {
+                               port);
+            if (sock2 < 0) {
                 isc_throw(Unexpected, "Failed to open multicast socket on "
                           << " interface " << iface->getFullName());
                 iface->delSocket(sock1); // delete previously opened socket
@@ -277,6 +295,7 @@ bool IfaceMgr::openSockets6() {
 
 void
 IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) {
+
     for (IfaceCollection::const_iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
@@ -302,11 +321,11 @@ IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) {
 
 IfaceMgr::Iface*
 IfaceMgr::getIface(int ifindex) {
-    for (IfaceCollection::iterator iface=ifaces_.begin();
-         iface!=ifaces_.end();
-         ++iface) {
-        if (iface->getIndex() == ifindex)
+    for (IfaceCollection::iterator iface = ifaces_.begin();
+         iface != ifaces_.end(); ++iface) {
+        if (iface->getIndex() == ifindex) {
             return (&(*iface));
+        }
     }
 
     return (NULL); // not found
@@ -314,11 +333,11 @@ IfaceMgr::getIface(int ifindex) {
 
 IfaceMgr::Iface*
 IfaceMgr::getIface(const std::string& ifname) {
-    for (IfaceCollection::iterator iface=ifaces_.begin();
-         iface!=ifaces_.end();
-         ++iface) {
-        if (iface->getName() == ifname)
+    for (IfaceCollection::iterator iface = ifaces_.begin();
+         iface != ifaces_.end(); ++iface) {
+        if (iface->getName() == ifname) {
             return (&(*iface));
+        }
     }
 
     return (NULL); // not found
@@ -369,8 +388,8 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
                   << "/port=" << port);
     }
 
-    // if there is no support for IP_PKTINFO, we are really out of luck
-    // it will be difficult to undersand, where this packet came from
+    // If there is no support for IP_PKTINFO, we are really out of luck.
+    // It will be difficult to understand, where this packet came from.
 #if defined(IP_PKTINFO)
     int flag = 1;
     if (setsockopt(sock, IPPROTO_IP, IP_PKTINFO, &flag, sizeof(flag)) != 0) {
@@ -382,8 +401,7 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     cout << "Created socket " << sock << " on " << iface.getName() << "/" <<
         addr.toText() << "/port=" << port << endl;
 
-    SocketInfo info(sock, addr, port);
-    iface.addSocket(info);
+    iface.addSocket(SocketInfo(sock, addr, port));
 
     return (sock);
 }
@@ -415,8 +433,8 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
         isc_throw(Unexpected, "Failed to create UDP6 socket.");
     }
 
-    /* Set the REUSEADDR option so that we don't fail to start if
-       we're being restarted. */
+    // Set the REUSEADDR option so that we don't fail to start if
+    // we're being restarted.
     int flag = 1;
     if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
                    (char *)&flag, sizeof(flag)) < 0) {
@@ -430,14 +448,14 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
                   << "/port=" << port);
     }
 #ifdef IPV6_RECVPKTINFO
-    /* RFC3542 - a new way */
+    // RFC3542 - a new way
     if (setsockopt(sock, IPPROTO_IPV6, IPV6_RECVPKTINFO,
                    &flag, sizeof(flag)) != 0) {
         close(sock);
         isc_throw(Unexpected, "setsockopt: IPV6_RECVPKTINFO failed.");
     }
 #else
-    /* RFC2292 - an old way */
+    // RFC2292 - an old way
     if (setsockopt(sock, IPPROTO_IPV6, IPV6_PKTINFO,
                    &flag, sizeof(flag)) != 0) {
         close(sock);
@@ -462,8 +480,7 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
     cout << "Created socket " << sock << " on " << iface.getName() << "/" <<
         addr.toText() << "/port=" << port << endl;
 
-    SocketInfo info(sock, addr, port);
-    iface.addSocket(info);
+    iface.addSocket(SocketInfo(sock, addr, port));
 
     return (sock);
 }
@@ -509,14 +526,10 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
 
     memset(&control_buf_[0], 0, control_buf_len_);
 
-    /*
-     * Initialize our message header structure.
-     */
+    // Initialize our message header structure.
     memset(&m, 0, sizeof(m));
 
-    /*
-     * Set the target address we're sending to.
-     */
+    // Set the target address we're sending to.
     sockaddr_in6 to;
     memset(&to, 0, sizeof(to));
     to.sin6_family = AF_INET6;
@@ -529,24 +542,20 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
     m.msg_name = &to;
     m.msg_namelen = sizeof(to);
 
-    /*
-     * Set the data buffer we're sending. (Using this wacky
-     * "scatter-gather" stuff... we only have a single chunk
-     * of data to send, so we declare a single vector entry.)
-     */
+    // Set the data buffer we're sending. (Using this wacky
+    // "scatter-gather" stuff... we only have a single chunk
+    // of data to send, so we declare a single vector entry.)
     v.iov_base = (char *) &pkt->data_[0];
     v.iov_len = pkt->data_len_;
     m.msg_iov = &v;
     m.msg_iovlen = 1;
 
-    /*
-     * Setting the interface is a bit more involved.
-     *
-     * We have to create a "control message", and set that to
-     * define the IPv6 packet information. We could set the
-     * source address if we wanted, but we can safely let the
-     * kernel decide what that should be.
-     */
+    // Setting the interface is a bit more involved.
+    //
+    // We have to create a "control message", and set that to
+    // define the IPv6 packet information. We could set the
+    // source address if we wanted, but we can safely let the
+    // kernel decide what that should be.
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
     cmsg = CMSG_FIRSTHDR(&m);
@@ -820,35 +829,27 @@ IfaceMgr::receive6() {
     memset(&from, 0, sizeof(from));
     memset(&to_addr, 0, sizeof(to_addr));
 
-    /*
-     * Initialize our message header structure.
-     */
+    // Initialize our message header structure.
     memset(&m, 0, sizeof(m));
 
-    /*
-     * Point so we can get the from address.
-     */
+    // Point so we can get the from address.
     m.msg_name = &from;
     m.msg_namelen = sizeof(from);
 
-    /*
-     * Set the data buffer we're receiving. (Using this wacky
-     * "scatter-gather" stuff... but we that doesn't really make
-     * sense for us, so we use a single vector entry.)
-     */
+    // Set the data buffer we're receiving. (Using this wacky
+    // "scatter-gather" stuff... but we that doesn't really make
+    // sense for us, so we use a single vector entry.)
     v.iov_base = (void*)&pkt->data_[0];
     v.iov_len = pkt->data_len_;
     m.msg_iov = &v;
     m.msg_iovlen = 1;
 
-    /*
-     * Getting the interface is a bit more involved.
-     *
-     * We set up some space for a "control message". We have
-     * previously asked the kernel to give us packet
-     * information (when we initialized the interface), so we
-     * should get the destination address from that.
-     */
+    // Getting the interface is a bit more involved.
+    //
+    // We set up some space for a "control message". We have
+    // previously asked the kernel to give us packet
+    // information (when we initialized the interface), so we
+    // should get the destination address from that.
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
 
@@ -886,14 +887,12 @@ IfaceMgr::receive6() {
     result = recvmsg(candidate->sockfd_, &m, 0);
 
     if (result >= 0) {
-        /*
-         * If we did read successfully, then we need to loop
-         * through the control messages we received and
-         * find the one with our destination address.
-         *
-         * We also keep a flag to see if we found it. If we
-         * didn't, then we consider this to be an error.
-         */
+        // If we did read successfully, then we need to loop
+        // through the control messages we received and
+        // find the one with our destination address.
+        //
+        // We also keep a flag to see if we found it. If we
+        // didn't, then we consider this to be an error.
         int found_pktinfo = 0;
         cmsg = CMSG_FIRSTHDR(&m);
         while (cmsg != NULL) {
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index dfe5050..0fcaaca 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -46,6 +46,12 @@ public:
         isc::asiolink::IOAddress addr_; /// bound address
         uint16_t port_;   /// socket port
         uint16_t family_; /// IPv4 or IPv6
+
+        /// @brief SocketInfo constructor.
+        ///
+        /// @param sockfd socket descriptor
+        /// @param addr an address the socket is bound to
+        /// @param port a port the socket is bound to
         SocketInfo(uint16_t sockfd, const isc::asiolink::IOAddress& addr,
                    uint16_t port)
         :sockfd_(sockfd), addr_(addr), port_(port), family_(addr.getFamily()) { }
@@ -59,7 +65,8 @@ public:
     /// Iface structure represents network interface with all useful
     /// information, like name, interface index, MAC address and
     /// list of assigned addresses
-    struct Iface {
+    class Iface {
+    public:
         /// @brief Iface constructor.
         ///
         /// Creates Iface object that represents network interface.
@@ -248,24 +255,31 @@ public:
     void
     printIfaces(std::ostream& out = std::cout);
 
-    /// @brief Sends a packet.
+    /// @brief Sends an IPv6 packet.
     ///
-    /// Sends a packet. All parameters for actual transmission are specified in
+    /// Sends an IPv6 packet. All parameters for actual transmission are specified in
     /// Pkt6 structure itself. That includes destination address, src/dst port
     /// and interface over which data will be sent.
     ///
     /// @param pkt packet to be sent
     ///
     /// @return true if sending was successful
-    bool
-    send(boost::shared_ptr<Pkt6>& pkt);
+    bool send(boost::shared_ptr<Pkt6>& pkt);
 
-    bool
-    send(boost::shared_ptr<Pkt4>& pkt);
+    /// @brief Sends an IPv4 packet.
+    ///
+    /// Sends an IPv4 packet. All parameters for actual transmission are specified
+    /// in Pkt4 structure itself. That includes destination address, src/dst
+    /// port and interface over which data will be sent.
+    ///
+    /// @param pkt a packet to be sent
+    ///
+    /// @return true if sending was successful
+    bool send(boost::shared_ptr<Pkt4>& pkt);
 
-    /// @brief Tries to receive packet over open sockets.
+    /// @brief Tries to receive IPv6 packet over open IPv6 sockets.
     ///
-    /// Attempts to receive a single packet of any of the open sockets.
+    /// Attempts to receive a single IPv6 packet of any of the open IPv6 sockets.
     /// If reception is successful and all information about its sender
     /// are obtained, Pkt6 object is created and returned.
     ///
@@ -276,9 +290,19 @@ public:
     /// @return Pkt6 object representing received packet (or NULL)
     boost::shared_ptr<Pkt6> receive6();
 
+    /// @brief Tries to receive IPv4 packet over open IPv4 sockets.
+    ///
+    /// Attempts to receive a single IPv4 packet of any of the open IPv4 sockets.
+    /// If reception is successful and all information about its sender
+    /// are obtained, Pkt4 object is created and returned.
+    ///
+    /// TODO Start using select() and add timeout to be able
+    /// to not wait infinitely, but rather do something useful
+    /// (e.g. remove expired leases)
+    ///
+    /// @return Pkt4 object representing received packet (or NULL)
     boost::shared_ptr<Pkt4> receive4();
 
-    ///
     /// Opens UDP/IP socket and binds it to address, interface and port.
     ///
     /// Specific type of socket (UDP/IPv4 or UDP/IPv6) depends on passed addr
@@ -288,8 +312,11 @@ public:
     /// @param addr address to be bound.
     /// @param port UDP port.
     ///
+    /// Method will throw if socket creation, socket binding or multicast
+    /// join fails.
+    ///
     /// @return socket descriptor, if socket creation, binding and multicast
-    /// group join were all successful. -1 otherwise.
+    /// group join were all successful.
     uint16_t openSocket(const std::string& ifname,
                         const isc::asiolink::IOAddress& addr, int port);
 
@@ -299,29 +326,62 @@ public:
     uint16_t countIfaces() { return ifaces_.size(); }
 
     /// Opens IPv6 sockets on detected interfaces.
-    bool
-    openSockets6();
+    /// Opens IPv6 sockets on detected interfaces.
+    ///
+    /// Will throw exception if socket creation fails.
+    ///
+    /// @param port specifies port number (usually DHCP6_SERVER_PORT)
+    bool openSockets6(uint16_t port);
 
 
     /// Opens IPv4 sockets on detected interfaces.
     bool
     openSockets4();
 
+    /// @brief Closes all open sockets.
+    /// Is used in destructor, but also from Dhcpv4_srv and Dhcpv6_srv classes.
+    void closeSockets();
+
     // don't use private, we need derived classes in tests
 protected:
 
     /// @brief Protected constructor.
     ///
     /// Protected constructor. This is a singleton class. We don't want
-    /// anyone to create instances of IfaceMgr. Use instance() method
+    /// anyone to create instances of IfaceMgr. Use instance() method instead.
     IfaceMgr();
 
     ~IfaceMgr();
 
+    /// @brief Opens IPv4 socket.
+    ///
+    /// Please do not use this method directly. Use openSocket instead.
+    ///
+    /// This method may throw exception if socket creation fails.
+    ///
+    /// @param iface reference to interface structure.
+    /// @param addr an address the created socket should be bound to
+    /// @param port a port that created socket should be bound to
+    ///
+    /// @return socket descriptor
     uint16_t openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
 
+    /// @brief Opens IPv6 socket.
+    ///
+    /// Please do not use this method directly. Use openSocket instead.
+    ///
+    /// This method may throw exception if socket creation fails.
+    ///
+    /// @param iface reference to interface structure.
+    /// @param addr an address the created socket should be bound to
+    /// @param port a port that created socket should be bound to
+    ///
+    /// @return socket descriptor
     uint16_t openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
 
+    /// @brief Adds an interface to list of known interfaces.
+    ///
+    /// @param iface reference to Iface object.
     void addInterface(const Iface& iface) {
         ifaces_.push_back(iface);
     }




More information about the bind10-changes mailing list