BIND 10 trac1238, updated. 075e3787986676c7491f157931b6f7da1773db0a [1238] Socket management refactored.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Nov 9 18:58:25 UTC 2011


The branch, trac1238 has been updated
       via  075e3787986676c7491f157931b6f7da1773db0a (commit)
      from  7d2f07481169780071bf564223a20a219b550385 (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 075e3787986676c7491f157931b6f7da1773db0a
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Nov 9 19:57:51 2011 +0100

    [1238] Socket management refactored.
    
    - Now IfaceMgr supports both IPv4 and IPv6 sockets.
    - Socket information management implemented.
    - getIface(), setIface() in Pkt4 implemented.

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

Summary of changes:
 src/bin/dhcp6/iface_mgr.cc                |  155 ++++++++++++++++++++++++-----
 src/bin/dhcp6/iface_mgr.h                 |  122 ++++++++++++++++++++---
 src/bin/dhcp6/tests/iface_mgr_unittest.cc |   32 ++++---
 src/lib/dhcp/pkt4.h                       |   13 +++-
 src/lib/dhcp/tests/pkt4_unittest.cc       |   13 +++
 5 files changed, 281 insertions(+), 54 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/iface_mgr.cc b/src/bin/dhcp6/iface_mgr.cc
index df9be63..b7a2661 100644
--- a/src/bin/dhcp6/iface_mgr.cc
+++ b/src/bin/dhcp6/iface_mgr.cc
@@ -18,9 +18,9 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
-#include "dhcp/dhcp6.h"
-#include "dhcp6/iface_mgr.h"
-#include "exceptions/exceptions.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp6/iface_mgr.h>
+#include <exceptions/exceptions.h>
 
 using namespace std;
 using namespace isc;
@@ -79,6 +79,19 @@ IfaceMgr::Iface::getPlainMac() const {
     return (tmp.str());
 }
 
+bool IfaceMgr::Iface::delSocket(uint16_t sockfd) {
+    list<SocketInfo>::iterator sock = sockets_.begin();
+    while (sock!=sockets_.end()) {
+        if (sock->sockfd_ == sockfd) {
+            close(sockfd);
+            sockets_.erase(sock);
+            return (true); //socket found
+        }
+        ++sock;
+    }
+    return (false); // socket not found
+}
+
 IfaceMgr::IfaceMgr()
     :control_buf_len_(CMSG_SPACE(sizeof(struct in6_pktinfo))),
      control_buf_(new char[control_buf_len_])
@@ -153,7 +166,7 @@ IfaceMgr::detectIfaces() {
 
 void
 IfaceMgr::openSockets() {
-    int sock;
+    int sock1, sock2;
 
     for (IfaceCollection::iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
@@ -165,24 +178,28 @@ IfaceMgr::openSockets() {
              addr != addrs.end();
              ++addr) {
 
-            sock = openSocket(iface->getName(), *addr,
-                              DHCP6_SERVER_PORT);
-            if (sock<0) {
+            sock1 = openSocket(iface->getName(), *addr, DHCP6_SERVER_PORT);
+            if (sock1<0) {
                 isc_throw(Unexpected, "Failed to open unicast socket on "
                           << " interface " << iface->getFullName());
             }
-            sendsock_ = sock;
 
-            sock = openSocket(iface->getName(),
-                              IOAddress(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
-                              DHCP6_SERVER_PORT);
-            if (sock<0) {
+            if ( !joinMcast(sock1, iface->getName(),
+                             string(ALL_DHCP_RELAY_AGENTS_AND_SERVERS) ) ) {
+                close(sock1);
+                isc_throw(Unexpected, "Failed to join " << ALL_DHCP_RELAY_AGENTS_AND_SERVERS
+                          << " multicast group.");
+            }
+
+            // 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) {
                 isc_throw(Unexpected, "Failed to open multicast socket on "
                           << " interface " << iface->getFullName());
-                close(sendsock_);
-                sendsock_ = 0;
+                iface->delSocket(sock1); // delete previously opened socket
             }
-            recvsock_ = sock;
         }
     }
 }
@@ -308,7 +325,9 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     cout << "Created socket " << sock << " on " << iface.getName() << "/" <<
         addr.toText() << "/port=" << port << endl;
 
-    // TODO: Add socket to iface interface
+    SocketInfo info(sock, addr, port);
+    iface.addSocket(info);
+
     return (sock);
 }
 
@@ -370,7 +389,6 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
 #endif
 
     // multicast stuff
-
     if (addr.getAddress().to_v6().is_multicast()) {
         // both mcast (ALL_DHCP_RELAY_AGENTS_AND_SERVERS and ALL_DHCP_SERVERS)
         // are link and site-scoped, so there is no sense to join those groups
@@ -387,7 +405,8 @@ IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
     cout << "Created socket " << sock << " on " << iface.getName() << "/" <<
         addr.toText() << "/port=" << port << endl;
 
-    // TODO: Add socket to iface interface
+    SocketInfo info(sock, addr, port);
+    iface.addSocket(info);
 
     return (sock);
 }
@@ -424,6 +443,13 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
     int result;
     struct in6_pktinfo *pktinfo;
     struct cmsghdr *cmsg;
+
+    Iface* iface = getIface(pkt->iface_);
+    if (!iface) {
+        isc_throw(BadValue, "Unable to send Pkt6. Invalid interface ("
+                  << pkt->iface_ << ") specified.");
+    }
+
     memset(&control_buf_[0], 0, control_buf_len_);
 
     /*
@@ -475,14 +501,12 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
     pktinfo->ipi6_ifindex = pkt->ifindex_;
     m.msg_controllen = cmsg->cmsg_len;
 
-    result = sendmsg(sendsock_, &m, 0);
+    result = sendmsg(getSocket(*pkt), &m, 0);
     if (result < 0) {
         cout << "Send packet failed." << endl;
     }
-    cout << "Sent " << result << " bytes." << endl;
-
-    cout << "Sent " << pkt->data_len_ << " bytes over "
-         << pkt->iface_ << "/" << pkt->ifindex_ << " interface: "
+    cout << "Sent " << pkt->data_len_ << " bytes over socket " << getSocket(*pkt)
+         << " on " << iface->getFullName() << " interface: "
          << " dst=" << pkt->remote_addr_.toText()
          << ", src=" << pkt->local_addr_.toText()
          << endl;
@@ -559,7 +583,34 @@ IfaceMgr::receive6() {
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
 
-    result = recvmsg(recvsock_, &m, 0);
+    /// TODO: Need to move to select() and pool over
+    /// all available sockets. For now, we just take the
+    /// first interface and use first socket from it.
+    IfaceCollection::const_iterator iface = ifaces_.begin();
+    if (iface == ifaces_.end()) {
+        isc_throw(Unexpected, "No interfaces detected. Can't receive anything.");
+    }
+    SocketCollection::const_iterator s = iface->sockets_.begin();
+    const SocketInfo* candidate = 0;
+    while (s != iface->sockets_.end()) {
+        if (s->addr_.getAddress().to_v6().is_multicast()) {
+            candidate = &(*s);
+            break;
+        }
+        if (!candidate) {
+            candidate = &(*s); // it's not multicast, but it's better than none
+        }
+        ++s;
+    }
+    if (!candidate) {
+        isc_throw(Unexpected, "Interface " << iface->getFullName()
+                  << " does not have any sockets open.");
+    }
+
+    cout << "Trying to receive over socket " << candidate->sockfd_ << " bound to "
+         << candidate->addr_.toText() << "/port=" << candidate->port_ << " on "
+         << iface->getFullName() << endl;
+    result = recvmsg(candidate->sockfd_, &m, 0);
 
     if (result >= 0) {
         /*
@@ -624,4 +675,60 @@ IfaceMgr::receive6() {
     return (pkt);
 }
 
+uint16_t
+IfaceMgr::getSocket(isc::dhcp::Pkt6 const& pkt) {
+    Iface* iface = getIface(pkt.iface_);
+    if (!iface) {
+        isc_throw(BadValue, "Tried to find socket for non-existent interface "
+                  << pkt.iface_);
+    }
+
+    SocketCollection::const_iterator s;
+    for (s = iface->sockets_.begin(); s != iface->sockets_.end(); ++s) {
+        if (s->family_ != AF_INET6) {
+            // don't use IPv4 sockets
+            continue;
+        }
+        if (s->addr_.getAddress().to_v6().is_multicast()) {
+            // don't use IPv6 sockets bound to multicast address
+            continue;
+        }
+        /// TODO: Add more checks here later. If remote address is
+        /// not link-local, we can't use link local bound socket
+        /// to send data.
+
+        return (s->sockfd_);
+    }
+
+    isc_throw(Unexpected, "Interface " << iface->getFullName()
+              << " does not have any suitable IPv6 sockets open.");
+}
+
+uint16_t
+IfaceMgr::getSocket(isc::dhcp::Pkt4 const& pkt) {
+    Iface* iface = getIface(pkt.getIface());
+    if (!iface) {
+        isc_throw(BadValue, "Tried to find socket for non-existent interface "
+                  << pkt.getIface());
+    }
+
+    SocketCollection::const_iterator s;
+    for (s = iface->sockets_.begin(); s != iface->sockets_.end(); ++s) {
+        if (s->family_ != AF_INET) {
+            // don't use IPv4 sockets
+            continue;
+        }
+        /// TODO: Add more checks here later. If remote address is
+        /// not link-local, we can't use link local bound socket
+        /// to send data.
+
+        return (s->sockfd_);
+    }
+
+    isc_throw(Unexpected, "Interface " << iface->getFullName()
+              << " does not have any suitable IPv4 sockets open.");
+}
+
+
+
 }
diff --git a/src/bin/dhcp6/iface_mgr.h b/src/bin/dhcp6/iface_mgr.h
index 2c306ef..6c8340e 100644
--- a/src/bin/dhcp6/iface_mgr.h
+++ b/src/bin/dhcp6/iface_mgr.h
@@ -40,35 +40,106 @@ public:
     /// maximum MAC address length (Infiniband uses 20 bytes)
     static const unsigned int MAX_MAC_LEN = 20;
 
+    /// Holds information about socket.
+    struct SocketInfo {
+        uint16_t sockfd_; /// socket descriptor
+        isc::asiolink::IOAddress addr_; /// bound address
+        uint16_t port_;   /// socket port
+        uint16_t family_; /// IPv4 or IPv6
+        SocketInfo(uint16_t sockfd, const isc::asiolink::IOAddress& addr,
+                   uint16_t port)
+        :sockfd_(sockfd), addr_(addr), port_(port), family_(addr.getFamily()) { }
+    };
+
+    /// type that holds a list of socket informations
+    typedef std::list<SocketInfo> SocketCollection;
+
     /// @brief represents a single network interface
     ///
     /// Iface structure represents network interface with all useful
     /// information, like name, interface index, MAC address and
     /// list of assigned addresses
     struct Iface {
-        /// constructor
+        /// @brief Iface constructor.
+        ///
+        /// Creates Iface object that represents network interface.
+        ///
+        /// @param name name of the interface
+        /// @param ifindex interface index (unique integer identifier)
         Iface(const std::string& name, int ifindex);
 
-        /// returns full interface name in format ifname/ifindex
+        /// @brief Returns full interface name as "ifname/ifindex" string.
+        ///
+        /// @return string with interface name
         std::string getFullName() const;
 
-        /// returns link-layer address a plain text
+        /// @brief Returns link-layer address a plain text.
+        ///
+        /// @return MAC address as a plain text (string)
         std::string getPlainMac() const;
 
+        /// @brief Returns interface index.
+        ///
+        /// @return interface index
         uint16_t getIndex() const { return ifindex_; }
 
-        std::string getName() const {
-            return name_;
-        };
-
+        /// @brief Returns interface name.
+        ///
+        /// @return interface name
+        std::string getName() const { return name_; };
+
+        /// @brief Returns all interfaces available on an interface.
+        ///
+        /// Care should be taken to not use this collection after Iface object
+        /// ceases to exist. That is easy in most cases as Iface objects are
+        /// created by IfaceMgr that is a singleton an is expected to be
+        /// available at all time. We may revisit this if we ever decide to
+        /// implement dynamic interface detection, but such fancy feature would
+        /// mostly be useful for clients with wifi/vpn/virtual interfaces.
+        ///
+        /// @return collection of addresses
         const AddressCollection& getAddresses() const { return addrs_; }
 
+        /// @brief Adds an address to an interface.
+        ///
+        /// This only adds an address to collection, it does not physically
+        /// configure address on actual network interface.
+        ///
+        /// @param addr address to be added
         void addAddress(const isc::asiolink::IOAddress& addr) {
             addrs_.push_back(addr);
         }
 
+        /// @brief Deletes an address from an interface.
+        ///
+        /// This only deletes address from collection, it does not physically
+        /// remove address configuration from actual network interface.
+        ///
+        /// @param addr address to be removed.
+        ///
+        /// @return true if removal was successful (address was in collection),
+        ///         false otherwise
         bool delAddress(const isc::asiolink::IOAddress& addr);
 
+        /// @brief Adds socket descriptor to an interface.
+        ///
+        /// @param socket SocketInfo structure that describes socket.
+        void addSocket(const SocketInfo& sock)
+            { sockets_.push_back(sock); }
+
+        /// @brief Closes socket.
+        ///
+        /// Closes socket and removes corresponding SocketInfo structure
+        /// from an interface.
+        ///
+        /// @param socket descriptor to be closed/removed.
+        /// @return true if there was such socket, false otherwise
+        bool delSocket(uint16_t sockfd);
+
+        /// socket used to sending data
+        /// TODO: this should be protected
+        SocketCollection sockets_;
+
     protected:
         /// network interface name
         std::string name_;
@@ -84,12 +155,6 @@ public:
 
         /// length of link-layer address (usually 6)
         int mac_len_;
-
-        /// socket used to sending data
-        uint16_t sendsock_;
-
-        /// socket used for receiving data
-        uint16_t recvsock_;
     };
 
     // TODO performance improvement: we may change this into
@@ -125,6 +190,32 @@ public:
     Iface*
     getIface(const std::string& ifname);
 
+    /// @brief Return most suitable socket for transmitting specified IPv6 packet.
+    ///
+    /// This method takes Pkt6 (see overloaded implementation that takes
+    /// Pkt4) and chooses appropriate socket to send it. This method
+    /// may throw BadValue if specified packet does not have outbound
+    /// interface specified, no such interface exists, or specified
+    /// interface does not have any appropriate sockets open.
+    ///
+    /// @param pkt a packet to be transmitted
+    ///
+    /// @return a socket descriptor
+    uint16_t getSocket(const isc::dhcp::Pkt6& pkt);
+
+    /// @brief Return most suitable socket for transmitting specified IPv6 packet.
+    ///
+    /// This method takes Pkt4 (see overloaded implementation that takes
+    /// Pkt6) and chooses appropriate socket to send it. This method
+    /// may throw BadValue if specified packet does not have outbound
+    /// interface specified, no such interface exists, or specified
+    /// interface does not have any appropriate sockets open.
+    ///
+    /// @param pkt a packet to be transmitted
+    ///
+    /// @return a socket descriptor
+    uint16_t getSocket(const isc::dhcp::Pkt4& pkt);
+
     /// debugging method that prints out all available interfaces
     ///
     /// @param out specifies stream to print list of interfaces to
@@ -218,8 +309,9 @@ protected:
     // TODO: Also keep this interface on Iface once interface detection
     // is implemented. We may need it e.g. to close all sockets on
     // specific interface
-    int recvsock_; // TODO: should be fd_set eventually, but we have only
-    int sendsock_; // 2 sockets for now. Will do for until next release
+    //int recvsock_; // TODO: should be fd_set eventually, but we have only
+    //int sendsock_; // 2 sockets for now. Will do for until next release
+
     // we can't use the same socket, as receiving socket
     // is bound to multicast address. And we all know what happens
     // to people who try to use multicast as source address.
diff --git a/src/bin/dhcp6/tests/iface_mgr_unittest.cc b/src/bin/dhcp6/tests/iface_mgr_unittest.cc
index f276509..4edac6e 100644
--- a/src/bin/dhcp6/tests/iface_mgr_unittest.cc
+++ b/src/bin/dhcp6/tests/iface_mgr_unittest.cc
@@ -41,8 +41,6 @@ class NakedIfaceMgr: public IfaceMgr {
 public:
     NakedIfaceMgr() { }
     IfaceCollection & getIfacesLst() { return ifaces_; }
-    void setSendSock(int sock) { sendsock_ = sock; }
-    void setRecvSock(int sock) { recvsock_ = sock; }
 };
 
 // dummy class for now, but this will be expanded when needed
@@ -253,10 +251,15 @@ TEST_F(IfaceMgrTest, sockets6) {
 
     IOAddress loAddr("::1");
 
+    Pkt6 pkt6(128);
+    pkt6.iface_ = LOOPBACK;
+
     // bind multicast socket to port 10547
     int socket1 = ifacemgr->openSocket(LOOPBACK, loAddr, 10547);
     EXPECT_GT(socket1, 0); // socket > 0
 
+    EXPECT_EQ(socket1, ifacemgr->getSocket(pkt6));
+
     // bind unicast socket to port 10548
     int socket2 = ifacemgr->openSocket(LOOPBACK, loAddr, 10548);
     EXPECT_GT(socket2, 0);
@@ -311,17 +314,11 @@ TEST_F(IfaceMgrTest, DISABLED_sockets6Mcast) {
     delete ifacemgr;
 }
 
-// TODO: disabled due to other naming on various systems
-// (lo in Linux, lo0 in BSD systems)
-// Fix for this is available on 1186 branch, will reenable
-// this test once 1186 is merged
 TEST_F(IfaceMgrTest, sendReceive6) {
+
     // testing socket operation in a portable way is tricky
     // without interface detection implemented
-
-    fstream fakeifaces(INTERFACE_FILE, ios::out|ios::trunc);
-    fakeifaces << LOOPBACK << " ::1";
-    fakeifaces.close();
+    createLoInterfacesTxt();
 
     NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
 
@@ -333,9 +330,6 @@ TEST_F(IfaceMgrTest, sendReceive6) {
         socket2 = ifacemgr->openSocket(LOOPBACK, loAddr, 10546);
     );
 
-    ifacemgr->setSendSock(socket2);
-    ifacemgr->setRecvSock(socket1);
-
     boost::shared_ptr<Pkt6> sendPkt(new Pkt6(128) );
 
     // prepare dummy payload
@@ -362,7 +356,12 @@ TEST_F(IfaceMgrTest, sendReceive6) {
                         rcvPkt->data_len_) );
 
     EXPECT_EQ(sendPkt->remote_addr_.toText(), rcvPkt->remote_addr_.toText());
-    EXPECT_EQ(rcvPkt->remote_port_, 10546);
+
+    // since we opened 2 sockets on the same interface and none of them is multicast,
+    // none is preferred over the other for sending data, so we really should not
+    // assume the one or the other will always be choosen for sending data. Therefore
+    // we should accept both values as source ports.
+    EXPECT_TRUE( (rcvPkt->remote_port_ == 10546) || (rcvPkt->remote_port_ == 10547) );
 
     delete ifacemgr;
 }
@@ -383,6 +382,11 @@ TEST_F(IfaceMgrTest, socket4) {
 
     EXPECT_GT(socket1, 0);
 
+    Pkt4 pkt(DHCPDISCOVER, 1234);
+    pkt.setIface(LOOPBACK);
+
+    EXPECT_EQ(socket1, ifacemgr->getSocket(pkt));
+
     close(socket1);
 
     delete ifacemgr;
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index 8517091..db26fa5 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -299,10 +299,21 @@ public:
     ///
     /// @return returns option of requested type (or NULL)
     ///         if no such option is present
-
     boost::shared_ptr<Option>
     getOption(uint8_t opt_type);
 
+
+    /// @brief set interface over which packet should be sent
+    ///
+    /// @param interface defines outbound interface
+    void setIface(const std::string& interface){ iface_ = interface; }
+
+    /// @brief gets interface over which packet was received or
+    ///        will be transmitted
+    ///
+    /// @return name of the interface
+    std::string getIface() const { return iface_; }
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index c89743f..0ec872f 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -559,4 +559,17 @@ TEST(Pkt4Test, unpackOptions) {
     EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+22, 3)); // data len=3
 }
 
+// This test verifies methods that are used for manipulating meta fields
+// i.e. fields that are not part of DHCPv4 (e.g. interface name).
+TEST(Pkt4Ttest, metaFields) {
+    Pkt4 pkt(DHCPDISCOVER, 1234);
+
+    pkt.setIface("lo0");
+
+    EXPECT_EQ("lo0", pkt.getIface());
+
+    /// TODO: Expand this test once additonal getters/setters are
+    /// implemented.
+}
+
 } // end of anonymous namespace




More information about the bind10-changes mailing list