From: Stephen Morris Date: Wed, 18 Apr 2012 18:10:56 +0000 (+0100) Subject: [1528] Modifications to some of the comments X-Git-Tag: trac2351_base~240^2~11 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7eaf3a710ceb20f4822a636054aef7d8a6f7e6dd;p=thirdparty%2Fkea.git [1528] Modifications to some of the comments --- diff --git a/src/lib/dhcp/iface_mgr_linux.cc b/src/lib/dhcp/iface_mgr_linux.cc index a6edfe1d4a..0ee56049e3 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 #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 RTattribPtrs; + typedef boost::array 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(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(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(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(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(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(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(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);