[PATCH] (take 3) Add active/inactive flag to ncf_num_of_interfaces() & ncf_list_interfaces().

[Laine Stump]

This is a rebase of the previous submission, plus changing the name of
the flags slightly, closing ioctl_fd if there's an error in drv_init,
and fixing the flag naming in test-initscripts.c (all suggested by
lutter in IRC just now)

---
 src/drv_initscripts.c    |  101 ++++++++++++++++++++++++++++++++++++++-------
 src/internal.h           |    4 +-
 src/ncftool.c            |   16 ++++++-
 src/netcf.c              |    8 ++--
 src/netcf.h              |   21 +++++++---
 tests/test-initscripts.c |    4 +-
 6 files changed, 122 insertions(+), 32 deletions(-)

diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index d0a9af8..c63dc90 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -30,6 +30,11 @@
 #include <stdbool.h>
 #include <string.h>
 #include <unistd.h>
+
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <net/if.h>
+
 #include "safe-alloc.h"
 #include "ref.h"
 #include "list.h"
@@ -86,6 +91,7 @@ struct driver {
     xsltStylesheetPtr  put;
     xsltStylesheetPtr  get;
     xmlRelaxNGPtr      rng;
+    int                ioctl_fd;
 };
 
 /* Entries in a ifcfg file that tell us that the interface
@@ -214,6 +220,18 @@ static int is_slave(struct netcf *ncf, const char *intf) {
     return 0;
 }
 
+static int is_active(struct netcf *ncf, const char *intf) {
+    struct ifreq ifr;
+
+    MEMZERO(&ifr, 1);
+    strncpy(ifr.ifr_name, intf, sizeof(ifr.ifr_name));
+    ifr.ifr_name[sizeof(ifr.ifr_name)] = '\0';
+    if (ioctl(ncf->driver->ioctl_fd, SIOCGIFFLAGS, &ifr))  {
+        return 0;
+    }
+    return ((ifr.ifr_flags & IFF_UP) == IFF_UP);
+}
+
 static int list_interfaces(struct netcf *ncf, char ***intf) {
     int nint = 0, result = 0;
     struct augeas *aug = NULL;
@@ -422,11 +440,35 @@ static char *xml_prop(xmlNodePtr node, const char *name) {
     return (char *) xmlGetProp(node, BAD_CAST name);
 }
 
+static int init_ioctl_fd(struct netcf *ncf) {
+
+    int ioctl_fd;
+    int flags;
+
+    ioctl_fd = socket(AF_INET, SOCK_STREAM, 0);
+    ERR_THROW(ioctl_fd < 0, ncf, EINTERNAL, "failed to open socket for interface
ioctl");
+
+    flags = fcntl(ioctl_fd, F_GETFD);
+    ERR_THROW(flags < 0, ncf, EINTERNAL, "failed to get flags for ioctl
socket");
+
+    flags = fcntl(ioctl_fd, F_SETFD, flags | FD_CLOEXEC);
+    ERR_THROW(flags < 0, ncf, EINTERNAL, "failed to set FD_CLOEXEC flag on ioctl
socket");
+    return ioctl_fd;
+
+error:
+    if (ioctl_fd >= 0)
+        close(ioctl_fd);
+    return -1;
+}
+
 int drv_init(struct netcf *ncf) {
     int r;
 
     if (ALLOC(ncf->driver) < 0)
         return -1;
+
+    ncf->driver->ioctl_fd = -1;
+
     // FIXME: Check for errors
     xsltInit();
     r = xslt_ext_register();
@@ -436,8 +478,16 @@ int drv_init(struct netcf *ncf) {
     ncf->driver->rng = rng_parse(ncf, "interface.rng");
     /* We undconditionally bridge physdevs; could be more discriminating */
     bridge_physdevs(ncf);
+
+    /* open a socket for interface ioctls */
+    ncf->driver->ioctl_fd = init_ioctl_fd(ncf);
+    if (ncf->driver->ioctl_fd < 0)
+        goto error;
     return 0;
+
  error:
+    if (ncf->driver->ioctl_fd >= 0)
+        close(ncf->driver->ioctl_fd);
     FREE(ncf->driver);
     return -1;
 }
@@ -447,49 +497,68 @@ void drv_close(struct netcf *ncf) {
     xsltFreeStylesheet(ncf->driver->put);
     xslt_ext_unregister();
     xsltCleanupGlobals();
+    if (ncf->driver->ioctl_fd >= 0)
+        close(ncf->driver->ioctl_fd);
     FREE(ncf->driver);
 }
 
-int drv_num_of_interfaces(struct netcf *ncf) {
-    int nint = 0;
-    char **intf = NULL;
-
-    nint = list_interfaces(ncf, &intf);
-    free_matches(nint, &intf);
-    return nint;
-}
-
 static int list_interface_ids(struct netcf *ncf,
                               int maxnames, char **names,
+                              unsigned int flags,
                               const char *id_attr) {
     struct augeas *aug = NULL;
-    int nint = 0, nmatches = 0, result = 0, r;
+    int nint = 0, nmatches = 0, nqualified = 0, result = 0, r;
     char **intf = NULL, **matches = NULL;
 
     aug = get_augeas(ncf);
     ERR_BAIL(ncf);
     nint = list_interfaces(ncf, &intf);
-    for (result = 0; result < nint && result < maxnames; result++) {
+    if (!names) {
+        maxnames = nint;    /* if not returning list, ignore maxnames too */
+    }
+    for (result = 0; (result < nint) && (nqualified < maxnames); result++)
{
         nmatches = aug_submatch(ncf, intf[result], id_attr, &matches);
         if (nmatches > 0) {
             const char *name;
+            int is_qualified = ((flags & (NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE))
+                                == (NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE));
+
             r = aug_get(aug, matches[nmatches-1], &name);
             ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            names[result] = strdup(name);
-            ERR_COND_BAIL(names[result] == NULL, ncf, ENOMEM);
+
+            if (!is_qualified) {
+                int if_is_active = is_active(ncf, name);
+                if ((if_is_active && (flags & NETCF_IFACE_ACTIVE))
+                    || ((!if_is_active) && (flags & NETCF_IFACE_INACTIVE)))
{
+
+                    is_qualified = 1;
+                }
+            }
+
+            if (is_qualified) {
+                if (names) {
+                    names[nqualified] = strdup(name);
+                    ERR_COND_BAIL(names[nqualified] == NULL, ncf, ENOMEM);
+                }
+                nqualified++;
+            }
         }
         free_matches(nmatches, &matches);
     }
     free_matches(nint, &intf);
-    return result;
+    return nqualified;
  error:
     free_matches(nmatches, &matches);
     free_matches(nint, &intf);
     return -1;
 }
 
-int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names) {
-    return list_interface_ids(ncf, maxnames, names, "DEVICE");
+int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names, unsigned int
flags) {
+    return list_interface_ids(ncf, maxnames, names, flags, "DEVICE");
+}
+
+int drv_num_of_interfaces(struct netcf *ncf, unsigned int flags) {
+    return list_interface_ids(ncf, 0, NULL, flags, "DEVICE");
 }
 
 static struct netcf_if *make_netcf_if(struct netcf *ncf, char *name) {
diff --git a/src/internal.h b/src/internal.h
index dc10a62..56c6038 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -152,8 +152,8 @@ void vreport_error(struct netcf *ncf, netcf_errcode_t errcode,
  */
 int drv_init(struct netcf *netcf);
 void drv_close(struct netcf *netcf);
-int drv_num_of_interfaces(struct netcf *ncf);
-int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names);
+int drv_num_of_interfaces(struct netcf *ncf, unsigned int flags);
+int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names, unsigned int
flags);
 struct netcf_if *drv_lookup_by_name(struct netcf *ncf, const char *name);
 int drv_lookup_by_mac_string(struct netcf *, const char *mac,
                              int maxifaces, struct netcf_if **ifaces);
diff --git a/src/ncftool.c b/src/ncftool.c
index cee1fce..a1362c2 100644
--- a/src/ncftool.c
+++ b/src/ncftool.c
@@ -137,13 +137,21 @@ static const char *arg_value(const struct command *cmd, const char
*name) {
 static int cmd_list(ATTRIBUTE_UNUSED const struct command *cmd) {
     int nint;
     char **intf;
+    unsigned int flags = NETCF_IFACE_ACTIVE;
 
-    nint = ncf_num_of_interfaces(ncf);
+    if (opt_present(cmd, "inactive")) {
+        flags = NETCF_IFACE_INACTIVE;
+    }
+    if (opt_present(cmd, "all")) {
+        flags = NETCF_IFACE_ACTIVE | NETCF_IFACE_INACTIVE;
+    }
+
+    nint = ncf_num_of_interfaces(ncf, flags);
     if (nint < 0)
         return CMD_RES_ERR;
     if (ALLOC_N(intf, nint) < 0)
         return CMD_RES_ENOMEM;
-    nint = ncf_list_interfaces(ncf, nint, intf);
+    nint = ncf_list_interfaces(ncf, nint, intf, flags);
     if (nint < 0)
         return CMD_RES_ERR;
     for (int i=0; i < nint; i++) {
@@ -175,6 +183,10 @@ static int cmd_list(ATTRIBUTE_UNUSED const struct command *cmd) {
 static const struct command_opt_def cmd_list_opts[] = {
     { .tag = CMD_OPT_BOOL, .name = "macs",
       .help = "show MAC addresses" },
+    { .tag = CMD_OPT_BOOL, .name = "all",
+      .help = "show all (up & down) interfaces" },
+    { .tag = CMD_OPT_BOOL, .name = "inactive",
+      .help = "show only inactive (down) interfaces" },
     CMD_OPT_DEF_LAST
 };
 
diff --git a/src/netcf.c b/src/netcf.c
index 4d9943c..906db7d 100644
--- a/src/netcf.c
+++ b/src/netcf.c
@@ -102,17 +102,17 @@ void ncf_close(struct netcf *ncf) {
  *
  * Maybe we should just list them as STRUCT NETCF_IF *
  */
-int ncf_num_of_interfaces(struct netcf *ncf) {
+int ncf_num_of_interfaces(struct netcf *ncf, unsigned int flags) {
     ERR_RESET(ncf);
-    return drv_num_of_interfaces(ncf);
+    return drv_num_of_interfaces(ncf, flags);
 }
 
-int ncf_list_interfaces(struct netcf *ncf, int maxnames, char **names) {
+int ncf_list_interfaces(struct netcf *ncf, int maxnames, char **names, unsigned int
flags) {
     int result;
 
     ERR_RESET(ncf);
     MEMZERO(names, maxnames);
-    result = drv_list_interfaces(ncf, maxnames, names);
+    result = drv_list_interfaces(ncf, maxnames, names, flags);
     if (result < 0)
         for (int i=0; i < maxnames; i++)
             FREE(names[i]);
diff --git a/src/netcf.h b/src/netcf.h
index 377b196..c7bfffd 100644
--- a/src/netcf.h
+++ b/src/netcf.h
@@ -24,10 +24,6 @@
 #define NETCF_H_
 
 /*
- * FIXME: We need a way to distinguish between 'active' interfaces (i.e.,
- * ones that are up) and ones that are merely defined; maybe punt that to
- * clients ?
- *
  * FIXME: NM needs a way to be notified of changes to the underlying
  * network files, either we provide a way to register callbacks for an
  * interface, or we hand out a list of files that contain the configs for
@@ -53,6 +49,18 @@ typedef enum {
     NETCF_EEXEC          /* external program execution failed or returned non-0 */
 } netcf_errcode_t;
 
+
+/*
+ * flags accepted by ncf_num_of_interfaces and ncf_list_interfaces.
+ * IMPORTANT NOTE: These are bits, so you should assign only powers of two
+ * (Default behavior is to match NO interfaces)
+ */
+typedef enum {
+    NETCF_IFACE_INACTIVE = 1,     /* match down interfaces */
+    NETCF_IFACE_ACTIVE = 2,       /* match up interfaces */
+} netcf_if_flag_t;
+
+
 /*
  * Initialize netcf. This function must be called before any other netcf
  * function can be called.
@@ -70,9 +78,10 @@ void ncf_close(struct netcf *);
  * identified by their name.
  */
 int
-ncf_num_of_interfaces(struct netcf *);
+ncf_num_of_interfaces(struct netcf *, unsigned int flags);
 int
-ncf_list_interfaces(struct netcf *, int maxnames, char **names);
+ncf_list_interfaces(struct netcf *, int maxnames, char **names, unsigned int flags);
+
 
 /* Look up interfaces by UUID, name and hwaddr (MAC-48) */
 struct netcf_if *
diff --git a/tests/test-initscripts.c b/tests/test-initscripts.c
index 3f0545e..abaee9b 100644
--- a/tests/test-initscripts.c
+++ b/tests/test-initscripts.c
@@ -182,11 +182,11 @@ static void testListInterfaces(CuTest *tc) {
     static const char *const exp_names[] = { "br0", "bond0",
"lo" };
     static const int exp_nint = ARRAY_CARDINALITY(exp_names);
 
-    nint = ncf_num_of_interfaces(ncf);
+    nint = ncf_num_of_interfaces(ncf, NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE);
     CuAssertIntEquals(tc, exp_nint, nint);
     if (ALLOC_N(names, nint) < 0)
         die("allocation failed");
-    nint = ncf_list_interfaces(ncf, nint, names);
+    nint = ncf_list_interfaces(ncf, nint, names,
NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE);
     CuAssertIntEquals(tc, exp_nint, nint);
     for (int i=0; i < exp_nint; i++) {
         int found = 0;
-- 
1.6.0.6

[David Lutterkort]

On Tue, 2009-06-30 at 18:57 -0400, Laine Stump wrote:
>  This is a rebase of the previous submission, plus changing the name
> of
>  the flags slightly, closing ioctl_fd if there's an error in drv_init,
>  and fixing the flag naming in test-initscripts.c (all suggested by
>  lutter in IRC just now)
>  
>  ---
>   src/drv_initscripts.c    |  101 ++++++++++++++++++++++++++++++++++++++-------
>   src/internal.h           |    4 +-
>   src/ncftool.c            |   16 ++++++-
>   src/netcf.c              |    8 ++--
>   src/netcf.h              |   21 +++++++---
>   tests/test-initscripts.c |    4 +-
>   6 files changed, 122 insertions(+), 32 deletions(-) 
Excellent - I committed this with one small fix:

>  diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
>  index d0a9af8..c63dc90 100644
>  --- a/src/drv_initscripts.c
>  +++ b/src/drv_initscripts.c
>  @@ -30,6 +30,11 @@
>   #include <stdbool.h>
>   #include <string.h>
>   #include <unistd.h>
>  +
>  +#include <fcntl.h>
>  +#include <sys/ioctl.h>
>  +#include <net/if.h>
>  +
>   #include "safe-alloc.h"
>   #include "ref.h"
>   #include "list.h"
>  @@ -86,6 +91,7 @@ struct driver {
>       xsltStylesheetPtr  put;
>       xsltStylesheetPtr  get;
>       xmlRelaxNGPtr      rng;
>  +    int                ioctl_fd;
>   };
>   
>   /* Entries in a ifcfg file that tell us that the interface
>  @@ -214,6 +220,18 @@ static int is_slave(struct netcf *ncf, const char *intf) {
>       return 0;
>   }
>   
>  +static int is_active(struct netcf *ncf, const char *intf) {
>  +    struct ifreq ifr;
>  +
>  +    MEMZERO(&ifr, 1);
>  +    strncpy(ifr.ifr_name, intf, sizeof(ifr.ifr_name));
>  +    ifr.ifr_name[sizeof(ifr.ifr_name)] = '\0'; 
There's an off-by-one error here: that should be
ifr.ifr_name[sizeof(ifr.ifr_name) - 1]

David