BIND 10 trac1528, updated. 7eaf3a710ceb20f4822a636054aef7d8a6f7e6dd [1528] Modifications to some of the comments

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Apr 18 18:11:14 UTC 2012


The branch, trac1528 has been updated
       via  7eaf3a710ceb20f4822a636054aef7d8a6f7e6dd (commit)
      from  1202068e5f61ed72afde9b204fd5cd011ecd190f (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 7eaf3a710ceb20f4822a636054aef7d8a6f7e6dd
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Apr 18 19:10:56 2012 +0100

    [1528] Modifications to some of the comments

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

Summary of changes:
 src/lib/dhcp/iface_mgr_linux.cc |  137 ++++++++++++++++++++-------------------
 1 files changed, 71 insertions(+), 66 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/iface_mgr_linux.cc b/src/lib/dhcp/iface_mgr_linux.cc
index a6edfe1..0ee5604 100644
--- a/src/lib/dhcp/iface_mgr_linux.cc
+++ b/src/lib/dhcp/iface_mgr_linux.cc
@@ -12,6 +12,21 @@
 // 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>
 
 #if defined(OS_LINUX)
@@ -34,8 +49,8 @@ namespace {
 
 /// @brief This class offers utility methods for netlink connection.
 ///
-/// See IfaceMgr::detectIfaces() (Linux implementation) for example
-/// usage.
+/// See IfaceMgr::detectIfaces() (Linux implementation, towards the end of this
+/// file) for example usage.
 class Netlink
 {
 public:
@@ -65,10 +80,9 @@ public:
 ///     unsigned short<>rta_len;
 ///     unsigned short<>rta_type;
 /// };
-///
-    typedef boost::array<struct rtattr*, IFLA_MAX+1> RTattribPtrs;
+    typedef boost::array<struct rtattr*, IFLA_MAX + 1> RTattribPtrs;
 
-    Netlink() :fd_(-1), seq_(0), dump_(0) {
+    Netlink() : fd_(-1), seq_(0), dump_(0) {
         memset(&local_, 0, sizeof(struct sockaddr_nl));
         memset(&peer_, 0, sizeof(struct sockaddr_nl));
     }
@@ -87,11 +101,11 @@ public:
     void rtnl_close_socket();
 
 private:
-    int fd_; // netlink file descriptor
-    sockaddr_nl local_; // local and remote addresses
-    sockaddr_nl peer_;
-    uint32_t seq_; // counter used for generating unique sequence numbers
-    uint32_t dump_; // number of expected message response
+    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
 };
 
 const size_t sndbuf = 32768;
@@ -100,10 +114,8 @@ const size_t rcvbuf = 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 Netlink::rtnl_open_socket() {
-    // equivalent of rtnl_open
+
     fd_ = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
     if (fd_ < 0) {
         isc_throw(Unexpected, "Failed to create NETLINK socket.");
@@ -136,6 +148,7 @@ void Netlink::rtnl_open_socket() {
     }
 }
 
+/// @brief Closes netlink communication socket
 void Netlink::rtnl_close_socket() {
     if (fd_ != -1) {
         close(fd_);
@@ -145,9 +158,8 @@ void Netlink::rtnl_close_socket() {
 
 /// @brief Sends request over NETLINK socket.
 ///
-/// @param handle structure that contains necessary information about netlink
-/// @param family requested information family
-/// @param type request type (RTM_GETLINK or RTM_GETADDR)
+/// @param family requested information family.
+/// @param type request type (RTM_GETLINK or RTM_GETADDR).
 void Netlink::rtnl_send_request(int family, int type) {
     struct {
         nlmsghdr netlink_header;
@@ -155,26 +167,25 @@ void Netlink::rtnl_send_request(int family, int type) {
     } req;
     struct sockaddr_nl nladdr;
 
-    // This doesn't work as gcc is confused with coma appearing in
-    // the expression and thinks that there are 2 parameters passed to
+    // This doesn't work as gcc is confused with a comma appearing in
+    // the expression and thinks that there are two parameters passed to
     // BOOST_STATIC_ASSERT macro, while it only takes one.
-    // BOOST_STATIC_ASSERT(sizeof(nlmsghdr) == offsetof(req,generic) );
-
+    // 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
-    // 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, but still it better to obey man page suggestion
-    // and just set this to monotonically increasing numbers.
+    // 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
+    // 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)
+    // as the request we sent).
     dump_ = seq_;
 
     memset(&req, 0, sizeof(req));
@@ -200,8 +211,9 @@ void Netlink::rtnl_send_request(int family, int type) {
 /// This method copies pointed nlmsg to a newly allocated memory
 /// and adds it to storage.
 ///
-/// @param storage a vector that holds netlink messages
-/// @param msg a netlink message to be added
+/// @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)
 {
     // we need to make a copy of this message. We really can't allocate
@@ -210,7 +222,7 @@ void Netlink::rtnl_store_reply(NetlinkMessages& storage, const struct nlmsghdr *
     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 object
+    // push_back copies only pointer content, not the pointed-to object.
     storage.push_back(copy);
 }
 
@@ -221,10 +233,10 @@ void Netlink::rtnl_store_reply(NetlinkMessages& storage, const struct nlmsghdr *
 /// 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)
+/// @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)
 {
     std::fill(table.begin(), table.end(), static_cast<struct rtattr*>(NULL));
     // RTA_OK and RTA_NEXT() are macros defined in linux/rtnetlink.h
@@ -246,7 +258,7 @@ void Netlink::parse_rtattr(RTattribPtrs& table, struct rtattr * rta, int len)
 
 /// @brief Parses addr_info and appends appropriate addresses to Iface object.
 ///
-/// Netlink is a fine, but convoluted interface. It returns concatenated
+/// 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
@@ -262,7 +274,7 @@ void Netlink::ipaddrs_get(IfaceMgr::Iface& iface, NetlinkMessages& addr_info) {
          msg != addr_info.end(); ++msg) {
         ifaddrmsg* ifa = static_cast<ifaddrmsg*>(NLMSG_DATA(*msg));
 
-        // these are not the addresses you are looking for
+        // These are not the addresses you are looking for
         if (ifa->ifa_index != iface.getIndex()) {
             continue;
         }
@@ -289,13 +301,13 @@ void Netlink::ipaddrs_get(IfaceMgr::Iface& iface, NetlinkMessages& addr_info) {
 
 /// @brief Processes reply received over netlink socket.
 ///
-/// This method parses received buffer (a collection of concatenated
+/// 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 info container.
+/// memory and stores pointers to it in the "info" container.
 ///
-/// Make sure to release this memory, e.g. using release_info() function.
-///
-/// @param info received netlink messages will be stored here
+/// @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;
@@ -352,7 +364,7 @@ void Netlink::rtnl_process_reply(NetlinkMessages& info) {
                 } 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;
             }
 
@@ -372,7 +384,7 @@ void Netlink::rtnl_process_reply(NetlinkMessages& info) {
 
 /// @brief releases nlmsg structure
 ///
-/// @param messages first element of the list to be released
+/// @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) {
@@ -389,17 +401,10 @@ namespace isc {
 
 namespace dhcp {
 
-/// @brief Detect available interfaces on Linux systesm.
+/// @brief Detect available interfaces on Linux systems.
 ///
-/// For detailed information about netlink interface, please refer to
-/// http://en.wikipedia.org/wiki/Netlink and RFC3549.  Following
-/// comments in the core is an overview on how netlink interface is
-/// used here. Please note that this interface is very robust and
-/// allows many operations: add/get/set/delete links, addresses,
-/// routes, queuing, manipulate traffic classes, manipulate
-/// neithborhood tables and even do something with address
-/// labels. Getting list of interfaces with addresses configured on it
-/// is just a small subset of all possible actions.
+/// Uses the socket-based netlink protocol to retrieve the list of interfaces
+/// from the Linux kernel.
 void IfaceMgr::detectIfaces() {
     cout << "Linux: detecting interfaces." << endl;
 
@@ -412,16 +417,16 @@ void IfaceMgr::detectIfaces() {
     // Socket descriptors and other rtnl-related parameters.
     Netlink nl;
 
-    // table with pointers to address attributes
+    // Table with pointers to address attributes.
     Netlink::RTattribPtrs attribs_table;
     std::fill(attribs_table.begin(), attribs_table.end(),
               static_cast<struct rtattr*>(NULL));
 
-    // open socket
+    // Open socket
     nl.rtnl_open_socket();
 
-    // now we have open functional socket, let's use it!
-    // ask for list of network interfaces...
+    // 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:
@@ -447,15 +452,15 @@ void IfaceMgr::detectIfaces() {
     // Now build list with interface names
     for (Netlink::NetlinkMessages::iterator msg = link_info.begin();
          msg != link_info.end(); ++msg) {
-        // required to display information about interface
+        // 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, I've split the whole interface
-        // definition into three separate steps for easier debugging.
+        // 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);
@@ -463,14 +468,14 @@ void IfaceMgr::detectIfaces() {
         iface.setHWType(interface_info->ifi_type);
         iface.setFlags(interface_info->ifi_flags);
 
-        // Does inetface has LL_ADDR?
+        // 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
+            // Tunnels can have no LL_ADDR. RTA_PAYLOAD doesn't check it and
+            // try to dereference it in this manner
         }
 
         nl.ipaddrs_get(iface, addr_info);



More information about the bind10-changes mailing list