BIND 10 trac878, updated. b2cab7978ff20eff1d4fcb4cf60fc8a4421fc24c [trac878] Changes after Jinmei's pre-review: - coding standards improved - better exception handling

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Aug 4 20:13:41 UTC 2011


The branch, trac878 has been updated
       via  b2cab7978ff20eff1d4fcb4cf60fc8a4421fc24c (commit)
      from  600d77cc8af4625a30dceda2033c4aadbbbe71ff (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 b2cab7978ff20eff1d4fcb4cf60fc8a4421fc24c
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Thu Aug 4 22:11:10 2011 +0200

    [trac878] Changes after Jinmei's pre-review:
    - coding standards improved
    - better exception handling

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

Summary of changes:
 src/bin/dhcp6/addr6.cc                    |   38 ++-
 src/bin/dhcp6/addr6.h                     |   24 +-
 src/bin/dhcp6/dhcp6.h                     |    4 +-
 src/bin/dhcp6/dhcp6_srv.cc                |   17 +-
 src/bin/dhcp6/iface_mgr.cc                |  500 ++++++++++++++++-------------
 src/bin/dhcp6/iface_mgr.h                 |   19 +-
 src/bin/dhcp6/main.cc                     |    9 +-
 src/bin/dhcp6/pkt6.cc                     |   63 +++--
 src/bin/dhcp6/pkt6.h                      |   14 +-
 src/bin/dhcp6/tests/addr6_unittest.cc     |    3 +
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc |    9 +-
 src/bin/dhcp6/tests/iface_mgr_unittest.cc |   18 +-
 src/bin/dhcp6/tests/pkt6_unittest.cc      |    6 +-
 13 files changed, 418 insertions(+), 306 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/addr6.cc b/src/bin/dhcp6/addr6.cc
index 4256e13..c191b78 100644
--- a/src/bin/dhcp6/addr6.cc
+++ b/src/bin/dhcp6/addr6.cc
@@ -19,7 +19,8 @@
 
 #include "dhcp6/addr6.h"
 
-std::ostream & isc::operator << (std::ostream & out, const isc::Addr6& addr) {
+std::ostream&
+isc::operator << (std::ostream & out, const isc::Addr6& addr) {
     out << addr.getPlain();
     return out;
 }
@@ -27,7 +28,7 @@ std::ostream & isc::operator << (std::ostream & out, const isc::Addr6& addr) {
 using namespace std;
 using namespace isc;
 
-Addr6::Addr6(const char *addr, bool plain /*=false*/) {
+Addr6::Addr6(const char* addr, bool plain /*=false*/) {
     if (plain) {
         inet_pton(AF_INET6, addr, addr_);
     } else {
@@ -47,37 +48,44 @@ Addr6::Addr6(sockaddr_in6* addr) {
     memcpy(addr_, &addr->sin6_addr, 16);
 }
 
-bool Addr6::linkLocal() const {
+bool
+Addr6::linkLocal() const {
     if ( ( (unsigned char)addr_[0]==0xfe) &&
          ( (unsigned char)addr_[1]==0x80) ) {
-        return true;
+	return (true);
     } else {
-        return false;
+        return (false);
     }
 }
 
-bool Addr6::multicast() const {
+bool
+Addr6::multicast() const {
     if ( (unsigned char)addr_[0]==0xff) {
-        return true;
+        return (true);
     } else {
-        return false;
+        return (false);
     }
 }
 
-std::string Addr6::getPlain() const {
+std::string
+Addr6::getPlain() const {
     char buf[MAX_ADDRESS_STRING_LEN];
 
     inet_ntop(AF_INET6, addr_, buf, MAX_ADDRESS_STRING_LEN);
-    return string(buf);
+    return (string(buf));
 }
 
-
-bool Addr6::operator==(const Addr6& other) const {
+bool
+Addr6::equals(const Addr6& other) const {
     if (!memcmp(addr_, other.addr_, 16)) {
-        return true;
+        return (true);
     } else {
-        return false;
+        return (false);
     }
-
     // return !memcmp() would be shorter, but less readable
 }
+
+bool
+Addr6::operator==(const Addr6& other) const {
+    return (equals(other));
+}
diff --git a/src/bin/dhcp6/addr6.h b/src/bin/dhcp6/addr6.h
index 3de59e7..07bcd7b 100644
--- a/src/bin/dhcp6/addr6.h
+++ b/src/bin/dhcp6/addr6.h
@@ -24,15 +24,26 @@ namespace isc {
     static const int MAX_ADDRESS_STRING_LEN =
         sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255");
 
+
+    /// \brief The implementation class for IPv6 address.
+    ///
+    /// There are no virtual methods to avoid virtual funtions
+    /// table. There are no extra fields, other than the address
+    /// itself. As a result, instances of this class are memory
+    /// optimal (sizeof(Addr6) == 16).
+    ///
+    /// Extra care should be taken, when extending this class
+    /// to keep low memory footprint.
     class Addr6 {
     public:
-        Addr6(const char * addr, bool plain=false);
+        Addr6(const char* addr, bool plain=false);
         Addr6(struct in6_addr* addr);
-        Addr6(struct sockaddr_in6 * addr);
+        Addr6(struct sockaddr_in6* addr);
         Addr6();
         inline const char * get() const { return addr_; }
         std::string getPlain() const;
-        char * get() { return addr_; }
+        char* getAddr() { return addr_; }
+        bool equals(const Addr6& other) const;
         bool operator==(const Addr6& other) const;
 
         bool linkLocal() const;
@@ -40,14 +51,13 @@ namespace isc {
 
         // no dtor necessary (no allocations done)
     private:
-        char addr_[MAX_ADDRESS_STRING_LEN];
+        char addr_[16];
     };
 
-    std::ostream & operator << (std::ostream & out, const Addr6& addr);
+    std::ostream& operator << (std::ostream & out, const Addr6& addr);
 
+    // TODO may need to also define map for faster access
     typedef std::list<Addr6> Addr6Lst;
-
 };
 
-
 #endif
diff --git a/src/bin/dhcp6/dhcp6.h b/src/bin/dhcp6/dhcp6.h
index 4228730..b5512f3 100644
--- a/src/bin/dhcp6/dhcp6.h
+++ b/src/bin/dhcp6/dhcp6.h
@@ -126,8 +126,8 @@ extern const int dhcpv6_type_name_max;
 /* 
  * DHCPv6 well-known multicast addressess, from section 5.1 of RFC 3315 
  */
-#define ALL_DHCP_RELAY_AGENTS_AND_SERVERS "FF02::1:2"
-#define ALL_DHCP_SERVERS "FF05::1:3"
+#define ALL_DHCP_RELAY_AGENTS_AND_SERVERS "ff02::1:2"
+#define ALL_DHCP_SERVERS "ff05::1:3"
 
 #define DHCP6_CLIENT_PORT 546
 #define DHCP6_SERVER_PORT 547
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 751bc0b..8e17d54 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -22,29 +22,36 @@ using namespace isc;
 
 Dhcpv6Srv::Dhcpv6Srv() {
     cout << "Initialization" << endl;
+
+    // first call to instance() will create IfaceMgr (it's a singleton)
+    // it may throw something if things go wrong
+    IfaceMgr::instance();
 }
 
 Dhcpv6Srv::~Dhcpv6Srv() {
     cout << "DHCPv6 Srv shutdown." << endl;
 }
 
-bool Dhcpv6Srv::run() {
+bool
+Dhcpv6Srv::run() {
     while (true) {
-        Pkt6 * pkt;
+        Pkt6* pkt;
 
         pkt = IfaceMgr::instance().receive();
 
         if (pkt) {
-            Addr6 client = pkt->remoteAddr;
-            cout << "Received " << pkt->dataLen_ << " bytes, echoing back."
+            Addr6 client = pkt->remote_addr_;
+            cout << "Received " << pkt->data_len_ << " bytes, echoing back."
                  << endl;
             IfaceMgr::instance().send(*pkt);
             delete pkt;
         }
 
+	// TODO add support for config session (see src/bin/auth/main.cc)
+	//      so this daemon can be controlled from bob
         sleep(1);
 
     }
 
-    return true;
+    return (true);
 }
diff --git a/src/bin/dhcp6/iface_mgr.cc b/src/bin/dhcp6/iface_mgr.cc
index 8153d94..8ca2bc2 100644
--- a/src/bin/dhcp6/iface_mgr.cc
+++ b/src/bin/dhcp6/iface_mgr.cc
@@ -22,67 +22,85 @@
 #include "addr6.h"
 #include "dhcp6/iface_mgr.h"
 #include "dhcp6/dhcp6.h"
+#include "exceptions/exceptions.h"
 
 using namespace std;
 using namespace isc;
 
+namespace isc {
+
 // IfaceMgr is a singleton implementation
-IfaceMgr * IfaceMgr::instance_ = 0;
+IfaceMgr* IfaceMgr::instance_ = 0;
 
-void IfaceMgr::instanceCreate() {
+void
+IfaceMgr::instanceCreate() {
     if (instance_) {
-        // XXX: throw exception here
+        // TODO throw exception here
         return;
     }
     instance_ = new IfaceMgr();
 }
 
-IfaceMgr& IfaceMgr::instance() {
+IfaceMgr&
+IfaceMgr::instance() {
     if (instance_ == 0)
         instanceCreate();
-    return *instance_;
+    return (*instance_);
 }
 
-
 IfaceMgr::Iface::Iface()
-    : name_(""), ifindex_(0), macLen_(0) {
+    : name_(""), ifindex_(0), mac_len_(0) {
     memset(mac_, 0, 20);
 }
 
-IfaceMgr::Iface::Iface(const std::string name, int ifindex)
-    :name_(name), ifindex_(ifindex), macLen_(0) {
+IfaceMgr::Iface::Iface(const std::string& name, int ifindex)
+    :name_(name), ifindex_(ifindex), mac_len_(0) {
     memset(mac_, 0, 20);
 }
 
-std::string IfaceMgr::Iface::getFullName() const {
+std::string
+IfaceMgr::Iface::getFullName() const {
     ostringstream tmp;
     tmp << name_ << "/" << ifindex_;
-    return tmp.str();
+    return (tmp.str());
 }
 
-std::string IfaceMgr::Iface::getPlainMac() const {
+std::string
+IfaceMgr::Iface::getPlainMac() const {
     ostringstream tmp;
-    for (int i=0; i<macLen_; i++) {
+    for (int i=0; i<mac_len_; i++) {
         tmp.fill('0');
         tmp.width(2);
         tmp << (hex) << (int) mac_[i];
-        if (i<macLen_-1) {
+        if (i<mac_len_-1) {
             tmp << ":";
         }
     }
-    return tmp.str();
+    return (tmp.str());
 }
 
 IfaceMgr::IfaceMgr() {
 
-    control_buf_len_ = CMSG_SPACE(sizeof(struct in6_pktinfo));
-    control_buf_ = (char*) new char[control_buf_len_];
+    cout << "IfaceMgr initialization." << endl;
 
-   cout << "IfaceMgr initialization." << endl;
+    try {
+        control_buf_len_ = CMSG_SPACE(sizeof(struct in6_pktinfo));
+        control_buf_ = new char[control_buf_len_];
 
-   detectIfaces();
+        detectIfaces();
 
-   openSockets();
+        if (!openSockets()) {
+            isc_throw(Unexpected, "Failed to open/bind sockets.");
+        }
+    } catch (std::exception& ex) {
+        cout << "IfaceMgr creation failed:" << ex.what() << endl;
+
+        // TODO Uncomment this (or call LOG_FATAL) once
+        // interface detection is implemented. Otherwise
+        // it is not possible to run tests in a portable
+        // way (see detectIfaces() method).
+        // throw ex;
+    }
 }
 
 IfaceMgr::~IfaceMgr() {
@@ -93,36 +111,49 @@ IfaceMgr::~IfaceMgr() {
     }
 }
 
-void IfaceMgr::detectIfaces() {
+void
+IfaceMgr::detectIfaces() {
     string ifaceName, linkLocal;
 
-    // XXX: do the actual detection
+    // TODO do the actual detection. Currently interface detection is faked
+    //      by reading a text file.
 
     cout << "Interface detection is not implemented yet. "
          << "Reading interfaces.txt file instead." << endl;
     cout << "Please use format: interface-name link-local-address" << endl;
 
-    ifstream interfaces("interfaces.txt");
-    interfaces >> ifaceName;
-    interfaces >> linkLocal;
+    try {
+        ifstream interfaces("interfaces.txt");
 
-    cout << "Detected interface " << ifaceName << "/" << linkLocal << endl;
-
-    Iface iface(ifaceName, if_nametoindex( ifaceName.c_str() ) );
-    Addr6 addr(linkLocal.c_str(), true);
-    iface.addrs_.push_back(addr);
-    ifaces_.push_back(iface);
-    interfaces.close();
+        if (!interfaces.good()) {
+            cout << "Failed to read interfaces.txt file." << endl;
+            isc_throw(Unexpected, "Failed to read interfaces.txt");
+        }
+        interfaces >> ifaceName;
+        interfaces >> linkLocal;
+
+        cout << "Detected interface " << ifaceName << "/" << linkLocal << endl;
+
+        Iface iface(ifaceName, if_nametoindex( ifaceName.c_str() ) );
+        Addr6 addr(linkLocal.c_str(), true);
+        iface.addrs_.push_back(addr);
+        ifaces_.push_back(iface);
+        interfaces.close();
+    } catch (std::exception& ex) {
+        // TODO Do LOG_FATAL here
+        std::cerr << "Interface detection failed." << std::endl;
+        throw ex;
+    }
 }
 
-bool IfaceMgr::openSockets() {
+bool
+IfaceMgr::openSockets() {
     int sock;
 
     for (IfaceLst::iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
 
-
         for (Addr6Lst::iterator addr=iface->addrs_.begin();
              addr!=iface->addrs_.end();
              ++addr) {
@@ -131,7 +162,7 @@ bool IfaceMgr::openSockets() {
                               DHCP6_SERVER_PORT, false);
             if (sock<0) {
                 cout << "Failed to open unicast socket." << endl;
-                return false;
+                return (false);
             }
             sendsock_ = sock;
 
@@ -140,16 +171,17 @@ bool IfaceMgr::openSockets() {
                               DHCP6_SERVER_PORT, true);
             if (sock<0) {
                 cout << "Failed to open multicast socket." << endl;
-                return false;
+                return (false);
             }
             recvsock_ = sock;
         }
     }
 
-    return true;
+    return (true);
 }
 
-void IfaceMgr::printIfaces() {
+void
+IfaceMgr::printIfaces() {
     for (IfaceLst::const_iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
@@ -164,29 +196,32 @@ void IfaceMgr::printIfaces() {
     }
 }
 
-IfaceMgr::Iface* IfaceMgr::getIface(int ifindex) {
+IfaceMgr::Iface*
+IfaceMgr::getIface(int ifindex) {
     for (IfaceLst::iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
         if (iface->ifindex_ == ifindex)
-            return &(*iface);
+            return (&(*iface));
     }
 
     return 0; // not found
 }
 
-IfaceMgr::Iface* IfaceMgr::getIface(const std::string &ifname) {
+IfaceMgr::Iface*
+IfaceMgr::getIface(const std::string &ifname) {
     for (IfaceLst::iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
         if (iface->name_ == ifname)
-            return &(*iface);
+            return (&(*iface));
     }
 
-    return 0; // not found
+    return (0); // not found
 }
 
-int IfaceMgr::openSocket(const std::string &ifname,
+int
+IfaceMgr::openSocket(const std::string &ifname,
                          const Addr6 &addr,
                          int port,
                          bool mcast) {
@@ -194,7 +229,7 @@ int IfaceMgr::openSocket(const std::string &ifname,
     int name_len;
     struct sockaddr_in6 *addr6;
 
-    cout << "Creating socket on " << ifname << "/" << addr << "/port=" 
+    cout << "Creating socket on " << ifname << "/" << addr << "/port="
          << port << endl;
 
     memset(&name, 0, sizeof(name));
@@ -216,7 +251,7 @@ int IfaceMgr::openSocket(const std::string &ifname,
     int sock = socket(AF_INET6, SOCK_DGRAM, 0);
     if (sock < 0) {
         cout << "Failed to create UDP6 socket." << endl;
-        return -1;
+        return (-1);
     }
 
     /* Set the REUSEADDR option so that we don't fail to start if
@@ -225,13 +260,13 @@ int IfaceMgr::openSocket(const std::string &ifname,
     if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
                    (char *)&flag, sizeof(flag)) < 0) {
         cout << "Can't set SO_REUSEADDR option on dhcpv6 socket." << endl;
-        return -1;
+        return (-1);
     }
 
     if (bind(sock, (struct sockaddr *)&name, name_len) < 0) {
         cout << "Failed to bind socket " << sock << " to " << addr.getPlain()
              << "/port=" << port << endl;
-        return -1;
+        return (-1);
     }
 
 #ifdef IPV6_RECVPKTINFO
@@ -239,14 +274,14 @@ int IfaceMgr::openSocket(const std::string &ifname,
     if (setsockopt(sock, IPPROTO_IPV6, IPV6_RECVPKTINFO,
                    &flag, sizeof(flag)) != 0) {
         cout << "setsockopt: IPV6_RECVPKTINFO failed." << endl;
-        return -1;
+        return (-1);
     }
 #else
     /* RFC2292 - an old way */
     if (setsockopt(sock, IPPROTO_IPV6, IPV6_PKTINFO,
                    &flag, sizeof(flag)) != 0) {
         cout << "setsockopt: IPV6_PKTINFO: failed." << endl;
-        return -1;
+        return (-1);
     }
 #endif
 
@@ -257,212 +292,225 @@ int IfaceMgr::openSocket(const std::string &ifname,
         // are link and site-scoped, so there is no sense to join those them
         // with global addressed.
 
-        if ( !joinMcast( sock, ifname, 
+        if ( !joinMcast( sock, ifname,
                          string(ALL_DHCP_RELAY_AGENTS_AND_SERVERS) ) ) {
             close(sock);
-            return -1;
+            return (-1);
         }
     }
 
-    cout << "Created socket " << sock << " on " << ifname << "/" << addr 
+    cout << "Created socket " << sock << " on " << ifname << "/" << addr
          << "/port=" << port << endl;
 
-    return sock;
+    return (sock);
 }
 
-bool IfaceMgr::joinMcast(int sock, const std::string& ifname,
-                         const std::string & mcast) {
+bool
+IfaceMgr::joinMcast(int sock, const std::string& ifname,
+const std::string & mcast) {
 
     struct ipv6_mreq mreq;
 
-        if (inet_pton(AF_INET6, mcast.c_str(),
-                      &mreq.ipv6mr_multiaddr) <= 0) {
-            cout << "Failed to convert " << ifname
-                 << " to IPv6 multicast address." << endl;
-            return false;
-        }
-
-        mreq.ipv6mr_interface = if_nametoindex(ifname.c_str());
-        if (setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP,
-                       &mreq, sizeof(mreq)) < 0) {
-            cout << "Failed to join " << mcast << " multicast group." << endl;
-            return false;
-        }
-
-        cout << "Joined multicast " << mcast << " group." << endl;
-
-        return true;
+    if (inet_pton(AF_INET6, mcast.c_str(),
+                  &mreq.ipv6mr_multiaddr) <= 0) {
+        cout << "Failed to convert " << ifname
+             << " to IPv6 multicast address." << endl;
+        return (false);
     }
 
-    bool IfaceMgr::send(Pkt6 &pkt) {
-        struct msghdr m;
-        struct iovec v;
-        int result;
-        struct in6_pktinfo *pktinfo;
-        struct cmsghdr *cmsg;
-        memset(control_buf_, 0, control_buf_len_);
-
-        /*
-         * Initialize our message header structure.
-         */
-        memset(&m, 0, sizeof(m));
-
-        /*
-         * Set the target address we're sending to.
-         */
-        sockaddr_in6 to;
-        memset(&to, 0, sizeof(to));
-        to.sin6_family = AF_INET6;
-        to.sin6_port = htons(pkt.remotePort);
-        memcpy(&to.sin6_addr, pkt.remoteAddr.get(), 16);
-        to.sin6_scope_id = pkt.ifindex;
-
-        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.)
-         */
-        v.iov_base = (char *) pkt.data_;
-        v.iov_len = pkt.dataLen_;
-        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.
-         */
-        m.msg_control = control_buf_;
-        m.msg_controllen = control_buf_len_;
-        cmsg = CMSG_FIRSTHDR(&m);
-        cmsg->cmsg_level = IPPROTO_IPV6;
-        cmsg->cmsg_type = IPV6_PKTINFO;
-        cmsg->cmsg_len = CMSG_LEN(sizeof(*pktinfo));
-        pktinfo = (struct in6_pktinfo *)CMSG_DATA(cmsg);
-        memset(pktinfo, 0, sizeof(*pktinfo));
-        pktinfo->ipi6_ifindex = pkt.ifindex;
-        m.msg_controllen = cmsg->cmsg_len;
-
-        result = sendmsg(sendsock_, &m, 0);
-        if (result < 0) {
-            cout << "Send packet failed." << endl;
-        }
-        cout << "Sent " << result << " bytes." << endl;
-
-        cout << "Sent " << pkt.dataLen_ << " bytes over "
-             << pkt.iface << "/" << pkt.ifindex << " interface: "
-             << " dst=" << pkt.remoteAddr << ", src=" << pkt.localAddr
-             << endl;
-
-        return result;
+    mreq.ipv6mr_interface = if_nametoindex(ifname.c_str());
+    if (setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP,
+                   &mreq, sizeof(mreq)) < 0) {
+        cout << "Failed to join " << mcast << " multicast group." << endl;
+        return (false);
     }
 
-    Pkt6 * IfaceMgr::receive() {
-        struct msghdr m;
-        struct iovec v;
-        int result;
-        struct cmsghdr *cmsg;
-        struct in6_pktinfo *pktinfo;
-        int found_pktinfo;
-        struct sockaddr_in6 from;
-        struct in6_addr to_addr;
-
-        Pkt6 * pkt = new Pkt6(1500);
+    cout << "Joined multicast " << mcast << " group." << endl;
 
-        memset(control_buf_, 0, control_buf_len_);
+    return (true);
+}
 
-        memset(&from, 0, sizeof(from));
-        memset(&to_addr, 0, sizeof(to_addr));
+bool
+IfaceMgr::send(Pkt6 &pkt) {
+    struct msghdr m;
+    struct iovec v;
+    int result;
+    struct in6_pktinfo *pktinfo;
+    struct cmsghdr *cmsg;
+    memset(control_buf_, 0, control_buf_len_);
+
+    /*
+     * Initialize our message header structure.
+     */
+    memset(&m, 0, sizeof(m));
+
+    /*
+     * Set the target address we're sending to.
+     */
+    sockaddr_in6 to;
+    memset(&to, 0, sizeof(to));
+    to.sin6_family = AF_INET6;
+    to.sin6_port = htons(pkt.remote_port_);
+    memcpy(&to.sin6_addr, pkt.remote_addr_.get(), 16);
+    to.sin6_scope_id = pkt.ifindex_;
+
+    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.)
+     */
+    v.iov_base = (char *) pkt.data_;
+    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.
+     */
+    m.msg_control = control_buf_;
+    m.msg_controllen = control_buf_len_;
+    cmsg = CMSG_FIRSTHDR(&m);
+    cmsg->cmsg_level = IPPROTO_IPV6;
+    cmsg->cmsg_type = IPV6_PKTINFO;
+    cmsg->cmsg_len = CMSG_LEN(sizeof(*pktinfo));
+    pktinfo = (struct in6_pktinfo *)CMSG_DATA(cmsg);
+    memset(pktinfo, 0, sizeof(*pktinfo));
+    pktinfo->ipi6_ifindex = pkt.ifindex_;
+    m.msg_controllen = cmsg->cmsg_len;
+
+    result = sendmsg(sendsock_, &m, 0);
+    if (result < 0) {
+        cout << "Send packet failed." << endl;
+    }
+    cout << "Sent " << result << " bytes." << endl;
 
-        /*
-         * Initialize our message header structure.
-         */
-        memset(&m, 0, sizeof(m));
+    cout << "Sent " << pkt.data_len_ << " bytes over "
+         << pkt.iface_ << "/" << pkt.ifindex_ << " interface: "
+         << " dst=" << pkt.remote_addr_ << ", src=" << pkt.local_addr_
+         << endl;
 
-        /*
-         * Point so we can get the from address.
-         */
-        m.msg_name = &from;
-        m.msg_namelen = sizeof(from);
+    return (result);
+}
 
-        /*
-         * 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 = pkt->data_;
-        v.iov_len = pkt->dataLen_;
-        m.msg_iov = &v;
-        m.msg_iovlen = 1;
+Pkt6*
+IfaceMgr::receive() {
+    struct msghdr m;
+    struct iovec v;
+    int result;
+    struct cmsghdr* cmsg;
+    struct in6_pktinfo* pktinfo;
+    struct sockaddr_in6 from;
+    struct in6_addr to_addr;
+    Pkt6* pkt;
+
+    try {
+        pkt = new Pkt6(1500);
+    } catch (std::exception& ex) {
+        cout << "Failed to create new packet." << endl;
+        return (0);
+    }
 
+    memset(control_buf_, 0, control_buf_len_);
+
+    memset(&from, 0, sizeof(from));
+    memset(&to_addr, 0, sizeof(to_addr));
+
+    /*
+     * Initialize our message header structure.
+     */
+    memset(&m, 0, sizeof(m));
+
+    /*
+     * 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.)
+     */
+    v.iov_base = pkt->data_;
+    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.
+     */
+    m.msg_control = control_buf_;
+    m.msg_controllen = control_buf_len_;
+
+    result = recvmsg(recvsock_, &m, 0);
+
+    if (result >= 0) {
         /*
-         * Getting the interface is a bit more involved.
+         * 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 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.
+         * We also keep a flag to see if we found it. If we
+         * didn't, then we consider this to be an error.
          */
-        m.msg_control = control_buf_;
-        m.msg_controllen = control_buf_len_;
-
-        result = recvmsg(recvsock_, &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.
-             */
-            found_pktinfo = 0;
-            cmsg = CMSG_FIRSTHDR(&m);
-            while (cmsg != NULL) {
-                if ((cmsg->cmsg_level == IPPROTO_IPV6) &&
-                    (cmsg->cmsg_type == IPV6_PKTINFO)) {
-                    pktinfo = (struct in6_pktinfo *)CMSG_DATA(cmsg);
-                    to_addr = pktinfo->ipi6_addr;
-                    pkt->ifindex = pktinfo->ipi6_ifindex;
-                    found_pktinfo = 1;
-                }
-                cmsg = CMSG_NXTHDR(&m, cmsg);
-            }
-            if (!found_pktinfo) {
-                cout << "Unable to find pktinfo" << endl;
-                delete pkt;
-                return 0;
+        int found_pktinfo = 0;
+        cmsg = CMSG_FIRSTHDR(&m);
+        while (cmsg != NULL) {
+            if ((cmsg->cmsg_level == IPPROTO_IPV6) &&
+                (cmsg->cmsg_type == IPV6_PKTINFO)) {
+                pktinfo = (struct in6_pktinfo*)CMSG_DATA(cmsg);
+                to_addr = pktinfo->ipi6_addr;
+                pkt->ifindex_ = pktinfo->ipi6_ifindex;
+                found_pktinfo = 1;
             }
-        } else {
-            cout << "Failed to receive data." << endl;
-
+            cmsg = CMSG_NXTHDR(&m, cmsg);
+        }
+        if (!found_pktinfo) {
+            cout << "Unable to find pktinfo" << endl;
             delete pkt;
-            return 0;
+            return (0);
         }
+    } else {
+        cout << "Failed to receive data." << endl;
+        delete pkt;
+        return (0);
+    }
 
-        pkt->localAddr = Addr6(&to_addr);
-        pkt->remoteAddr = Addr6(&from);
-        pkt->remotePort = ntohs(from.sin6_port);
-
-        Iface * received = getIface(pkt->ifindex);
-        if (received) {
-            pkt->iface = received->name_;
-        }
+    pkt->local_addr_ = Addr6(&to_addr);
+    pkt->remote_addr_ = Addr6(&from);
+    pkt->remote_port_ = ntohs(from.sin6_port);
+
+    Iface* received = getIface(pkt->ifindex_);
+    if (received) {
+        pkt->iface_ = received->name_;
+    } else {
+        cout << "Received packet over unknown interface (ifindex="
+             << pkt->ifindex_ << endl;
+        pkt->iface_ = "[unknown]";
+    }
 
-        pkt->dataLen_ = result;
+    pkt->data_len_ = result;
 
-        cout << "Received " << pkt->dataLen_ << " bytes over "
-             << pkt->iface << "/" << pkt->ifindex << " interface: "
-             << " src=" << pkt->remoteAddr << ", dst=" << pkt->localAddr
-             << endl;
+    // TODO Move this to LOG_DEBUG
+    cout << "Received " << pkt->data_len_ << " bytes over "
+         << pkt->iface_ << "/" << pkt->ifindex_ << " interface: "
+         << " src=" << pkt->remote_addr_ << ", dst=" << pkt->local_addr_
+         << endl;
 
-        return pkt;
+    return (pkt);
+}
 
-    }
+}
diff --git a/src/bin/dhcp6/iface_mgr.h b/src/bin/dhcp6/iface_mgr.h
index 48009f0..c72a712 100644
--- a/src/bin/dhcp6/iface_mgr.h
+++ b/src/bin/dhcp6/iface_mgr.h
@@ -34,34 +34,37 @@ namespace isc {
             int ifindex_;
             Addr6Lst addrs_;
             char mac_[20]; //
-            int macLen_;
+            int mac_len_;
 
             Iface();
-            Iface(const std::string name, int ifindex);
+            Iface(const std::string& name, int ifindex);
             std::string getFullName() const;
             std::string getPlainMac() const;
 
             // next field is not needed, let's keep it in cointainers
         };
+
+	// TODO performance improvement: we may change this into
+	//      2 maps (ifindex-indexed and name-indexed)
         typedef std::list<Iface> IfaceLst;
 
         static IfaceMgr& instance();
         static void instanceCreate();
 
         Iface * getIface(int ifindex);
-        Iface * getIface(const std::string &ifname);
+        Iface * getIface(const std::string& ifname);
 
         bool openSockets();
         void printIfaces();
 
-        int openSocket(const std::string &ifname,
-                       const Addr6 &addr,
+        int openSocket(const std::string& ifname,
+                       const Addr6& addr,
                        int port, bool multicast);
-        bool joinMcast(int sock, const std::string &ifname,
+        bool joinMcast(int sock, const std::string& ifname,
                        const std::string& mcast);
 
-        bool send(Pkt6 &pkt);
-        Pkt6 * receive();
+        bool send(Pkt6& pkt);
+        Pkt6* receive();
 
 	// don't use private, we need derived classes in tests
     protected:
diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc
index 5dbfc5d..45ff376 100644
--- a/src/bin/dhcp6/main.cc
+++ b/src/bin/dhcp6/main.cc
@@ -88,13 +88,11 @@ main(int argc, char* argv[]) {
 
     int ret = 0;
 
-    // XXX: we should eventually pass io_service here.
+    // TODO remainder of auth to dhcp6 code copy. We need to enable this in
+    //      dhcp6 eventually
 #if 0
     Session* cc_session = NULL;
-    Session* xfrin_session = NULL;
     Session* statistics_session = NULL;
-    bool xfrin_session_established = false; // XXX (see Trac #287)
-    bool statistics_session_established = false; // XXX (see Trac #287)
     ModuleCCSession* config_session = NULL;
 #endif
     try {
@@ -110,9 +108,8 @@ main(int argc, char* argv[]) {
         // auth_server->setVerbose(verbose_mode);
         cout << "[b10-dhcp6] Initiating DHCPv6 operation." << endl;
 
-        Dhcpv6Srv *srv = new Dhcpv6Srv();
+        Dhcpv6Srv* srv = new Dhcpv6Srv();
 
-        //srv->init();
         srv->run();
 
     } catch (const std::exception& ex) {
diff --git a/src/bin/dhcp6/pkt6.cc b/src/bin/dhcp6/pkt6.cc
index 63b5eae..33025c3 100644
--- a/src/bin/dhcp6/pkt6.cc
+++ b/src/bin/dhcp6/pkt6.cc
@@ -15,32 +15,57 @@
 
 #include "dhcp6/dhcp6.h"
 #include "dhcp6/pkt6.h"
+#include <iostream>
 
 namespace isc {
 
-    /**
-     * constructor.
-     *
-     * Note: Pkt6 will take ownership of any data passed
-     *
-     * @param data
-     * @param dataLen
-     */
-    Pkt6::Pkt6(char * data, int dataLen) {
-	data_ = data;
-	dataLen_ = dataLen;
-    }
+///
+/// constructor
+///
+/// This constructor is used during packet reception.
+///
+/// Note: Pkt6 will take ownership of any data passed
+/// (due to performance reasons). Copying data on creation
+/// would be more elegant, but slower.
+///
+/// \param data
+/// \param dataLen
+///
+Pkt6::Pkt6(char * data, int dataLen) {
+    data_ = data;
+    data_len_ = dataLen;
+}
 
-    Pkt6::Pkt6(int dataLen) {
+///
+/// constructor
+///
+/// This constructor is used for generated packets.
+///
+/// Note: Pkt6 will take ownership of any data passed
+/// (due to performance reasons). Copying data on creation
+/// would be more elegant, but slower.
+///
+/// \param dataLen - length of the data to be allocated
+///
+Pkt6::Pkt6(int dataLen) {
+    try {
 	data_ = new char[dataLen];
-	dataLen_ = dataLen;
+	data_len_ = dataLen;
+    } catch (const std::exception& ex) {
+	// TODO move to LOG_FATAL()
+	// let's continue with empty pkt for now
+        std::cout << "Failed to allocate " << dataLen << " bytes."
+                  << std::endl;
+        data_ = 0;
+        data_len_ = 0;
     }
+}
 
-    Pkt6::~Pkt6() {
-	if (data_) {
-	    delete [] data_;
-	}
-
+Pkt6::~Pkt6() {
+    if (data_) {
+        delete [] data_;
+        data_ = 0;
     }
+}
 
 };
diff --git a/src/bin/dhcp6/pkt6.h b/src/bin/dhcp6/pkt6.h
index 0cf4782..5508fda 100644
--- a/src/bin/dhcp6/pkt6.h
+++ b/src/bin/dhcp6/pkt6.h
@@ -29,16 +29,16 @@ namespace isc {
         // XXX: probably need getter/setter wrappers
         //      and hide fields as protected
         char * data_;
-        int dataLen_;
+        int data_len_;
 
-        Addr6 localAddr;
-        Addr6 remoteAddr;
+        Addr6 local_addr_;
+        Addr6 remote_addr_;
 
-        std::string iface;
-        int ifindex;
+        std::string iface_;
+        int ifindex_;
 
-        int localPort;
-        int remotePort;
+        int local_port_;
+        int remote_port_;
 
         // XXX: add *a lot* here
 
diff --git a/src/bin/dhcp6/tests/addr6_unittest.cc b/src/bin/dhcp6/tests/addr6_unittest.cc
index 1f150d9..dbbca08 100644
--- a/src/bin/dhcp6/tests/addr6_unittest.cc
+++ b/src/bin/dhcp6/tests/addr6_unittest.cc
@@ -25,6 +25,7 @@
 using namespace std;
 using namespace isc;
 
+namespace {
 // empty class for now, but may be extended once Addr6 becomes bigger
 class Addr6Test : public ::testing::Test {
 public:
@@ -86,3 +87,5 @@ TEST_F(Addr6Test, stream) {
 
     EXPECT_STREQ( tmp.str().c_str(), plain.c_str() );
 }
+
+}
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 7869b74..96c767e 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -25,6 +25,7 @@
 using namespace std;
 using namespace isc;
 
+namespace {
 class Dhcpv6SrvTest : public ::testing::Test {
 public:
     Dhcpv6SrvTest() {
@@ -36,11 +37,17 @@ TEST_F(Dhcpv6SrvTest, basic) {
     // 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();
 
 	delete srv;
-    });
+	});
     
 }
 
+}
diff --git a/src/bin/dhcp6/tests/iface_mgr_unittest.cc b/src/bin/dhcp6/tests/iface_mgr_unittest.cc
index b2fd81b..7f2b583 100644
--- a/src/bin/dhcp6/tests/iface_mgr_unittest.cc
+++ b/src/bin/dhcp6/tests/iface_mgr_unittest.cc
@@ -27,6 +27,7 @@
 using namespace std;
 using namespace isc;
 
+namespace {
 class NakedIfaceMgr: public IfaceMgr {
     // "naked" Interface Manager, exposes internal fields 
 public:
@@ -169,10 +170,10 @@ TEST_F(IfaceMgrTest, sendReceive) {
 	sendPkt.data_[i] = i;
     }
 
-    sendPkt.remotePort = 10547;
-    sendPkt.remoteAddr = Addr6("::1", true);
-    sendPkt.ifindex = 1;
-    sendPkt.iface = "lo";
+    sendPkt.remote_port_ = 10547;
+    sendPkt.remote_addr_ = Addr6("::1", true);
+    sendPkt.ifindex_ = 1;
+    sendPkt.iface_ = "lo";
     
     Pkt6 * rcvPkt;
 
@@ -183,14 +184,15 @@ TEST_F(IfaceMgrTest, sendReceive) {
     ASSERT_TRUE( rcvPkt != NULL ); // received our own packet
 
     // let's check that we received what was sent
-    EXPECT_EQ(sendPkt.dataLen_, rcvPkt->dataLen_);
-    EXPECT_EQ(0, memcmp(sendPkt.data_, rcvPkt->data_, rcvPkt->dataLen_) );
+    EXPECT_EQ(sendPkt.data_len_, rcvPkt->data_len_);
+    EXPECT_EQ(0, memcmp(sendPkt.data_, rcvPkt->data_, rcvPkt->data_len_) );
 
-    EXPECT_EQ(sendPkt.remoteAddr, rcvPkt->remoteAddr);
-    EXPECT_EQ(rcvPkt->remotePort, 10546);
+    EXPECT_EQ(sendPkt.remote_addr_, rcvPkt->remote_addr_);
+    EXPECT_EQ(rcvPkt->remote_port_, 10546);
 
     delete rcvPkt;
 
     delete ifacemgr;
 }
 
+}
diff --git a/src/bin/dhcp6/tests/pkt6_unittest.cc b/src/bin/dhcp6/tests/pkt6_unittest.cc
index ad98c6c..0fa393b 100644
--- a/src/bin/dhcp6/tests/pkt6_unittest.cc
+++ b/src/bin/dhcp6/tests/pkt6_unittest.cc
@@ -25,6 +25,7 @@
 using namespace std;
 using namespace isc;
 
+namespace {
 // empty class for now, but may be extended once Addr6 becomes bigger
 class Pkt6Test : public ::testing::Test {
 public:
@@ -35,17 +36,18 @@ public:
 TEST_F(Pkt6Test, constructor) {
     Pkt6 * pkt1 = new Pkt6(17);
     
-    ASSERT_EQ(pkt1->dataLen_, 17);
+    ASSERT_EQ(pkt1->data_len_, 17);
 
     char * buf = new char[23];
     // can't use char buf[23], as Pkt6 takes ownership of the data
 
     Pkt6 * pkt2 = new Pkt6(buf, 23);
 
-    ASSERT_EQ(pkt2->dataLen_, 23);
+    ASSERT_EQ(pkt2->data_len_, 23);
     ASSERT_EQ(pkt2->data_, buf);
 
     delete pkt1;
     delete pkt2;
 }
 
+}




More information about the bind10-changes mailing list