[1238] Socket management refactored.

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

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.");
+}
+
+
+
 }

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.

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;

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

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