BIND 10 master, updated. fe58fab6bdad19c5ae7191b9ee1e3b5629f1045b [1528] ChangeLog commit-id update.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed May 9 12:43:16 UTC 2012


The branch, master has been updated
       via  fe58fab6bdad19c5ae7191b9ee1e3b5629f1045b (commit)
       via  221f5649496821d19a40863e53e72685524b9ab2 (commit)
       via  67a08ea405d64a5767215d57b936b0a6b0f7b802 (commit)
       via  3b48a25b8e175427773d9463593b02361333f282 (commit)
       via  7eaf3a710ceb20f4822a636054aef7d8a6f7e6dd (commit)
       via  1202068e5f61ed72afde9b204fd5cd011ecd190f (commit)
       via  d8db35358ab97a466ea61e9624edf8d736364211 (commit)
       via  ce7abc70e21119826398943593b327f12b4bc751 (commit)
       via  dbedca057de99f85a373b918cae849174128e17b (commit)
       via  cc72ac90f8858d6e24da1816d5e70d38b0db8e9c (commit)
       via  d53ab0d663c6ead51bdde2d2b16365ff6adab424 (commit)
       via  6207354d2f7830511008d49e36aac658ec378348 (commit)
       via  44b275d27c8089167cff36195d1102f54c99047c (commit)
       via  7f9ddb4963264b1ac022e7d550930dfbc825d71c (commit)
      from  028d5ab64c816945dc310a6d8b4d159e22ba9184 (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 fe58fab6bdad19c5ae7191b9ee1e3b5629f1045b
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed May 9 14:23:06 2012 +0200

    [1528] ChangeLog commit-id update.

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

Summary of changes:
 ChangeLog                                |    5 +
 src/bin/dhcp6/dhcp6_srv.cc               |    8 +-
 src/lib/dhcp/iface_mgr.cc                |   23 +-
 src/lib/dhcp/iface_mgr.h                 |   40 ++-
 src/lib/dhcp/iface_mgr_linux.cc          |  540 ++++++++++++++++++------------
 src/lib/dhcp/tests/iface_mgr_unittest.cc |   74 ++++-
 6 files changed, 463 insertions(+), 227 deletions(-)

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 4c3586f..1821340 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+434.	[func]		tomek
+	libdhcp++: Linux interface detection refactored. The code is
+	now cleaner. Tests better support certain versions of ifconfig.
+	(Trac #1528, git 221f5649496821d19a40863e53e72685524b9ab2)
+
 433.	[func]		tomek
 	libdhcp++: Option6 and Pkt6 now follow the same design as
 	options and packet for DHCPv4. General code refactoring after
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index f4a535e..cc4219e 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -169,7 +169,7 @@ void Dhcpv6Srv::setServerID() {
         // WiFi, Infiniband, etc.) have 6 bytes long MAC address. We want to
         // base our DUID on real hardware address, rather than virtual
         // interface that pretends that underlying IP address is its MAC.
-        if (iface->mac_len_ < MIN_MAC_LEN) {
+        if (iface->getMacLen() < MIN_MAC_LEN) {
             continue;
         }
 
@@ -186,7 +186,7 @@ void Dhcpv6Srv::setServerID() {
         // some interfaces (like lo on Linux) report 6-bytes long
         // MAC adress 00:00:00:00:00:00. Let's not use such weird interfaces
         // to generate DUID.
-        if (isRangeZero(iface->mac_, iface->mac_ + iface->mac_len_)) {
+        if (isRangeZero(iface->getMac(), iface->getMac() + iface->getMacLen())) {
             continue;
         }
 
@@ -198,11 +198,11 @@ void Dhcpv6Srv::setServerID() {
         time_t seconds = time(NULL);
         seconds -= DUID_TIME_EPOCH;
 
-        OptionBuffer srvid(8 + iface->mac_len_);
+        OptionBuffer srvid(8 + iface->getMacLen());
         writeUint16(DUID_LLT, &srvid[0]);
         writeUint16(HWTYPE_ETHERNET, &srvid[2]);
         writeUint32(static_cast<uint32_t>(seconds), &srvid[4]);
-        memcpy(&srvid[0]+8, iface->mac_, iface->mac_len_);
+        memcpy(&srvid[0]+8, iface->getMac(), iface->getMacLen());
 
         serverid_ = OptionPtr(new Option(Option::V6, D6O_SERVERID,
                                          srvid.begin(), srvid.end()));
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index c394da5..990e987 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -55,9 +55,9 @@ IfaceMgr::instance() {
 }
 
 IfaceMgr::Iface::Iface(const std::string& name, int ifindex)
-    :name_(name), ifindex_(ifindex), mac_len_(0), flag_loopback_(false),
-     flag_up_(false), flag_running_(false), flag_multicast_(false),
-     flag_broadcast_(false), flags_(0), hardware_type_(0)
+    :name_(name), ifindex_(ifindex), mac_len_(0), hardware_type_(0),
+     flag_loopback_(false), flag_up_(false), flag_running_(false),
+     flag_multicast_(false), flag_broadcast_(false), flags_(0)
 {
     memset(mac_, 0, sizeof(mac_));
 }
@@ -84,6 +84,17 @@ IfaceMgr::Iface::getPlainMac() const {
     return (tmp.str());
 }
 
+void IfaceMgr::Iface::setMac(const uint8_t* mac, size_t len) {
+    if (len > IfaceMgr::MAX_MAC_LEN) {
+        isc_throw(OutOfRange, "Interface " << getFullName()
+                  << " was detected to have link address of length "
+                  << len << ", but maximum supported length is "
+                  << IfaceMgr::MAX_MAC_LEN);
+    }
+    mac_len_ = len;
+    memcpy(mac_, mac, len);
+}
+
 bool IfaceMgr::Iface::delAddress(const isc::asiolink::IOAddress& addr) {
     for (AddressCollection::iterator a = addrs_.begin();
          a!=addrs_.end(); ++a) {
@@ -185,7 +196,7 @@ void IfaceMgr::stubDetectIfaces() {
         iface.flag_loopback_ = false;
         iface.flag_multicast_ = true;
         iface.flag_broadcast_ = true;
-        iface.hardware_type_ = HWTYPE_ETHERNET;
+        iface.setHWType(HWTYPE_ETHERNET);
         IOAddress addr(linkLocal);
         iface.addAddress(addr);
         addInterface(iface);
@@ -322,8 +333,8 @@ IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) {
         const AddressCollection& addrs = iface->getAddresses();
 
         out << "Detected interface " << iface->getFullName()
-             << ", hwtype=" << iface->hardware_type_ << ", maclen=" << iface->mac_len_
-             << ", mac=" << iface->getPlainMac();
+            << ", hwtype=" << iface->getHWType()
+            << ", mac=" << iface->getPlainMac();
         out << ", flags=" << hex << iface->flags_ << dec << "("
             << (iface->flag_loopback_?"LOOPBACK ":"")
             << (iface->flag_up_?"UP ":"")
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index aa11e59..b09a1ac 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -96,6 +96,23 @@ public:
         /// @return MAC address as a plain text (string)
         std::string getPlainMac() const;
 
+        /// @brief Sets MAC address of the interface.
+        ///
+        /// @param mac pointer to MAC address buffer
+        /// @param macLen length of mac address
+        void setMac(const uint8_t* mac, size_t macLen);
+
+        /// @brief Returns MAC length.
+        ///
+        /// @return length of MAC address
+        size_t getMacLen() const { return mac_len_; }
+
+        /// @brief Returns pointer to MAC address.
+        ///
+        /// Note: Returned pointer is only valid as long as the interface object
+        /// that returned it.
+        const uint8_t* getMac() const { return mac_; }
+
         /// @brief Sets flag_*_ fields based on bitmask value returned by OS
         ///
         /// Note: Implementation of this method is OS-dependent as bits have
@@ -114,6 +131,16 @@ public:
         /// @return interface name
         std::string getName() const { return name_; };
 
+        /// @brief Sets up hardware type of the interface.
+        ///
+        /// @param type hardware type
+        void setHWType(uint16_t type ) { hardware_type_ = type; }
+
+        /// @brief Returns hardware type of the interface.
+        ///
+        /// @return hardware type
+        uint16_t getHWType() const { return hardware_type_; }
+
         /// @brief Returns all interfaces available on an interface.
         ///
         /// Care should be taken to not use this collection after Iface object
@@ -176,12 +203,18 @@ public:
         /// list of assigned addresses
         AddressCollection addrs_;
 
-    public:
         /// link-layer address
         uint8_t mac_[MAX_MAC_LEN];
 
         /// length of link-layer address (usually 6)
-        int mac_len_;
+        size_t mac_len_;
+
+        /// hardware type
+        uint16_t hardware_type_;
+
+    public:
+        /// @todo: Make those fields protected once we start supporting more
+        /// than just Linux
 
         /// specifies if selected interface is loopback
         bool flag_loopback_;
@@ -202,9 +235,6 @@ public:
         /// interface flags (this value is as is returned by OS,
         /// it may mean different things on different OSes)
         uint32_t flags_;
-
-        /// hardware type
-        uint16_t hardware_type_;
     };
 
     // TODO performance improvement: we may change this into
diff --git a/src/lib/dhcp/iface_mgr_linux.cc b/src/lib/dhcp/iface_mgr_linux.cc
index a04708b..3faac02 100644
--- a/src/lib/dhcp/iface_mgr_linux.cc
+++ b/src/lib/dhcp/iface_mgr_linux.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -12,6 +12,20 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+/// @file
+/// Access to interface information on Linux is via netlink, a socket-based
+/// method for transferring information between the kernel and user processes.
+///
+/// For detailed information about netlink interface, please refer to
+/// http://en.wikipedia.org/wiki/Netlink and RFC3549.  Comments in the
+/// detectIfaces() method (towards the end of this file) provide an overview
+/// on how the netlink interface is used here.
+///
+/// Note that this interface is very robust and allows many operations:
+/// add/get/set/delete links, addresses, routes, queuing, manipulation of
+/// traffic classes, manipulation of neighbourhood tables and even the ability
+/// to do something with address labels. Getting a list of interfaces with
+/// addresses configured on it is just a small subset of all possible actions.
 
 #include <config.h>
 
@@ -23,146 +37,230 @@
 #include <stdint.h>
 #include <net/if.h>
 #include <linux/rtnetlink.h>
+#include <boost/array.hpp>
+#include <boost/static_assert.hpp>
+#include <dhcp/iface_mgr.h>
+#include <exceptions/exceptions.h>
+#include <asiolink/io_address.h>
+#include <util/io/sockaddr_util.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using namespace isc::util::io::internal;
 
-/// @brief This code uses netlink interface. See RFC3549 for details.
+BOOST_STATIC_ASSERT(IFLA_MAX>=IFA_MAX);
+
+namespace {
+
+/// @brief This class offers utility methods for netlink connection.
 ///
-/// For detailed description, see http://en.wikipedia.org/wiki/Netlink
-/// or RFC3549. According to Wikipedia: "Netlink was designed for and
-/// is used to transfer miscellaneous networking information between
-/// the Linux kernel space and user space processes. Many networking
-/// utilities use Netlink to communicate with the Linux kernel from
-/// user space, for example iproute2. Netlink consists of a standard
-/// socket-based interface for user space processes and an internal
-/// kernel API for kernel modules. It is designed to be a more
-/// flexible successor to ioctl. Originally, Netlink uses the
-/// AF_NETLINK socket family."
-
-/// This is a structure that defines context for netlink connection.
-struct rtnl_handle
+/// See IfaceMgr::detectIfaces() (Linux implementation, towards the end of this
+/// file) for example usage.
+class Netlink
 {
-    int fd;
-    struct sockaddr_nl local;
-    struct sockaddr_nl peer;
-    uint32_t seq;
-    uint32_t dump;
-};
+public:
 
-struct nlmsg_list
-{
-    struct nlmsg_list *next;
-    struct nlmsghdr h;
-};
+/// @brief Holds pointers to netlink messages.
+///
+/// netlink (a Linux interface for getting information about network
+/// interfaces) uses memory aliasing. Linux kernel returns a memory
+/// blob that should be interpreted as series of nlmessages. There
+/// are different nlmsg structures defined with varying size. They
+/// have one thing common - inital fields are laid out in the same
+/// way as nlmsghdr. Therefore different messages can be represented
+/// as nlmsghdr with followed variable number of bytes that are
+/// message-specific. The only reasonable way to represent this in
+/// C++ is to use vector of pointers to nlmsghdr (the common structure).
+    typedef vector<nlmsghdr*> NetlinkMessages;
+
+/// @brief Holds pointers to interface or address attributes.
+///
+/// Note that to get address info, a shorter (IFA_MAX rather than IFLA_MAX)
+/// table could be used, but we will use the bigger one anyway to
+/// make the code reusable.
+///
+/// rtattr is a generic structure, similar to sockaddr. It is defined
+/// in linux/rtnetlink.h and shown here for documentation purposes only:
+///
+/// struct rtattr {
+///     unsigned short<>rta_len;
+///     unsigned short<>rta_type;
+/// };
+    typedef boost::array<struct rtattr*, IFLA_MAX + 1> RTattribPtrs;
+
+    Netlink() : fd_(-1), seq_(0), dump_(0) {
+        memset(&local_, 0, sizeof(struct sockaddr_nl));
+        memset(&peer_, 0, sizeof(struct sockaddr_nl));
+    }
 
-const int SNDBUFSIZE = 32768;
-const int RCVBUFSIZE = 32768;
+    ~Netlink() {
+        rtnl_close_socket();
+    }
 
-namespace isc {
 
+    void rtnl_open_socket();
+    void rtnl_send_request(int family, int type);
+    void rtnl_store_reply(NetlinkMessages& storage, const nlmsghdr* msg);
+    void parse_rtattr(RTattribPtrs& table, rtattr* rta, int len);
+    void ipaddrs_get(IfaceMgr::Iface& iface, NetlinkMessages& addr_info);
+    void rtnl_process_reply(NetlinkMessages& info);
+    void release_list(NetlinkMessages& messages);
+    void rtnl_close_socket();
+
+private:
+    int fd_;            // Netlink file descriptor
+    sockaddr_nl local_; // Local addresses
+    sockaddr_nl peer_;  // Remote address
+    uint32_t seq_;      // Counter used for generating unique sequence numbers
+    uint32_t dump_;     // Number of expected message response
+};
+
+/// @brief defines a size of a sent netlink buffer
+const static size_t SNDBUF_SIZE = 32768;
+
+/// @brief defines a size of a received netlink buffer
+const static size_t RCVBUF_SIZE = 32768;
 
 /// @brief Opens netlink socket and initializes handle structure.
 ///
 /// @exception Unexpected Thrown if socket configuration fails.
-///
-/// @param handle Context will be stored in this structure.
-void rtnl_open_socket(struct rtnl_handle& handle) {
-    // equivalent of rtnl_open
-    handle.fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
-    if (handle.fd < 0) {
+void Netlink::rtnl_open_socket() {
+
+    fd_ = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+    if (fd_ < 0) {
         isc_throw(Unexpected, "Failed to create NETLINK socket.");
     }
 
-    if (setsockopt(handle.fd, SOL_SOCKET, SO_SNDBUF, &SNDBUFSIZE, sizeof(SNDBUFSIZE)) < 0) {
+    if (setsockopt(fd_, SOL_SOCKET, SO_SNDBUF, &SNDBUF_SIZE, sizeof(SNDBUF_SIZE)) < 0) {
         isc_throw(Unexpected, "Failed to set send buffer in NETLINK socket.");
     }
 
-    if (setsockopt(handle.fd, SOL_SOCKET, SO_RCVBUF, &RCVBUFSIZE, sizeof(RCVBUFSIZE)) < 0) {
+    if (setsockopt(fd_, SOL_SOCKET, SO_RCVBUF, &RCVBUF_SIZE, sizeof(RCVBUF_SIZE)) < 0) {
         isc_throw(Unexpected, "Failed to set receive buffer in NETLINK socket.");
     }
 
-    memset(&handle.local, 0, sizeof(handle.local));
-    handle.local.nl_family = AF_NETLINK;
-    handle.local.nl_groups = 0;
+    local_.nl_family = AF_NETLINK;
+    local_.nl_groups = 0;
 
-    if (bind(handle.fd, (struct sockaddr*)&handle.local, sizeof(handle.local)) < 0) {
+    if (bind(fd_, convertSockAddr(&local_), sizeof(local_)) < 0) {
         isc_throw(Unexpected, "Failed to bind netlink socket.");
     }
 
-    socklen_t addr_len = sizeof(handle.local);
-    if (getsockname(handle.fd, (struct sockaddr*)&handle.local, &addr_len) < 0) {
+    socklen_t addr_len = sizeof(local_);
+    if (getsockname(fd_, convertSockAddr(&local_), &addr_len) < 0) {
         isc_throw(Unexpected, "Getsockname for netlink socket failed.");
     }
 
     // just 2 sanity checks and we are done
-    if ( (addr_len != sizeof(handle.local)) ||
-         (handle.local.nl_family != AF_NETLINK) ) {
+    if ( (addr_len != sizeof(local_)) ||
+         (local_.nl_family != AF_NETLINK) ) {
         isc_throw(Unexpected, "getsockname() returned unexpected data for netlink socket.");
     }
 }
 
+/// @brief Closes netlink communication socket
+void Netlink::rtnl_close_socket() {
+    if (fd_ != -1) {
+        close(fd_);
+    }
+    fd_ = -1;
+}
+
 /// @brief Sends request over NETLINK socket.
 ///
-/// @param handle context structure
-/// @param family requested information family
-/// @param type request type (RTM_GETLINK or RTM_GETADDR)
-void rtnl_send_request(struct rtnl_handle& handle, int family, int type) {
-    struct {
-        struct nlmsghdr nlh;
-        struct rtgenmsg g;
-    } req;
+/// @param family requested information family.
+/// @param type request type (RTM_GETLINK or RTM_GETADDR).
+void Netlink::rtnl_send_request(int family, int type) {
+    struct Req {
+        nlmsghdr netlink_header;
+        rtgenmsg generic;
+    };
+    Req req; // we need this type named for offsetof() used in assert
     struct sockaddr_nl nladdr;
 
+    // do a sanity check. Verify that Req structure is aligned properly
+    BOOST_STATIC_ASSERT(sizeof(nlmsghdr) == offsetof(Req, generic));
+
     memset(&nladdr, 0, sizeof(nladdr));
     nladdr.nl_family = AF_NETLINK;
 
+    // According to netlink(7) manpage, mlmsg_seq must be set to a sequence
+    // number and is used to track messages. That is just a value that is
+    // opaque to kernel, and user-space code is supposed to use it to match
+    // incoming responses to sent requests. That is not really useful as we
+    // send a single request and get a single response at a time. However, we
+    // obey the man page suggestion and just set this to monotonically
+    // increasing numbers.
+    seq_++;
+
+    // This will be used to finding correct response (responses
+    // sent by kernel are supposed to have the same sequence number
+    // as the request we sent).
+    dump_ = seq_;
+
     memset(&req, 0, sizeof(req));
-    req.nlh.nlmsg_len = sizeof(req);
-    req.nlh.nlmsg_type = type;
-    req.nlh.nlmsg_flags = NLM_F_ROOT|NLM_F_MATCH|NLM_F_REQUEST;
-    req.nlh.nlmsg_pid = 0;
-    req.nlh.nlmsg_seq = handle.dump = ++handle.seq;
-    req.g.rtgen_family = family;
+    req.netlink_header.nlmsg_len = sizeof(req);
+    req.netlink_header.nlmsg_type = type;
+    req.netlink_header.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
+    req.netlink_header.nlmsg_pid = 0;
+    req.netlink_header.nlmsg_seq = seq_;
+    req.generic.rtgen_family = family;
 
-    int status =  sendto(handle.fd, (void*)&req, sizeof(req), 0,
-                         (struct sockaddr*)&nladdr, sizeof(nladdr));
+    int status =  sendto(fd_, static_cast<void*>(&req), sizeof(req), 0,
+                         static_cast<struct sockaddr*>(static_cast<void*>(&nladdr)),
+                         sizeof(nladdr));
 
     if (status<0) {
-        isc_throw(Unexpected, "Failed to send " << sizeof(nladdr) << " bytes over netlink socket.");
+        isc_throw(Unexpected, "Failed to send " << sizeof(nladdr)
+                  << " bytes over netlink socket.");
     }
 }
 
-/// @brief Appends nlmsg to a list
+/// @brief Appends nlmsg to a storage.
+///
+/// This method copies pointed nlmsg to a newly allocated memory
+/// and adds it to storage.
 ///
-/// @param n a message to be added
-/// @param link_info a list
-void rtnl_store_reply(struct nlmsghdr *n, struct nlmsg_list** link_info)
+/// @param storage A vector that holds pointers to netlink messages. The caller
+///        is responsible for freeing the pointed-to messages.
+/// @param msg A netlink message to be added.
+void Netlink::rtnl_store_reply(NetlinkMessages& storage, const struct nlmsghdr *msg)
 {
-    struct nlmsg_list *h;
-    struct nlmsg_list **lp;
-
-    h = (nlmsg_list*)malloc(n->nlmsg_len+sizeof(void*));
-    if (h == NULL) {
-        isc_throw(Unexpected, "Failed to allocate " << n->nlmsg_len+sizeof(void*)
-                  << " bytes.");
-    }
-
-    memcpy(&h->h, n, n->nlmsg_len);
-    h->next = NULL;
-
-    for (lp = link_info; *lp; lp = &(*lp)->next) /* NOTHING */;
-    *lp = h;
+    // we need to make a copy of this message. We really can't allocate
+    // nlmsghdr directly as it is only part of the structure. There are
+    // many message types with varying lengths and a common header.
+    struct nlmsghdr* copy = reinterpret_cast<struct nlmsghdr*>(new char[msg->nlmsg_len]);
+    memcpy(copy, msg, msg->nlmsg_len);
+
+    // push_back copies only pointer content, not the pointed-to object.
+    storage.push_back(copy);
 }
 
-void parse_rtattr(struct rtattr *tb[], int max, struct rtattr *rta, int len)
+/// @brief Parses rtattr message.
+///
+/// Some netlink messages represent address information. Such messages
+/// are concatenated collection of rtaddr structures. This function
+/// iterates over that list and stores pointers to those messages in
+/// flat array (table).
+///
+/// @param table rtattr Messages will be stored here
+/// @param rta Pointer to first rtattr object
+/// @param len Length (in bytes) of concatenated rtattr list.
+void Netlink::parse_rtattr(RTattribPtrs& table, struct rtattr* rta, int len)
 {
-    memset(tb, 0, sizeof(struct rtattr *) * (max + 1));
+    std::fill(table.begin(), table.end(), static_cast<struct rtattr*>(NULL));
+    // RTA_OK and RTA_NEXT() are macros defined in linux/rtnetlink.h
+    // they are used to handle rtattributes. RTA_OK checks if the structure
+    // pointed by rta is reasonable and passes all sanity checks.
+    // RTA_NEXT() returns pointer to the next rtattr structure that
+    // immediately follows pointed rta structure. See aforementioned
+    // header for details.
     while (RTA_OK(rta, len)) {
-        if (rta->rta_type <= max)
-            tb[rta->rta_type] = rta;
+        if (rta->rta_type < table.size()) {
+            table[rta->rta_type] = rta;
+        }
         rta = RTA_NEXT(rta,len);
     }
     if (len) {
@@ -170,114 +268,122 @@ void parse_rtattr(struct rtattr *tb[], int max, struct rtattr *rta, int len)
     }
 }
 
-void ipaddrs_get(IfaceMgr::Iface& iface, struct nlmsg_list *addr_info) {
-    uint8_t addr[16];
-    struct rtattr * rta_tb[IFA_MAX+1];
-
-    for ( ;addr_info ;  addr_info = addr_info->next) {
-        struct nlmsghdr *n = &addr_info->h;
-        struct ifaddrmsg *ifa = (ifaddrmsg*)NLMSG_DATA(n);
-
-        // these are not the addresses you are looking for
-        if ( ifa->ifa_index != iface.getIndex()) {
+/// @brief Parses addr_info and appends appropriate addresses to Iface object.
+///
+/// Netlink is a fine, but convoluted interface. It returns a concatenated
+/// collection of netlink messages. Some of those messages convey information
+/// about addresses. Those messages are in fact appropriate header followed
+/// by concatenated lists of rtattr structures that define various pieces
+/// of address information.
+///
+/// @param iface interface representation (addresses will be added here)
+/// @param addr_info collection of parsed netlink messages
+void Netlink::ipaddrs_get(IfaceMgr::Iface& iface, NetlinkMessages& addr_info) {
+    uint8_t addr[V6ADDRESS_LEN];
+    RTattribPtrs rta_tb;
+
+    for (NetlinkMessages::const_iterator msg = addr_info.begin();
+         msg != addr_info.end(); ++msg) {
+        ifaddrmsg* ifa = static_cast<ifaddrmsg*>(NLMSG_DATA(*msg));
+
+        // These are not the addresses you are looking for
+        if (ifa->ifa_index != iface.getIndex()) {
             continue;
         }
 
-        if ( ifa->ifa_family == AF_INET6 ) {
-            memset(rta_tb, 0, sizeof(rta_tb));
-            parse_rtattr(rta_tb, IFA_MAX, IFA_RTA(ifa), n->nlmsg_len - NLMSG_LENGTH(sizeof(*ifa)));
-            if (!rta_tb[IFA_LOCAL])
-                rta_tb[IFA_LOCAL]   = rta_tb[IFA_ADDRESS];
-            if (!rta_tb[IFA_ADDRESS])
+        if ((ifa->ifa_family == AF_INET6) || (ifa->ifa_family == AF_INET)) {
+            std::fill(rta_tb.begin(), rta_tb.end(), static_cast<rtattr*>(NULL));
+            parse_rtattr(rta_tb, IFA_RTA(ifa), (*msg)->nlmsg_len - NLMSG_LENGTH(sizeof(*ifa)));
+            if (!rta_tb[IFA_LOCAL]) {
+                rta_tb[IFA_LOCAL] = rta_tb[IFA_ADDRESS];
+            }
+            if (!rta_tb[IFA_ADDRESS]) {
                 rta_tb[IFA_ADDRESS] = rta_tb[IFA_LOCAL];
+            }
 
-            memcpy(addr,(char*)RTA_DATA(rta_tb[IFLA_ADDRESS]),16);
-            IOAddress a = IOAddress::from_bytes(AF_INET6, addr);
+            memcpy(addr, RTA_DATA(rta_tb[IFLA_ADDRESS]),
+                   ifa->ifa_family==AF_INET?V4ADDRESS_LEN:V6ADDRESS_LEN);
+            IOAddress a = IOAddress::from_bytes(ifa->ifa_family, addr);
             iface.addAddress(a);
 
-            /// TODO: Read lifetimes of configured addresses
-        }
-
-        if ( ifa->ifa_family == AF_INET ) {
-            memset(rta_tb, 0, sizeof(rta_tb));
-            parse_rtattr(rta_tb, IFA_MAX, IFA_RTA(ifa), n->nlmsg_len - NLMSG_LENGTH(sizeof(*ifa)));
-            if (!rta_tb[IFA_LOCAL])
-                rta_tb[IFA_LOCAL]   = rta_tb[IFA_ADDRESS];
-            if (!rta_tb[IFA_ADDRESS])
-                rta_tb[IFA_ADDRESS] = rta_tb[IFA_LOCAL];
-
-            memcpy(addr,(char*)RTA_DATA(rta_tb[IFLA_ADDRESS]),4);
-            IOAddress a = IOAddress::from_bytes(AF_INET, addr);
-            iface.addAddress(a);
+            /// TODO: Read lifetimes of configured IPv6 addresses
         }
     }
 }
 
-
-void rtnl_process_reply(struct rtnl_handle &rth, struct nlmsg_list *&info) {
-
-    struct sockaddr_nl nladdr;
-    struct iovec iov;
-    struct msghdr msg;
-    memset(&msg, 0, sizeof(struct msghdr));
+/// @brief Processes reply received over netlink socket.
+///
+/// This method parses the received buffer (a collection of concatenated
+/// netlink messages), copies each received message to newly allocated
+/// memory and stores pointers to it in the "info" container.
+///
+/// @param info received netlink messages will be stored here.  It is the
+///        caller's responsibility to release the memory associated with the
+///        messages by calling the release_list() method.
+void Netlink::rtnl_process_reply(NetlinkMessages& info) {
+    sockaddr_nl nladdr;
+    iovec iov;
+    msghdr msg;
+    memset(&msg, 0, sizeof(msghdr));
     msg.msg_name = &nladdr;
     msg.msg_namelen = sizeof(nladdr);
     msg.msg_iov = &iov;
     msg.msg_iovlen = 1;
 
-    char buf[RCVBUFSIZE];
+    char buf[RCVBUF_SIZE];
 
     iov.iov_base = buf;
-    while (1) {
-        int status;
-        struct nlmsghdr *h;
-
-        iov.iov_len = sizeof(buf);
-        status = recvmsg(rth.fd, &msg, 0);
+    iov.iov_len = sizeof(buf);
+    while (true) {
+        int status = recvmsg(fd_, &msg, 0);
 
         if (status < 0) {
-            if (errno == EINTR)
+            if (errno == EINTR) {
                 continue;
-            isc_throw(Unexpected, "Overrun while processing reply from netlink socket.");
+            }
+            isc_throw(Unexpected, "Error " << errno
+                      << " while processing reply from netlink socket.");
         }
 
         if (status == 0) {
             isc_throw(Unexpected, "EOF while reading netlink socket.");
         }
 
-        h = (struct nlmsghdr*)buf;
-        while (NLMSG_OK(h, status)) {
+        nlmsghdr* header = static_cast<nlmsghdr*>(static_cast<void*>(buf));
+        while (NLMSG_OK(header, status)) {
 
-            // why we received this anyway?
+            // Received a message not addressed to our process, or not
+            // with a sequence number we are expecting.  Ignore, and
+            // look at the next one.
             if (nladdr.nl_pid != 0 ||
-                h->nlmsg_pid != rth.local.nl_pid ||
-                h->nlmsg_seq != rth.dump) {
-                h = NLMSG_NEXT(h, status);
+                header->nlmsg_pid != local_.nl_pid ||
+                header->nlmsg_seq != dump_) {
+                header = NLMSG_NEXT(header, status);
                 continue;
             }
 
-            if (h->nlmsg_type == NLMSG_DONE) {
-                // end of message
+            if (header->nlmsg_type == NLMSG_DONE) {
+                // End of message.
                 return;
             }
 
-            if (h->nlmsg_type == NLMSG_ERROR) {
-                struct nlmsgerr *err = (struct nlmsgerr*)NLMSG_DATA(h);
-                if (h->nlmsg_len < NLMSG_LENGTH(sizeof(struct nlmsgerr))) {
-                    // we are really out of luck here. We can't even say what is
+            if (header->nlmsg_type == NLMSG_ERROR) {
+                nlmsgerr* err = static_cast<nlmsgerr*>(NLMSG_DATA(header));
+                if (header->nlmsg_len < NLMSG_LENGTH(sizeof(struct nlmsgerr))) {
+                    // We are really out of luck here. We can't even say what is
                     // wrong as error message is truncated. D'oh.
                     isc_throw(Unexpected, "Netlink reply read failed.");
                 } else {
                     isc_throw(Unexpected, "Netlink reply read error " << -err->error);
                 }
-                // never happens we throw before we reach here
+                // Never happens we throw before we reach here
                 return;
             }
 
             // store the data
-            rtnl_store_reply(h, &info);
+            rtnl_store_reply(info, header);
 
-            h = NLMSG_NEXT(h, status);
+            header = NLMSG_NEXT(header, status);
         }
         if (msg.msg_flags & MSG_TRUNC) {
             isc_throw(Unexpected, "Message received over netlink truncated.");
@@ -288,87 +394,108 @@ void rtnl_process_reply(struct rtnl_handle &rth, struct nlmsg_list *&info) {
     }
 }
 
-/// @brief releases nlmsg list
+/// @brief releases nlmsg structure
 ///
-/// @param head first element of the list to be released
-void release_list(struct nlmsg_list *head) {
-    struct nlmsg_list *tmp;
-    while (head) {
-        tmp = head->next;
-        free(head);
-        head = tmp;
+/// @param messages Set of messages to be freed.
+void Netlink::release_list(NetlinkMessages& messages) {
+    // let's free local copies of stored messages
+    for (NetlinkMessages::iterator msg = messages.begin(); msg != messages.end(); ++msg) {
+        delete (*msg);
     }
+
+    // ang get rid of the message pointers as well
+    messages.clear();
 }
 
+} // end of anonymous namespace
+
+namespace isc {
+
+namespace dhcp {
+
+/// @brief Detect available interfaces on Linux systems.
+///
+/// Uses the socket-based netlink protocol to retrieve the list of interfaces
+/// from the Linux kernel.
 void IfaceMgr::detectIfaces() {
     cout << "Linux: detecting interfaces." << endl;
 
-    struct nlmsg_list* link_info = NULL; // link info list
-    struct nlmsg_list* addr_info = NULL; // address info list
-    struct nlmsg_list* l = NULL;
-    struct rtnl_handle rth;
+    // Copies of netlink messages about links will be stored here.
+    Netlink::NetlinkMessages link_info;
+
+    // Copies of netlink messages about addresses will be stored here.
+    Netlink::NetlinkMessages addr_info;
 
-    // required to display information about interface
-    struct ifinfomsg* ifi = NULL;
-    struct rtattr* tb[IFLA_MAX+1];
-    int len = 0;
-    memset(tb, 0, sizeof(tb));
-    memset(&rth,0, sizeof(rth));
+    // Socket descriptors and other rtnl-related parameters.
+    Netlink nl;
 
-    // open socket
-    rtnl_open_socket(rth);
+    // Table with pointers to address attributes.
+    Netlink::RTattribPtrs attribs_table;
+    std::fill(attribs_table.begin(), attribs_table.end(),
+              static_cast<struct rtattr*>(NULL));
 
-    // now we have open functional socket, let's use it!
-    // ask for list of interface...
-    rtnl_send_request(rth, AF_PACKET, RTM_GETLINK);
+    // Open socket
+    nl.rtnl_open_socket();
 
-    // Get reply and store it in link_info list.
-    rtnl_process_reply(rth, link_info);
+    // Now we have open functional socket, let's use it!
+    // Ask for list of network interfaces...
+    nl.rtnl_send_request(AF_PACKET, RTM_GETLINK);
+
+    // Get reply and store it in link_info list:
+    // response is received as with any other socket - just a series
+    // of bytes. They are representing collection of netlink messages
+    // concatenated together. rtnl_process_reply will parse this
+    // buffer, copy each message to a newly allocated memory and
+    // store pointers to it in link_info. This allocated memory will
+    // be released later. See release_info(link_info) below.
+    nl.rtnl_process_reply(link_info);
 
     // Now ask for list of addresses (AF_UNSPEC = of any family)
-    rtnl_send_request(rth, AF_UNSPEC, RTM_GETADDR);
+    // Let's repeat, but this time ask for any addresses.
+    // That includes IPv4, IPv6 and any other address families that
+    // are happen to be supported by this system.
+    nl.rtnl_send_request(AF_UNSPEC, RTM_GETADDR);
 
     // Get reply and store it in addr_info list.
-    rtnl_process_reply(rth, addr_info);
+    // Again, we will allocate new memory and store messages in
+    // addr_info. It will be released later using release_info(addr_info).
+    nl.rtnl_process_reply(addr_info);
 
     // Now build list with interface names
-    for (l=link_info; l; l = l->next) {
-        ifi = (ifinfomsg*)NLMSG_DATA(&l->h);
-        len = (&l->h)->nlmsg_len;
-        len -= NLMSG_LENGTH(sizeof(*ifi));
-        parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-
-        Iface* iface = new Iface(string( (char*)RTA_DATA(tb[IFLA_IFNAME])), ifi->ifi_index);
-
-        iface->hardware_type_ = ifi->ifi_type;
-        iface->setFlags(ifi->ifi_flags);
-
-        iface->mac_len_ = 0;
-        memset(iface->mac_, 0, IfaceMgr::MAX_MAC_LEN);
-        // Does inetface has LL_ADDR?
-        if (tb[IFLA_ADDRESS]) {
-            iface->mac_len_ = RTA_PAYLOAD(tb[IFLA_ADDRESS]);
-            if (iface->mac_len_ > IfaceMgr::MAX_MAC_LEN) {
-                iface->mac_len_ = 0;
-                isc_throw(Unexpected, "Interface " << iface->getFullName()
-                          << " was detected to have link address of length " << RTA_PAYLOAD(tb[IFLA_ADDRESS])
-                          << ", but maximum supported length is " << IfaceMgr::MAX_MAC_LEN);
-            }
-            memcpy(iface->mac_, RTA_DATA(tb[IFLA_ADDRESS]), iface->mac_len_);
+    for (Netlink::NetlinkMessages::iterator msg = link_info.begin();
+         msg != link_info.end(); ++msg) {
+        // Required to display information about interface
+        struct ifinfomsg* interface_info = static_cast<ifinfomsg*>(NLMSG_DATA(*msg));
+        int len = (*msg)->nlmsg_len;
+        len -= NLMSG_LENGTH(sizeof(*interface_info));
+        nl.parse_rtattr(attribs_table, IFLA_RTA(interface_info), len);
+
+        // valgrind reports *possible* memory leak in the line below, but it is
+        // bogus. Nevertheless, the whole interface definition has been split
+        // into three separate steps for easier debugging.
+        const char* tmp = static_cast<const char*>(RTA_DATA(attribs_table[IFLA_IFNAME]));
+        string iface_name(tmp); // <--- bogus valgrind warning here
+        Iface iface = Iface(iface_name, interface_info->ifi_index);
+
+        iface.setHWType(interface_info->ifi_type);
+        iface.setFlags(interface_info->ifi_flags);
+
+        // Does inetface have LL_ADDR?
+        if (attribs_table[IFLA_ADDRESS]) {
+            iface.setMac(static_cast<const uint8_t*>(RTA_DATA(attribs_table[IFLA_ADDRESS])),
+                         RTA_PAYLOAD(attribs_table[IFLA_ADDRESS]));
         }
         else {
-            // Tunnels can have no LL_ADDR. RTA_PAYLOAD doesn't check it and try to
-            // dereference it in this manner
-            // #define RTA_PAYLOAD(rta) ((int)((rta)->rta_len) - RTA_LENGTH(0))
-            iface->mac_len_ = 0;
+            // Tunnels can have no LL_ADDR. RTA_PAYLOAD doesn't check it and
+            // try to dereference it in this manner
         }
 
-        ipaddrs_get(*iface, addr_info);
-        ifaces_.push_back(*iface);
+        nl.ipaddrs_get(iface, addr_info);
+        ifaces_.push_back(iface);
     }
 
-    release_list(link_info);
-    release_list(addr_info);
+    nl.release_list(link_info);
+    nl.release_list(addr_info);
 
     printIfaces();
 }
@@ -444,6 +571,7 @@ bool IfaceMgr::os_receive4(struct msghdr& m, Pkt4Ptr& pkt) {
     return (false);
 }
 
-}
+} // end of isc::dhcp namespace
+} // end of isc namespace
 
 #endif // if defined(LINUX)
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 98fc3bf..83ba231 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -535,6 +535,39 @@ TEST_F(IfaceMgrTest, iface) {
     );
 }
 
+TEST_F(IfaceMgrTest, iface_methods) {
+    IfaceMgr::Iface iface("foo", 1234);
+
+    iface.setHWType(42);
+    EXPECT_EQ(42, iface.getHWType());
+
+    uint8_t mac[IfaceMgr::MAX_MAC_LEN+10];
+    for (int i = 0; i < IfaceMgr::MAX_MAC_LEN + 10; i++)
+        mac[i] = 255 - i;
+
+    EXPECT_EQ("foo", iface.getName());
+    EXPECT_EQ(1234, iface.getIndex());
+
+    // MAC is too long. Exception should be thrown and
+    // MAC length should not be set.
+    EXPECT_THROW(
+        iface.setMac(mac, IfaceMgr::MAX_MAC_LEN + 1),
+        OutOfRange
+    );
+
+    // MAC length should stay not set as excep
+    EXPECT_EQ(0, iface.getMacLen());
+
+    // Setting maximum length MAC should be ok.
+    iface.setMac(mac, IfaceMgr::MAX_MAC_LEN);
+
+    // For some reason constants cannot be used directly in EXPECT_EQ
+    // as this produces linking error.
+    size_t len = IfaceMgr::MAX_MAC_LEN;
+    EXPECT_EQ(len, iface.getMacLen());
+    EXPECT_EQ(0, memcmp(mac, iface.getMac(), iface.getMacLen()));
+}
+
 TEST_F(IfaceMgrTest, socketInfo) {
 
     // check that socketinfo for IPv4 socket is functional
@@ -682,6 +715,8 @@ size_t parse_mac(const std::string& textMac, uint8_t* mac, size_t macLen) {
 /// of text that ifconfig prints and robustness to handle slight differences
 /// in ifconfig output.
 ///
+/// @todo: Consider using isc::util::str::tokens here.
+///
 /// @param textFile name of a text file that holds output of ifconfig -a
 /// @param ifaces empty list of interfaces to be filled
 void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifaces) {
@@ -711,6 +746,11 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
             if (offset == string::npos) {
                 isc_throw(BadValue, "Malformed output of ifconfig");
             }
+
+            // ifconfig in Gentoo prints out eth0: instead of eth0
+            if (line[offset - 1] == ':') {
+                offset--;
+            }
             string name = line.substr(0, offset);
 
             // sadly, ifconfig does not return ifindex
@@ -726,19 +766,41 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
                 mac = line.substr(offset, string::npos);
                 mac = mac.substr(0, mac.find_first_of(" "));
 
-                iface->mac_len_ = parse_mac(mac, iface->mac_, IfaceMgr::MAX_MAC_LEN);
+                uint8_t buf[IfaceMgr::MAX_MAC_LEN];
+                int mac_len = parse_mac(mac, buf, IfaceMgr::MAX_MAC_LEN);
+                iface->setMac(buf, mac_len);
             }
         }
 
         if (line.find("inet6") != string::npos) {
             // IPv6 address
-            string addr = line.substr(line.find("inet6")+12, string::npos);
+            string addr;
+            if (line.find("addr:", line.find("inet6")) != string::npos) {
+                // Ubuntu style format: inet6 addr: ::1/128 Scope:Host
+                addr = line.substr(line.find("addr:") + 6, string::npos);
+            } else {
+                // Gentoo style format: inet6 fe80::6ef0:49ff:fe96:ba17  prefixlen 64  scopeid 0x20<link>
+                addr = line.substr(line.find("inet6") + 6, string::npos);
+            }
+
+            // handle Ubuntu format: inet6 addr: fe80::f66d:4ff:fe96:58f2/64 Scope:Link
             addr = addr.substr(0, addr.find("/"));
+
+            // handle inet6 fe80::ca3a:35ff:fed4:8f1d  prefixlen 64  scopeid 0x20<link>
+            addr = addr.substr(0, addr.find(" "));
             IOAddress a(addr);
             iface->addAddress(a);
         } else if(line.find("inet") != string::npos) {
             // IPv4 address
-            string addr = line.substr(line.find("inet")+10, string::npos);
+            string addr;
+            if (line.find("addr:", line.find("inet")) != string::npos) {
+                // Ubuntu style format: inet addr:127.0.0.1  Mask:255.0.0.0
+                addr = line.substr(line.find("addr:") + 5, string::npos);
+            } else {
+                // Gentoo style format: inet 10.53.0.4  netmask 255.255.255.0
+                addr = line.substr(line.find("inet") + 5, string::npos);
+            }
+
             addr = addr.substr(0, addr.find_first_of(" "));
             IOAddress a(addr);
             iface->addAddress(a);
@@ -789,7 +851,7 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
     // list of interfaces parsed from ifconfig
     IfaceMgr::IfaceCollection parsedIfaces;
 
-    EXPECT_NO_THROW(
+    ASSERT_NO_THROW(
         parse_ifconfig(textFile, parsedIfaces);
     );
     unlink(textFile.c_str());
@@ -848,8 +910,8 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
             // skip MAC comparison for loopback as netlink returns MAC
             // 00:00:00:00:00:00 for lo
             if (!detected->flag_loopback_) {
-                ASSERT_EQ(detected->mac_len_, i->mac_len_);
-                EXPECT_EQ(0, memcmp(detected->mac_, i->mac_, i->mac_len_));
+                ASSERT_EQ(detected->getMacLen(), i->getMacLen());
+                EXPECT_EQ(0, memcmp(detected->getMac(), i->getMac(), i->getMacLen()));
             }
 
             EXPECT_EQ(detected->getAddresses().size(), i->getAddresses().size());



More information about the bind10-changes mailing list