Rewrite the netlink handling code to use libmnl. The size cost is very small,

and it is hard to do netlink completely correctly as it is poorly documented.

CMakeLists.txt

@@ -7,6 +7,8 @@ set(CMAKE_CXX_FLAGS "${CMAKE_C_FLAGS} -s -std=gnu99 -pedantic -Wall -lcap -DHAVE
 
 include_directories("${PROJECT_SOURCE_DIR}/ncmlib")
 add_subdirectory(ncmlib)
+include_directories("${PROJECT_SOURCE_DIR}/libmnl")
+add_subdirectory(libmnl)
 
 add_subdirectory(ifchd)
 add_subdirectory(ndhc)

ndhc/CMakeLists.txt

@@ -5,4 +5,4 @@ cmake_minimum_required (VERSION 2.6)
 file(GLOB NDHC_SRCS "*.c")
 
 add_executable(ndhc ${NDHC_SRCS})
-target_link_libraries(ndhc ncmlib)
+target_link_libraries(ndhc ncmlib libmnl)

ndhc/ndhc.c

@@ -314,7 +314,5 @@ int main(int argc, char **argv)
     ifchange(NULL, IFCHANGE_DECONFIG);
 
     do_work();
-
+    return EXIT_SUCCESS; // Never reached.
-    nl_close(&cs);
-    return EXIT_SUCCESS;
 }

ndhc/netlink.c

@@ -1,9 +1,6 @@
 /* netlink.c - netlink physical link notification handling and info retrieval
  *
  * (c) 2011 Nicholas J. Kain <njkain at gmail dot com>
- * (c) 2006-2007 Stefan Rompf <sux@loplof.de>
- *
- * This code was largely taken from Stefan Rompf's dhcpclient.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -34,14 +31,15 @@
 #include <string.h>
 #include <sys/select.h>
 #include <fcntl.h>
+#include <time.h>
+#include <libmnl/libmnl.h>
+#include <errno.h>
 
 #include "netlink.h"
 #include "ifchange.h"
 #include "arp.h"
 #include "log.h"
 
-#define NLMSG_RECVSIZE 8192
-
 enum {
     IFS_NONE = 0,
     IFS_UP,
@@ -50,76 +48,34 @@ enum {
     IFS_REMOVED
 };
 
-static unsigned int nl_seq;
+static struct mnl_socket *mls;
 
-/* internal callback handling */
+static void nl_close(struct client_state_t *cs)
-static void (*nlcb_function)(struct nlmsghdr *msg, void **args);
+{
-static void *nlcb_args[3];
+    mnl_socket_close(mls);
-static __u32 nlcb_pid;
+    cs->nlFd = -1;
-static unsigned int nlcb_seq;
+}
-static char nlcb_run;
 
 int nl_open(struct client_state_t *cs)
 {
-    struct sockaddr_nl nlsock = {
-        .nl_family = AF_NETLINK,
-        .nl_pad = 0,
-        .nl_pid = getpid(),
-        .nl_groups = RTMGRP_LINK
-    };
-
-    nlcb_pid = nlsock.nl_pid;
-
     assert(cs->nlFd == -1);
-
+    if ((mls = mnl_socket_open(NETLINK_ROUTE)) == (struct mnl_socket *)-1)
-    cs->nlFd = socket(AF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE);
-
-    if (cs->nlFd == -1)
         return -1;
-
+    cs->nlFd = mnl_socket_get_fd(mls);
-    if (bind(cs->nlFd, (const struct sockaddr *)&nlsock, sizeof(nlsock)))
-        goto err_close;
-
     if (fcntl(cs->nlFd, F_SETFD, FD_CLOEXEC))
         goto err_close;
-
+    if (mnl_socket_bind(mls, RTMGRP_LINK, 0))
+        goto err_close;
     return 0;
-
   err_close:
     nl_close(cs);
     return -1;
 }
 
-void nl_close(struct client_state_t *cs)
-{
-    close(cs->nlFd);
-    cs->nlFd = -1;
-}
-
-void nl_queryifstatus(int ifidx, struct client_state_t *cs)
-{
-    struct {
-        struct nlmsghdr hdr;
-        struct ifinfomsg ifinfo;
-    } req;
-
-    req.hdr.nlmsg_len = sizeof req;
-    req.hdr.nlmsg_type = RTM_GETLINK;
-    req.hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_ROOT;
-    req.hdr.nlmsg_seq = ++nl_seq;
-    req.hdr.nlmsg_pid = nlcb_pid;
-    req.ifinfo.ifi_family = AF_UNSPEC;
-    req.ifinfo.ifi_index = ifidx; /* Doesn't work... */
-    req.ifinfo.ifi_flags = IFF_UP;
-    req.ifinfo.ifi_change = 0xffffffff;
-
-    send(cs->nlFd, &req, sizeof req, 0);
-}
-
 static void takedown_if(struct client_state_t *cs)
 {
     log_line("nl: taking down interface");
-    // Same as packet.c: line 258
+    // XXX: Same as packet.c - merge somehow?
     ifchange(NULL, IFCHANGE_DECONFIG);
     cs->dhcpState = DS_SELECTING;
     cs->timeout = 0;
@@ -128,53 +84,99 @@ static void takedown_if(struct client_state_t *cs)
     set_listen_raw(cs);
 }
 
-// Decode netlink messages and process them
+static int data_attr_cb(const struct nlattr *attr, void *data)
-static void nl_handlemsg(struct nlmsghdr *msg, unsigned int len,
-                         struct client_state_t *cs)
 {
-    if (len < sizeof(*msg)) return;
+    const struct nlattr **tb = data;
+    int type = mnl_attr_get_type(attr);
 
-    while(NLMSG_OK(msg,len)) {
+    /* skip unsupported attribute in user-space */
-        if (nlcb_run &&
+    if (mnl_attr_type_valid(attr, IFLA_MAX) < 0)
-            nlcb_pid == msg->nlmsg_pid &&
+        return MNL_CB_OK;
-            nlcb_seq == msg->nlmsg_seq) {
+    switch (type) {
-            nlcb_function(msg, nlcb_args);
+        case IFLA_IFNAME:
-
+            if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0) {
-            if (msg->nlmsg_type == NLMSG_DONE ||
+                log_warning("nl: IFLA_IFNAME failed validation.");
-                msg->nlmsg_type == NLMSG_ERROR) nlcb_run = 0;
+                return MNL_CB_ERROR;
             }
-
-        if (NLMSG_PAYLOAD(msg, msg->nlmsg_len) >= sizeof(struct ifinfomsg)) {
-            struct ifinfomsg *ifinfo = NLMSG_DATA(msg);
-
-            switch(msg->nlmsg_type) {
-                case RTM_NEWLINK:
-                    if (ifinfo->ifi_index != client_config.ifindex)
             break;
-                    if (ifinfo->ifi_flags & IFF_UP) {
+        case IFLA_ADDRESS:
-                        if (ifinfo->ifi_flags & IFF_RUNNING) {
+            if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0) {
+                log_warning("nl: IFLA_ADDRESS failed validation.");
+                return MNL_CB_ERROR;
+            }
+            break;
+    }
+    tb[type] = attr;
+    return MNL_CB_OK;
+}
+
+static void get_if_index_and_mac(const struct nlmsghdr *nlh,
+                                 struct ifinfomsg *ifm,
+                                 const struct nlattr **tb)
+{
+    mnl_attr_parse(nlh, sizeof(*ifm), data_attr_cb, tb);
+    if (!tb[IFLA_IFNAME])
+        return;
+    if (!strcmp(client_config.interface, mnl_attr_get_str(tb[IFLA_IFNAME]))) {
+        client_config.ifindex = ifm->ifi_index;
+        if (!tb[IFLA_ADDRESS])
+            suicide("FATAL: adapter %s lacks a hardware address");
+        int maclen = mnl_attr_get_len(tb[IFLA_ADDRESS]) - 4;
+        if (maclen != 6)
+            suicide("FATAL: adapter hardware address length should be 6, but is %u",
+                    maclen);
+
+        const unsigned char *mac =
+            (unsigned char *)mnl_attr_get_str(tb[IFLA_ADDRESS]);
+        log_line("%s hardware address %x:%x:%x:%x:%x:%x",
+                 client_config.interface,
+                 mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
+        memcpy(client_config.arp, mac, 6);
+    }
+}
+
+// XXX: Rather than exit, go into RELEASE state until a new hardware event
+// forces wakeup.
+static int data_cb(const struct nlmsghdr *nlh, void *data)
+{
+    struct nlattr *tb[IFLA_MAX+1] = {0};
+    struct ifinfomsg *ifm = mnl_nlmsg_get_payload(nlh);
+    struct client_state_t *cs = data;
+
+    switch(nlh->nlmsg_type) {
+        case RTM_NEWLINK:
+            if (!client_config.ifindex)
+                get_if_index_and_mac(nlh, ifm, (const struct nlattr **)tb);
+            if (ifm->ifi_index != client_config.ifindex)
+                break;
+            if (ifm->ifi_flags & IFF_UP) {
+                if (ifm->ifi_flags & IFF_RUNNING) {
                     if (cs->ifsPrevState != IFS_UP) {
                         cs->ifsPrevState = IFS_UP;
-                                /*
+                        // If we have a lease, then check to see
-                                 * If we have a lease, then check to see
+                        // if our gateway is still valid (via ARP).
-                                 * if our gateway is still valid (via ARP).
+                        // If it fails, state -> SELECTING.
-                                 * If it fails, state -> INIT.
-                                 *
-                                 * If we don't have a lease, state -> INIT.
-                                 */
                         if (cs->dhcpState == DS_BOUND) {
                             if (arp_gw_check(cs) == -1)
-                                        log_warning("arp_gw_check could not make arp socket, assuming lease is still OK");
+                                log_warning("nl: arp_gw_check could not make arp socket, assuming lease is still OK");
-                                } else if (cs->dhcpState != DS_SELECTING)
+                            else
+                                log_line("nl: interface back, revalidating lease");
+                        // If we don't have a lease, state -> SELECTING.
+                        } else if (cs->dhcpState != DS_SELECTING) {
+                            log_line("nl: interface back, querying for new lease");
                             takedown_if(cs);
                         }
+                    }
                 } else {
                     if (cs->ifsPrevState != IFS_DOWN) {
+                        // Interface was marked up but not running.
+                        // Get a new lease from scratch.
                         cs->ifsPrevState = IFS_DOWN;
                         takedown_if(cs);
                     }
                 }
             } else {
+                // No hardware carrier.
                 if (cs->ifsPrevState != IFS_SHUT) {
                     cs->ifsPrevState = IFS_SHUT;
                     log_line("Interface shut down; exiting.");
@@ -183,7 +185,7 @@ static void nl_handlemsg(struct nlmsghdr *msg, unsigned int len,
             }
             break;
         case RTM_DELLINK:
-                    if (ifinfo->ifi_index != client_config.ifindex)
+            if (ifm->ifi_index != client_config.ifindex)
                 break;
             if (cs->ifsPrevState != IFS_REMOVED) {
                 cs->ifsPrevState = IFS_REMOVED;
@@ -194,95 +196,41 @@ static void nl_handlemsg(struct nlmsghdr *msg, unsigned int len,
         default:
             break;
     }
-        }
+    return MNL_CB_OK;
-        msg = NLMSG_NEXT(msg,len);
-    }
 }
 
 void handle_nl_message(struct client_state_t *cs)
 {
-    char c[NLMSG_RECVSIZE];
+    char buf[MNL_SOCKET_BUFFER_SIZE];
-    struct nlmsghdr *msg = (struct nlmsghdr *)c;
+    int ret;
-    int n;
-
     assert(cs->nlFd != -1);
-    n = recv(cs->nlFd, c, NLMSG_RECVSIZE, 0);
+    do {
-    nl_handlemsg(msg, n, cs);
+        ret = mnl_socket_recvfrom(mls, buf, sizeof buf);
+        ret = mnl_cb_run(buf, ret, 0, 0, data_cb, cs);
+    } while (ret > 0);
+    if (ret == -1)
+        log_line("nl callback function returned error: %s", strerror(errno));
 }
 
-// Wait for and synchronously process netlink replies until a callback completes
-static void nl_sync_dump(struct client_state_t *cs)
-{
-    char c[NLMSG_RECVSIZE];
-    struct nlmsghdr *msg = (struct nlmsghdr *)c;
-    int n;
-
-    nlcb_seq = nl_seq;
-    for(nlcb_run = 1; nlcb_run;) {
-        n = recv(cs->nlFd, c, NLMSG_RECVSIZE, 0);
-        assert(n >= 0);
-        nl_handlemsg(msg, n, cs);
-    }
-}
-
-// Callback function for getting interface mac address and index.
-static void copy_ifdata(struct nlmsghdr *msg, void **args)
-{
-    struct ifinfomsg *ifinfo = NLMSG_DATA(msg);
-    struct rtattr *rta = IFLA_RTA(ifinfo);
-    int len = NLMSG_PAYLOAD(msg, sizeof(*ifinfo));
-    int found = 0;
-
-    if (msg->nlmsg_type != RTM_NEWLINK)
-        return;
-    if (client_config.ifindex)
-        return;
-
-    for(; RTA_OK(rta, len); rta = RTA_NEXT(rta, len)) {
-        switch(rta->rta_type) {
-            case IFLA_IFNAME:
-                if (!strncmp(client_config.interface,
-                             (char *)RTA_DATA(rta), RTA_PAYLOAD(rta))) {
-                    client_config.ifindex = ifinfo->ifi_index;
-                    found |= 1;
-                }
-                break;
-            case IFLA_ADDRESS:
-                if (found != 1)
-                    break;
-                /* We can only handle ethernet like devices with 6 octet MAC */
-                if (RTA_PAYLOAD(rta) == 6) {
-                    memcpy(client_config.arp, RTA_DATA(rta), 6);
-                    found |= 2;
-                }
-                break;
-        }
-    }
-    if (found == 3)
-        nlcb_args[0] = (void *)1;
-}
-
-// Gets interface mac address and index (synchronous).
 int nl_getifdata(const char *ifname, struct client_state_t *cs)
 {
-    struct {
+    char buf[MNL_SOCKET_BUFFER_SIZE];
-        struct nlmsghdr hdr;
+    struct nlmsghdr *nlh;
-        struct ifinfomsg ifinfo;
+    struct ifinfomsg *ifinfo;
-    } req;
+    unsigned int seq;
 
-    req.hdr.nlmsg_len = sizeof(req);
+    nlh = mnl_nlmsg_put_header(buf);
-    req.hdr.nlmsg_type = RTM_GETLINK;
+    nlh->nlmsg_type = RTM_GETLINK;
-    req.hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_ROOT;
+    nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ROOT;
-    req.hdr.nlmsg_seq = ++nl_seq;
+    nlh->nlmsg_seq = seq = time(NULL);
-    req.hdr.nlmsg_pid = nlcb_pid;
-    req.ifinfo.ifi_family = AF_UNSPEC;
 
-    if (send(cs->nlFd, &req, sizeof(req), 0) != sizeof(req)) return -1;
+    ifinfo = mnl_nlmsg_put_extra_header(nlh, sizeof(struct ifinfomsg));
+    ifinfo->ifi_family = AF_UNSPEC;
 
-    nlcb_function = copy_ifdata;
+    if (mnl_socket_sendto(mls, nlh, nlh->nlmsg_len) < 0)
-    nlcb_args[0] = NULL;
+        return -1;
 
-    nl_sync_dump(cs);
+    handle_nl_message(cs);
-
+    return 0;
-    return nlcb_args[0]?0:-1;
 }
+

ndhc/netlink.h

@@ -1,9 +1,6 @@
 /* netlink.h - netlink physical link notification handling and info retrieval
  *
  * (c) 2011 Nicholas J. Kain <njkain at gmail dot com>
- * (c) 2006-2007 Stefan Rompf <sux@loplof.de>
- *
- * This code was largely taken from Stefan Rompf's dhcpclient.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -26,8 +23,6 @@
 #include "state.h"
 
 int nl_open(struct client_state_t *cs);
-void nl_close(struct client_state_t *cs);
-void nl_queryifstatus(int ifidx, struct client_state_t *cs);
 void handle_nl_message(struct client_state_t *cs);
 int nl_getifdata(const char *ifname, struct client_state_t *cs);